Re: [PATCH v2 1/2] x86/hvm: don't let EFLAGS.IF block NMI/#MC due to nested state

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

On 11/10/26 8:57 AM, chunjie.zhu@xxxxxxxxxx wrote:
> From: Chunjie Zhu <chunjie.zhu@xxxxxxxxxx>
>
> nhvm_interrupt_blocked() reports why an interrupt is blocked, but both
> callers, nestedsvm_vcpu_interrupt() and hvm_interrupt_blocked(), treat any
> reason as blocking every event source.  EFLAGS.IF (hvm_intblk_rflags_ie)
> doesn't mask NMI or #MC, so NMI/#MC get held pending needlessly, e.g. when
> L1 sets V_INTR_MASKING and L1's IF is clear.
>
> Have nhvm_interrupt_blocked() take the intack and report no block for
> NMI/#MC when the reason is hvm_intblk_rflags_ie.
>
> nvmx_intr_blocked() used hvm_intblk_rflags_ie for vmexit/vmentry in
> progress and a valid VM_ENTRY_INTR_INFO.  Those block every event source
> and have nothing to do with RFLAGS.IF, so report hvm_intblk_arch instead.
> This doesn't change behaviour as all callers only compare against
> hvm_intblk_none and hvm_intblk_tpr.
>
> Signed-off-by: Chunjie Zhu <chunjie.zhu@xxxxxxxxxx>
> ---
> v2:
>   * New patch.  Handle the blocking reason in callers, as suggested by Ross.
>   * Report hvm_intblk_arch from nvmx_intr_blocked() so the NMI/#MC exemption
>     doesn't affect VMX.
>
>   xen/arch/x86/hvm/hvm.c             |  2 +-
>   xen/arch/x86/hvm/svm/nestedsvm.c   |  2 +-
>   xen/arch/x86/hvm/vmx/intr.c        |  6 +++---
>   xen/arch/x86/include/asm/hvm/hvm.h | 12 ++++++++++--
>   4 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index ef9c7cd05777..8fd095a4f8e6 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3897,7 +3897,7 @@ enum hvm_intblk hvm_interrupt_blocked(struct vcpu *v, 
> struct hvm_intack intack)
>       if ( nestedhvm_enabled(v->domain) ) {
>           enum hvm_intblk intr;
>   
> -        intr = nhvm_interrupt_blocked(v);
> +        intr = nhvm_interrupt_blocked(v, intack);
>           if ( intr != hvm_intblk_none )
>               return intr;
>       }
> diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
> index 66b77c6efe7d..a93681e9bbcd 100644
> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> @@ -1496,7 +1496,7 @@ nestedsvm_vcpu_interrupt(struct vcpu *v, const struct 
> hvm_intack intack)
>       uint64_t exitinfo2 = 0;
>       ASSERT(nestedhvm_vcpu_in_guestmode(v));
>   
> -    intr = nhvm_interrupt_blocked(v);
> +    intr = nhvm_interrupt_blocked(v, intack);
>       if ( intr != hvm_intblk_none )
>           return NSVM_INTR_MASKED;
>   
> diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
> index 6c220c4c2e49..38be2387739d 100644
> --- a/xen/arch/x86/hvm/vmx/intr.c
> +++ b/xen/arch/x86/hvm/vmx/intr.c
> @@ -142,18 +142,18 @@ enum hvm_intblk cf_check nvmx_intr_blocked(struct vcpu *v)
>       {
>           if ( nvcpu->nv_vmexit_pending ||
>                nvcpu->nv_vmswitch_in_progress )
> -            r = hvm_intblk_rflags_ie;
> +            r = hvm_intblk_arch;
>           else
>           {
>               unsigned long intr_info;
>   
>               __vmread(VM_ENTRY_INTR_INFO, &intr_info);
>               if ( intr_info & INTR_INFO_VALID_MASK )
> -                r = hvm_intblk_rflags_ie;
> +                r = hvm_intblk_arch;
>           }
>       }
>       else if ( nvcpu->nv_vmentry_pending )
> -        r = hvm_intblk_rflags_ie;
> +        r = hvm_intblk_arch;
>   
>       return r;
>   }
> diff --git a/xen/arch/x86/include/asm/hvm/hvm.h 
> b/xen/arch/x86/include/asm/hvm/hvm.h
> index 02d9c44373ff..3e03a08a4818 100644
> --- a/xen/arch/x86/include/asm/hvm/hvm.h
> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> @@ -682,9 +682,17 @@ static inline bool nhvm_vmcx_hap_enabled(struct vcpu *v)
>   }
>   
>   /* interrupt */
> -static inline enum hvm_intblk nhvm_interrupt_blocked(struct vcpu *v)
> +static inline enum hvm_intblk nhvm_interrupt_blocked(
> +    struct vcpu *v, const struct hvm_intack intack)
>   {
> -    return alternative_call(hvm_funcs.nhvm_intr_blocked, v);
> +    enum hvm_intblk intr = alternative_call(hvm_funcs.nhvm_intr_blocked, v);
> +
> +    /* EFLAGS.IF doesn't mask NMI or #MC. */
> +    if ( intr == hvm_intblk_rflags_ie &&
> +         (intack.source == hvm_intsrc_nmi || intack.source == hvm_intsrc_mce) )
> +        return hvm_intblk_none;
> +
> +    return intr;
>   }
Not specifically for this patch, but I note that hvm_interrupt_blocked()
similarly checks the interrupt flag but only ignores it for NMIs, not MCEs.
That should probably be fixed.

Ross