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

Groups > linux.kernel > #1510046 > unrolled thread

[PATCH v6 1/2] x86/msr: Add write msr notrace

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

  [PATCH v6 1/2] x86/msr: Add write msr notrace  Wanpeng Li <kernellwp@gmail.com> - 2016-10-27 06:40 +0200
    Re: [PATCH v6 1/2] x86/msr: Add write msr notrace Borislav Petkov <bp@alien8.de> - 2016-10-28 18:50 +0200
      Re: [PATCH v6 1/2] x86/msr: Add write msr notrace Wanpeng Li <kernellwp@gmail.com> - 2016-10-31 00:40 +0100
        Re: [PATCH v6 1/2] x86/msr: Add write msr notrace Borislav Petkov <bp@alien8.de> - 2016-10-31 00:50 +0100
          Re: [PATCH v6 1/2] x86/msr: Add write msr notrace Wanpeng Li <kernellwp@gmail.com> - 2016-10-31 02:50 +0100

#1510046 — [PATCH v6 1/2] x86/msr: Add write msr notrace

From: Wanpeng Li <wanpeng.li@hotmail.com>

Add write msr notrace, it will be used by later patch.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/include/asm/msr.h | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
index b5fee97..eec29a7 100644
--- a/arch/x86/include/asm/msr.h
+++ b/arch/x86/include/asm/msr.h
@@ -115,17 +115,29 @@ static inline unsigned long long native_read_msr_safe(unsigned int msr,
 }
 
 /* Can be uninlined because referenced by paravirt */
-notrace static inline void native_write_msr(unsigned int msr,
+notrace static inline void __native_write_msr_notrace(unsigned int msr,
 					    unsigned low, unsigned high)
 {
 	asm volatile("1: wrmsr\n"
 		     "2:\n"
 		     _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_wrmsr_unsafe)
 		     : : "c" (msr), "a"(low), "d" (high) : "memory");
+}
+
+/* Can be uninlined because referenced by paravirt */
+notrace static inline void native_write_msr(unsigned int msr,
+					    unsigned low, unsigned high)
+{
+	__native_write_msr_notrace(msr, low, high);
 	if (msr_tracepoint_active(__tracepoint_write_msr))
 		do_trace_write_msr(msr, ((u64)high << 32 | low), 0);
 }
 
+static inline void wrmsr_notrace(unsigned msr, unsigned low, unsigned high)
+{
+	__native_write_msr_notrace(msr, low, high);
+}
+
 /* Can be uninlined because referenced by paravirt */
 notrace static inline int native_write_msr_safe(unsigned int msr,
 					unsigned low, unsigned high)
-- 
1.9.1

[toc] | [next] | [standalone]

#1511230

On Thu, Oct 27, 2016 at 12:38:41PM +0800, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> Add write msr notrace, it will be used by later patch.
> 
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  arch/x86/include/asm/msr.h | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
> index b5fee97..eec29a7 100644
> --- a/arch/x86/include/asm/msr.h
> +++ b/arch/x86/include/asm/msr.h
> @@ -115,17 +115,29 @@ static inline unsigned long long native_read_msr_safe(unsigned int msr,
>  }
>  
>  /* Can be uninlined because referenced by paravirt */
> -notrace static inline void native_write_msr(unsigned int msr,
> +notrace static inline void __native_write_msr_notrace(unsigned int msr,
>  					    unsigned low, unsigned high)
					    ^^^^^^^
Align arguments on an opening brace.

Also, please fix that in a patch ontop of this one:

WARNING: storage class should be at the beginning of the declaration
#43: FILE: arch/x86/include/asm/msr.h:118:
+notrace static inline void __native_write_msr_notrace(unsigned int msr,

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
#54: FILE: arch/x86/include/asm/msr.h:129:
+                                           unsigned low, unsigned high)

And because we know what those are, you can convert them directly to u32.

IOW, the end result should be something like this:

static inline void notrace
__native_write_msr_notrace(unsigned int msr, u32 low, u32 high)

And yes, I suggested using the "_notrace" suffix for the name but then
it would look funny if we end up using it in code.

So maybe we should make that lower-level helper simply:

static inline void notrace
__native_write_msr(unsigned int msr, u32 low, u32 high)

to denote that it does purely the WRMSR operation and nothing else.

Yap, that looks the cleanest to me.

Thanks.

-- 
Regards/Gruss,
    Boris.

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

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

#1512238

2016-10-29 0:47 GMT+08:00 Borislav Petkov <bp@alien8.de>:
[...]
>>
>>  /* Can be uninlined because referenced by paravirt */
>> -notrace static inline void native_write_msr(unsigned int msr,
>> +notrace static inline void __native_write_msr_notrace(unsigned int msr,
>>                                           unsigned low, unsigned high)
>                                             ^^^^^^^
> Align arguments on an opening brace.

Other functions like native_write_msr() and native_write_msr_safe()
etc are also not aligned, so your suggestion maybe result in
inconsistent.

>
> Also, please fix that in a patch ontop of this one:
>
> WARNING: storage class should be at the beginning of the declaration
> #43: FILE: arch/x86/include/asm/msr.h:118:
> +notrace static inline void __native_write_msr_notrace(unsigned int msr,
>
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> #54: FILE: arch/x86/include/asm/msr.h:129:
> +                                           unsigned low, unsigned high)
>
> And because we know what those are, you can convert them directly to u32.

Ditto.

>
> IOW, the end result should be something like this:
>
> static inline void notrace
> __native_write_msr_notrace(unsigned int msr, u32 low, u32 high)
>
> And yes, I suggested using the "_notrace" suffix for the name but then
> it would look funny if we end up using it in code.
>
> So maybe we should make that lower-level helper simply:
>
> static inline void notrace
> __native_write_msr(unsigned int msr, u32 low, u32 high)
>
> to denote that it does purely the WRMSR operation and nothing else.
>
> Yap, that looks the cleanest to me.

Agreed.

Regards,
Wanpeng Li

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

#1512240

On Mon, Oct 31, 2016 at 07:30:33AM +0800, Wanpeng Li wrote:
> Other functions like native_write_msr() and native_write_msr_safe()
> etc are also not aligned, so your suggestion maybe result in
> inconsistent.

So align them too, while you're at it.

> > And because we know what those are, you can convert them directly to u32.
> 
> Ditto.

You could convert the rest to u32/u64 in another patch, ontop or if you
don't feel like it, I can take care of it.

Thanks.

-- 
Regards/Gruss,
    Boris.

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

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

#1512282

2016-10-31 7:46 GMT+08:00 Borislav Petkov <bp@alien8.de>:
> On Mon, Oct 31, 2016 at 07:30:33AM +0800, Wanpeng Li wrote:
>> Other functions like native_write_msr() and native_write_msr_safe()
>> etc are also not aligned, so your suggestion maybe result in
>> inconsistent.
>
> So align them too, while you're at it.
>
>> > And because we know what those are, you can convert them directly to u32.
>>
>> Ditto.
>
> You could convert the rest to u32/u64 in another patch, ontop or if you
> don't feel like it, I can take care of it.

I just fix my own "storage class should be at the beginning of the
declaration" coding style issue. Please feel free to handle align/u32
issues which existing before the patchset if you like. :)

Regards,
Wanpeng Li

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web