| Message ID | 1477543122-4908-2-git-send-email-wanpeng.li@hotmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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)