lkml.org 
[lkml]   [2016]   [Mar]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [Xen-devel] [PATCH v4 0/5] [PATCH v3 0/5] Improve non-"safe" MSR access failure handling
From
Date
On 03/12/2016 01:08 PM, Andy Lutomirski wrote:
> 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 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 (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(-)
>

I don't see any issues as far as Xen is concerned but let me run this
through our nightly. I'll wait for the next version (which I think you
might have based on the comments). Please copy me.

-boris

\
 
 \ /
  Last update: 2016-03-14 16:21    [W:0.081 / U:0.804 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site