Re: [PATCH v2 2/8] x86/msr: Consolidate rdmsr() definitions

[Yan Zhao <yan.y.zhao@intel.com>]

On Wed, May 06, 2026 at 10:03:26AM +0800, Yan Zhao wrote:
> On Wed, Apr 29, 2026 at 11:45:21AM -0700, Dave Hansen wrote:
> > +/*
> > + * Common paravirt and native helpers:
> > + */
> > +#define rdmsr(msr, low, high)					\
> > +do {								\
> > +	u64 __val = paravirt_read_msr((msr));			\
> > +	(void)((low) = (u32)__val);				\
> > +	(void)((high) = (u32)(__val >> 32));			\
> > +} while (0)
> Rather than direct all (paravirt and native) invocations of rdmsr() to
> paravirt_*(), does it make sense to first introduce the common version of
> helpers and direct the common version of helpers to paravirt_* or native_*?
> So, we can have
> 
> +/*
> + * Common paravirt and native helpers:
> + */
> +#define rdmsr(msr, low, high)                                  \
> +do {                                                           \
> +       u64 __val = common_read_msr((msr));                     \
> +       (void)((low) = (u32)__val);                             \
> +       (void)((high) = (u32)(__val >> 32));                    \
> +} while (0)
> 
Or maybe s/common/trampoline ?


> with below changes in patch 1.
> 
> diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h
> index 9c2ea29e12a9..7ba5bc921526 100644
> --- a/arch/x86/include/asm/msr.h
> +++ b/arch/x86/include/asm/msr.h
> @@ -171,8 +171,19 @@ static inline u64 native_read_pmc(int counter)
> 
>  #ifdef CONFIG_PARAVIRT_XXL
>  #include <asm/paravirt.h>
> +#define common_read_msr        paravirt_read_msr
> +#define common_read_msr_safe   paravirt_read_msr_safe
> +#define common_write_msr       paravirt_write_msr
> +#define common_write_msr_safe  paravirt_write_msr_safe
>  #else
>  #include <linux/errno.h>
> +
> +/* Short-circuit the paravirt MSR infrastructure when it is disabled: */
> +#define common_read_msr        native_read_msr
> +#define common_read_msr_safe   native_read_msr_safe
> +#define common_write_msr       native_write_msr
> +#define common_write_msr_safe  native_write_msr_safe
> +
> 
>  /*
>   * Access to machine-specific registers (available on 586 and better only)
>   * Note: the rd* operations modify the parameters directly (without using
> @@ -181,35 +192,35 @@ static inline u64 native_read_pmc(int counter)
> 
>  #define rdmsr(msr, low, high)                                  \
>  do {                                                           \
> -       u64 __val = native_read_msr((msr));                     \
> +       u64 __val = common_read_msr((msr));                     \
>         (void)((low) = (u32)__val);                             \
>         (void)((high) = (u32)(__val >> 32));                    \
>  } while (0)
> 
>  static inline void wrmsr(u32 msr, u32 low, u32 high)
>  {
> -       native_write_msr(msr, (u64)high << 32 | low);
> +       common_write_msr(msr, (u64)high << 32 | low);
>  }
> 
>  #define rdmsrq(msr, val)                       \
> -       ((val) = native_read_msr((msr)))
> +       ((val) = common_read_msr((msr)))
> 
>  static inline void wrmsrq(u32 msr, u64 val)
>  {
> -       native_write_msr(msr, val);
> +       common_write_msr(msr, val);
>  }
> 
>  /* wrmsr with exception handling */
>  static inline int wrmsrq_safe(u32 msr, u64 val)
>  {
> -       return native_write_msr_safe(msr, val);
> +       return common_write_msr_safe(msr, val);
>  }
> 
>  /* rdmsr with exception handling */
>  #define rdmsr_safe(msr, low, high)                             \
>  ({                                                             \
>         u64 __val;                                              \
> -       int __err = native_read_msr_safe((msr), &__val);        \
> +       int __err = common_read_msr_safe((msr), &__val);                \
>         (*low) = (u32)__val;                                    \
>         (*high) = (u32)(__val >> 32);                           \
>         __err;                                                  \
> @@ -217,7 +228,7 @@ static inline int wrmsrq_safe(u32 msr, u64 val)
> 
>  static inline int rdmsrq_safe(u32 msr, u64 *p)
>  {
> -       return native_read_msr_safe(msr, p);
> +       return common_read_msr_safe(msr, p);
>  }
> 
>  static __always_inline u64 rdpmc(int counter)
> 
>