diff mbox series

[v3,2/3] x86/vmx: introduce helper to set VMX_INTR_SHADOW_NMI

Message ID 20220701131648.34292-3-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show
Series x86/vmx: implement Bus Lock and VM Notify | expand

Commit Message

Roger Pau Monné July 1, 2022, 1:16 p.m. UTC 
Introduce a small helper to OR VMX_INTR_SHADOW_NMI in
GUEST_INTERRUPTIBILITY_INFO in order to help dealing with the NMI
unblocked by IRET case.  Replace the existing usage in handling
EXIT_REASON_EXCEPTION_NMI and also add such handling to EPT violations
and page-modification log-full events.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Changes since v1:
 - New in this version.
---
 xen/arch/x86/hvm/vmx/vmx.c             | 28 +++++++++++++++++++-------
 xen/arch/x86/include/asm/hvm/vmx/vmx.h |  3 +++
 2 files changed, 24 insertions(+), 7 deletions(-)

Comments

Tian, Kevin July 19, 2022, 7:33 a.m. UTC | #1 
> From: Roger Pau Monne <roger.pau@citrix.com>
> Sent: Friday, July 1, 2022 9:17 PM
> 
> @@ -225,6 +225,9 @@ static inline void pi_clear_sn(struct pi_desc *pi_desc)
> 
>  /*
>   * Interruption-information format
> + *
> + * Note INTR_INFO_NMI_UNBLOCKED_BY_IRET is also used with Exit
> Qualification
> + * field under some circumstances.
>   */
>  #define INTR_INFO_VECTOR_MASK           0xff            /* 7:0 */
>  #define INTR_INFO_INTR_TYPE_MASK        0x700           /* 10:8 */

Overall this is good. But I wonder whether the readability is slightly better
by defining a dedicate flag bit for exit qualification with a clear comment
on which events it makes sense to...
Roger Pau Monné Dec. 13, 2022, 2:42 p.m. UTC | #2 
On Tue, Jul 19, 2022 at 07:33:47AM +0000, Tian, Kevin wrote:
> > From: Roger Pau Monne <roger.pau@citrix.com>
> > Sent: Friday, July 1, 2022 9:17 PM
> > 
> > @@ -225,6 +225,9 @@ static inline void pi_clear_sn(struct pi_desc *pi_desc)
> > 
> >  /*
> >   * Interruption-information format
> > + *
> > + * Note INTR_INFO_NMI_UNBLOCKED_BY_IRET is also used with Exit
> > Qualification
> > + * field under some circumstances.
> >   */
> >  #define INTR_INFO_VECTOR_MASK           0xff            /* 7:0 */
> >  #define INTR_INFO_INTR_TYPE_MASK        0x700           /* 10:8 */
> 
> Overall this is good. But I wonder whether the readability is slightly better
> by defining a dedicate flag bit for exit qualification with a clear comment
> on which events it makes sense to...

I've expanded the comment to:

"Note INTR_INFO_NMI_UNBLOCKED_BY_IRET is also used with Exit Qualification
field for EPT violations, PML full and SPP-related event vmexits."

I leave the creation of the specific flag to a separate commit, to not
block the series on that.

Thanks, Roger.
diff mbox series

Patch

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 853f3a9111..d69c02b00a 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3935,6 +3935,15 @@  static int vmx_handle_apic_write(void)
     return vlapic_apicv_write(current, exit_qualification & 0xfff);
 }
 
+static void undo_nmis_unblocked_by_iret(void)
+{
+    unsigned long guest_info;
+
+    __vmread(GUEST_INTERRUPTIBILITY_INFO, &guest_info);
+    __vmwrite(GUEST_INTERRUPTIBILITY_INFO,
+              guest_info | VMX_INTR_SHADOW_NMI);
+}
+
 void vmx_vmexit_handler(struct cpu_user_regs *regs)
 {
     unsigned long exit_qualification, exit_reason, idtv_info, intr_info = 0;
@@ -4134,13 +4143,7 @@  void vmx_vmexit_handler(struct cpu_user_regs *regs)
         if ( unlikely(intr_info & INTR_INFO_NMI_UNBLOCKED_BY_IRET) &&
              !(idtv_info & INTR_INFO_VALID_MASK) &&
              (vector != TRAP_double_fault) )
-        {
-            unsigned long guest_info;
-
-            __vmread(GUEST_INTERRUPTIBILITY_INFO, &guest_info);
-            __vmwrite(GUEST_INTERRUPTIBILITY_INFO,
-                      guest_info | VMX_INTR_SHADOW_NMI);
-        }
+            undo_nmis_unblocked_by_iret();
 
         perfc_incra(cause_vector, vector);
 
@@ -4506,6 +4509,11 @@  void vmx_vmexit_handler(struct cpu_user_regs *regs)
 
         __vmread(GUEST_PHYSICAL_ADDRESS, &gpa);
         __vmread(EXIT_QUALIFICATION, &exit_qualification);
+
+        if ( unlikely(exit_qualification & INTR_INFO_NMI_UNBLOCKED_BY_IRET) &&
+             !(idtv_info & INTR_INFO_VALID_MASK) )
+            undo_nmis_unblocked_by_iret();
+
         ept_handle_violation(exit_qualification, gpa);
         break;
     }
@@ -4550,6 +4558,12 @@  void vmx_vmexit_handler(struct cpu_user_regs *regs)
         break;
 
     case EXIT_REASON_PML_FULL:
+        __vmread(EXIT_QUALIFICATION, &exit_qualification);
+
+        if ( unlikely(exit_qualification & INTR_INFO_NMI_UNBLOCKED_BY_IRET) &&
+             !(idtv_info & INTR_INFO_VALID_MASK) )
+            undo_nmis_unblocked_by_iret();
+
         vmx_vcpu_flush_pml_buffer(v);
         break;
 
diff --git a/xen/arch/x86/include/asm/hvm/vmx/vmx.h b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
index 03995701a1..bc0caad6fb 100644
--- a/xen/arch/x86/include/asm/hvm/vmx/vmx.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmx.h
@@ -225,6 +225,9 @@  static inline void pi_clear_sn(struct pi_desc *pi_desc)
 
 /*
  * Interruption-information format
+ *
+ * Note INTR_INFO_NMI_UNBLOCKED_BY_IRET is also used with Exit Qualification
+ * field under some circumstances.
  */
 #define INTR_INFO_VECTOR_MASK           0xff            /* 7:0 */
 #define INTR_INFO_INTR_TYPE_MASK        0x700           /* 10:8 */