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

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

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)


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)