* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <CA+55aFzKCi6pf0RP8HjDQYDsms6reB5AihuCAHEkVJtoOHk_Yw@mail.gmail.com>
@ 2015-09-21 16:49 ` Arjan van de Ven
[not found] ` <56003506.1050100@linux.intel.com>
` (6 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Arjan van de Ven @ 2015-09-21 16:49 UTC (permalink / raw)
To: Linus Torvalds, Ingo Molnar
Cc: KVM list, Peter Zijlstra, the arch/x86 maintainers,
Linux Kernel Mailing List, xen-devel, Paolo Bonzini,
Thomas Gleixner, Andy Lutomirski, Andrew Morton
On 9/21/2015 9:36 AM, Linus Torvalds wrote:
> On Mon, Sep 21, 2015 at 1:46 AM, Ingo Molnar <mingo@kernel.org> wrote:
>>
>> Linus, what's your preference?
>
> So quite frankly, is there any reason we don't just implement
> native_read_msr() as just
>
> unsigned long long native_read_msr(unsigned int msr)
> {
> int err;
> unsigned long long val;
>
> val = native_read_msr_safe(msr, &err);
> WARN_ON_ONCE(err);
> return val;
> }
>
> Note: no inline, no nothing. Just put it in arch/x86/lib/msr.c, and be
> done with it. I don't see the downside.
>
> How many msr reads are <i>so</i> critical that the function call
> overhead would matter?
if anything qualifies it'd be switch_to() and friends.
note that I'm not entirely happy about the notion of "safe" MSRs.
They're safe as in "won't fault".
Reading random MSRs isn't a generic safe operation though, but the name sort of gives people
the impression that it is. Even with _safe variants, you still need to KNOW the MSR exists (by means
of CPUID or similar) unfortunately.
^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <56003506.1050100@linux.intel.com>]
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <56003506.1050100@linux.intel.com>
@ 2015-09-21 17:27 ` Linus Torvalds
2015-09-21 17:43 ` Andy Lutomirski
[not found] ` <CALCETrUSEm_QLxGaBdmy3tQn=WnagvzTaYpy+i14sGUeaOUvTQ@mail.gmail.com>
2 siblings, 0 replies; 22+ messages in thread
From: Linus Torvalds @ 2015-09-21 17:27 UTC (permalink / raw)
To: Arjan van de Ven
Cc: Andrew Morton, KVM list, Peter Zijlstra,
the arch/x86 maintainers, Linux Kernel Mailing List, xen-devel,
Paolo Bonzini, Thomas Gleixner, Andy Lutomirski, Ingo Molnar
On Mon, Sep 21, 2015 at 9:49 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
>>
>> How many msr reads are <i>so</i> critical that the function call
>> overhead would matter?
>
> if anything qualifies it'd be switch_to() and friends.
Is there anything else than the FS/GS_BASE thing (possibly hidden
behind inlines etc that I didn't get from a quick grep)? And why is
that sometimes using the "safe" version (in do_arch_prctl()), and
sometimes not (switch_to())?
I'm not convinced that mess is a good argument for the status quo ;)
> note that I'm not entirely happy about the notion of "safe" MSRs.
> They're safe as in "won't fault".
I wouldn't object to renaming them.
Linus
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <56003506.1050100@linux.intel.com>
2015-09-21 17:27 ` Linus Torvalds
@ 2015-09-21 17:43 ` Andy Lutomirski
[not found] ` <CALCETrUSEm_QLxGaBdmy3tQn=WnagvzTaYpy+i14sGUeaOUvTQ@mail.gmail.com>
2 siblings, 0 replies; 22+ messages in thread
From: Andy Lutomirski @ 2015-09-21 17:43 UTC (permalink / raw)
To: Arjan van de Ven
Cc: Andrew Morton, KVM list, Peter Zijlstra,
the arch/x86 maintainers, Linux Kernel Mailing List, xen-devel,
Paolo Bonzini, Thomas Gleixner, Linus Torvalds, Ingo Molnar
On Mon, Sep 21, 2015 at 9:49 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
> On 9/21/2015 9:36 AM, Linus Torvalds wrote:
>>
>> On Mon, Sep 21, 2015 at 1:46 AM, Ingo Molnar <mingo@kernel.org> wrote:
>>>
>>>
>>> Linus, what's your preference?
>>
>>
>> So quite frankly, is there any reason we don't just implement
>> native_read_msr() as just
>>
>> unsigned long long native_read_msr(unsigned int msr)
>> {
>> int err;
>> unsigned long long val;
>>
>> val = native_read_msr_safe(msr, &err);
>> WARN_ON_ONCE(err);
>> return val;
>> }
>>
>> Note: no inline, no nothing. Just put it in arch/x86/lib/msr.c, and be
>> done with it. I don't see the downside.
>>
>> How many msr reads are <i>so</i> critical that the function call
>> overhead would matter?
>
>
> if anything qualifies it'd be switch_to() and friends.
And maybe the KVM user return notifier. Unfortunately, switch_to
might gain another two MSR accesses at some point if we decide to fix
the bugs in there. Sigh.
>
> note that I'm not entirely happy about the notion of "safe" MSRs.
> They're safe as in "won't fault".
> Reading random MSRs isn't a generic safe operation though, but the name sort
> of gives people
> the impression that it is. Even with _safe variants, you still need to KNOW
> the MSR exists (by means
> of CPUID or similar) unfortunately.
>
I tend to agree.
Anyway, the fully out-of-line approach isn't obviously a bad idea, and
it simplifies the whole mess (we can drop most of the paravirt
patches, too). I'll give it a try and see what happens.
--Andy
^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CALCETrUSEm_QLxGaBdmy3tQn=WnagvzTaYpy+i14sGUeaOUvTQ@mail.gmail.com>]
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <CALCETrUSEm_QLxGaBdmy3tQn=WnagvzTaYpy+i14sGUeaOUvTQ@mail.gmail.com>
@ 2015-09-22 8:12 ` Paolo Bonzini
0 siblings, 0 replies; 22+ messages in thread
From: Paolo Bonzini @ 2015-09-22 8:12 UTC (permalink / raw)
To: Andy Lutomirski, Arjan van de Ven
Cc: Andrew Morton, KVM list, Peter Zijlstra,
the arch/x86 maintainers, Linux Kernel Mailing List, xen-devel,
Thomas Gleixner, Linus Torvalds, Ingo Molnar
On 21/09/2015 19:43, Andy Lutomirski wrote:
> And maybe the KVM user return notifier.
No, not really. If anything, the place in KVM where it makes a
difference is vmx_save_host_state, which is also only using
always-present MSRs. But don't care about KVM.
First clean it up, then we can add back inline versions like __rdmsr or
rdmsr_fault or rdmsr_unsafe or whatever.
Paolo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <CA+55aFzKCi6pf0RP8HjDQYDsms6reB5AihuCAHEkVJtoOHk_Yw@mail.gmail.com>
2015-09-21 16:49 ` Arjan van de Ven
[not found] ` <56003506.1050100@linux.intel.com>
@ 2015-09-21 18:16 ` Andy Lutomirski
[not found] ` <CALCETrUBcDxvtjKC-m97GPwCOHvcoDdGjnMJLukSue8ha55mng@mail.gmail.com>
` (4 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Andy Lutomirski @ 2015-09-21 18:16 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, KVM list, Peter Zijlstra,
the arch/x86 maintainers, Linux Kernel Mailing List, xen-devel,
Borislav Petkov, Paolo Bonzini, Thomas Gleixner,
Arjan van de Ven, Ingo Molnar
On Mon, Sep 21, 2015 at 9:36 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Mon, Sep 21, 2015 at 1:46 AM, Ingo Molnar <mingo@kernel.org> wrote:
>>
>> Linus, what's your preference?
>
> So quite frankly, is there any reason we don't just implement
> native_read_msr() as just
>
> unsigned long long native_read_msr(unsigned int msr)
> {
> int err;
> unsigned long long val;
>
> val = native_read_msr_safe(msr, &err);
> WARN_ON_ONCE(err);
> return val;
> }
>
> Note: no inline, no nothing. Just put it in arch/x86/lib/msr.c, and be
> done with it. I don't see the downside.
In the interest of sanity, I want to drop the "native_", too, since
there appear to be few or no good use cases for native_read_msr as
such. I'm tempted to add new functions read_msr and write_msr that
forward to rdmsrl_safe and wrmsrl_safe.
It looks like the msr helpers are every bit as bad as the TSC helpers
used to be :(
--Andy
^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CALCETrUBcDxvtjKC-m97GPwCOHvcoDdGjnMJLukSue8ha55mng@mail.gmail.com>]
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <CALCETrUBcDxvtjKC-m97GPwCOHvcoDdGjnMJLukSue8ha55mng@mail.gmail.com>
@ 2015-09-21 18:36 ` Borislav Petkov
2015-09-21 18:47 ` Linus Torvalds
1 sibling, 0 replies; 22+ messages in thread
From: Borislav Petkov @ 2015-09-21 18:36 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andrew Morton, KVM list, Peter Zijlstra, Linus Torvalds,
the arch/x86 maintainers, Linux Kernel Mailing List, xen-devel,
Paolo Bonzini, Thomas Gleixner, Arjan van de Ven, Ingo Molnar
On Mon, Sep 21, 2015 at 11:16:30AM -0700, Andy Lutomirski wrote:
> In the interest of sanity, I want to drop the "native_", too, since
> there appear to be few or no good use cases for native_read_msr as
> such. I'm tempted to add new functions read_msr and write_msr that
> forward to rdmsrl_safe and wrmsrl_safe.
Just change the msr_read/msr_write() ones in arch/x86/lib/msr.c to take
a u64 and you're there.
> It looks like the msr helpers are every bit as bad as the TSC helpers
> used to be :(
Yap.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <CALCETrUBcDxvtjKC-m97GPwCOHvcoDdGjnMJLukSue8ha55mng@mail.gmail.com>
2015-09-21 18:36 ` Borislav Petkov
@ 2015-09-21 18:47 ` Linus Torvalds
1 sibling, 0 replies; 22+ messages in thread
From: Linus Torvalds @ 2015-09-21 18:47 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andrew Morton, KVM list, Peter Zijlstra,
the arch/x86 maintainers, Linux Kernel Mailing List, xen-devel,
Borislav Petkov, Paolo Bonzini, Thomas Gleixner,
Arjan van de Ven, Ingo Molnar
On Mon, Sep 21, 2015 at 11:16 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> In the interest of sanity, I want to drop the "native_", too
Yes. I think the only reason it exists is to have that wrapper layer
for PV. And that argument just goes away if you just make the
non-inline helper function do all the PV logic directly.
I really suspect we should do this for a *lot* of the PV ops. Yeah,
some are so performance-critical that we probably do have a good
reason for the inline indirections etc (historical example: native
spin-unlock, which traditionally could be done as a single store
instruction), but I suspect a lot of the PV indirection is for this
kind of "historical wrapper model" reason, and it often makes it
really hard to see what is going on because you have to go through
several layers of indirection, often in different files.
Linus
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <CA+55aFzKCi6pf0RP8HjDQYDsms6reB5AihuCAHEkVJtoOHk_Yw@mail.gmail.com>
` (3 preceding siblings ...)
[not found] ` <CALCETrUBcDxvtjKC-m97GPwCOHvcoDdGjnMJLukSue8ha55mng@mail.gmail.com>
@ 2015-09-22 7:14 ` Ingo Molnar
2015-09-30 13:10 ` Peter Zijlstra
` (2 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Ingo Molnar @ 2015-09-22 7:14 UTC (permalink / raw)
To: Linus Torvalds
Cc: KVM list, Peter Zijlstra, the arch/x86 maintainers,
Linux Kernel Mailing List, xen-devel, Paolo Bonzini,
Thomas Gleixner, Andy Lutomirski, Arjan van de Ven,
Andrew Morton
* Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Mon, Sep 21, 2015 at 1:46 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > Linus, what's your preference?
>
> So quite frankly, is there any reason we don't just implement
> native_read_msr() as just
>
> unsigned long long native_read_msr(unsigned int msr)
> {
> int err;
> unsigned long long val;
>
> val = native_read_msr_safe(msr, &err);
> WARN_ON_ONCE(err);
> return val;
> }
>
> Note: no inline, no nothing. Just put it in arch/x86/lib/msr.c, and be
> done with it. I don't see the downside.
Absolutely!
> How many msr reads are <i>so</i> critical that the function call overhead would
> matter? Get rid of the inline version of the _safe() thing too, and put that
> thing there too.
Only a very low number of them is performance critical (because even
hw-accelerated MSR accesses are generally slow so we try to avoid MSR accesses in
fast paths as much as possible, via shadowing, etc.) - and in the few cases where
we have to access an MSR in a fast path we can do those separately.
I'm only worried about the 'default' APIs, i.e. rdmsr() that is used throughout
arch/x86/ over a hundred times, not about performance critical code paths that get
enough testing and enough attention in general.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <CA+55aFzKCi6pf0RP8HjDQYDsms6reB5AihuCAHEkVJtoOHk_Yw@mail.gmail.com>
` (4 preceding siblings ...)
2015-09-22 7:14 ` Ingo Molnar
@ 2015-09-30 13:10 ` Peter Zijlstra
[not found] ` <20150930131002.GK2881@worktop.programming.kicks-ass.net>
2015-09-30 18:32 ` H. Peter Anvin
7 siblings, 0 replies; 22+ messages in thread
From: Peter Zijlstra @ 2015-09-30 13:10 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, KVM list, the arch/x86 maintainers,
Linux Kernel Mailing List, xen-devel, Paolo Bonzini,
Thomas Gleixner, Andy Lutomirski, Arjan van de Ven, Ingo Molnar
On Mon, Sep 21, 2015 at 09:36:15AM -0700, Linus Torvalds wrote:
> On Mon, Sep 21, 2015 at 1:46 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > Linus, what's your preference?
>
> So quite frankly, is there any reason we don't just implement
> native_read_msr() as just
>
> unsigned long long native_read_msr(unsigned int msr)
> {
> int err;
> unsigned long long val;
>
> val = native_read_msr_safe(msr, &err);
> WARN_ON_ONCE(err);
> return val;
> }
>
> Note: no inline, no nothing. Just put it in arch/x86/lib/msr.c, and be
> done with it. I don't see the downside.
>
> How many msr reads are <i>so</i> critical that the function call
> overhead would matter? Get rid of the inline version of the _safe()
> thing too, and put that thing there too.
There are a few in the perf code, and esp. on cores without a stack
engine the call overhead is noticeable. Also note that the perf MSRs are
generally optimized MSRs and less slow (we cannot say fast, they're
still MSRs) than regular MSRs.
^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20150930131002.GK2881@worktop.programming.kicks-ass.net>]
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <20150930131002.GK2881@worktop.programming.kicks-ass.net>
@ 2015-09-30 14:01 ` Ingo Molnar
[not found] ` <20150930140122.GB3285@gmail.com>
1 sibling, 0 replies; 22+ messages in thread
From: Ingo Molnar @ 2015-09-30 14:01 UTC (permalink / raw)
To: Peter Zijlstra
Cc: KVM list, Arjan van de Ven, the arch/x86 maintainers,
Linux Kernel Mailing List, xen-devel, Paolo Bonzini,
Thomas Gleixner, Andy Lutomirski, Linus Torvalds, Andrew Morton
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Sep 21, 2015 at 09:36:15AM -0700, Linus Torvalds wrote:
> > On Mon, Sep 21, 2015 at 1:46 AM, Ingo Molnar <mingo@kernel.org> wrote:
> > >
> > > Linus, what's your preference?
> >
> > So quite frankly, is there any reason we don't just implement
> > native_read_msr() as just
> >
> > unsigned long long native_read_msr(unsigned int msr)
> > {
> > int err;
> > unsigned long long val;
> >
> > val = native_read_msr_safe(msr, &err);
> > WARN_ON_ONCE(err);
> > return val;
> > }
> >
> > Note: no inline, no nothing. Just put it in arch/x86/lib/msr.c, and be
> > done with it. I don't see the downside.
> >
> > How many msr reads are <i>so</i> critical that the function call
> > overhead would matter? Get rid of the inline version of the _safe()
> > thing too, and put that thing there too.
>
> There are a few in the perf code, and esp. on cores without a stack engine the
> call overhead is noticeable. Also note that the perf MSRs are generally
> optimized MSRs and less slow (we cannot say fast, they're still MSRs) than
> regular MSRs.
These could still be open coded in an inlined fashion, like the scheduler usage.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20150930140122.GB3285@gmail.com>]
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <20150930140122.GB3285@gmail.com>
@ 2015-09-30 18:04 ` Andy Lutomirski
[not found] ` <CALCETrVMV2_3ywQ_t+0rsqzxMm6D9PvDEmOdrie67rRzfj-W_Q@mail.gmail.com>
1 sibling, 0 replies; 22+ messages in thread
From: Andy Lutomirski @ 2015-09-30 18:04 UTC (permalink / raw)
To: Ingo Molnar
Cc: KVM list, Peter Zijlstra, Linus Torvalds,
the arch/x86 maintainers, Linux Kernel Mailing List, xen-devel,
Paolo Bonzini, Thomas Gleixner, Arjan van de Ven, Andrew Morton
On Wed, Sep 30, 2015 at 7:01 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Peter Zijlstra <peterz@infradead.org> wrote:
>
>> On Mon, Sep 21, 2015 at 09:36:15AM -0700, Linus Torvalds wrote:
>> > On Mon, Sep 21, 2015 at 1:46 AM, Ingo Molnar <mingo@kernel.org> wrote:
>> > >
>> > > Linus, what's your preference?
>> >
>> > So quite frankly, is there any reason we don't just implement
>> > native_read_msr() as just
>> >
>> > unsigned long long native_read_msr(unsigned int msr)
>> > {
>> > int err;
>> > unsigned long long val;
>> >
>> > val = native_read_msr_safe(msr, &err);
>> > WARN_ON_ONCE(err);
>> > return val;
>> > }
>> >
>> > Note: no inline, no nothing. Just put it in arch/x86/lib/msr.c, and be
>> > done with it. I don't see the downside.
>> >
>> > How many msr reads are <i>so</i> critical that the function call
>> > overhead would matter? Get rid of the inline version of the _safe()
>> > thing too, and put that thing there too.
>>
>> There are a few in the perf code, and esp. on cores without a stack engine the
>> call overhead is noticeable. Also note that the perf MSRs are generally
>> optimized MSRs and less slow (we cannot say fast, they're still MSRs) than
>> regular MSRs.
>
> These could still be open coded in an inlined fashion, like the scheduler usage.
>
We could have a raw_rdmsr for those.
OTOH, I'm still not 100% convinced that this warn-but-don't-die
behavior is worth the effort. This isn't a frequent source of bugs to
my knowledge, and we don't try to recover from incorrect cr writes,
out-of-bounds MMIO, etc, so do we really gain much by rigging a
recovery mechanism for rdmsr and wrmsr failures for code that doesn't
use the _safe variants?
--Andy
> Thanks,
>
> Ingo
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CALCETrVMV2_3ywQ_t+0rsqzxMm6D9PvDEmOdrie67rRzfj-W_Q@mail.gmail.com>]
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <CALCETrVMV2_3ywQ_t+0rsqzxMm6D9PvDEmOdrie67rRzfj-W_Q@mail.gmail.com>
@ 2015-10-01 7:15 ` Ingo Molnar
[not found] ` <20151001071505.GA21542@gmail.com>
1 sibling, 0 replies; 22+ messages in thread
From: Ingo Molnar @ 2015-10-01 7:15 UTC (permalink / raw)
To: Andy Lutomirski
Cc: KVM list, Peter Zijlstra, Linus Torvalds,
the arch/x86 maintainers, Linux Kernel Mailing List, xen-devel,
Paolo Bonzini, Thomas Gleixner, Arjan van de Ven, Andrew Morton
* Andy Lutomirski <luto@amacapital.net> wrote:
> > These could still be open coded in an inlined fashion, like the scheduler usage.
>
> We could have a raw_rdmsr for those.
>
> OTOH, I'm still not 100% convinced that this warn-but-don't-die behavior is
> worth the effort. This isn't a frequent source of bugs to my knowledge, and we
> don't try to recover from incorrect cr writes, out-of-bounds MMIO, etc, so do we
> really gain much by rigging a recovery mechanism for rdmsr and wrmsr failures
> for code that doesn't use the _safe variants?
It's just the general principle really: don't crash the kernel on bootup. There's
few things more user hostile than that.
Also, this would maintain the status quo: since we now (accidentally) don't crash
the kernel on distro kernels (but silently and unsafely ignore the faulting
instruction), we should not regress that behavior (by adding the chance to crash
again), but improve upon it.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20151001071505.GA21542@gmail.com>]
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <20151001071505.GA21542@gmail.com>
@ 2016-03-11 16:48 ` Andy Lutomirski
[not found] ` <CALCETrUFakwGL9zpj9TwKX9KbG9czq8fpEViU3nWaCvnpGurew@mail.gmail.com>
1 sibling, 0 replies; 22+ messages in thread
From: Andy Lutomirski @ 2016-03-11 16:48 UTC (permalink / raw)
To: Ingo Molnar
Cc: KVM list, Peter Zijlstra, Linus Torvalds,
the arch/x86 maintainers, Linux Kernel Mailing List, xen-devel,
Paolo Bonzini, Thomas Gleixner, Arjan van de Ven, Andrew Morton
On Thu, Oct 1, 2015 at 12:15 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Andy Lutomirski <luto@amacapital.net> wrote:
>
>> > These could still be open coded in an inlined fashion, like the scheduler usage.
>>
>> We could have a raw_rdmsr for those.
>>
>> OTOH, I'm still not 100% convinced that this warn-but-don't-die behavior is
>> worth the effort. This isn't a frequent source of bugs to my knowledge, and we
>> don't try to recover from incorrect cr writes, out-of-bounds MMIO, etc, so do we
>> really gain much by rigging a recovery mechanism for rdmsr and wrmsr failures
>> for code that doesn't use the _safe variants?
>
> It's just the general principle really: don't crash the kernel on bootup. There's
> few things more user hostile than that.
>
> Also, this would maintain the status quo: since we now (accidentally) don't crash
> the kernel on distro kernels (but silently and unsafely ignore the faulting
> instruction), we should not regress that behavior (by adding the chance to crash
> again), but improve upon it.
Just a heads up: the extable improvements in tip:ras/core make it
straightforward to get the best of all worlds: explicit failure
handling (written in C!), no fast path overhead whatsoever, and no new
garbage in the exception handlers.
Patches coming once I test them.
>
> Thanks,
>
> Ingo
--
Andy Lutomirski
AMA Capital Management, LLC
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <CALCETrUFakwGL9zpj9TwKX9KbG9czq8fpEViU3nWaCvnpGurew@mail.gmail.com>]
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <CALCETrUFakwGL9zpj9TwKX9KbG9czq8fpEViU3nWaCvnpGurew@mail.gmail.com>
@ 2016-03-12 16:02 ` Ingo Molnar
0 siblings, 0 replies; 22+ messages in thread
From: Ingo Molnar @ 2016-03-12 16:02 UTC (permalink / raw)
To: Andy Lutomirski
Cc: KVM list, Peter Zijlstra, Linus Torvalds,
the arch/x86 maintainers, Linux Kernel Mailing List, xen-devel,
Paolo Bonzini, Thomas Gleixner, Arjan van de Ven, Andrew Morton
* Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Oct 1, 2015 at 12:15 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Andy Lutomirski <luto@amacapital.net> wrote:
> >
> >> > These could still be open coded in an inlined fashion, like the scheduler usage.
> >>
> >> We could have a raw_rdmsr for those.
> >>
> >> OTOH, I'm still not 100% convinced that this warn-but-don't-die behavior is
> >> worth the effort. This isn't a frequent source of bugs to my knowledge, and we
> >> don't try to recover from incorrect cr writes, out-of-bounds MMIO, etc, so do we
> >> really gain much by rigging a recovery mechanism for rdmsr and wrmsr failures
> >> for code that doesn't use the _safe variants?
> >
> > It's just the general principle really: don't crash the kernel on bootup. There's
> > few things more user hostile than that.
> >
> > Also, this would maintain the status quo: since we now (accidentally) don't crash
> > the kernel on distro kernels (but silently and unsafely ignore the faulting
> > instruction), we should not regress that behavior (by adding the chance to crash
> > again), but improve upon it.
>
> Just a heads up: the extable improvements in tip:ras/core make it
> straightforward to get the best of all worlds: explicit failure
> handling (written in C!), no fast path overhead whatsoever, and no new
> garbage in the exception handlers.
I _knew_ I should have merged them into tip:x86/mm, not tip:ras/core ;-)
I had a quick look at your new MSR series and I'm very happy with that direction!
Thanks,
Ingo
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops
[not found] ` <CA+55aFzKCi6pf0RP8HjDQYDsms6reB5AihuCAHEkVJtoOHk_Yw@mail.gmail.com>
` (6 preceding siblings ...)
[not found] ` <20150930131002.GK2881@worktop.programming.kicks-ass.net>
@ 2015-09-30 18:32 ` H. Peter Anvin
7 siblings, 0 replies; 22+ messages in thread
From: H. Peter Anvin @ 2015-09-30 18:32 UTC (permalink / raw)
To: Linus Torvalds, Ingo Molnar
Cc: KVM list, Peter Zijlstra, the arch/x86 maintainers,
Linux Kernel Mailing List, xen-devel, Paolo Bonzini,
Thomas Gleixner, Andy Lutomirski, Arjan van de Ven,
Andrew Morton
On 09/21/2015 09:36 AM, Linus Torvalds wrote:
>
> How many msr reads are <i>so</i> critical that the function call
> overhead would matter? Get rid of the inline version of the _safe()
> thing too, and put that thing there too.
>
Probably only the ones that may go in the context switch path.
-hpa
^ permalink raw reply [flat|nested] 22+ messages in thread