All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Maxim Levitsky <mlevitsk@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Santosh Shukla <santosh.shukla@amd.com>
Subject: Re: [PATCH] KVM: SVM: Don't intercept IRET when injecting NMI and vNMI is enabled
Date: Tue, 10 Oct 2023 10:50:35 -0700	[thread overview]
Message-ID: <ZSWO6x-mFg37uIpq@google.com> (raw)
In-Reply-To: <1ca607bcb4931b7f5e14e6c064264d86e58fd3ce.camel@redhat.com>

On Tue, Oct 10, 2023, Maxim Levitsky wrote:
> У вт, 2023-10-10 у 07:46 -0700, Sean Christopherson пише:
> > On Tue, Oct 10, 2023, Maxim Levitsky wrote:
> > > У пн, 2023-10-09 у 14:29 -0700, Sean Christopherson пише:
> > > > Note, per the APM, hardware sets the BLOCKING flag when software directly
> > > > directly injects an NMI:
> > > > 
> > > >   If Event Injection is used to inject an NMI when NMI Virtualization is
> > > >   enabled, VMRUN sets V_NMI_MASK in the guest state.
> > > 
> > > I think that this comment is not needed in the commit message. It describes
> > > a different unrelated concern and can be put somewhere in the code but
> > > not in the commit message.
> > 
> > I strongly disagree, this blurb in the APM directly affects the patch.  If hardware
> > didn't set V_NMI_MASK, then the patch would need to be at least this:
> 
> I don't see how 'the blurb in the APM' relates to the removal of the 
> IRET intercept, which is what this patch is about.

No, it's not *just* about IRET interception.  This patch also guards:

	svm->nmi_masked = true;

If the reader doesn't already know that hardware sets V_NMI_BLOCK_MASK on direct
injection, as was the case for me when I stumbled upon this issue, it's not at
all obvious that not doing something analogous to setting nmi_masked is correct.

I mentioned only IRET interception in the shortlog because that's the only practical
impact of the change.  I can massage the shortlog if it's confusing/misleading,
but I really don't want to drop the reference to hardware setting V_NMI_BLOCK_MASK.

  reply	other threads:[~2023-10-10 17:50 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-09 21:29 [PATCH] KVM: SVM: Don't intercept IRET when injecting NMI and vNMI is enabled Sean Christopherson
2023-10-10 12:03 ` Maxim Levitsky
2023-10-10 14:46   ` Sean Christopherson
2023-10-10 16:06     ` Maxim Levitsky
2023-10-10 17:50       ` Sean Christopherson [this message]
2023-10-14 10:16     ` Santosh Shukla
2023-10-14 14:49       ` Santosh Shukla

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=ZSWO6x-mFg37uIpq@google.com \
    --to=seanjc@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mlevitsk@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=santosh.shukla@amd.com \
    /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.