xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/9] Improve non-"safe" MSR access failure handling
@ 2016-04-02 14:01 Andy Lutomirski
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Lutomirski @ 2016-04-02 14:01 UTC (permalink / raw)
  To: X86 ML
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, linux-kernel,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

There are two parts here:

***** FIRST PART: EARLY EXCEPTIONS *****

The first few patches move some early panic code into C, add pt_regs
to early exception handling, and make fancy exception handlers work early.

***** SECOND PART: MSRs *****

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 once 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.

Changes from v4:
 - Fix a missing \n (Joe Perches)
 - No more panic_on_oops
 - Works early

Changes from v3:
 - WARN_ONCE instead of WARN (Ingo)
 - In the warning text, s/unsafe/unchecked/ (Ingo, sort of)

Changes from earlier versions: lots of changes!

Andy Lutomirski (9):
  x86/head: Pass a real pt_regs and trapnr to early_fixup_exception
  x86/head: Move the early NMI fixup into C
  x86/head: Move early exception panic code into early_fixup_exception
  x86/traps: Enable all exception handler callbacks early
  x86/paravirt: Add _safe to the read_msr and write_msr PV hooks
  x86/msr: Carry on after a non-"safe" MSR access fails
  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/include/asm/uaccess.h        |   2 +-
 arch/x86/kernel/head_32.S             | 116 ++++++++++++++--------------------
 arch/x86/kernel/head_64.S             |  95 ++++++++--------------------
 arch/x86/kernel/paravirt.c            |   6 +-
 arch/x86/mm/extable.c                 |  64 +++++++++++++++----
 arch/x86/xen/enlighten.c              |  27 +++++++-
 9 files changed, 208 insertions(+), 181 deletions(-)

-- 
2.5.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling
       [not found] <cover.1459605520.git.luto@kernel.org>
                   ` (2 preceding siblings ...)
  2016-04-04 16:23 ` Borislav Petkov
@ 2016-04-05 15:38 ` Boris Ostrovsky
  3 siblings, 0 replies; 6+ messages in thread
From: Boris Ostrovsky @ 2016-04-05 15:38 UTC (permalink / raw)
  To: Andy Lutomirski, X86 ML
  Cc: KVM list, Peter Zijlstra, Arjan van de Ven, linux-kernel,
	xen-devel, Borislav Petkov, Paolo Bonzini, Andrew Morton,
	Linus Torvalds

On 04/02/2016 10:01 AM, Andy Lutomirski wrote:
>
> Andy Lutomirski (9):
>    x86/head: Pass a real pt_regs and trapnr to early_fixup_exception
>    x86/head: Move the early NMI fixup into C
>    x86/head: Move early exception panic code into early_fixup_exception
>    x86/traps: Enable all exception handler callbacks early
>    x86/paravirt: Add _safe to the read_msr and write_msr PV hooks
>    x86/msr: Carry on after a non-"safe" MSR access fails
>    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

With Xen (on top of eb1af3b):

Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling
       [not found] <cover.1459605520.git.luto@kernel.org>
  2016-04-02 14:24 ` Linus Torvalds
       [not found] ` <CA+55aFwi2E0A9e7krCG9dkTq7vMZnxsBwtF6_hw6QO0EVObf=g@mail.gmail.com>
@ 2016-04-04 16:23 ` Borislav Petkov
  2016-04-05 15:38 ` Boris Ostrovsky
  3 siblings, 0 replies; 6+ messages in thread
From: Borislav Petkov @ 2016-04-04 16:23 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, Linus Torvalds, X86 ML, linux-kernel,
	xen-devel, Paolo Bonzini, Andrew Morton, Arjan van de Ven

On Sat, Apr 02, 2016 at 07:01:31AM -0700, Andy Lutomirski wrote:
> There are two parts here:
> 
> ***** FIRST PART: EARLY EXCEPTIONS *****
> 
> The first few patches move some early panic code into C, add pt_regs
> to early exception handling, and make fancy exception handlers work early.
> 
> ***** SECOND PART: MSRs *****
> 
> 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.


...

FWIW:

Reviewed-by: Borislav Petkov <bp@suse.de>

Definitely a step in the right direction, regardless of how we're going
to be doing early_printk(), which is a tangential topic. This pile could
be taken for a spin in tip.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling
       [not found]   ` <CALCETrWP33SR5TCA-baZi8xBrHpWe=wzkuvUM=y4x_MPt1XgYg@mail.gmail.com>
