All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Hemanth Selam" <hemanth.selam@gmail.com>
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: selftests: Add a test for nested virtual NMI support
Date: Fri, 11 Sep 2026 10:30:40 +0000	[thread overview]
Message-ID: <20260911103041.4E7A71F00899@smtp.kernel.org> (raw)
In-Reply-To: <20260911102105.1927773-1-hemanth.selam@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing memory barrier before VM-exit allows compiler to reorder global variable store, causing test flakiness.
--

commit 369b941892ff4fd633ee8a8bca1eef89e730d9a0
Author: Hemanth Selam <hemanth.selam@gmail.com>

KVM: selftests: Add a test for nested virtual NMI support

KVM's virtual NMI support for nested guests has no test coverage.
This commit adds a test covering the vNMI controls L1 provides in vmcb12,
including delivery of virtual NMIs to L2 and verifying the state of
V_NMI_PENDING and V_NMI_BLOCKING flags.

> diff --git a/tools/testing/selftests/kvm/x86/svm_nested_vnmi_test.c b/tools/testing/selftests/kvm/x86/svm_nested_vnmi_test.c
> new file mode 100644
> index 0000000000000..4e9916a837ddd
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/svm_nested_vnmi_test.c

[ ... ]

> +static unsigned int nmi_fired;
> +
> +static void guest_nmi_handler(struct ex_regs *regs)
> +{
> +	nmi_fired++;
> +
> +	/*
> +	 * Exit to L1 from NMI context, i.e. before this handler's IRET, so
> +	 * that L1 can observe V_NMI_BLOCKING while the NMI is in service.
> +	 */
> +	vmmcall();
> +}

[Severity: Medium]
Could an optimizing compiler reorder the store to nmi_fired after the
vmmcall()?

Since nmi_fired is a plain global variable and vmmcall() is implemented as
an inline assembly block without a "memory" clobber, the C standard allows
the compiler to reorder the memory store across the vmmcall() instruction.

If instruction scheduling places the memory store after the vmmcall(), L2
will execute the VM-exit before the store is committed, causing the host (L1)
to read a stale value and making the GUEST_ASSERT_EQ(nmi_fired, i) assertion
fail sporadically.

Would adding a compiler barrier or using WRITE_ONCE() before the vmmcall()
help ensure the store is committed before exiting to the host?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911102105.1927773-1-hemanth.selam@gmail.com?part=1

  reply	other threads:[~2026-09-11 10:30 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-11 10:21 [PATCH] KVM: selftests: Add a test for nested virtual NMI support Hemanth Selam
2026-09-11 10:30 ` sashiko-bot [this message]
2026-09-15  5:46 ` [PATCH v2] " Hemanth Selam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260911103041.4E7A71F00899@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=hemanth.selam@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.