Re: [PATCH V3 1/2] x86/msr: add msr_set/clear_bit_on_cpu/cpus access functions

[Thomas Gleixner]

On Mon, 27 Mar 2017, kan.liang@xxxxxxxxx wrote:

> From: Kan Liang <Kan.liang@xxxxxxxxx>
> 
> To flip a MSR bit on many CPUs or specific CPU, currently it has to do
> read-modify-write operation on the MSR through rd/wrmsr_on_cpu(s).
> It actually sends two IPIs to the given CPU.

The IPIs are the least of the problems, really. The real problem is that

       rdmsr_on_cpu()
       wrmsr_on_cpu()

is not atomic. That's what wants to be solved. The reduction of IPIs just a
side effect.

>  #else  /*  CONFIG_SMP  */
> +static inline int msr_set_bit_on_cpu(unsigned int cpu, u32 msr, u8 bit)
> +{
> +	return msr_set_bit(msr, bit);
> +}
> +
> +static inline int msr_clear_bit_on_cpu(unsigned int cpu, u32 msr, u8 bit)
> +{
> +	return msr_clear_bit(msr, bit);
> +}
> +
> +static inline void msr_set_bit_on_cpus(const struct cpumask *mask, u32 msr, u8 bit)
> +{
> +	msr_set_bit(msr, bit);
> +}
> +
> +static inline void msr_clear_bit_on_cpus(const struct cpumask *mask, u32 msr, u8 bit)
> +{
> +	msr_clear_bit(msr, bit);
> +}

This is utter crap because it's fundamentaly different from the SMP
version.

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?

Thanks,

	tglx