All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] x86/asm/msr: Make wrmsrl() a function
@ 2015-07-23 19:14 Andy Lutomirski
  2015-07-23 19:14 ` [PATCH v2] x86/entry/32: Remove duplicate initialization of tss.ss1 Andy Lutomirski
  2015-08-23 11:45 ` [tip:x86/asm] x86/asm/msr: Make wrmsrl() a function tip-bot for Andy Lutomirski
  0 siblings, 2 replies; 8+ messages in thread
From: Andy Lutomirski @ 2015-07-23 19:14 UTC (permalink / raw)
  To: X86 ML, linux-kernel
  Cc: Brian Gerst, Steven Rostedt, Willy Tarreau, Borislav Petkov,
	Thomas Gleixner, Peter Zijlstra, Linus Torvalds, Andy Lutomirski

As of cf991de2f614 ("x86/asm/msr: Make wrmsrl_safe() a function"),
wrmsrl_safe is a function, but wrmsrl is still a macro.  The wrmsrl
macro performs invalid shifts if the value argument is 32 bits.
This makes it unnecessarily awkward to write code that puts an
unsigned long into an MSR.

To make this work, syscall_init needs tweaking to stop passing
a function pointer to wrmsrl.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---

Changes since v1: Fix one more warning.

arch/x86/include/asm/msr.h      | 6 ++++--
 arch/x86/include/asm/paravirt.h | 6 +++++-
 arch/x86/kernel/cpu/common.c    | 6 +++---
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index 131eec2ca137..714c80755dae 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -185,8 +185,10 @@ static inline void wrmsr(unsigned msr, unsigned low, unsigned high)
 #define rdmsrl(msr, val)			\
 	((val) = native_read_msr((msr)))
 
-#define wrmsrl(msr, val)						\
-	native_write_msr((msr), (u32)((u64)(val)), (u32)((u64)(val) >> 32))
+static inline void wrmsrl(unsigned msr, u64 val)
+{
+	native_write_msr(msr, (u32)val, (u32)(val >> 32));
+}
 
 /* wrmsr with exception handling */
 static inline int wrmsr_safe(unsigned msr, unsigned low, unsigned high)
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index c2be0375bcad..10d0596433f8 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -153,7 +153,11 @@ do {						\
 	val = paravirt_read_msr(msr, &_err);	\
 } while (0)
 
-#define wrmsrl(msr, val)	wrmsr(msr, (u32)((u64)(val)), ((u64)(val))>>32)
+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)
 
 /* rdmsr with exception handling */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index ffb0020ada5f..e2ed2513a51e 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1184,13 +1184,13 @@ void syscall_init(void)
 	 * set CS/DS but only a 32bit target. LSTAR sets the 64bit rip.
 	 */
 	wrmsrl(MSR_STAR,  ((u64)__USER32_CS)<<48  | ((u64)__KERNEL_CS)<<32);
-	wrmsrl(MSR_LSTAR, entry_SYSCALL_64);
+	wrmsrl(MSR_LSTAR, (unsigned long)entry_SYSCALL_64);
 
 #ifdef CONFIG_IA32_EMULATION
-	wrmsrl(MSR_CSTAR, entry_SYSCALL_compat);
+	wrmsrl(MSR_CSTAR, (unsigned long)entry_SYSCALL_compat);
 	enable_sep_cpu();
 #else
-	wrmsrl(MSR_CSTAR, ignore_sysret);
+	wrmsrl(MSR_CSTAR, (unsigned long)ignore_sysret);
 	wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)GDT_ENTRY_INVALID_SEG);
 	wrmsrl_safe(MSR_IA32_SYSENTER_ESP, 0ULL);
 	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, 0ULL);
-- 
2.4.3


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

* [PATCH v2] x86/entry/32: Remove duplicate initialization of tss.ss1
  2015-07-23 19:14 [PATCH v2] x86/asm/msr: Make wrmsrl() a function Andy Lutomirski
