Re: [PATCH 0/3] Virtual NMI

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/02/2026 3:53 pm, Abdelkareem@xxxxxxxxx wrote:
> Abdelkareem Abdelsaamad (3):
>   svm/vnmi: introduce the vnmi bit support in the cpuid feature set
>   svm/vnmi: add the definitions for the svm vnmi management bits in the
>     VMCB
>   svm/vnmi: Add support for the SVM Virtual NMI
>
>  xen/arch/x86/hvm/hvm.c             | 29 +++++++++++++++++++++--------
>  xen/arch/x86/hvm/svm/intr.c        | 16 ++++++++++++++--
>  xen/arch/x86/hvm/svm/svm.c         | 25 ++++++++++++++++++++++++-
>  xen/arch/x86/hvm/svm/vmcb.c        |  3 +++
>  xen/arch/x86/hvm/svm/vmcb.h        | 12 ++++++++----
>  xen/arch/x86/include/asm/hvm/hvm.h | 12 +++++++++++-
>  xen/arch/x86/include/asm/hvm/svm.h |  2 ++
>  7 files changed, 83 insertions(+), 16 deletions(-)

Patches 1 and 2 want merging.  They're both enumerations and
configuration bits, although the very first hunk of patch 1 (the P())
wants delaying until the final patch; we shouldn't print out the
capability until it's being used.

The patch subjects want to be:

    x86/svm: Enumerations for virtual NMI
and
    x86/svm: Use virtual NMI when available


Everything here is local to SVM.  Notably there should be no edits to
hvm.c or hvm.h.  By introducing hvm_intblk_vnmi, you break NMI injection
in other cases.  vNMI is just a hardware-optimised way of handling the
hvm_intblk_nmi_iret case.



svm_inject_nmi() wants to gain a check to see whether vNMI is enabled,
and in the case that it is, simply set vnmi_pending.  You have this
partially, but it needs to be dependent on the VMCB vNMI setting, not
some global idea of enablement.

svm_get_interrupt_shadow() needs a similar adjustment to read
vnmi_blocked rather than unconditionally depending on INTERCEPT_IRET.

In construct_vmcb(), you need to check cpu_has_svm_vnmi.  I think this
change is simple enough to be enabled unconditionally.  (We'll need to
change this in due course, but that's going to take other infrastructure
which we don't have yet.)

I think that's everything that needs altering.


A couple of other minor notes:

In the vintr_t union, use an anonymous 3 bit field (literally "u64 :3;",
which is valid syntax) instead of renumbering the rsvd$N fields.  That
will shrink the diff.

Xen's style has spaces inside the outermost brackets for control
structures, and {'s on new lines.  For the functions you're modifying,
just copy the surrounding style.

~Andrew