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)
>
>