@ 2015-07-23 19:14 ` Andy Lutomirski
  2015-08-23 11:26   ` Ingo Molnar
  2015-08-23 11:45 ` [tip:x86/asm] x86/asm/msr: Make wrmsrl() a function tip-bot for Andy Lutomirski
  1 sibling, 1 reply; 8+ messages in thread
From: Andy Lutomirski @ 2015-07-23 19:14 UTC (permalink / raw)
  To: X86 ML, linux-kernel
  Cc: Brian Gerst, Steven Rostedt, Willy Tarreau, Borislav Petkov,
	Thomas Gleixner, Peter Zijlstra, Linus Torvalds, Andy Lutomirski

It's statically initialized, so we don't need to dynamically
initialize it too.

Reported-by: Brian Gerst <brgerst@gmail.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---

Changes since v1: Delete the code :)

arch/x86/kernel/cpu/common.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index e2ed2513a51e..e08eee98a5f8 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1005,14 +1005,6 @@ void enable_sep_cpu(void)
 	if (IS_ENABLED(CONFIG_X86_32) && !boot_cpu_has(X86_FEATURE_SEP))
 		goto out;
 
-#ifdef CONFIG_X86_32
-	/*
-	 * We cache MSR_IA32_SYSENTER_CS's value in the TSS's ss1 field --
-	 * see the big comment in struct x86_hw_tss's definition.
-	 */
-	tss->x86_tss.ss1 = __KERNEL_CS;
-#endif
-
 	wrmsrl_safe(MSR_IA32_SYSENTER_CS, __KERNEL_CS);
 	wrmsrl_safe(MSR_IA32_SYSENTER_ESP,
 		    (unsigned long)tss +
-- 
2.4.3


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

* Re: [PATCH v2] x86/entry/32: Remove duplicate initialization of tss.ss1
  2015-07-23 19:14 ` [PATCH v2] x86/entry/32: Remove duplicate initialization of tss.ss1 Andy Lutomirski
@ 2015-08-23 11:26   ` Ingo Molnar
  2015-08-23 15:43     ` Andy Lutomirski
  0 siblings, 1 reply; 8+ messages in thread
From: Ingo Molnar @ 2015-08-23 11:26 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: X86 ML, linux-kernel, Brian Gerst, Steven Rostedt, Willy Tarreau,
	Borislav Petkov, Thomas Gleixner, Peter Zijlstra, Linus Torvalds


* Andy Lutomirski <luto@kernel.org> wrote:

