All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Andy Lutomirski <luto@kernel.org>
Cc: x86@kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	KVM list <kvm@vger.kernel.org>,
	Arjan van de Ven <arjan@linux.intel.com>,
	xen-devel <Xen-devel@lists.xen.org>,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
Date: Fri, 18 Sep 2015 09:14:04 +0200	[thread overview]
Message-ID: <20150918071403.GA1172@gmail.com> (raw)
In-Reply-To: <203bb8a52efae1781281fb70ccd45c3e164fbce2.1442523997.git.luto@kernel.org>


* Andy Lutomirski <luto@kernel.org> wrote:

> This demotes an OOPS and likely panic due to a failed non-"safe" MSR
> access to a WARN_ON_ONCE and a return of poisoned values (in the
> RDMSR case).  We still write a pr_info entry unconditionally for
> debugging.
> 
> To be clear, this type of failure should *not* happen.  This patch
> exists to minimize the chance of nasty undebuggable failures due on
> systems that used to work due to a now-fixed CONFIG_PARAVIRT=y bug.

> +	if (opcode == 0x320f) {
> +		/* RDMSR */
> +		pr_info("bad kernel RDMSR from non-existent MSR 0x%x",
> +			(unsigned int)regs->cx);
> +		if (!panic_on_oops) {
> +			WARN_ON_ONCE(true);
> +
> +			/* Patch it up with deterministic poison. */
> +			regs->ax = 0x5aadc0de;
> +			regs->dx = 0x8badf00d;
> +			regs->ip += 2;
> +			return true;

IMHO this should really not poison the result, but use zero as the result.

The poison might randomly indicate 'present' feature in various registers that 
might be accessed in a buggy way. Don't send the code further down into la-la-land 
by giving it a 'success'.

And yes, zero can mean success too, but we have to pick a side here ...

The warning will be enough to fix these ups, people (and in particular distro 
testing people) will be watching out for them.

Thanks,

	Ingo

  reply	other threads:[~2015-09-18  7:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-17 21:11 [PATCH 0/2] x86/msr: MSR access failure changes Andy Lutomirski
2015-09-17 21:11 ` [PATCH 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops Andy Lutomirski
2015-09-18  7:14   ` Ingo Molnar [this message]
2015-09-18  7:14   ` Ingo Molnar
2015-09-17 21:11 ` Andy Lutomirski
2015-09-17 21:11 ` [PATCH 2/2] x86/msr: Set the return value to zero when native_rdmsr_safe fails Andy Lutomirski
2015-09-17 21:11 ` Andy Lutomirski

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=20150918071403.GA1172@gmail.com \
    --to=mingo@kernel.org \
    --cc=Xen-devel@lists.xen.org \
    --cc=akpm@linux-foundation.org \
    --cc=arjan@linux.intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --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 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.