Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1369949 > unrolled thread

[PATCH v5 0/9] Improve non-"safe" MSR access failure handling

Back to article view | Back to linux.kernel

Contents

  [PATCH v5 0/9] Improve non-"safe" MSR access failure handling Andy Lutomirski <luto@kernel.org> - 2016-04-02 16:10 +0200
    [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks Andy Lutomirski <luto@kernel.org> - 2016-04-02 16:10 +0200
      Re: [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and  write_msr PV hooks Borislav Petkov <bp@alien8.de> - 2016-04-03 10:50 +0200
        Re: [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and  write_msr PV hooks Andy Lutomirski <luto@amacapital.net> - 2016-04-03 15:30 +0200
          Re: [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and  write_msr PV hooks Borislav Petkov <bp@alien8.de> - 2016-04-03 16:10 +0200
      [tip:x86/asm] x86/paravirt: Add _safe to the read_ms()r and  write_msr() PV callbacks tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-04-13 13:50 +0200
    [PATCH v5 8/9] x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y Andy Lutomirski <luto@kernel.org> - 2016-04-02 16:10 +0200
      [tip:x86/asm] x86/paravirt: Make "unsafe" MSR accesses unsafe even  if PARAVIRT=y tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-04-13 13:50 +0200
    [PATCH v5 2/9] x86/head: Move the early NMI fixup into C Andy Lutomirski <luto@kernel.org> - 2016-04-02 16:10 +0200
      [tip:x86/asm] x86/head: Move the early NMI fixup into C tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-04-13 13:50 +0200
    [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception Andy Lutomirski <luto@kernel.org> - 2016-04-02 16:10 +0200
      Re: [PATCH v5 3/9] x86/head: Move early exception panic code into  early_fixup_exception Borislav Petkov <bp@alien8.de> - 2016-04-02 20:40 +0200
        Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception Andy Lutomirski <luto@amacapital.net> - 2016-04-02 22:20 +0200
          Re: [PATCH v5 3/9] x86/head: Move early exception panic code into  early_fixup_exception Borislav Petkov <bp@alien8.de> - 2016-04-02 22:50 +0200
            Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception Andy Lutomirski <luto@amacapital.net> - 2016-04-02 23:00 +0200
              Re: [PATCH v5 3/9] x86/head: Move early exception panic code into  early_fixup_exception Jan Kara <jack@suse.cz> - 2016-04-04 14:00 +0200
                Re: [PATCH v5 3/9] x86/head: Move early exception panic code into  early_fixup_exception Peter Zijlstra <peterz@infradead.org> - 2016-04-04 14:50 +0200
                Re: [PATCH v5 3/9] x86/head: Move early exception panic code into  early_fixup_exception Arjan van de Ven <arjan@linux.intel.com> - 2016-04-04 17:40 +0200
                Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception Andy Lutomirski <luto@amacapital.net> - 2016-04-04 17:40 +0200
                  Re: [PATCH v5 3/9] x86/head: Move early exception panic code into  early_fixup_exception Peter Zijlstra <peterz@infradead.org> - 2016-04-04 18:10 +0200
                    Re: [PATCH v5 3/9] x86/head: Move early exception panic code into  early_fixup_exception Borislav Petkov <bp@alien8.de> - 2016-04-04 21:40 +0200
                      Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception Andy Lutomirski <luto@amacapital.net> - 2016-04-04 23:40 +0200
                        Re: [PATCH v5 3/9] x86/head: Move early exception panic code into  early_fixup_exception Borislav Petkov <bp@alien8.de> - 2016-04-04 23:50 +0200
      [tip:x86/asm] x86/head: Move early exception panic code into  early_fixup_exception() tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-04-13 13:50 +0200
    [PATCH v5 7/9] x86/paravirt: Add paravirt_{read,write}_msr Andy Lutomirski <luto@kernel.org> - 2016-04-02 16:10 +0200
      Re: [Xen-devel] [PATCH v5 7/9] x86/paravirt: Add paravirt_{read,  write}_msr David Vrabel <david.vrabel@citrix.com> - 2016-04-04 18:40 +0200
        Re: [Xen-devel] [PATCH v5 7/9] x86/paravirt: Add paravirt_{read, write}_msr Andy Lutomirski <luto@amacapital.net> - 2016-04-04 18:50 +0200
      [tip:x86/asm] x86/paravirt: Add paravirt_{read,write}_msr() tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-04-13 13:50 +0200
    [PATCH v5 1/9] x86/head: Pass a real pt_regs and trapnr to early_fixup_exception Andy Lutomirski <luto@kernel.org> - 2016-04-02 16:10 +0200
      [tip:x86/asm] x86/head: Pass a real pt_regs and trapnr to  early_fixup_exception() tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-04-13 13:50 +0200
    [PATCH v5 6/9] x86/msr: Carry on after a non-"safe" MSR access fails Andy Lutomirski <luto@kernel.org> - 2016-04-02 16:10 +0200
      [tip:x86/asm] x86/msr: Carry on after a non-"safe" MSR access fails tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-04-13 13:50 +0200
    [PATCH v5 9/9] x86/msr: Set the return value to zero when native_rdmsr_safe fails Andy Lutomirski <luto@kernel.org> - 2016-04-02 16:10 +0200
      [tip:x86/asm] x86/msr: Set the return value to zero when  native_rdmsr_safe() fails tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-04-13 13:50 +0200
    Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-02 16:30 +0200
      Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling Andy Lutomirski <luto@amacapital.net> - 2016-04-02 17:20 +0200
        Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling Linus Torvalds <torvalds@linux-foundation.org> - 2016-04-02 17:30 +0200
    Re: [PATCH v5 0/9] Improve non-"safe" MSR access failure handling Borislav Petkov <bp@alien8.de> - 2016-04-04 18:30 +0200
    Re: [Xen-devel] [PATCH v5 0/9] Improve non-"safe" MSR access failure  handling Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-04-05 17:40 +0200

Page 1 of 2  [1] 2  Next page →

#1369949 — [PATCH v5 0/9] Improve non-"safe" MSR access failure handling

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

[toc] | [next] | [standalone]

#1369950 — [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks

These hooks match the _safe variants, so name them accordingly.
This will make room for unsafe PV hooks.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/paravirt.h       | 33 +++++++++++++++++----------------
 arch/x86/include/asm/paravirt_types.h |  8 ++++----
 arch/x86/kernel/paravirt.c            |  4 ++--
 arch/x86/xen/enlighten.c              |  4 ++--
 4 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index f6192502149e..2e49228ed9a3 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -129,34 +129,35 @@ static inline void wbinvd(void)
 
 #define get_kernel_rpl()  (pv_info.kernel_rpl)
 
-static inline u64 paravirt_read_msr(unsigned msr, int *err)
+static inline u64 paravirt_read_msr_safe(unsigned msr, int *err)
 {
-	return PVOP_CALL2(u64, pv_cpu_ops.read_msr, msr, err);
+	return PVOP_CALL2(u64, pv_cpu_ops.read_msr_safe, msr, err);
 }
 
-static inline int paravirt_write_msr(unsigned msr, unsigned low, unsigned high)
+static inline int paravirt_write_msr_safe(unsigned msr,
+					  unsigned low, unsigned high)
 {
-	return PVOP_CALL3(int, pv_cpu_ops.write_msr, msr, low, high);
+	return PVOP_CALL3(int, pv_cpu_ops.write_msr_safe, msr, low, high);
 }
 
 /* These should all do BUG_ON(_err), but our headers are too tangled. */
 #define rdmsr(msr, val1, val2)			\
 do {						\
 	int _err;				\
-	u64 _l = paravirt_read_msr(msr, &_err);	\
+	u64 _l = paravirt_read_msr_safe(msr, &_err);	\
 	val1 = (u32)_l;				\
 	val2 = _l >> 32;			\
 } while (0)
 
 #define wrmsr(msr, val1, val2)			\
 do {						\
-	paravirt_write_msr(msr, val1, val2);	\
+	paravirt_write_msr_safe(msr, val1, val2);	\
 } while (0)
 
 #define rdmsrl(msr, val)			\
 do {						\
 	int _err;				\
-	val = paravirt_read_msr(msr, &_err);	\
+	val = paravirt_read_msr_safe(msr, &_err);	\
 } while (0)
 
 static inline void wrmsrl(unsigned msr, u64 val)
@@ -164,23 +165,23 @@ static inline void wrmsrl(unsigned msr, u64 val)
 	wrmsr(msr, (u32)val, (u32)(val>>32));
 }
 
-#define wrmsr_safe(msr, a, b)	paravirt_write_msr(msr, a, b)
+#define wrmsr_safe(msr, a, b)	paravirt_write_msr_safe(msr, a, b)
 
 /* rdmsr with exception handling */
-#define rdmsr_safe(msr, a, b)			\
-({						\
-	int _err;				\
-	u64 _l = paravirt_read_msr(msr, &_err);	\
-	(*a) = (u32)_l;				\
-	(*b) = _l >> 32;			\
-	_err;					\
+#define rdmsr_safe(msr, a, b)				\
+({							\
+	int _err;					\
+	u64 _l = paravirt_read_msr_safe(msr, &_err);	\
+	(*a) = (u32)_l;					\
+	(*b) = _l >> 32;				\
+	_err;						\
 })
 
 static inline int rdmsrl_safe(unsigned msr, unsigned long long *p)
 {
 	int err;
 
-	*p = paravirt_read_msr(msr, &err);
+	*p = paravirt_read_msr_safe(msr, &err);
 	return err;
 }
 
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 77db5616a473..5a06cccd36f0 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -155,10 +155,10 @@ struct pv_cpu_ops {
 	void (*cpuid)(unsigned int *eax, unsigned int *ebx,
 		      unsigned int *ecx, unsigned int *edx);
 
-	/* MSR, PMC and TSR operations.
-	   err = 0/-EFAULT.  wrmsr returns 0/-EFAULT. */
-	u64 (*read_msr)(unsigned int msr, int *err);
-	int (*write_msr)(unsigned int msr, unsigned low, unsigned high);
+	/* MSR operations.
+	   err = 0/-EIO.  wrmsr returns 0/-EIO. */
+	u64 (*read_msr_safe)(unsigned int msr, int *err);
+	int (*write_msr_safe)(unsigned int msr, unsigned low, unsigned high);
 
 	u64 (*read_pmc)(int counter);
 
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index f08ac28b8136..8aad95478ae5 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -339,8 +339,8 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
 	.write_cr8 = native_write_cr8,
 #endif
 	.wbinvd = native_wbinvd,
-	.read_msr = native_read_msr_safe,
-	.write_msr = native_write_msr_safe,
+	.read_msr_safe = native_read_msr_safe,
+	.write_msr_safe = native_write_msr_safe,
 	.read_pmc = native_read_pmc,
 	.load_tr_desc = native_load_tr_desc,
 	.set_ldt = native_set_ldt,
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index d09e4c9d7cc5..ff2df20d0067 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1222,8 +1222,8 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
 
 	.wbinvd = native_wbinvd,
 
-	.read_msr = xen_read_msr_safe,
-	.write_msr = xen_write_msr_safe,
+	.read_msr_safe = xen_read_msr_safe,
+	.write_msr_safe = xen_write_msr_safe,
 
 	.read_pmc = xen_read_pmc,
 
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1370129 — Re: [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks

On Sat, Apr 02, 2016 at 07:01:36AM -0700, Andy Lutomirski wrote:
> These hooks match the _safe variants, so name them accordingly.
> This will make room for unsafe PV hooks.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/include/asm/paravirt.h       | 33 +++++++++++++++++----------------
>  arch/x86/include/asm/paravirt_types.h |  8 ++++----
>  arch/x86/kernel/paravirt.c            |  4 ++--
>  arch/x86/xen/enlighten.c              |  4 ++--
>  4 files changed, 25 insertions(+), 24 deletions(-)

...

> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
> index 77db5616a473..5a06cccd36f0 100644
> --- a/arch/x86/include/asm/paravirt_types.h
> +++ b/arch/x86/include/asm/paravirt_types.h
> @@ -155,10 +155,10 @@ struct pv_cpu_ops {
>  	void (*cpuid)(unsigned int *eax, unsigned int *ebx,
>  		      unsigned int *ecx, unsigned int *edx);
>  
> -	/* MSR, PMC and TSR operations.
> -	   err = 0/-EFAULT.  wrmsr returns 0/-EFAULT. */
> -	u64 (*read_msr)(unsigned int msr, int *err);
> -	int (*write_msr)(unsigned int msr, unsigned low, unsigned high);
> +	/* MSR operations.
> +	   err = 0/-EIO.  wrmsr returns 0/-EIO. */

Please reformat this comment properly, while you're at it:

	/*
	 * A sentence.
	 * Another sentence.
	 */

> +	u64 (*read_msr_safe)(unsigned int msr, int *err)Please reformat this
> comment properly, while you're at it:
> +	int (*write_msr_safe)(unsigned int msr, unsigned low, unsigned high);
>  
>  	u64 (*read_pmc)(int counter);
>  

-- 
Regards/Gruss,
    Boris.

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

[toc] | [prev] | [next] | [standalone]

#1370215 — Re: [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks

On Sun, Apr 3, 2016 at 1:41 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Sat, Apr 02, 2016 at 07:01:36AM -0700, Andy Lutomirski wrote:
>> These hooks match the _safe variants, so name them accordingly.
>> This will make room for unsafe PV hooks.
>>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>  arch/x86/include/asm/paravirt.h       | 33 +++++++++++++++++----------------
>>  arch/x86/include/asm/paravirt_types.h |  8 ++++----
>>  arch/x86/kernel/paravirt.c            |  4 ++--
>>  arch/x86/xen/enlighten.c              |  4 ++--
>>  4 files changed, 25 insertions(+), 24 deletions(-)
>
> ...
>
>> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
>> index 77db5616a473..5a06cccd36f0 100644
>> --- a/arch/x86/include/asm/paravirt_types.h
>> +++ b/arch/x86/include/asm/paravirt_types.h
>> @@ -155,10 +155,10 @@ struct pv_cpu_ops {
>>       void (*cpuid)(unsigned int *eax, unsigned int *ebx,
>>                     unsigned int *ecx, unsigned int *edx);
>>
>> -     /* MSR, PMC and TSR operations.
>> -        err = 0/-EFAULT.  wrmsr returns 0/-EFAULT. */
>> -     u64 (*read_msr)(unsigned int msr, int *err);
>> -     int (*write_msr)(unsigned int msr, unsigned low, unsigned high);
>> +     /* MSR operations.
>> +        err = 0/-EIO.  wrmsr returns 0/-EIO. */
>
> Please reformat this comment properly, while you're at it:
>
>         /*
>          * A sentence.
>          * Another sentence.
>          */

You already caught that one.  It's fixed in "x86/paravirt: Add
paravirt_{read,write}_msr".

Congrats on being deterministic :)

--Andy

[toc] | [prev] | [next] | [standalone]

#1370226 — Re: [PATCH v5 5/9] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks

On Sun, Apr 03, 2016 at 06:23:36AM -0700, Andy Lutomirski wrote:
> You already caught that one.  It's fixed in "x86/paravirt: Add
> paravirt_{read,write}_msr".
> 
> Congrats on being deterministic :)

LOL.

TBH, I had a strange deja-vu while typing... :-)

-- 
Regards/Gruss,
    Boris.

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

[toc] | [prev] | [next] | [standalone]

#1377832 — [tip:x86/asm] x86/paravirt: Add _safe to the read_ms()r and write_msr() PV callbacks

Commit-ID:  c2ee03b2a94d7ba692cf6206bbe069d5bfcc20ed
Gitweb:     http://git.kernel.org/tip/c2ee03b2a94d7ba692cf6206bbe069d5bfcc20ed
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Sat, 2 Apr 2016 07:01:36 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 13 Apr 2016 11:37:45 +0200

x86/paravirt: Add _safe to the read_ms()r and write_msr() PV callbacks

These callbacks match the _safe variants, so name them accordingly.
This will make room for unsafe PV callbacks.

Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: KVM list <kvm@vger.kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: xen-devel <Xen-devel@lists.xen.org>
Link: http://lkml.kernel.org/r/9ee3fb6a196a514c93325bdfa15594beecf04876.1459605520.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/paravirt.h       | 33 +++++++++++++++++----------------
 arch/x86/include/asm/paravirt_types.h |  8 ++++----
 arch/x86/kernel/paravirt.c            |  4 ++--
 arch/x86/xen/enlighten.c              |  4 ++--
 4 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 601f1b8..81ef2d5 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -130,34 +130,35 @@ static inline void wbinvd(void)
 
 #define get_kernel_rpl()  (pv_info.kernel_rpl)
 
-static inline u64 paravirt_read_msr(unsigned msr, int *err)
+static inline u64 paravirt_read_msr_safe(unsigned msr, int *err)
 {
-	return PVOP_CALL2(u64, pv_cpu_ops.read_msr, msr, err);
+	return PVOP_CALL2(u64, pv_cpu_ops.read_msr_safe, msr, err);
 }
 
-static inline int paravirt_write_msr(unsigned msr, unsigned low, unsigned high)
+static inline int paravirt_write_msr_safe(unsigned msr,
+					  unsigned low, unsigned high)
 {
-	return PVOP_CALL3(int, pv_cpu_ops.write_msr, msr, low, high);
+	return PVOP_CALL3(int, pv_cpu_ops.write_msr_safe, msr, low, high);
 }
 
 /* These should all do BUG_ON(_err), but our headers are too tangled. */
 #define rdmsr(msr, val1, val2)			\
 do {						\
 	int _err;				\
-	u64 _l = paravirt_read_msr(msr, &_err);	\
+	u64 _l = paravirt_read_msr_safe(msr, &_err);	\
 	val1 = (u32)_l;				\
 	val2 = _l >> 32;			\
 } while (0)
 
 #define wrmsr(msr, val1, val2)			\
 do {						\
-	paravirt_write_msr(msr, val1, val2);	\
+	paravirt_write_msr_safe(msr, val1, val2);	\
 } while (0)
 
 #define rdmsrl(msr, val)			\
 do {						\
 	int _err;				\
-	val = paravirt_read_msr(msr, &_err);	\
+	val = paravirt_read_msr_safe(msr, &_err);	\
 } while (0)
 
 static inline void wrmsrl(unsigned msr, u64 val)
@@ -165,23 +166,23 @@ static inline void wrmsrl(unsigned msr, u64 val)
 	wrmsr(msr, (u32)val, (u32)(val>>32));
 }
 
-#define wrmsr_safe(msr, a, b)	paravirt_write_msr(msr, a, b)
+#define wrmsr_safe(msr, a, b)	paravirt_write_msr_safe(msr, a, b)
 
 /* rdmsr with exception handling */
-#define rdmsr_safe(msr, a, b)			\
-({						\
-	int _err;				\
-	u64 _l = paravirt_read_msr(msr, &_err);	\
-	(*a) = (u32)_l;				\
-	(*b) = _l >> 32;			\
-	_err;					\
+#define rdmsr_safe(msr, a, b)				\
+({							\
+	int _err;					\
+	u64 _l = paravirt_read_msr_safe(msr, &_err);	\
+	(*a) = (u32)_l;					\
+	(*b) = _l >> 32;				\
+	_err;						\
 })
 
 static inline int rdmsrl_safe(unsigned msr, unsigned long long *p)
 {
 	int err;
 
-	*p = paravirt_read_msr(msr, &err);
+	*p = paravirt_read_msr_safe(msr, &err);
 	return err;
 }
 
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index e8c2326..09c9e1d 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -155,10 +155,10 @@ struct pv_cpu_ops {
 	void (*cpuid)(unsigned int *eax, unsigned int *ebx,
 		      unsigned int *ecx, unsigned int *edx);
 
-	/* MSR, PMC and TSR operations.
-	   err = 0/-EFAULT.  wrmsr returns 0/-EFAULT. */
-	u64 (*read_msr)(unsigned int msr, int *err);
-	int (*write_msr)(unsigned int msr, unsigned low, unsigned high);
+	/* MSR operations.
+	   err = 0/-EIO.  wrmsr returns 0/-EIO. */
+	u64 (*read_msr_safe)(unsigned int msr, int *err);
+	int (*write_msr_safe)(unsigned int msr, unsigned low, unsigned high);
 
 	u64 (*read_pmc)(int counter);
 
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index f08ac28..8aad954 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -339,8 +339,8 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
 	.write_cr8 = native_write_cr8,
 #endif
 	.wbinvd = native_wbinvd,
-	.read_msr = native_read_msr_safe,
-	.write_msr = native_write_msr_safe,
+	.read_msr_safe = native_read_msr_safe,
+	.write_msr_safe = native_write_msr_safe,
 	.read_pmc = native_read_pmc,
 	.load_tr_desc = native_load_tr_desc,
 	.set_ldt = native_set_ldt,
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 9b8f1ea..13f756f 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1222,8 +1222,8 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
 
 	.wbinvd = native_wbinvd,
 
-	.read_msr = xen_read_msr_safe,
-	.write_msr = xen_write_msr_safe,
+	.read_msr_safe = xen_read_msr_safe,
+	.write_msr_safe = xen_write_msr_safe,
 
 	.read_pmc = xen_read_pmc,
 

[toc] | [prev] | [next] | [standalone]

#1369951 — [PATCH v5 8/9] x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y

Enabling CONFIG_PARAVIRT had an unintended side effect: rdmsr turned
into rdmsr_safe and wrmsr turned into wrmsr_safe, even on bare
metal.  Undo that by using the new unsafe paravirt MSR hooks.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/paravirt.h | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 68297d87e85c..0c99f10874e4 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -151,24 +151,21 @@ static inline int paravirt_write_msr_safe(unsigned msr,
 	return PVOP_CALL3(int, pv_cpu_ops.write_msr_safe, msr, low, high);
 }
 
-/* These should all do BUG_ON(_err), but our headers are too tangled. */
 #define rdmsr(msr, val1, val2)			\
 do {						\
-	int _err;				\
-	u64 _l = paravirt_read_msr_safe(msr, &_err);	\
+	u64 _l = paravirt_read_msr(msr);	\
 	val1 = (u32)_l;				\
 	val2 = _l >> 32;			\
 } while (0)
 
 #define wrmsr(msr, val1, val2)			\
 do {						\
-	paravirt_write_msr_safe(msr, val1, val2);	\
+	paravirt_write_msr(msr, val1, val2);	\
 } while (0)
 
 #define rdmsrl(msr, val)			\
 do {						\
-	int _err;				\
-	val = paravirt_read_msr_safe(msr, &_err);	\
+	val = paravirt_read_msr(msr);		\
 } while (0)
 
 static inline void wrmsrl(unsigned msr, u64 val)
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1377847 — [tip:x86/asm] x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y

Commit-ID:  4985ce15a397e9b6541548efe3b9ffac2dda9127
Gitweb:     http://git.kernel.org/tip/4985ce15a397e9b6541548efe3b9ffac2dda9127
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Sat, 2 Apr 2016 07:01:39 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 13 Apr 2016 11:37:46 +0200

x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y

Enabling CONFIG_PARAVIRT had an unintended side effect: rdmsr() turned
into rdmsr_safe() and wrmsr() turned into wrmsr_safe(), even on bare
metal.  Undo that by using the new unsafe paravirt MSR callbacks.

Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: KVM list <kvm@vger.kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: xen-devel <Xen-devel@lists.xen.org>
Link: http://lkml.kernel.org/r/414fabd6d3527703077c6c2a797223d0a9c3b081.1459605520.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/paravirt.h | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 97839fa..3c73141 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -152,24 +152,21 @@ static inline int paravirt_write_msr_safe(unsigned msr,
 	return PVOP_CALL3(int, pv_cpu_ops.write_msr_safe, msr, low, high);
 }
 
-/* These should all do BUG_ON(_err), but our headers are too tangled. */
 #define rdmsr(msr, val1, val2)			\
 do {						\
-	int _err;				\
-	u64 _l = paravirt_read_msr_safe(msr, &_err);	\
+	u64 _l = paravirt_read_msr(msr);	\
 	val1 = (u32)_l;				\
 	val2 = _l >> 32;			\
 } while (0)
 
 #define wrmsr(msr, val1, val2)			\
 do {						\
-	paravirt_write_msr_safe(msr, val1, val2);	\
+	paravirt_write_msr(msr, val1, val2);	\
 } while (0)
 
 #define rdmsrl(msr, val)			\
 do {						\
-	int _err;				\
-	val = paravirt_read_msr_safe(msr, &_err);	\
+	val = paravirt_read_msr(msr);		\
 } while (0)
 
 static inline void wrmsrl(unsigned msr, u64 val)

[toc] | [prev] | [next] | [standalone]

#1369953 — [PATCH v5 2/9] x86/head: Move the early NMI fixup into C

C is nicer than asm.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kernel/head_32.S | 7 -------
 arch/x86/kernel/head_64.S | 6 ------
 arch/x86/mm/extable.c     | 5 +++++
 3 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index bef3e6c49b56..5e6ce845813a 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -555,9 +555,6 @@ early_idt_handler_common:
 	 */
 	cld
 
-	cmpl $2,(%esp)		# X86_TRAP_NMI
-	je .Lis_nmi		# Ignore NMI
-
 	cmpl $2,%ss:early_recursion_flag
 	je hlt_loop
 	incl %ss:early_recursion_flag
@@ -636,10 +633,6 @@ early_idt_handler_common:
 hlt_loop:
 	hlt
 	jmp hlt_loop
-
-.Lis_nmi:
-	addl $8,%esp		/* drop vector number and error code */
-	iret
 ENDPROC(early_idt_handler_common)
 
 /* This is the default interrupt "handler" :-) */
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index f8d6dad41e8d..af87896b6a23 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -354,9 +354,6 @@ early_idt_handler_common:
 	 */
 	cld
 
-	cmpl $2,(%rsp)		# X86_TRAP_NMI
-	je .Lis_nmi		# Ignore NMI
-
 	cmpl $2,early_recursion_flag(%rip)
 	jz  1f
 	incl early_recursion_flag(%rip)
@@ -425,9 +422,6 @@ early_idt_handler_common:
 20:	/* Exception table entry found or page table generated */
 	decl early_recursion_flag(%rip)
 	jmp restore_regs_and_iret
-.Lis_nmi:
-	addq $16,%rsp		# drop vector number and error code
-	INTERRUPT_RETURN
 ENDPROC(early_idt_handler_common)
 
 	__INITDATA
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 176e48de25d4..d6e4e6fb4002 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -2,6 +2,7 @@
 #include <linux/spinlock.h>
 #include <linux/sort.h>
 #include <asm/uaccess.h>
+#include <asm/traps.h>
 
 typedef bool (*ex_handler_t)(const struct exception_table_entry *,
 			    struct pt_regs *, int);
@@ -96,6 +97,10 @@ int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 	unsigned long new_ip;
 	ex_handler_t handler;
 
+	/* Ignore early NMIs. */
+	if (trapnr == X86_TRAP_NMI)
+		return 1;
+
 	e = search_exception_tables(regs->ip);
 	if (!e)
 		return 0;
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1377836 — [tip:x86/asm] x86/head: Move the early NMI fixup into C

Commit-ID:  0d0efc07f3df677d7622bb760f8e2920b5e33f42
Gitweb:     http://git.kernel.org/tip/0d0efc07f3df677d7622bb760f8e2920b5e33f42
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Sat, 2 Apr 2016 07:01:33 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 13 Apr 2016 11:37:44 +0200

x86/head: Move the early NMI fixup into C

C is nicer than asm.

Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: KVM list <kvm@vger.kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: xen-devel <Xen-devel@lists.xen.org>
Link: http://lkml.kernel.org/r/dd068269f8d59fe44e9e43a50d0efd67da65c2b5.1459605520.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/head_32.S | 7 -------
 arch/x86/kernel/head_64.S | 6 ------
 arch/x86/mm/extable.c     | 5 +++++
 3 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 0904536..184291c 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -561,9 +561,6 @@ early_idt_handler_common:
 	 */
 	cld
 
-	cmpl $2,(%esp)		# X86_TRAP_NMI
-	je .Lis_nmi		# Ignore NMI
-
 	cmpl $2,%ss:early_recursion_flag
 	je hlt_loop
 	incl %ss:early_recursion_flag
@@ -642,10 +639,6 @@ early_idt_handler_common:
 hlt_loop:
 	hlt
 	jmp hlt_loop
-
-.Lis_nmi:
-	addl $8,%esp		/* drop vector number and error code */
-	iret
 ENDPROC(early_idt_handler_common)
 
 /* This is the default interrupt "handler" :-) */
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 9e8636d..2308437 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -351,9 +351,6 @@ early_idt_handler_common:
 	 */
 	cld
 
-	cmpl $2,(%rsp)		# X86_TRAP_NMI
-	je .Lis_nmi		# Ignore NMI
-
 	cmpl $2,early_recursion_flag(%rip)
 	jz  1f
 	incl early_recursion_flag(%rip)
@@ -422,9 +419,6 @@ early_idt_handler_common:
 20:	/* Exception table entry found or page table generated */
 	decl early_recursion_flag(%rip)
 	jmp restore_regs_and_iret
-.Lis_nmi:
-	addq $16,%rsp		# drop vector number and error code
-	INTERRUPT_RETURN
 ENDPROC(early_idt_handler_common)
 
 	__INITDATA
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 1366e06..4be0419 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -1,5 +1,6 @@
 #include <linux/module.h>
 #include <asm/uaccess.h>
+#include <asm/traps.h>
 
 typedef bool (*ex_handler_t)(const struct exception_table_entry *,
 			    struct pt_regs *, int);
@@ -89,6 +90,10 @@ int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 	unsigned long new_ip;
 	ex_handler_t handler;
 
+	/* Ignore early NMIs. */
+	if (trapnr == X86_TRAP_NMI)
+		return 1;
+
 	e = search_exception_tables(regs->ip);
 	if (!e)
 		return 0;

[toc] | [prev] | [next] | [standalone]

#1369955 — [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception

This removes a bunch of assembly and adds some C code instead.  It
changes the actual printouts on both 32-bit and 64-bit kernels, but
they still seem okay.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/include/asm/uaccess.h |  2 +-
 arch/x86/kernel/head_32.S      | 49 +++++-------------------------------------
 arch/x86/kernel/head_64.S      | 45 ++------------------------------------
 arch/x86/mm/extable.c          | 29 ++++++++++++++++++++-----
 4 files changed, 32 insertions(+), 93 deletions(-)

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index a3afb7259751..83fd2cf187d2 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -111,7 +111,7 @@ struct exception_table_entry {
 
 extern int fixup_exception(struct pt_regs *regs, int trapnr);
 extern bool ex_has_fault_handler(unsigned long ip);
-extern int early_fixup_exception(struct pt_regs *regs, int trapnr);
+extern void early_fixup_exception(struct pt_regs *regs, int trapnr);
 
 /*
  * These are the main single-value transfer routines.  They automatically
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 5e6ce845813a..411dce93fee9 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -555,8 +555,6 @@ early_idt_handler_common:
 	 */
 	cld
 
-	cmpl $2,%ss:early_recursion_flag
-	je hlt_loop
 	incl %ss:early_recursion_flag
 
 	/* The vector number is in pt_regs->gs */
@@ -588,13 +586,8 @@ early_idt_handler_common:
 	movw	%gs, PT_GS(%esp)
 	movw	$0, PT_GS+2(%esp)
 
-	cmpl $(__KERNEL_CS),PT_CS(%esp)
-	jne 10f
-
 	movl	%esp, %eax	/* args are pt_regs (EAX), trapnr (EDX) */
 	call	early_fixup_exception
-	andl	%eax,%eax
-	jz	10f		/* Exception wasn't fixed up */
 
 	popl	%ebx		/* pt_regs->bx */
 	popl	%ecx		/* pt_regs->cx */
@@ -610,29 +603,6 @@ early_idt_handler_common:
 	decl	%ss:early_recursion_flag
 	addl	$4, %esp	/* pop pt_regs->orig_ax */
 	iret
-
-10:
-#ifdef CONFIG_PRINTK
-	xorl %eax,%eax
-	movw %ax,PT_FS+2(%esp)	/* clean up the segment values on some cpus */
-	movw %ax,PT_DS+2(%esp)
-	movw %ax,PT_ES+2(%esp)
-	leal  40(%esp),%eax
-	pushl %eax		/* %esp before the exception */
-	pushl %ebx
-	pushl %ebp
-	pushl %esi
-	pushl %edi
-	movl %cr2,%eax
-	pushl %eax
-	pushl (20+6*4)(%esp)	/* trapno */
-	pushl $fault_msg
-	call printk
-#endif
-	call dump_stack
-hlt_loop:
-	hlt
-	jmp hlt_loop
 ENDPROC(early_idt_handler_common)
 
 /* This is the default interrupt "handler" :-) */
@@ -668,10 +638,14 @@ ignore_int:
 	popl %eax
 #endif
 	iret
+
+hlt_loop:
+	hlt
+	jmp hlt_loop
 ENDPROC(ignore_int)
 __INITDATA
 	.align 4
-early_recursion_flag:
+GLOBAL(early_recursion_flag)
 	.long 0
 
 __REFDATA
@@ -736,19 +710,6 @@ __INITRODATA
 int_msg:
 	.asciz "Unknown interrupt or fault at: %p %p %p\n"
 
-fault_msg:
-/* fault info: */
-	.ascii "BUG: Int %d: CR2 %p\n"
-/* regs pushed in early_idt_handler: */
-	.ascii "     EDI %p  ESI %p  EBP %p  EBX %p\n"
-	.ascii "     ESP %p   ES %p   DS %p\n"
-	.ascii "     EDX %p  ECX %p  EAX %p\n"
-/* fault frame: */
-	.ascii "     vec %p  err %p  EIP %p   CS %p  flg %p\n"
-	.ascii "Stack: %p %p %p %p %p %p %p %p\n"
-	.ascii "       %p %p %p %p %p %p %p %p\n"
-	.asciz "       %p %p %p %p %p %p %p %p\n"
-
 #include "../../x86/xen/xen-head.S"
 
 /*
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index af87896b6a23..c39b6437cf03 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -354,8 +354,6 @@ early_idt_handler_common:
 	 */
 	cld
 
-	cmpl $2,early_recursion_flag(%rip)
-	jz  1f
 	incl early_recursion_flag(%rip)
 
 	/* The vector number is currently in the pt_regs->di slot. */
@@ -376,9 +374,6 @@ early_idt_handler_common:
 	pushq %r14				/* pt_regs->r14 */
 	pushq %r15				/* pt_regs->r15 */
 
-	cmpl $__KERNEL_CS,CS(%rsp)
-	jne 11f
-
 	cmpq $14,%rsi		/* Page fault? */
 	jnz 10f
 	GET_CR2_INTO(%rdi)	/* Can clobber any volatile register if pv */
@@ -389,37 +384,8 @@ early_idt_handler_common:
 10:
 	movq %rsp,%rdi		/* RDI = pt_regs; RSI is already trapnr */
 	call early_fixup_exception
-	andl %eax,%eax
-	jnz 20f			# Found an exception entry
-
-11:
-#ifdef CONFIG_EARLY_PRINTK
-	/*
-	 * On paravirt kernels, GET_CR2_INTO clobbers callee-clobbered regs.
-	 * We only care about RSI, so we need to save it.
-	 */
-	movq %rsi,%rbx		/* Save vector number */
-	GET_CR2_INTO(%r9)
-	movq ORIG_RAX(%rsp),%r8	/* error code */
-	movq %rbx,%rsi		/* vector number */
-	movq CS(%rsp),%rdx
-	movq RIP(%rsp),%rcx
-	xorl %eax,%eax
-	leaq early_idt_msg(%rip),%rdi
-	call early_printk
-	cmpl $2,early_recursion_flag(%rip)
-	jz  1f
-	call dump_stack
-#ifdef CONFIG_KALLSYMS	
-	leaq early_idt_ripmsg(%rip),%rdi
-	movq RIP(%rsp),%rsi	# %rip again
-	call __print_symbol
-#endif
-#endif /* EARLY_PRINTK */
-1:	hlt
-	jmp 1b
 
-20:	/* Exception table entry found or page table generated */
+20:
 	decl early_recursion_flag(%rip)
 	jmp restore_regs_and_iret
 ENDPROC(early_idt_handler_common)
@@ -427,16 +393,9 @@ ENDPROC(early_idt_handler_common)
 	__INITDATA
 
 	.balign 4
-early_recursion_flag:
+GLOBAL(early_recursion_flag)
 	.long 0
 
-#ifdef CONFIG_EARLY_PRINTK
-early_idt_msg:
-	.asciz "PANIC: early exception %02lx rip %lx:%lx error %lx cr2 %lx\n"
-early_idt_ripmsg:
-	.asciz "RIP %s\n"
-#endif /* CONFIG_EARLY_PRINTK */
-
 #define NEXT_PAGE(name) \
 	.balign	PAGE_SIZE; \
 GLOBAL(name)
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index d6e4e6fb4002..8997022abebc 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -90,8 +90,10 @@ int fixup_exception(struct pt_regs *regs, int trapnr)
 	return handler(e, regs, trapnr);
 }
 
+extern unsigned int early_recursion_flag;
+
 /* Restricted version used during very early boot */
-int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
+void __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 {
 	const struct exception_table_entry *e;
 	unsigned long new_ip;
@@ -99,21 +101,38 @@ int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
 
 	/* Ignore early NMIs. */
 	if (trapnr == X86_TRAP_NMI)
-		return 1;
+		return;
+
+	if (early_recursion_flag > 2)
+		goto halt_loop;
+
+	if (regs->cs != __KERNEL_CS)
+		goto fail;
 
 	e = search_exception_tables(regs->ip);
 	if (!e)
-		return 0;
+		goto fail;
 
 	new_ip  = ex_fixup_addr(e);
 	handler = ex_fixup_handler(e);
 
 	/* special handling not supported during early boot */
 	if (handler != ex_handler_default)
-		return 0;
+		goto fail;
 
 	regs->ip = new_ip;
-	return 1;
+	return;
+
+fail:
+	early_printk("PANIC: early exception 0x%02x IP %lx:%lx error %lx cr2 0x%lx\n",
+		     (unsigned)trapnr, (unsigned long)regs->cs, regs->ip,
+		     regs->orig_ax, read_cr2());
+
+	show_regs(regs);
+
+halt_loop:
+	while (true)
+		halt();
 }
 
 /*
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1370031 — Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception

On Sat, Apr 02, 2016 at 07:01:34AM -0700, Andy Lutomirski wrote:
> This removes a bunch of assembly and adds some C code instead.  It
> changes the actual printouts on both 32-bit and 64-bit kernels, but
> they still seem okay.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/include/asm/uaccess.h |  2 +-
>  arch/x86/kernel/head_32.S      | 49 +++++-------------------------------------
>  arch/x86/kernel/head_64.S      | 45 ++------------------------------------
>  arch/x86/mm/extable.c          | 29 ++++++++++++++++++++-----
>  4 files changed, 32 insertions(+), 93 deletions(-)

...

> @@ -99,21 +101,38 @@ int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
>  
>  	/* Ignore early NMIs. */
>  	if (trapnr == X86_TRAP_NMI)
> -		return 1;
> +		return;
> +
> +	if (early_recursion_flag > 2)
> +		goto halt_loop;
> +
> +	if (regs->cs != __KERNEL_CS)
> +		goto fail;
>  
>  	e = search_exception_tables(regs->ip);
>  	if (!e)
> -		return 0;
> +		goto fail;
>  
>  	new_ip  = ex_fixup_addr(e);
>  	handler = ex_fixup_handler(e);
>  
>  	/* special handling not supported during early boot */
>  	if (handler != ex_handler_default)
> -		return 0;
> +		goto fail;
>  
>  	regs->ip = new_ip;
> -	return 1;
> +	return;
> +
> +fail:
> +	early_printk("PANIC: early exception 0x%02x IP %lx:%lx error %lx cr2 0x%lx\n",
> +		     (unsigned)trapnr, (unsigned long)regs->cs, regs->ip,
> +		     regs->orig_ax, read_cr2());
> +
> +	show_regs(regs);

To make this even better, it could be something called early_show_regs()
or so and be a simplified version of __show_regs() on both bitness but
which calls early_printk().

This way you'll be able to get out stuff to the console as early as
possible.

Btw, you don't need to dump rIP, CR2, etc in the PANIC message above
since you're going to early_show_regs() anyway.

-- 
Regards/Gruss,
    Boris.

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

[toc] | [prev] | [next] | [standalone]

#1370043 — Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception

On Sat, Apr 2, 2016 at 11:39 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Sat, Apr 02, 2016 at 07:01:34AM -0700, Andy Lutomirski wrote:
>> This removes a bunch of assembly and adds some C code instead.  It
>> changes the actual printouts on both 32-bit and 64-bit kernels, but
>> they still seem okay.
>>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>  arch/x86/include/asm/uaccess.h |  2 +-
>>  arch/x86/kernel/head_32.S      | 49 +++++-------------------------------------
>>  arch/x86/kernel/head_64.S      | 45 ++------------------------------------
>>  arch/x86/mm/extable.c          | 29 ++++++++++++++++++++-----
>>  4 files changed, 32 insertions(+), 93 deletions(-)
>
> ...
>
>> @@ -99,21 +101,38 @@ int __init early_fixup_exception(struct pt_regs *regs, int trapnr)
>>
>>       /* Ignore early NMIs. */
>>       if (trapnr == X86_TRAP_NMI)
>> -             return 1;
>> +             return;
>> +
>> +     if (early_recursion_flag > 2)
>> +             goto halt_loop;
>> +
>> +     if (regs->cs != __KERNEL_CS)
>> +             goto fail;
>>
>>       e = search_exception_tables(regs->ip);
>>       if (!e)
>> -             return 0;
>> +             goto fail;
>>
>>       new_ip  = ex_fixup_addr(e);
>>       handler = ex_fixup_handler(e);
>>
>>       /* special handling not supported during early boot */
>>       if (handler != ex_handler_default)
>> -             return 0;
>> +             goto fail;
>>
>>       regs->ip = new_ip;
>> -     return 1;
>> +     return;
>> +
>> +fail:
>> +     early_printk("PANIC: early exception 0x%02x IP %lx:%lx error %lx cr2 0x%lx\n",
>> +                  (unsigned)trapnr, (unsigned long)regs->cs, regs->ip,
>> +                  regs->orig_ax, read_cr2());
>> +
>> +     show_regs(regs);
>
> To make this even better, it could be something called early_show_regs()
> or so and be a simplified version of __show_regs() on both bitness but
> which calls early_printk().
>
> This way you'll be able to get out stuff to the console as early as
> possible.
>
> Btw, you don't need to dump rIP, CR2, etc in the PANIC message above
> since you're going to early_show_regs() anyway.

Given that I this isn't really a regression with my patches (it
probably never worked much better on 32-bit and the regs never would
have shown at all on 64-bit), I propose a different approach: make
printk work earlier.  Something like:

if (early) {
    early_printk(args);
}

or early_vprintk or whatever.

If the cost of a branch mattered, this could be alternative-patched
out later on, but that seems silly.  I also bet that a more sensible
fallback could be created in which printk would try to use an early
console if there's no real console.

--Andy

[toc] | [prev] | [next] | [standalone]

#1370050 — Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception

On Sat, Apr 02, 2016 at 01:13:37PM -0700, Andy Lutomirski wrote:
> Given that I this isn't really a regression with my patches (it
> probably never worked much better on 32-bit and the regs never would
> have shown at all on 64-bit),

You're right. That thing calls printk *and* early_printk, WTF:

#ifdef CONFIG_EARLY_PRINTK

	call early_printk
	...

	call dump_stack

	...

	call __print_symbol

those last two call printk. Great.

> I propose a different approach: make
> printk work earlier.  Something like:
> 
> if (early) {
>     early_printk(args);
> }
> 
> or early_vprintk or whatever.
> 
> If the cost of a branch mattered, this could be alternative-patched
> out later on, but that seems silly.  I also bet that a more sensible
> fallback could be created in which printk would try to use an early
> console if there's no real console.

So how about this:

printk() does

	vprintk_func = this_cpu_read(printk_func);

and that's

DEFINE_PER_CPU(printk_func_t, printk_func) = vprintk_default

I guess we can make that function be early_printk-something and once
printk is initialized, we overwrite it with vprintk_default.

Elegant and no need for if branches and alternatives.

Hmmm.

-- 
Regards/Gruss,
    Boris.

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

[toc] | [prev] | [next] | [standalone]

#1370053 — Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception

[cc Jan Kara]

On Sat, Apr 2, 2016 at 1:47 PM, Borislav Petkov <bp@alien8.de> wrote:
> On Sat, Apr 02, 2016 at 01:13:37PM -0700, Andy Lutomirski wrote:
>> Given that I this isn't really a regression with my patches (it
>> probably never worked much better on 32-bit and the regs never would
>> have shown at all on 64-bit),
>
> You're right. That thing calls printk *and* early_printk, WTF:
>
> #ifdef CONFIG_EARLY_PRINTK
>
>         call early_printk
>         ...
>
>         call dump_stack
>
>         ...
>
>         call __print_symbol
>
> those last two call printk. Great.
>
>> I propose a different approach: make
>> printk work earlier.  Something like:
>>
>> if (early) {
>>     early_printk(args);
>> }
>>
>> or early_vprintk or whatever.
>>
>> If the cost of a branch mattered, this could be alternative-patched
>> out later on, but that seems silly.  I also bet that a more sensible
>> fallback could be created in which printk would try to use an early
>> console if there's no real console.
>
> So how about this:
>
> printk() does
>
>         vprintk_func = this_cpu_read(printk_func);
>
> and that's
>
> DEFINE_PER_CPU(printk_func_t, printk_func) = vprintk_default
>
> I guess we can make that function be early_printk-something and once
> printk is initialized, we overwrite it with vprintk_default.
>
> Elegant and no need for if branches and alternatives.
>
> Hmmm.

Jan, IIRC you were looking at printk recently-ish.  Any thoughts here?

--Andy

[toc] | [prev] | [next] | [standalone]

#1370581 — Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception

On Sat 02-04-16 13:58:19, Andy Lutomirski wrote:
> [cc Jan Kara]
> 
> On Sat, Apr 2, 2016 at 1:47 PM, Borislav Petkov <bp@alien8.de> wrote:
> > On Sat, Apr 02, 2016 at 01:13:37PM -0700, Andy Lutomirski wrote:
> >> Given that I this isn't really a regression with my patches (it
> >> probably never worked much better on 32-bit and the regs never would
> >> have shown at all on 64-bit),
> >
> > You're right. That thing calls printk *and* early_printk, WTF:
> >
> > #ifdef CONFIG_EARLY_PRINTK
> >
> >         call early_printk
> >         ...
> >
> >         call dump_stack
> >
> >         ...
> >
> >         call __print_symbol
> >
> > those last two call printk. Great.
> >
> >> I propose a different approach: make
> >> printk work earlier.  Something like:
> >>
> >> if (early) {
> >>     early_printk(args);
> >> }
> >>
> >> or early_vprintk or whatever.
> >>
> >> If the cost of a branch mattered, this could be alternative-patched
> >> out later on, but that seems silly.  I also bet that a more sensible
> >> fallback could be created in which printk would try to use an early
> >> console if there's no real console.
> >
> > So how about this:
> >
> > printk() does
> >
> >         vprintk_func = this_cpu_read(printk_func);
> >
> > and that's
> >
> > DEFINE_PER_CPU(printk_func_t, printk_func) = vprintk_default
> >
> > I guess we can make that function be early_printk-something and once
> > printk is initialized, we overwrite it with vprintk_default.
> >
> > Elegant and no need for if branches and alternatives.
> >
> > Hmmm.
> 
> Jan, IIRC you were looking at printk recently-ish.  Any thoughts here?

Sounds like a good idea to me. I've also consulted this with Petr Mladek
(added to CC) who is using printk_func per-cpu variable in his
printk-from-NMI patches and he also doesn't see a problem with this.

I was just wondering about one thing - this way we add more early printks
if I understand your intention right. Are we guaranteed that they happen
only from a single CPU? Because currently there is no locking in
early_printk() and thus we can end up writing to early console several
messages in parallel from different CPUs. Not sure what's going to happen
in that case...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [next] | [standalone]

#1370599 — Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception

On Mon, Apr 04, 2016 at 01:52:06PM +0200, Jan Kara wrote:
> Sounds like a good idea to me. I've also consulted this with Petr Mladek
> (added to CC) who is using printk_func per-cpu variable in his
> printk-from-NMI patches and he also doesn't see a problem with this.

There's a few printk() variants that do not go through this; which means
they're broken for a number of cases, including the kdb printk
redirection, this NMI stuff etc.

> I was just wondering about one thing - this way we add more early printks
> if I understand your intention right. Are we guaranteed that they happen
> only from a single CPU? Because currently there is no locking in
> early_printk() and thus we can end up writing to early console several
> messages in parallel from different CPUs. Not sure what's going to happen
> in that case...

You get lovely per char interleaving on you serial line ;-)

What I've done in the past was something like the below; that way you
only get the normal task->softirq->irq->nmi nesting, which is mostly
decipherable.

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index bfbf284e4218..c4c3269ff104 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1907,17 +1907,36 @@ struct console *early_console;
 asmlinkage __visible void early_printk(const char *fmt, ...)
 {
 	va_list ap;
+	static int print_cpu = -1;
 	char buf[512];
-	int n;
+	int n, cpu;
 
 	if (!early_console)
 		return;
 
+	preempt_disable();
+	cpu = raw_smp_processor_id();
+	for (;;) {
+		if (READ_ONCE(print_cpu) == cpu)
+			break;
+
+		if (READ_ONCE(print_cpu) == -1 &&
+		    cmpxchg(&print_cpu, -1, cpu) == -1) {
+			cpu = -1;
+			break;
+		}
+
+		cpu_relax();
+	}
+
 	va_start(ap, fmt);
 	n = vscnprintf(buf, sizeof(buf), fmt, ap);
 	va_end(ap);
 
 	early_console->write(early_console, buf, n);
+
+	smp_store_release(&print_cpu, cpu);
+	preempt_enable();
 }
 #endif
 

[toc] | [prev] | [next] | [standalone]

#1370701 — Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception

On 4/4/2016 8:32 AM, Andy Lutomirski wrote:
>
> Adding locking would be easy enough, wouldn't it?
>
> But do any platforms really boot a second CPU before switching to real
> printk?  Given that I see all the smpboot stuff in dmesg, I guess real
> printk happens first.  I admit I haven't actually checked.

adding locking also makes things more fragile in terms of getting the last thing out
before you go down in flaming death....

until it's a proven problem, this early, get the message out at all is more important
than getting it out perfectly, sometimes.

[toc] | [prev] | [next] | [standalone]

#1370703 — Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception

On Apr 4, 2016 4:51 AM, "Jan Kara" <jack@suse.cz> wrote:
>
> On Sat 02-04-16 13:58:19, Andy Lutomirski wrote:
> > [cc Jan Kara]
> >
> > On Sat, Apr 2, 2016 at 1:47 PM, Borislav Petkov <bp@alien8.de> wrote:
> > > On Sat, Apr 02, 2016 at 01:13:37PM -0700, Andy Lutomirski wrote:
> > >> Given that I this isn't really a regression with my patches (it
> > >> probably never worked much better on 32-bit and the regs never would
> > >> have shown at all on 64-bit),
> > >
> > > You're right. That thing calls printk *and* early_printk, WTF:
> > >
> > > #ifdef CONFIG_EARLY_PRINTK
> > >
> > >         call early_printk
> > >         ...
> > >
> > >         call dump_stack
> > >
> > >         ...
> > >
> > >         call __print_symbol
> > >
> > > those last two call printk. Great.
> > >
> > >> I propose a different approach: make
> > >> printk work earlier.  Something like:
> > >>
> > >> if (early) {
> > >>     early_printk(args);
> > >> }
> > >>
> > >> or early_vprintk or whatever.
> > >>
> > >> If the cost of a branch mattered, this could be alternative-patched
> > >> out later on, but that seems silly.  I also bet that a more sensible
> > >> fallback could be created in which printk would try to use an early
> > >> console if there's no real console.
> > >
> > > So how about this:
> > >
> > > printk() does
> > >
> > >         vprintk_func = this_cpu_read(printk_func);
> > >
> > > and that's
> > >
> > > DEFINE_PER_CPU(printk_func_t, printk_func) = vprintk_default
> > >
> > > I guess we can make that function be early_printk-something and once
> > > printk is initialized, we overwrite it with vprintk_default.
> > >
> > > Elegant and no need for if branches and alternatives.
> > >
> > > Hmmm.
> >
> > Jan, IIRC you were looking at printk recently-ish.  Any thoughts here?
>
> Sounds like a good idea to me. I've also consulted this with Petr Mladek
> (added to CC) who is using printk_func per-cpu variable in his
> printk-from-NMI patches and he also doesn't see a problem with this.
>
> I was just wondering about one thing - this way we add more early printks
> if I understand your intention right. Are we guaranteed that they happen
> only from a single CPU? Because currently there is no locking in
> early_printk() and thus we can end up writing to early console several
> messages in parallel from different CPUs. Not sure what's going to happen
> in that case...

Adding locking would be easy enough, wouldn't it?

But do any platforms really boot a second CPU before switching to real
printk?  Given that I see all the smpboot stuff in dmesg, I guess real
printk happens first.  I admit I haven't actually checked.

--Andy

>
>                                                                 Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

[toc] | [prev] | [next] | [standalone]

#1370723 — Re: [PATCH v5 3/9] x86/head: Move early exception panic code into early_fixup_exception

On Mon, Apr 04, 2016 at 08:32:21AM -0700, Andy Lutomirski wrote:

> Adding locking would be easy enough, wouldn't it?

See patch in this thread..

> But do any platforms really boot a second CPU before switching to real
> printk? 

I _only_ use early_printk() as printk() is a quagmire of fail :-)

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web