Messages in this thread |  | | From | Wanpeng Li <> | Date | Mon, 31 Oct 2016 07:30:33 +0800 | Subject | Re: [PATCH v6 1/2] x86/msr: Add write msr notrace |
| |
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
|  |