Re: [PATCH v2] KVM: SVM: Don't intercept IRET when injecting NMI and vNMI is enabled

[Santosh Shukla]

On 10/19/2023 12:50 AM, Sean Christopherson wrote:
> When vNMI is enabled, rely entirely on hardware to correctly handle NMI
> blocking, i.e. don't intercept IRET to detect when NMIs are no longer
> blocked.  KVM already correctly ignores svm->nmi_masked when vNMI is
> enabled, so the effect of the bug is essentially an unnecessary VM-Exit.
> 
> KVM intercepts IRET for two reasons:
>  - To track NMI masking to be able to know at any point of time if NMI
>    is masked.
>  - To track NMI windows (to inject another NMI after the guest executes
>    IRET, i.e. unblocks NMIs)
> 
> When vNMI is enabled, both cases are handled by hardware:
> - NMI masking state resides in int_ctl.V_NMI_BLOCKING and can be read by
>   KVM at will.
> - Hardware automatically "injects" pending virtual NMIs when virtual NMIs
>   become unblocked.
> 
> However, even though pending a virtual NMI for hardware to handle is the
> most common way to synthesize a guest NMI, KVM may still directly inject
> an NMI via when KVM is handling two "simultaneous" NMIs (see comments in
> process_nmi() for details on KVM's simultaneous NMI handling).  Per AMD's
> APM, hardware sets the BLOCKING flag when software directly injects an NMI
> as well, i.e. KVM doesn't need to manually mark vNMIs as blocked:
> 
>   If Event Injection is used to inject an NMI when NMI Virtualization is
>   enabled, VMRUN sets V_NMI_MASK in the guest state.
> 
> Note, it's still possible that KVM could trigger a spurious IRET VM-Exit.
> When running a nested guest, KVM disables vNMI for L2 and thus will enable
> IRET interception (in both vmcb01 and vmcb02) while running L2 reason.  If
> a nested VM-Exit happens before L2 executes IRET, KVM can end up running
> L1 with vNMI enable and IRET intercepted.  This is also a benign bug, and
> even less likely to happen, i.e. can be safely punted to a future fix.
> 
> Fixes: fa4c027a7956 ("KVM: x86: Add support for SVM's Virtual NMI")
> Link: https://lore.kernel.org/all/ZOdnuDZUd4mevCqe@xxxxxxxxxxx
> Cc: Santosh Shukla <santosh.shukla@xxxxxxx>
> Cc: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
> 
> v2: Expand changelog to explain the various behaviors and combos. [Maxim]
> 
> v1: https://lore.kernel.org/all/20231009212919.221810-1-seanjc@xxxxxxxxxx
> 

Tested-by: Santosh Shukla <santosh.shukla@xxxxxxx>

Thanks,