mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Maxim Levitsky <mlevitsk@redhat.com>
To: Jim Mattson <jmattson@google.com>
Cc: kvm@vger.kernel.org, Sean Christopherson <seanjc@google.com>,
	x86@kernel.org, Kees Cook <keescook@chromium.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	linux-kernel@vger.kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Borislav Petkov <bp@alien8.de>, Joerg Roedel <joro@8bytes.org>,
	Ingo Molnar <mingo@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>
Subject: Re: [PATCH v2 11/11] KVM: x86: emulator/smm: preserve interrupt shadow in SMRAM
Date: Thu, 30 Jun 2022 09:00:40 +0300	[thread overview]
Message-ID: <42da1631c8cdd282e5d9cfd0698b6df7deed2daf.camel@redhat.com> (raw)
In-Reply-To: <CALMp9eSe5jtvmOPWLYCcrMmqyVBeBkg90RwtR4bwxay99NAF3g@mail.gmail.com>

On Wed, 2022-06-29 at 09:31 -0700, Jim Mattson wrote:
> On Tue, Jun 21, 2022 at 8:09 AM Maxim Levitsky <mlevitsk@redhat.com> wrote:
> > When #SMI is asserted, the CPU can be in interrupt shadow
> > due to sti or mov ss.
> > 
> > It is not mandatory in  Intel/AMD prm to have the #SMI
> > blocked during the shadow, and on top of
> > that, since neither SVM nor VMX has true support for SMI
> > window, waiting for one instruction would mean single stepping
> > the guest.
> > 
> > Instead, allow #SMI in this case, but both reset the interrupt
> > window and stash its value in SMRAM to restore it on exit
> > from SMM.
> > 
> > This fixes rare failures seen mostly on windows guests on VMX,
> > when #SMI falls on the sti instruction which mainfest in
> > VM entry failure due to EFLAGS.IF not being set, but STI interrupt
> > window still being set in the VMCS.
> 
> I think you're just making stuff up! See Note #5 at
> https://sandpile.org/x86/inter.htm.
> 
> Can you reference the vendors' documentation that supports this change?
> 

First of all, just to note that the actual issue here was that 
we don't clear the shadow bits in the guest interruptability field 
in the vmcb on SMM entry, that triggered a consistency check because
we do clear EFLAGS.IF.
Preserving the interrupt shadow is just nice to have.


That what Intel's spec says for the 'STI':

"The IF flag and the STI and CLI instructions do not prohibit the generation of exceptions and nonmaskable inter-
rupts (NMIs). However, NMIs (and system-management interrupts) may be inhibited on the instruction boundary
following an execution of STI that begins with IF = 0."

Thus it is likely that #SMI are just blocked when in shadow, but it is easier to implement
it this way (avoids single stepping the guest) and without any user visable difference,
which I noted in the patch description, I noted that there are two ways to solve this,
and preserving the int shadow in SMRAM is just more simple way.


As for CPUS that neither block SMI nor preserve the int shadaw, in theory they can, but that would
break things, as noted in this mail

https://lore.kernel.org/lkml/1284913699-14986-1-git-send-email-avi@redhat.com/

It is possible though that real cpu supports HLT restart flag, which makes this a non issue,
still. I can't rule out that a real cpu doesn't preserve the interrupt shadow on SMI, but
I don't see why we can't do this to make things more robust.

Best regards,
	Maxim Levitsky


  reply	other threads:[~2022-06-30  6:00 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-21 15:08 [PATCH v2 00/11] SMM emulation and interrupt shadow fixes Maxim Levitsky
2022-06-21 15:08 ` [PATCH v2 01/11] KVM: x86: emulator: em_sysexit should update ctxt->mode Maxim Levitsky
2022-06-21 15:08 ` [PATCH v2 02/11] KVM: x86: emulator: introduce update_emulation_mode Maxim Levitsky
2022-07-20 23:44   ` Sean Christopherson
2022-07-21 11:52     ` Maxim Levitsky
2022-07-21 14:23       ` Sean Christopherson
2022-06-21 15:08 ` [PATCH v2 03/11] KVM: x86: emulator: remove assign_eip_near/far Maxim Levitsky
2022-07-20 23:51   ` Sean Christopherson
2022-07-21 11:52     ` Maxim Levitsky
2022-06-21 15:08 ` [PATCH v2 04/11] KVM: x86: emulator: update the emulation mode after rsm Maxim Levitsky
2022-06-21 15:08 ` [PATCH v2 05/11] KVM: x86: emulator: update the emulation mode after CR0 write Maxim Levitsky
2022-07-20 23:50   ` Sean Christopherson
2022-07-21 11:53     ` Maxim Levitsky
2022-07-21 14:11       ` Sean Christopherson
2022-07-21 14:57         ` Maxim Levitsky
2022-06-21 15:08 ` [PATCH v2 06/11] KVM: x86: emulator/smm: number of GPRs in the SMRAM image depends on the image format Maxim Levitsky
2022-07-21  0:06   ` Sean Christopherson
2022-07-21  0:09     ` Sean Christopherson
2022-06-21 15:08 ` [PATCH v2 07/11] KVM: x86: emulator/smm: add structs for KVM's smram layout Maxim Levitsky
2022-07-21  0:40   ` Sean Christopherson
2022-07-21 11:54     ` Maxim Levitsky
2022-06-21 15:08 ` [PATCH v2 08/11] KVM: x86: emulator/smm: use smram struct for 32 bit smram load/restore Maxim Levitsky
2022-06-21 15:09 ` [PATCH v2 09/11] KVM: x86: emulator/smm: use smram struct for 64 " Maxim Levitsky
2022-07-21  0:38   ` Sean Christopherson
2022-07-21 11:54     ` Maxim Levitsky
2022-06-21 15:09 ` [PATCH v2 10/11] KVM: x86: SVM: use smram structs Maxim Levitsky
2022-07-21  0:18   ` Sean Christopherson
2022-07-21 11:54     ` Maxim Levitsky
2022-07-21  0:39   ` Sean Christopherson
2022-07-21 11:54     ` Maxim Levitsky
2022-06-21 15:09 ` [PATCH v2 11/11] KVM: x86: emulator/smm: preserve interrupt shadow in SMRAM Maxim Levitsky
2022-06-29 16:31   ` Jim Mattson
2022-06-30  6:00     ` Maxim Levitsky [this message]
2022-06-30 16:00       ` Jim Mattson
2022-07-05 13:38         ` Maxim Levitsky
2022-07-05 13:40           ` Maxim Levitsky
2022-07-05 13:51             ` Maxim Levitsky
2022-07-06 18:13           ` Jim Mattson
2022-07-06 20:00             ` Maxim Levitsky
2022-07-06 20:38               ` Jim Mattson
2022-07-10 16:05                 ` Maxim Levitsky
2022-06-29  7:21 ` [PATCH v2 00/11] SMM emulation and interrupt shadow fixes Maxim Levitsky
2022-07-14 11:06 ` Maxim Levitsky
2022-07-20  8:47   ` Maxim Levitsky

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=42da1631c8cdd282e5d9cfd0698b6df7deed2daf.camel@redhat.com \
    --to=mlevitsk@redhat.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=keescook@chromium.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=x86@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®