LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Andy Lutomirski <luto@kernel.org>
Cc: "David P. Reed" <dpreed@deepplum.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	X86 ML <x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
	Allison Randal <allison@lohutok.net>,
	Enrico Weigelt <info@metux.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Kate Stewart <kstewart@linuxfoundation.org>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	Martin Molnar <martin.molnar.programming@gmail.com>,
	Alexandre Chartre <alexandre.chartre@oracle.com>,
	Jann Horn <jannh@google.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 3/3] Force all cpus to exit VMX root operation on crash/panic reliably
Date: Mon, 6 Jul 2020 22:29:26 -0700
Message-ID: <20200707052926.GG5208@linux.intel.com> (raw)
In-Reply-To: <CALCETrWnef+Q=Pyrf1h5tcPSrp7tW6eSVozjfONC+OsqbGcj-Q@mail.gmail.com>

On Sun, Jul 05, 2020 at 01:53:39PM -0700, Andy Lutomirski wrote:
> On Sun, Jul 5, 2020 at 1:00 PM David P. Reed <dpreed@deepplum.com> wrote:
> >
> > On Sunday, July 5, 2020 2:26pm, "Andy Lutomirski" <luto@kernel.org> said:
> > > As a minor caveat, doing cr4_clear_bits() in NMI context is not really
> > > okay, but we're about to reboot, so nothing too awful should happen.
> > > And this has very little to do with your patch.
> >
> > I had wondered why the bit is cleared, too. (I assumed it was OK or
> > desirable, because it was being cleared in NMI context before). Happy to
> > submit a separate patch to eliminate that issue as well, since the point of
> > emergency vmxoff is only to get out of VMX root mode - CR4.VMXE's state is
> > irrelevant. Of course, clearing it prevents any future emergency vmxoff
> > attempts. (there seemed to be some confusion about "enabling" VMX vs. "in
> > VMX operation" in the comments)  Should I?
> 
> I have a vague recollection of some firmwares that got upset if rebooted with
> CR4.VMXE set.  Sean?

Hmm, rebooting with CR4.VMXE=1 shouldn't be a problem.  VMXON does all the
special stuff that causes problems with reboot, e.g. blocks INIT, prevents
disabling paging, etc...

That being said, I think it makes sense to keep the clearing of CR4.VMXE out
of paranoia as BIOS will be BIOS, and there is no real downside.  Not only
is the system about to reboot, but the CPUs that call cr4_clear_bits() from
NMI context are also being put into an infinite loop by crash_nmi_callback(),
i.e. they never leave NMI context.  And we rely on that behavior, otherwise
KVM could set CR4.VMXE and do VMXON after the NMI and the whole thing would
be for naught.
 
> The real issue here is that the percpu CR4 machinery uses IRQ-offness as a
> lock, and NMI breaks this.

      reply index

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-25 14:45 [PATCH v2] Fix undefined operation VMXOFF during reboot and crash David P. Reed
2020-06-25 14:59 ` David P. Reed
2020-06-29 20:54   ` David P. Reed
2020-06-29 21:22     ` Andy Lutomirski
2020-06-29 21:49       ` Sean Christopherson
2020-06-29 22:46         ` David P. Reed
2020-07-04 20:38         ` [PATCH v3 0/3] " David P. Reed
2020-07-04 20:38           ` [PATCH v3 1/3] Correct asm VMXOFF side effects David P. Reed
2020-07-05  5:46             ` Randy Dunlap
2020-07-04 20:38           ` [PATCH v3 2/3] Fix undefined operation fault that can hang a cpu on crash or panic David P. Reed
2020-07-05 18:22             ` Andy Lutomirski
2020-07-05 19:52               ` David P. Reed
2020-07-05 20:55                 ` Andy Lutomirski
2020-07-05 22:07                   ` David P. Reed
2020-07-07  5:09             ` Sean Christopherson
2020-07-07 19:09               ` David P. Reed
2020-07-07 19:24                 ` Sean Christopherson
2020-07-07 19:52                   ` David P. Reed
2020-07-04 20:38           ` [PATCH v3 3/3] Force all cpus to exit VMX root operation on crash/panic reliably David P. Reed
2020-07-05 18:26             ` Andy Lutomirski
2020-07-05 20:00               ` David P. Reed
2020-07-05 20:53                 ` Andy Lutomirski
2020-07-07  5:29                   ` Sean Christopherson [this message]

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=20200707052926.GG5208@linux.intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=alexandre.chartre@oracle.com \
    --cc=allison@lohutok.net \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=dpreed@deepplum.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=info@metux.net \
    --cc=jannh@google.com \
    --cc=kstewart@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=martin.molnar.programming@gmail.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rdunlap@infradead.org \
    --cc=tglx@linutronix.de \
    --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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git