@ 2016-04-02 15:21     ` Linus Torvalds
  0 siblings, 0 replies; 6+ messages in thread
From: Linus Torvalds @ 2016-04-02 15:21 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, X86 ML, Linux Kernel Mailing List,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

On Sat, Apr 2, 2016 at 10:13 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> I also tried a bad wrmsrl at a couple early points.  Very very early
> it just works with not warning.  A little later and it prints the
> warning.

Ok, that sounds like the correct behavior - I'm sure the very very
early ones "warned" too, it just got dropped on the floor due to being
before the printing/logging was up and running.

And even then, it's obviously much better than having machines
mysteriously just reboot.

Thanks,
         Linus

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling
       [not found] ` <CA+55aFwi2E0A9e7krCG9dkTq7vMZnxsBwtF6_hw6QO0EVObf=g@mail.gmail.com>
@ 2016-04-02 15:13   ` Andy Lutomirski
       [not found]   ` <CALCETrWP33SR5TCA-baZi8xBrHpWe=wzkuvUM=y4x_MPt1XgYg@mail.gmail.com>
  1 sibling, 0 replies; 6+ messages in thread
From: Andy Lutomirski @ 2016-04-02 15:13 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: KVM list, Peter Zijlstra, X86 ML, Linux Kernel Mailing List,
	xen-devel, Borislav Petkov, Andy Lutomirski, Paolo Bonzini,
	Andrew Morton, Arjan van de Ven

On Sat, Apr 2, 2016 at 7:24 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> This patch series looks much nicer than the last one. I assume you
> tested that the early-trap handling actually worked too? I only looked
> at the patches..
>
> Ack to it all,

I injected some BUGs in various places on 32-bit an 64-bit and it
seemed to do the right thing.  Depending on how early they were, I
either got a clean hang or I got a printout, and whether it displayed
anything didn't seem to change with and without the patches.  I think
that early_printk doesn't work from the very very beginning.

I also tried a bad wrmsrl at a couple early points.  Very very early
it just works with not warning.  A little later and it prints the
warning.

--Andy

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling
       [not found] <cover.1459605520.git.luto@kernel.org>
@ 2016-04-02 14:24 ` Linus Torvalds
       [not found] ` <CA+55aFwi2E0A9e7krCG9dkTq7vMZnxsBwtF6_hw6QO0EVObf=g@mail.gmail.com>
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Linus Torvalds @ 2016-04-02 14:24 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: KVM list, Peter Zijlstra, X86 ML, Linux Kernel Mailing List,
	xen-devel, Borislav Petkov, Paolo Bonzini, Andrew Morton,
	Arjan van de Ven

This patch series looks much nicer than the last one. I assume you
tested that the early-trap handling actually worked too? I only looked
at the patches..

Ack to it all,

           Linus

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-04-05 15:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-02 14:01 [PATCH v5 0/9] Improve non-"safe" MSR access failure handling Andy Lutomirski
     [not found] <cover.1459605520.git.luto@kernel.org>
2016-04-02 14:24 ` Linus Torvalds
     [not found] ` <CA+55aFwi2E0A9e7krCG9dkTq7vMZnxsBwtF6_hw6QO0EVObf=g@mail.gmail.com>
2016-04-02 15:13   ` Andy Lutomirski
     [not found]   ` <CALCETrWP33SR5TCA-baZi8xBrHpWe=wzkuvUM=y4x_MPt1XgYg@mail.gmail.com>
2016-04-02 15:21     ` Linus Torvalds
2016-04-04 16:23 ` Borislav Petkov
2016-04-05 15:38 ` Boris Ostrovsky

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).