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
next prev parent reply other threads:[~2015-09-18 7:14 UTC|newest]
Thread overview: 4+ 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-17 21:11 ` [PATCH 2/2] x86/msr: Set the return value to zero when native_rdmsr_safe fails 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).