.
> > msr_set/clear_bit() are not protected by anyhting. And in your call
> > site this is invoked from fully preemptible context. What protects
> > against context switch and interrupts fiddling with DEBUGMSR?
>
> And thinking more about that whole interface. It's just overkill.
>
> diff --git a/arch/x86/lib/msr.c b/arch/x86/lib/msr.c index
> d1dee753b949..35763927adaa 100644
> --- a/arch/x86/lib/msr.c
> +++ b/arch/x86/lib/msr.c
> @@ -58,7 +58,7 @@ int msr_write(u32 msr, struct msr *m)
> return wrmsrl_safe(msr, m->q);
> }
>
> -static inline int __flip_bit(u32 msr, u8 bit, bool set)
> +int msr_flip_bit(u32 msr, u8 bit, bool set)
> {
> struct msr m, m1;
> int err = -EINVAL;
> @@ -85,6 +85,7 @@ static inline int __flip_bit(u32 msr, u8 bit, bool set)
>
> return 1;
> }
> +EXPORT_SYMBOL_GPL(msr_flip_bit);
>
> /**
> * Set @bit in a MSR @msr.
> @@ -96,7 +97,7 @@ static inline int __flip_bit(u32 msr, u8 bit, bool set)
> */
> int msr_set_bit(u32 msr, u8 bit)
> {
> - return __flip_bit(msr, bit, true);
> + return msr_flip_bit(msr, bit, true);
> }
>
> /**
> @@ -109,7 +110,7 @@ int msr_set_bit(u32 msr, u8 bit)
> */
> int msr_clear_bit(u32 msr, u8 bit)
> {
> - return __flip_bit(msr, bit, false);
> + return msr_flip_bit(msr, bit, false);
> }
>
> #ifdef CONFIG_TRACEPOINTS
>
> And in the driver:
>
> static void flip_smm_bit(void *data)
> {
> int val = *(int *)data;
>
> msr_flip_bit(DEBUGMSR, SMMBIT, val);
> }
>
> And in the write function:
>
> smp_call_function(flip_smm_bit, &val, 1);
>
> That avoids all the extra interfaces and requires less code and less text foot
> print when unused .....
>
Thanks. It simplify the code very much.
I think we still need to protect the smp_call_function in the driver, right?
Would be the following code enough?
get_online_cpus();
preempt_disable();
smp_call_function(flip_smm_bit, &val, 1);
preempt_enable();
put_online_cpus();
Thanks,
Kan
