linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: X86 ML <x86@kernel.org>
Cc: 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" <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andy Lutomirski <luto@kernel.org>
Subject: [PATCH v3 0/5] Improve non-"safe" MSR access failure handling
Date: Fri, 11 Mar 2016 11:06:45 -0800	[thread overview]
Message-ID: <cover.1457723023.git.luto@kernel.org> (raw)

Setting CONFIG_PARAVIRT=y has an unintended side effect: it silently
turns all rdmsr and wrmsr operations into the safe variants without
any checks that the operations actually succeed.

With CONFIG_PARAVIRT=n, unchecked MSR failures OOPS and probably
cause boot to fail if they happen before init starts.

Neither behavior is very good, and it's particularly unfortunate that
the behavior changes depending on CONFIG_PARAVIRT.

In particular, KVM guests might be unwittingly depending on the
PARAVIRT=y behavior because CONFIG_KVM_GUEST currently depends on
CONFIG_PARAVIRT, and, because accesses in that case are completely
unchecked, we wouldn't even see a warning.

This series changes the native behavior, regardless of
CONFIG_PARAVIRT.  A non-"safe" MSR failure will give an informative
warning and will be fixed up -- native_read_msr will return zero,
and both reads and writes will continue where they left off.

If panic_on_oops is set, they will still OOPS and panic.

By using the shiny new custom exception handler infrastructure,
there should be no overhead on the success paths.

I didn't change the behavior on Xen, but, with this series applied,
it would be straightforward for the Xen maintainers to make the
corresponding change -- knowledge of whether the access is "safe" is
now propagated into the pvops.

Doing this is probably a prerequisite to sanely decoupling
CONFIG_KVM_GUEST and CONFIG_PARAVIRT, which would probably make
Arjan and the rest of the Clear Containers people happy :)

There's also room to reduce the code size of the "safe" variants
using custom exception handlers in the future.

Andy Lutomirski (5):
  x86/paravirt: Add _safe to the read_msr and write_msr PV hooks
  x86/msr: Carry on after a non-"safe" MSR access fails without
    !panic_on_oops
  x86/paravirt: Add paravirt_{read,write}_msr
  x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y
  x86/msr: Set the return value to zero when native_rdmsr_safe fails

 arch/x86/include/asm/msr.h            | 20 ++++++++++++----
 arch/x86/include/asm/paravirt.h       | 45 +++++++++++++++++++++--------------
 arch/x86/include/asm/paravirt_types.h | 14 +++++++----
 arch/x86/kernel/paravirt.c            |  6 +++--
 arch/x86/mm/extable.c                 | 33 +++++++++++++++++++++++++
 arch/x86/xen/enlighten.c              | 27 +++++++++++++++++++--
 6 files changed, 114 insertions(+), 31 deletions(-)

-- 
2.5.0

             reply	other threads:[~2016-03-11 19:07 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-11 19:06 Andy Lutomirski [this message]
2016-03-11 19:06 ` [PATCH v3 1/5] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks Andy Lutomirski
2016-03-11 19:06 ` [PATCH v3 2/5] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops Andy Lutomirski
2016-03-12 15:31   ` Ingo Molnar
2016-03-12 15:36   ` Ingo Molnar
2016-03-12 17:32     ` Andy Lutomirski
2016-03-11 19:06 ` [PATCH v3 3/5] x86/paravirt: Add paravirt_{read,write}_msr Andy Lutomirski
2016-03-14 14:02   ` Paolo Bonzini
2016-03-14 16:53     ` Andy Lutomirski
     [not found]       ` <CA+55aFzdUHUTomsMU7YAYgYkUQvNXHAiNX765wdSFqrKyoLKpQ@mail.gmail.com>
2016-03-14 17:02         ` Andy Lutomirski
2016-03-15  8:49           ` Paolo Bonzini
2016-03-15  8:56       ` Paolo Bonzini
2016-03-11 19:06 ` [PATCH v3 4/5] x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y Andy Lutomirski
2016-03-11 19:06 ` [PATCH v3 5/5] 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=cover.1457723023.git.luto@kernel.org \
    --to=luto@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=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --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).