Re: [PATCH v4 1/4] x86/speculation: Add __update_spec_ctrl() helper

[Waiman Long]

On 7/3/23 06:28, Peter Zijlstra wrote:
> On Tue, Jun 27, 2023 at 10:25:51PM -0400, Waiman Long wrote:
> > Add a new __update_spec_ctrl() helper which is a variant of
> > update_spec_ctrl() that can be used in a noinstr function.
> >
> > Suggested-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > Signed-off-by: Waiman Long <longman@xxxxxxxxxx>
> > ---
> >   arch/x86/include/asm/nospec-branch.h | 11 ++++++++++-
> >   1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> > index 55388c9f7601..1d363fcea207 100644
> > --- a/arch/x86/include/asm/nospec-branch.h
> > +++ b/arch/x86/include/asm/nospec-branch.h
> > @@ -9,7 +9,7 @@
> >   
> >   #include <asm/alternative.h>
> >   #include <asm/cpufeatures.h>
> > -#include <asm/msr-index.h>
> > +#include <asm/msr.h>
> >   #include <asm/unwind_hints.h>
> >   #include <asm/percpu.h>
> >   #include <asm/current.h>
> > @@ -488,6 +488,15 @@ DECLARE_PER_CPU(u64, x86_spec_ctrl_current);
> >   extern void update_spec_ctrl_cond(u64 val);
> >   extern u64 spec_ctrl_current(void);
> >   
> > +/*
> > + * This can be used in noinstr function.
> > + */
> > +static __always_inline void __update_spec_ctrl(u64 val)
> > +{
> > +	__this_cpu_write(x86_spec_ctrl_current, val);
> > +	native_wrmsrl(MSR_IA32_SPEC_CTRL, val);
> > +}
> Should we also use this to implement update_spec_ctrl() ?

I also thought about that. However, I am a bit worry about losing some 
additional check done in this_cpu_write() and the paravirt support in 
wrmsrl(). I have no problem making update_spec_ctrl() an instrument-able 
wrapper on __update_spec_ctrl() if you think these are not valid concerns.

Cheers,
Longman