> It's statically initialized, so we don't need to dynamically
> initialize it too.
> 
> Reported-by: Brian Gerst <brgerst@gmail.com>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
> 
> Changes since v1: Delete the code :)
> 
> arch/x86/kernel/cpu/common.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index e2ed2513a51e..e08eee98a5f8 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -1005,14 +1005,6 @@ void enable_sep_cpu(void)
>  	if (IS_ENABLED(CONFIG_X86_32) && !boot_cpu_has(X86_FEATURE_SEP))
>  		goto out;
>  
> -#ifdef CONFIG_X86_32
> -	/*
> -	 * We cache MSR_IA32_SYSENTER_CS's value in the TSS's ss1 field --
> -	 * see the big comment in struct x86_hw_tss's definition.
> -	 */
> -	tss->x86_tss.ss1 = __KERNEL_CS;
> -#endif
> -
>  	wrmsrl_safe(MSR_IA32_SYSENTER_CS, __KERNEL_CS);
>  	wrmsrl_safe(MSR_IA32_SYSENTER_ESP,
>  		    (unsigned long)tss +

So this code changed substantially in tip:x86/asm - do we still need this patch?

Thanks,

	Ingo

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

* [tip:x86/asm] x86/asm/msr: Make wrmsrl() a function
  2015-07-23 19:14 [PATCH v2] x86/asm/msr: Make wrmsrl() a function Andy Lutomirski
  2015-07-23 19:14 ` [PATCH v2] x86/entry/32: Remove duplicate initialization of tss.ss1 Andy Lutomirski
@ 2015-08-23 11:45 ` tip-bot for Andy Lutomirski
  2015-08-23 19:23   ` H. Peter Anvin
  1 sibling, 1 reply; 8+ messages in thread
From: tip-bot for Andy Lutomirski @ 2015-08-23 11:45 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: dvlasenk, brgerst, bp, rostedt, hpa, luto, linux-kernel, luto,
	torvalds, tglx, mingo, peterz, w

Commit-ID:  47edb65178cb7056c2eea0b6c41a7d8c84547192
Gitweb:     http://git.kernel.org/tip/47edb65178cb7056c2eea0b6c41a7d8c84547192
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Thu, 23 Jul 2015 12:14:40 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sun, 23 Aug 2015 13:25:38 +0200

x86/asm/msr: Make wrmsrl() a function

As of cf991de2f614 ("x86/asm/msr: Make wrmsrl_safe() a
function"), wrmsrl_safe is a function, but wrmsrl is still a
macro.  The wrmsrl macro performs invalid shifts if the value
argument is 32 bits. This makes it unnecessarily awkward to
write code that puts an unsigned long into an MSR.

To make this work, syscall_init needs tweaking to stop passing
a function pointer to wrmsrl.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Willy Tarreau <w@1wt.eu>
Link: http://lkml.kernel.org/r/690f0c629a1085d054e2d1ef3da073cfb3f7db92.1437678821.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/msr.h      | 6 ++++--
 arch/x86/include/asm/paravirt.h | 6 +++++-
 arch/x86/kernel/cpu/common.c    | 6 +++---
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index 54e9f08..77d8b28 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -188,8 +188,10 @@ static inline void wrmsr(unsigned msr, unsigned low, unsigned high)
 #define rdmsrl(msr, val)			\
 	((val) = native_read_msr((msr)))
 
-#define wrmsrl(msr, val)						\
-	native_write_msr((msr), (u32)((u64)(val)), (u32)((u64)(val) >> 32))
+static inline void wrmsrl(unsigned msr, u64 val)
+{
+	native_write_msr(msr, (u32)val, (u32)(val >> 32));
+}
 
 /* wrmsr with exception handling */
 static inline int wrmsr_safe(unsigned msr, unsigned low, unsigned high)
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index c2be037..10d0596 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -153,7 +153,11 @@ do {						\
 	val = paravirt_read_msr(msr, &_err);	\
 } while (0)
 
-#define wrmsrl(msr, val)	wrmsr(msr, (u32)((u64)(val)), ((u64)(val))>>32)
+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)
 
 /* rdmsr with exception handling */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index cb9e5df..b128808 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1185,10 +1185,10 @@ void syscall_init(void)
 	 * set CS/DS but only a 32bit target. LSTAR sets the 64bit rip.
 	 */
 	wrmsrl(MSR_STAR,  ((u64)__USER32_CS)<<48  | ((u64)__KERNEL_CS)<<32);
-	wrmsrl(MSR_LSTAR, entry_SYSCALL_64);
+	wrmsrl(MSR_LSTAR, (unsigned long)entry_SYSCALL_64);
 
 #ifdef CONFIG_IA32_EMULATION
-	wrmsrl(MSR_CSTAR, entry_SYSCALL_compat);
+	wrmsrl(MSR_CSTAR, (unsigned long)entry_SYSCALL_compat);
 	/*
 	 * This only works on Intel CPUs.
 	 * On AMD CPUs these MSRs are 32-bit, CPU truncates MSR_IA32_SYSENTER_EIP.
@@ -1199,7 +1199,7 @@ void syscall_init(void)
 	wrmsrl_safe(MSR_IA32_SYSENTER_ESP, 0ULL);
 	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)entry_SYSENTER_compat);
 #else
-	wrmsrl(MSR_CSTAR, ignore_sysret);
+	wrmsrl(MSR_CSTAR, (unsigned long)ignore_sysret);
 	wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)GDT_ENTRY_INVALID_SEG);
 	wrmsrl_safe(MSR_IA32_SYSENTER_ESP, 0ULL);
 	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, 0ULL);

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

* Re: [PATCH v2] x86/entry/32: Remove duplicate initialization of tss.ss1
  2015-08-23 11:26   ` Ingo Molnar
@ 2015-08-23 15:43     ` Andy Lutomirski
  2015-08-24  6:48       ` Ingo Molnar
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Lutomirski @ 2015-08-23 15:43 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, X86 ML, linux-kernel, Brian Gerst,
	Steven Rostedt, Willy Tarreau, Borislav Petkov, Thomas Gleixner,
	Peter Zijlstra, Linus Torvalds

On Sun, Aug 23, 2015 at 4:26 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Andy Lutomirski <luto@kernel.org> wrote:
>
>> It's statically initialized, so we don't need to dynamically
>> initialize it too.
>>
>> Reported-by: Brian Gerst <brgerst@gmail.com>
>> Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> ---
>>
>> Changes since v1: Delete the code :)
>>
>> arch/x86/kernel/cpu/common.c | 8 --------
>>  1 file changed, 8 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
>> index e2ed2513a51e..e08eee98a5f8 100644
>> --- a/arch/x86/kernel/cpu/common.c
>> +++ b/arch/x86/kernel/cpu/common.c
>> @@ -1005,14 +1005,6 @@ void enable_sep_cpu(void)
>>       if (IS_ENABLED(CONFIG_X86_32) && !boot_cpu_has(X86_FEATURE_SEP))
>>               goto out;
>>
>> -#ifdef CONFIG_X86_32
>> -     /*
>> -      * We cache MSR_IA32_SYSENTER_CS's value in the TSS's ss1 field --
>> -      * see the big comment in struct x86_hw_tss's definition.
>> -      */
>> -     tss->x86_tss.ss1 = __KERNEL_CS;
>> -#endif
>> -
>>       wrmsrl_safe(MSR_IA32_SYSENTER_CS, __KERNEL_CS);
>>       wrmsrl_safe(MSR_IA32_SYSENTER_ESP,
>>                   (unsigned long)tss +
>
> So this code changed substantially in tip:x86/asm - do we still need this patch?
>

Yes, although I think it's actually the other way around -- I think
this patch may have applied on top of something that never made it
into tip/x86/asm.  I can re-check or I could just rebase the patch (or
you could apply it with the obvious fixup).

It's obviously not a critical fix.

--Andy

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

* Re: [tip:x86/asm] x86/asm/msr: Make wrmsrl() a function
  2015-08-23 11:45 ` [tip:x86/asm] x86/asm/msr: Make wrmsrl() a function tip-bot for Andy Lutomirski
@ 2015-08-23 19:23   ` H. Peter Anvin
  2015-08-24 18:11     ` Andy Lutomirski
  0 siblings, 1 reply; 8+ messages in thread
From: H. Peter Anvin @ 2015-08-23 19:23 UTC (permalink / raw)
  To: linux-kernel, luto, peterz, w, mingo, torvalds, tglx, bp,
	dvlasenk, brgerst, luto, rostedt, linux-tip-commits

On 08/23/2015 04:45 AM, tip-bot for Andy Lutomirski wrote:
> Commit-ID:  47edb65178cb7056c2eea0b6c41a7d8c84547192
> Gitweb:     http://git.kernel.org/tip/47edb65178cb7056c2eea0b6c41a7d8c84547192
> Author:     Andy Lutomirski <luto@kernel.org>
> AuthorDate: Thu, 23 Jul 2015 12:14:40 -0700
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Sun, 23 Aug 2015 13:25:38 +0200
> 
> x86/asm/msr: Make wrmsrl() a function
> 
> As of cf991de2f614 ("x86/asm/msr: Make wrmsrl_safe() a
> function"), wrmsrl_safe is a function, but wrmsrl is still a
> macro.  The wrmsrl macro performs invalid shifts if the value
> argument is 32 bits. This makes it unnecessarily awkward to
> write code that puts an unsigned long into an MSR.
> 

Looking at this: where do you see an invalid shift?  Everywhere I can
see we do the proper casting.  Still not side effect free, though.

	-hpa


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

* Re: [PATCH v2] x86/entry/32: Remove duplicate initialization of tss.ss1
  2015-08-23 15:43     ` Andy Lutomirski
@ 2015-08-24  6:48       ` Ingo Molnar
  0 siblings, 0 replies; 8+ messages in thread
From: Ingo Molnar @ 2015-08-24  6:48 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Andy Lutomirski, X86 ML, linux-kernel, Brian Gerst,
	Steven Rostedt, Willy Tarreau, Borislav Petkov, Thomas Gleixner,
	Peter Zijlstra, Linus Torvalds


* Andy Lutomirski <luto@amacapital.net> wrote:

> On Sun, Aug 23, 2015 at 4:26 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Andy Lutomirski <luto@kernel.org> wrote:
> >
> >> It's statically initialized, so we don't need to dynamically
> >> initialize it too.
> >>
> >> Reported-by: Brian Gerst <brgerst@gmail.com>
> >> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> >> ---
> >>
> >> Changes since v1: Delete the code :)
> >>
> >> arch/x86/kernel/cpu/common.c | 8 --------
> >>  1 file changed, 8 deletions(-)
> >>
> >> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> >> index e2ed2513a51e..e08eee98a5f8 100644
> >> --- a/arch/x86/kernel/cpu/common.c
> >> +++ b/arch/x86/kernel/cpu/common.c
> >> @@ -1005,14 +1005,6 @@ void enable_sep_cpu(void)
> >>       if (IS_ENABLED(CONFIG_X86_32) && !boot_cpu_has(X86_FEATURE_SEP))
> >>               goto out;
> >>
> >> -#ifdef CONFIG_X86_32
> >> -     /*
> >> -      * We cache MSR_IA32_SYSENTER_CS's value in the TSS's ss1 field --
> >> -      * see the big comment in struct x86_hw_tss's definition.
> >> -      */
> >> -     tss->x86_tss.ss1 = __KERNEL_CS;
> >> -#endif
> >> -
> >>       wrmsrl_safe(MSR_IA32_SYSENTER_CS, __KERNEL_CS);
> >>       wrmsrl_safe(MSR_IA32_SYSENTER_ESP,
> >>                   (unsigned long)tss +
> >
> > So this code changed substantially in tip:x86/asm - do we still need this patch?
> >
> 
> Yes, although I think it's actually the other way around -- I think
> this patch may have applied on top of something that never made it
> into tip/x86/asm.  I can re-check or I could just rebase the patch (or
> you could apply it with the obvious fixup).

Yeah, so I noticed that it fell amongst the cracks - please incorporate it into 
your next (v4.4 targeted) series once we get to that, so it doesn't get lost.

> It's obviously not a critical fix.

Yeah.

Thanks,

	Ingo

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

* Re: [tip:x86/asm] x86/asm/msr: Make wrmsrl() a function
  2015-08-23 19:23   ` H. Peter Anvin
@ 2015-08-24 18:11     ` Andy Lutomirski
  0 siblings, 0 replies; 8+ messages in thread
From: Andy Lutomirski @ 2015-08-24 18:11 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: linux-kernel, Peter Zijlstra, Willy Tarreau, Ingo Molnar,
	Linus Torvalds, Thomas Gleixner, Borislav Petkov, Denys Vlasenko,
	Brian Gerst, Andrew Lutomirski, Steven Rostedt,
	linux-tip-commits

On Sun, Aug 23, 2015 at 12:23 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 08/23/2015 04:45 AM, tip-bot for Andy Lutomirski wrote:
>> Commit-ID:  47edb65178cb7056c2eea0b6c41a7d8c84547192
>> Gitweb:     http://git.kernel.org/tip/47edb65178cb7056c2eea0b6c41a7d8c84547192
>> Author:     Andy Lutomirski <luto@kernel.org>
>> AuthorDate: Thu, 23 Jul 2015 12:14:40 -0700
>> Committer:  Ingo Molnar <mingo@kernel.org>
>> CommitDate: Sun, 23 Aug 2015 13:25:38 +0200
>>
>> x86/asm/msr: Make wrmsrl() a function
>>
>> As of cf991de2f614 ("x86/asm/msr: Make wrmsrl_safe() a
>> function"), wrmsrl_safe is a function, but wrmsrl is still a
>> macro.  The wrmsrl macro performs invalid shifts if the value
>> argument is 32 bits. This makes it unnecessarily awkward to
>> write code that puts an unsigned long into an MSR.
>>
>
> Looking at this: where do you see an invalid shift?  Everywhere I can
> see we do the proper casting.  Still not side effect free, though.

I'm having trouble reproducing it right now, but IIRC at one point I
got wrmsrl to warn that a constant turned into zero as a result of a
shift.

--Andy

>
>         -hpa
>



-- 
Andy Lutomirski
AMA Capital Management, LLC

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

end of thread, other threads:[~2015-08-24 18:11 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-23 19:14 [PATCH v2] x86/asm/msr: Make wrmsrl() a function Andy Lutomirski
2015-07-23 19:14 ` [PATCH v2] x86/entry/32: Remove duplicate initialization of tss.ss1 Andy Lutomirski
2015-08-23 11:26   ` Ingo Molnar
2015-08-23 15:43     ` Andy Lutomirski
2015-08-24  6:48       ` Ingo Molnar
2015-08-23 11:45 ` [tip:x86/asm] x86/asm/msr: Make wrmsrl() a function tip-bot for Andy Lutomirski
2015-08-23 19:23   ` H. Peter Anvin
2015-08-24 18:11     ` Andy Lutomirski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.