LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: "David P. Reed" <dpreed@deepplum.com>
Cc: Andy Lutomirski <luto@kernel.org>,
	Sean Christopherson <sean.j.christopherson@intel.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 2/3] Fix undefined operation fault that can hang a cpu on crash or panic
Date: Sun, 5 Jul 2020 13:55:42 -0700
Message-ID: <CALCETrVCEP6bLrwTYg7h_Rx-XHEsOXT4QZ=BnwpReM-n8eco6g@mail.gmail.com> (raw)
In-Reply-To: <1593978728.059424180@apps.rackspace.com>

On Sun, Jul 5, 2020 at 12:52 PM David P. Reed <dpreed@deepplum.com> wrote:
>
> Thanks, will handle these. 2 questions below.
>
> On Sunday, July 5, 2020 2:22pm, "Andy Lutomirski" <luto@kernel.org> said:
>
> > On Sat, Jul 4, 2020 at 1:38 PM David P. Reed <dpreed@deepplum.com> wrote:
> >>
> >> Fix: Mask undefined operation fault during emergency VMXOFF that must be
> >> attempted to force cpu exit from VMX root operation.
> >> Explanation: When a cpu may be in VMX root operation (only possible when
> >> CR4.VMXE is set), crash or panic reboot tries to exit VMX root operation
> >> using VMXOFF. This is necessary, because any INIT will be masked while cpu
> >> is in VMX root operation, but that state cannot be reliably
> >> discerned by the state of the cpu.
> >> VMXOFF faults if the cpu is not actually in VMX root operation, signalling
> >> undefined operation.
> >> Discovered while debugging an out-of-tree x-visor with a race. Can happen
> >> due to certain kinds of bugs in KVM.
> >
> > Can you re-wrap lines to 68 characters?  Also, the Fix: and
>
> I used 'scripts/checkpatch.pl' and it had me wrap to 75 chars:
> "WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)"
>
> Should I submit a fix to checkpatch.pl to say 68?

75 is probably fine too, but something is odd about your wrapping.
You have long lines mostly alternating with short lines.  It's as if
you wrote 120-ish character lines and then wrapped to 75 without
reflowing.

>
> > Explanation: is probably unnecessary.  You could say:
> >
> > Ignore a potential #UD failut during emergency VMXOFF ...
> >
> > When a cpu may be in VMX ...
> >
> >>
> >> Fixes: 208067 <https://bugzilla.kernel.org/show_bug.cgi?id=208067>
> >> Reported-by: David P. Reed <dpreed@deepplum.com>
> >
> > It's not really necessary to say that you, the author, reported the
> > problem, but I guess it's harmless.
> >
> >> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> >> Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
> >> Suggested-by: Andy Lutomirski <luto@kernel.org>
> >> Signed-off-by: David P. Reed <dpreed@deepplum.com>
> >> ---
> >>  arch/x86/include/asm/virtext.h | 20 ++++++++++++++------
> >>  1 file changed, 14 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
> >> index 0ede8d04535a..0e0900eacb9c 100644
> >> --- a/arch/x86/include/asm/virtext.h
> >> +++ b/arch/x86/include/asm/virtext.h
> >> @@ -30,11 +30,11 @@ static inline int cpu_has_vmx(void)
> >>  }
> >>
> >>
> >> -/* Disable VMX on the current CPU
> >> +/* Exit VMX root mode and isable VMX on the current CPU.
> >
> > s/isable/disable/
> >
> >
> >>  /* Disable VMX if it is supported and enabled on the current CPU
> >> --
> >> 2.26.2
> >>
> >
> > Other than that:
> >
> > Reviewed-by: Andy Lutomirski <luto@kernel.org>
>
> As a newbie, I have a process question - should I resend the patch with the 'Reviewed-by' line, as well as correcting the other wording? Thanks!

Probably.  Sometimes a maintainer will apply the patch and make these
types of cosmetic changes, but it's easier if you resubmit.  That
being said, for non-urgent patches, it's usually considered polite to
wait a day or two to give other people a chance to comment.

--Andy

  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 [this message]
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

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='CALCETrVCEP6bLrwTYg7h_Rx-XHEsOXT4QZ=BnwpReM-n8eco6g@mail.gmail.com' \
    --to=luto@kernel.org \
    --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=martin.molnar.programming@gmail.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rdunlap@infradead.org \
    --cc=sean.j.christopherson@intel.com \
    --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