| Message ID | 20220823234353.937002-1-pshier@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86/msr: add idle version of wrmsr trace | expand |
Dear FirstName LastName, pleae fix your MUA. On Tue, Aug 23, 2022 at 04:43:53PM -0700, FirstName LastName wrote: > From: Peter Shier <pshier@google.com> > > With commit bf5835bcdb963 ("intel_idle: Disable IBRS during long idle"), > enabling wrmsr trace with CONFIG_LOCKDEP causes "suspicious > rcu_dereference_check() usage" warning because do_trace_write_msr does not > use trace_write_msr_rcuidle. > No, the right thing here is to not do tracing at all. *sigh* I should go finish this series: https://lore.kernel.org/lkml/20220608142723.103523089@infradead.org/ --- diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 3e101719689a..6e82b2df29cb 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -187,12 +187,12 @@ static __cpuidle int intel_idle_ibrs(struct cpuidle_device *dev, int ret; if (smt_active) - wrmsrl(MSR_IA32_SPEC_CTRL, 0); + __native_wrmsr(MSR_IA32_SPEC_CTRL, 0); ret = __intel_idle(dev, drv, index); if (smt_active) - wrmsrl(MSR_IA32_SPEC_CTRL, spec_ctrl); + __native_wrmsr(MSR_IA32_SPEC_CTRL, spec_ctrl); return ret; }
On Wed, Aug 24, 2022 at 08:48:33AM +0200, Peter Zijlstra wrote: > diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c > index 3e101719689a..6e82b2df29cb 100644 > --- a/drivers/idle/intel_idle.c > +++ b/drivers/idle/intel_idle.c > @@ -187,12 +187,12 @@ static __cpuidle int intel_idle_ibrs(struct cpuidle_device *dev, > int ret; > > if (smt_active) > - wrmsrl(MSR_IA32_SPEC_CTRL, 0); > + __native_wrmsr(MSR_IA32_SPEC_CTRL, 0); > > ret = __intel_idle(dev, drv, index); > > if (smt_active) > - wrmsrl(MSR_IA32_SPEC_CTRL, spec_ctrl); > + __native_wrmsr(MSR_IA32_SPEC_CTRL, spec_ctrl); > > return ret; > } Clearly I should not be sending email before having wake-up-juice.. that should of course be native_wrmsr(SPEC_CTRL, val, 0) etc..
diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h index 65ec1965cd28..248cc58b7758 100644 --- a/arch/x86/include/asm/msr.h +++ b/arch/x86/include/asm/msr.h @@ -62,10 +62,12 @@ DECLARE_TRACEPOINT(read_msr); DECLARE_TRACEPOINT(write_msr); DECLARE_TRACEPOINT(rdpmc); extern void do_trace_write_msr(unsigned int msr, u64 val, int failed); +extern void do_trace_write_msr_idle(unsigned int msr, u64 val, int failed); extern void do_trace_read_msr(unsigned int msr, u64 val, int failed); extern void do_trace_rdpmc(unsigned int msr, u64 val, int failed); #else static inline void do_trace_write_msr(unsigned int msr, u64 val, int failed) {} +extern void do_trace_write_msr_idle(unsigned int msr, u64 val, int failed) {} static inline void do_trace_read_msr(unsigned int msr, u64 val, int failed) {} static inline void do_trace_rdpmc(unsigned int msr, u64 val, int failed) {} #endif @@ -148,6 +150,15 @@ native_write_msr(unsigned int msr, u32 low, u32 high) do_trace_write_msr(msr, ((u64)high << 32 | low), 0); } +static inline void notrace +native_write_msr_idle(unsigned int msr, u32 low, u32 high) +{ + __wrmsr(msr, low, high); + + if (tracepoint_enabled(write_msr)) + do_trace_write_msr_idle(msr, ((u64)high << 32 | low), 0); +} + /* Can be uninlined because referenced by paravirt */ static inline int notrace native_write_msr_safe(unsigned int msr, u32 low, u32 high) @@ -262,6 +273,11 @@ static inline void wrmsrl(unsigned int msr, u64 val) native_write_msr(msr, (u32)(val & 0xffffffffULL), (u32)(val >> 32)); } +static inline void wrmsrl_idle(unsigned int msr, u64 val) +{ + native_write_msr_idle(msr, (u32)(val & 0xffffffffULL), (u32)(val >> 32)); +} + /* wrmsr with exception handling */ static inline int wrmsr_safe(unsigned int msr, u32 low, u32 high) { diff --git a/arch/x86/lib/msr.c b/arch/x86/lib/msr.c index b09cd2ad426c..58fdf0f13850 100644 --- a/arch/x86/lib/msr.c +++ b/arch/x86/lib/msr.c @@ -121,6 +121,12 @@ void do_trace_write_msr(unsigned int msr, u64 val, int failed) EXPORT_SYMBOL(do_trace_write_msr); EXPORT_TRACEPOINT_SYMBOL(write_msr); +void do_trace_write_msr_idle(unsigned int msr, u64 val, int failed) +{ + trace_write_msr_rcuidle(msr, val, failed); +} +EXPORT_SYMBOL(do_trace_write_msr_idle); + void do_trace_read_msr(unsigned int msr, u64 val, int failed) { trace_read_msr(msr, val, failed); diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 3e101719689a..bdecd2638c59 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -187,12 +187,12 @@ static __cpuidle int intel_idle_ibrs(struct cpuidle_device *dev, int ret; if (smt_active) - wrmsrl(MSR_IA32_SPEC_CTRL, 0); + wrmsrl_idle(MSR_IA32_SPEC_CTRL, 0); ret = __intel_idle(dev, drv, index); if (smt_active) - wrmsrl(MSR_IA32_SPEC_CTRL, spec_ctrl); + wrmsrl_idle(MSR_IA32_SPEC_CTRL, spec_ctrl); return ret; }