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

Groups > linux.kernel > #1211608 > unrolled thread

[tip:x86/asm] x86/asm/msr: Make wrmsrl() a function

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [tip:x86/asm] x86/asm/msr: Make wrmsrl() a function tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2015-08-23 13:50 +0200
    Re: [tip:x86/asm] x86/asm/msr: Make wrmsrl() a function "H. Peter Anvin" <hpa@zytor.com> - 2015-08-23 21:30 +0200
      Re: [tip:x86/asm] x86/asm/msr: Make wrmsrl() a function Andy Lutomirski <luto@amacapital.net> - 2015-08-24 20:20 +0200

#1211608 — [tip:x86/asm] x86/asm/msr: Make wrmsrl() a function

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);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1211663

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1212394

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web