[PATCH v2 0/8] x86/msr: Consolidate native/paravirt MSR functions

[PATCH v2 0/8] x86/msr: Consolidate native/paravirt MSR functions

[Dave Hansen]

Changes from v1:
 - Remove "raw_" names. Just use "paravirt_" in the generic code.

I'm thinking I'll just apply this in the coming days if nobody
screams too loudly.

--

This is old cruft, but it appears that having two copies of these
MSR functions is enabling warnings to creep in[1].

I know there's also been some work to pare down the XXL code, but
it's obviously not merged yet and this is a good baby step.

Create helpers that both paravirt and native can use in common code
and remove the paravirt implementations of the helpers. This reduces
the amount of logic that is duplicated in the paravirt code.

The wonky thing about this solution is that it has the common code
always make literal "paravirt_" calls, even when paravirt is not in
use for MSRs. In that case, the calls just go directly to the
"native_" functions via #defines.

Conceptually:
 -   native: The bare-metal implementation. Might not be usable under
	     paravirt XXL.
 - paravirt: Call the native version directly if paravirt is compiled
 	     out. Call into paravirt ops when available, which might
	     ultimately call a native implementation.

1. https://lore.kernel.org/all/20260319152210.210854-1-aldocontelk@gmail.com/

 msr.h      |  124 +++++++++++++++++++++++++++++++------------------------------
 paravirt.h |   44 ---------------------
 2 files changed, 65 insertions(+), 103 deletions(-)

[PATCH v2 1/8] x86/msr: Use paravirt "calls" in common code

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

Currently, the paravirt and native code define a common set of MSR
functions. But, some of the code is duplicated between the two. For
instance, the packing and unpacking of the 64-bit MSR value into two
32-bit values is done in both.

Prepare to consolidate the two copies. Have common code use the
paravirt_{rd,wr}msr*() naming and short-circuit the paravirt
infrastructure with the preprocessor to do paravirt=>native without
any paravirt overhead.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/arch/x86/include/asm/msr.h |   21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff -puN arch/x86/include/asm/msr.h~raw_msr_names arch/x86/include/asm/msr.h
--- a/arch/x86/include/asm/msr.h~raw_msr_names	2026-04-01 14:32:55.572415984 -0700
+++ b/arch/x86/include/asm/msr.h	2026-04-01 14:32:55.575416096 -0700
@@ -173,6 +173,13 @@ static inline u64 native_read_pmc(int co
 #include <asm/paravirt.h>
 #else
 #include <linux/errno.h>
+
+/* Short-circuit the paravirt MSR infrastructure when it is disabled: */
+#define paravirt_read_msr	native_read_msr
+#define paravirt_read_msr_safe	native_read_msr_safe
+#define paravirt_write_msr	native_write_msr
+#define paravirt_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 +188,35 @@ static inline u64 native_read_pmc(int co
 
 #define rdmsr(msr, low, high)					\
 do {								\
-	u64 __val = native_read_msr((msr));			\
+	u64 __val = paravirt_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);
+	paravirt_write_msr(msr, (u64)high << 32 | low);
 }
 
 #define rdmsrq(msr, val)			\
-	((val) = native_read_msr((msr)))
+	((val) = paravirt_read_msr((msr)))
 
 static inline void wrmsrq(u32 msr, u64 val)
 {
-	native_write_msr(msr, val);
+	paravirt_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 paravirt_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 = paravirt_read_msr_safe((msr), &__val);		\
 	(*low) = (u32)__val;					\
 	(*high) = (u32)(__val >> 32);				\
 	__err;							\
@@ -217,7 +224,7 @@ static inline int wrmsrq_safe(u32 msr, u
 
 static inline int rdmsrq_safe(u32 msr, u64 *p)
 {
-	return native_read_msr_safe(msr, p);
+	return paravirt_read_msr_safe(msr, p);
 }
 
 static __always_inline u64 rdpmc(int counter)
_

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

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

The paravirt and native code define very similar rdmsr()
implementations. Move the non-paravirt one out to common code where
both the "native" and "paravirt" implementations can use it.

Remove the now duplicate paravirt rdmsr().

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/arch/x86/include/asm/msr.h      |   17 ++++++++++-------
 b/arch/x86/include/asm/paravirt.h |    7 -------
 2 files changed, 10 insertions(+), 14 deletions(-)

diff -puN arch/x86/include/asm/msr.h~rdmsr-dups-2 arch/x86/include/asm/msr.h
--- a/arch/x86/include/asm/msr.h~rdmsr-dups-2	2026-04-01 14:32:56.105435948 -0700
+++ b/arch/x86/include/asm/msr.h	2026-04-01 14:32:56.112436210 -0700
@@ -186,13 +186,6 @@ static inline u64 native_read_pmc(int co
  * pointer indirection), this allows gcc to optimize better
  */
 
-#define rdmsr(msr, low, high)					\
-do {								\
-	u64 __val = paravirt_read_msr((msr));			\
-	(void)((low) = (u32)__val);				\
-	(void)((high) = (u32)(__val >> 32));			\
-} while (0)
-
 static inline void wrmsr(u32 msr, u32 low, u32 high)
 {
 	paravirt_write_msr(msr, (u64)high << 32 | low);
@@ -234,6 +227,16 @@ static __always_inline u64 rdpmc(int cou
 
 #endif	/* !CONFIG_PARAVIRT_XXL */
 
+/*
+ * 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)
+
 /* Instruction opcode for WRMSRNS supported in binutils >= 2.40 */
 #define ASM_WRMSRNS _ASM_BYTES(0x0f,0x01,0xc6)
 
diff -puN arch/x86/include/asm/paravirt.h~rdmsr-dups-2 arch/x86/include/asm/paravirt.h
--- a/arch/x86/include/asm/paravirt.h~rdmsr-dups-2	2026-04-01 14:32:56.109436097 -0700
+++ b/arch/x86/include/asm/paravirt.h	2026-04-01 14:32:56.112436210 -0700
@@ -161,13 +161,6 @@ static inline int paravirt_write_msr_saf
 	return PVOP_CALL2(int, pv_ops, cpu.write_msr_safe, msr, val);
 }
 
-#define rdmsr(msr, val1, val2)			\
-do {						\
-	u64 _l = paravirt_read_msr(msr);	\
-	val1 = (u32)_l;				\
-	val2 = _l >> 32;			\
-} while (0)
-
 static __always_inline void wrmsr(u32 msr, u32 low, u32 high)
 {
 	paravirt_write_msr(msr, (u64)high << 32 | low);
_

[PATCH v2 3/8] x86/msr: Consolidate rdmsr_safe() implementations

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

Move the existing "native" rdmsr_safe() implementation out to common
code. Consolidate the two rdmsr_safe() implementations down to one
by removing the paravirt.h version.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/arch/x86/include/asm/msr.h      |   20 ++++++++++----------
 b/arch/x86/include/asm/paravirt.h |   10 ----------
 2 files changed, 10 insertions(+), 20 deletions(-)

diff -puN arch/x86/include/asm/msr.h~rdmsr-dups-4 arch/x86/include/asm/msr.h
--- a/arch/x86/include/asm/msr.h~rdmsr-dups-4	2026-04-01 14:32:56.670457110 -0700
+++ b/arch/x86/include/asm/msr.h	2026-04-01 14:32:56.676457335 -0700
@@ -205,16 +205,6 @@ static inline int wrmsrq_safe(u32 msr, u
 	return paravirt_write_msr_safe(msr, val);
 }
 
-/* rdmsr with exception handling */
-#define rdmsr_safe(msr, low, high)				\
-({								\
-	u64 __val;						\
-	int __err = paravirt_read_msr_safe((msr), &__val);		\
-	(*low) = (u32)__val;					\
-	(*high) = (u32)(__val >> 32);				\
-	__err;							\
-})
-
 static inline int rdmsrq_safe(u32 msr, u64 *p)
 {
 	return paravirt_read_msr_safe(msr, p);
@@ -237,6 +227,16 @@ do {								\
 	(void)((high) = (u32)(__val >> 32));			\
 } while (0)
 
+/* rdmsr with exception handling */
+#define rdmsr_safe(msr, low, high)				\
+({								\
+	u64 __val;						\
+	int __err = paravirt_read_msr_safe((msr), &__val);		\
+	(*low) = (u32)__val;					\
+	(*high) = (u32)(__val >> 32);				\
+	__err;							\
+})
+
 /* Instruction opcode for WRMSRNS supported in binutils >= 2.40 */
 #define ASM_WRMSRNS _ASM_BYTES(0x0f,0x01,0xc6)
 
diff -puN arch/x86/include/asm/paravirt.h~rdmsr-dups-4 arch/x86/include/asm/paravirt.h
--- a/arch/x86/include/asm/paravirt.h~rdmsr-dups-4	2026-04-01 14:32:56.673457222 -0700
+++ b/arch/x86/include/asm/paravirt.h	2026-04-01 14:32:56.676457335 -0700
@@ -181,16 +181,6 @@ static inline int wrmsrq_safe(u32 msr, u
 	return paravirt_write_msr_safe(msr, val);
 }
 
-/* rdmsr with exception handling */
-#define rdmsr_safe(msr, a, b)				\
-({							\
-	u64 _l;						\
-	int _err = paravirt_read_msr_safe((msr), &_l);	\
-	(*a) = (u32)_l;					\
-	(*b) = (u32)(_l >> 32);				\
-	_err;						\
-})
-
 static __always_inline int rdmsrq_safe(u32 msr, u64 *p)
 {
 	return paravirt_read_msr_safe(msr, p);
_

Re: [PATCH v2 3/8] x86/msr: Consolidate rdmsr_safe() implementations

[Maciej Wieczor-Retman]

On 2026-04-29 at 11:45:22 -0700, Dave Hansen wrote:
>
>From: Dave Hansen <dave.hansen@linux.intel.com>
>
>Move the existing "native" rdmsr_safe() implementation out to common
>code. Consolidate the two rdmsr_safe() implementations down to one
>by removing the paravirt.h version.
>
>Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
>---
>
> b/arch/x86/include/asm/msr.h      |   20 ++++++++++----------
> b/arch/x86/include/asm/paravirt.h |   10 ----------
> 2 files changed, 10 insertions(+), 20 deletions(-)
>
>diff -puN arch/x86/include/asm/msr.h~rdmsr-dups-4 arch/x86/include/asm/msr.h
>--- a/arch/x86/include/asm/msr.h~rdmsr-dups-4	2026-04-01 14:32:56.670457110 -0700
>+++ b/arch/x86/include/asm/msr.h	2026-04-01 14:32:56.676457335 -0700
>@@ -237,6 +227,16 @@ do {								\
> 	(void)((high) = (u32)(__val >> 32));			\
> } while (0)
> 
>+/* rdmsr with exception handling */
>+#define rdmsr_safe(msr, low, high)				\
>+({								\
>+	u64 __val;						\
>+	int __err = paravirt_read_msr_safe((msr), &__val);		\

I guess this backslash could be aligned with other ones?

Other than that everything looks ok to me, boot tested it too with and without
PARAVIRT_XXL on v7.1-rc1 without issues. So for the whole patchset:

Tested-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
Acked-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>

-- 
Kind regards
Maciej Wieczór-Retman

[PATCH v2 4/8] x86/msr: Consolidate rdmsrq() implementations

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

Consolidate the two rdmsrq() implementations down to one.

The paravirt.h implementation was probably better, but just stick
with the native one here for consistency.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/arch/x86/include/asm/msr.h      |    6 +++---
 b/arch/x86/include/asm/paravirt.h |    5 -----
 2 files changed, 3 insertions(+), 8 deletions(-)

diff -puN arch/x86/include/asm/msr.h~rdmsr-dups-5 arch/x86/include/asm/msr.h
--- a/arch/x86/include/asm/msr.h~rdmsr-dups-5	2026-04-01 14:32:57.236478310 -0700
+++ b/arch/x86/include/asm/msr.h	2026-04-01 14:32:57.242478534 -0700
@@ -191,9 +191,6 @@ static inline void wrmsr(u32 msr, u32 lo
 	paravirt_write_msr(msr, (u64)high << 32 | low);
 }
 
-#define rdmsrq(msr, val)			\
-	((val) = paravirt_read_msr((msr)))
-
 static inline void wrmsrq(u32 msr, u64 val)
 {
 	paravirt_write_msr(msr, val);
@@ -237,6 +234,9 @@ do {								\
 	__err;							\
 })
 
+#define rdmsrq(msr, val)			\
+	((val) = paravirt_read_msr((msr)))
+
 /* Instruction opcode for WRMSRNS supported in binutils >= 2.40 */
 #define ASM_WRMSRNS _ASM_BYTES(0x0f,0x01,0xc6)
 
diff -puN arch/x86/include/asm/paravirt.h~rdmsr-dups-5 arch/x86/include/asm/paravirt.h
--- a/arch/x86/include/asm/paravirt.h~rdmsr-dups-5	2026-04-01 14:32:57.239478422 -0700
+++ b/arch/x86/include/asm/paravirt.h	2026-04-01 14:32:57.242478534 -0700
@@ -166,11 +166,6 @@ static __always_inline void wrmsr(u32 ms
 	paravirt_write_msr(msr, (u64)high << 32 | low);
 }
 
-#define rdmsrq(msr, val)			\
-do {						\
-	val = paravirt_read_msr(msr);		\
-} while (0)
-
 static inline void wrmsrq(u32 msr, u64 val)
 {
 	paravirt_write_msr(msr, val);
_

[PATCH v2 5/8] x86/msr: Consolidate {rd,wr}msr[q]_safe() implementations

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

These should be very familiar by now. Move the native implementations
of the "safe" MSR functions out to common code and zap the paravirt.h
version. Do four at once now because these are quite straightforward.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/arch/x86/include/asm/msr.h      |   42 +++++++++++++++++++-------------------
 b/arch/x86/include/asm/paravirt.h |   20 ------------------
 2 files changed, 21 insertions(+), 41 deletions(-)

diff -puN arch/x86/include/asm/msr.h~rdmsr-dups-6 arch/x86/include/asm/msr.h
--- a/arch/x86/include/asm/msr.h~rdmsr-dups-6	2026-04-01 14:32:57.802499509 -0700
+++ b/arch/x86/include/asm/msr.h	2026-04-01 14:32:57.809499772 -0700
@@ -186,27 +186,6 @@ static inline u64 native_read_pmc(int co
  * pointer indirection), this allows gcc to optimize better
  */
 
-static inline void wrmsr(u32 msr, u32 low, u32 high)
-{
-	paravirt_write_msr(msr, (u64)high << 32 | low);
-}
-
-static inline void wrmsrq(u32 msr, u64 val)
-{
-	paravirt_write_msr(msr, val);
-}
-
-/* wrmsr with exception handling */
-static inline int wrmsrq_safe(u32 msr, u64 val)
-{
-	return paravirt_write_msr_safe(msr, val);
-}
-
-static inline int rdmsrq_safe(u32 msr, u64 *p)
-{
-	return paravirt_read_msr_safe(msr, p);
-}
-
 static __always_inline u64 rdpmc(int counter)
 {
 	return native_read_pmc(counter);
@@ -237,6 +216,27 @@ do {								\
 #define rdmsrq(msr, val)			\
 	((val) = paravirt_read_msr((msr)))
 
+static inline int rdmsrq_safe(u32 msr, u64 *p)
+{
+	return paravirt_read_msr_safe(msr, p);
+}
+
+/* wrmsr with exception handling */
+static inline int wrmsrq_safe(u32 msr, u64 val)
+{
+	return paravirt_write_msr_safe(msr, val);
+}
+
+static inline void wrmsr(u32 msr, u32 low, u32 high)
+{
+	paravirt_write_msr(msr, (u64)high << 32 | low);
+}
+
+static inline void wrmsrq(u32 msr, u64 val)
+{
+	paravirt_write_msr(msr, val);
+}
+
 /* Instruction opcode for WRMSRNS supported in binutils >= 2.40 */
 #define ASM_WRMSRNS _ASM_BYTES(0x0f,0x01,0xc6)
 
diff -puN arch/x86/include/asm/paravirt.h~rdmsr-dups-6 arch/x86/include/asm/paravirt.h
--- a/arch/x86/include/asm/paravirt.h~rdmsr-dups-6	2026-04-01 14:32:57.806499659 -0700
+++ b/arch/x86/include/asm/paravirt.h	2026-04-01 14:32:57.809499772 -0700
@@ -161,26 +161,6 @@ static inline int paravirt_write_msr_saf
 	return PVOP_CALL2(int, pv_ops, cpu.write_msr_safe, msr, val);
 }
 
-static __always_inline void wrmsr(u32 msr, u32 low, u32 high)
-{
-	paravirt_write_msr(msr, (u64)high << 32 | low);
-}
-
-static inline void wrmsrq(u32 msr, u64 val)
-{
-	paravirt_write_msr(msr, val);
-}
-
-static inline int wrmsrq_safe(u32 msr, u64 val)
-{
-	return paravirt_write_msr_safe(msr, val);
-}
-
-static __always_inline int rdmsrq_safe(u32 msr, u64 *p)
-{
-	return paravirt_read_msr_safe(msr, p);
-}
-
 static __always_inline u64 rdpmc(int counter)
 {
 	return PVOP_CALL1(u64, pv_ops, cpu.read_pmc, counter);
_

[PATCH v2 6/8] x86/msr: Consolidate rdpmc() implementations

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

Doing this is debatable. It does not actually remove any code. But,
this makes rdpmc() follow the same pattern as all of the MSR functions
where paravirt.h defines a paravirt_foo() function and then msr.h
uses that function in common code.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/arch/x86/include/asm/msr.h      |   11 ++++++-----
 b/arch/x86/include/asm/paravirt.h |    2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff -puN arch/x86/include/asm/msr.h~rdmsr-dups-9 arch/x86/include/asm/msr.h
--- a/arch/x86/include/asm/msr.h~rdmsr-dups-9	2026-04-01 14:32:58.366520634 -0700
+++ b/arch/x86/include/asm/msr.h	2026-04-01 14:32:58.373520897 -0700
@@ -179,6 +179,7 @@ static inline u64 native_read_pmc(int co
 #define paravirt_read_msr_safe	native_read_msr_safe
 #define paravirt_write_msr	native_write_msr
 #define paravirt_write_msr_safe	native_write_msr_safe
+#define paravirt_read_pmc		native_read_pmc
 
 /*
  * Access to machine-specific registers (available on 586 and better only)
@@ -186,11 +187,6 @@ static inline u64 native_read_pmc(int co
  * pointer indirection), this allows gcc to optimize better
  */
 
-static __always_inline u64 rdpmc(int counter)
-{
-	return native_read_pmc(counter);
-}
-
 #endif	/* !CONFIG_PARAVIRT_XXL */
 
 /*
@@ -237,6 +233,11 @@ static inline void wrmsrq(u32 msr, u64 v
 	paravirt_write_msr(msr, val);
 }
 
+static __always_inline u64 rdpmc(int counter)
+{
+	return paravirt_read_pmc(counter);
+}
+
 /* Instruction opcode for WRMSRNS supported in binutils >= 2.40 */
 #define ASM_WRMSRNS _ASM_BYTES(0x0f,0x01,0xc6)
 
diff -puN arch/x86/include/asm/paravirt.h~rdmsr-dups-9 arch/x86/include/asm/paravirt.h
--- a/arch/x86/include/asm/paravirt.h~rdmsr-dups-9	2026-04-01 14:32:58.370520784 -0700
+++ b/arch/x86/include/asm/paravirt.h	2026-04-01 14:32:58.373520897 -0700
@@ -161,7 +161,7 @@ static inline int paravirt_write_msr_saf
 	return PVOP_CALL2(int, pv_ops, cpu.write_msr_safe, msr, val);
 }
 
-static __always_inline u64 rdpmc(int counter)
+static __always_inline u64 paravirt_read_pmc(int counter)
 {
 	return PVOP_CALL1(u64, pv_ops, cpu.read_pmc, counter);
 }
_

[PATCH v2 7/8] x86/msr: Remove old crusty comment

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

I'm not sure any of this makes sense any more. The kernel only
runs on "586 and better". The comment about gcc optimization is
hopefully decades out of date too.

Really, the only reason to keep the wonky semantics where the
parameters get modified is to avoid all the churn to make them sane.
Not gcc. gcc was probably a bad reason, even back in the day because
MSRs are mostly very slow and have always been very slow. A few
extra bytes of register shuffling was probably never measurable.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/arch/x86/include/asm/msr.h |    6 ------
 1 file changed, 6 deletions(-)

diff -puN arch/x86/include/asm/msr.h~rdmsr-dups-10 arch/x86/include/asm/msr.h
--- a/arch/x86/include/asm/msr.h~rdmsr-dups-10	2026-04-01 14:32:58.971543295 -0700
+++ b/arch/x86/include/asm/msr.h	2026-04-01 14:32:58.974543407 -0700
@@ -181,12 +181,6 @@ static inline u64 native_read_pmc(int co
 #define paravirt_write_msr_safe	native_write_msr_safe
 #define paravirt_read_pmc		native_read_pmc
 
-/*
- * Access to machine-specific registers (available on 586 and better only)
- * Note: the rd* operations modify the parameters directly (without using
- * pointer indirection), this allows gcc to optimize better
- */
-
 #endif	/* !CONFIG_PARAVIRT_XXL */
 
 /*
_

[PATCH v2 8/8] x86/msr: Remove duplicate #include

[Dave Hansen]

From: Dave Hansen <dave.hansen@linux.intel.com>

errno.h is already included for C code at the top of the header.
Zap the duplicate.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/arch/x86/include/asm/msr.h |    1 -
 1 file changed, 1 deletion(-)

diff -puN arch/x86/include/asm/msr.h~rdmsr-dups-11 arch/x86/include/asm/msr.h
--- a/arch/x86/include/asm/msr.h~rdmsr-dups-11	2026-04-29 11:38:57.838732405 -0700
+++ b/arch/x86/include/asm/msr.h	2026-04-29 11:38:57.841732519 -0700
@@ -172,7 +172,6 @@ static inline u64 native_read_pmc(int co
 #ifdef CONFIG_PARAVIRT_XXL
 #include <asm/paravirt.h>
 #else
-#include <linux/errno.h>
 
 /* Short-circuit the paravirt MSR infrastructure when it is disabled: */
 #define paravirt_read_msr	native_read_msr
_

Re: [PATCH v2 8/8] x86/msr: Remove duplicate #include

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 1032 bytes --]

On 29.04.26 20:45, Dave Hansen wrote:
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> errno.h is already included for C code at the top of the header.

I'm seeing only asm/errno.h being included.

I don't say linux/errno.h is needed, but the reasoning is not really
convincing.


Juergen

> Zap the duplicate.
> 
> Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
> ---
> 
>   b/arch/x86/include/asm/msr.h |    1 -
>   1 file changed, 1 deletion(-)
> 
> diff -puN arch/x86/include/asm/msr.h~rdmsr-dups-11 arch/x86/include/asm/msr.h
> --- a/arch/x86/include/asm/msr.h~rdmsr-dups-11	2026-04-29 11:38:57.838732405 -0700
> +++ b/arch/x86/include/asm/msr.h	2026-04-29 11:38:57.841732519 -0700
> @@ -172,7 +172,6 @@ static inline u64 native_read_pmc(int co
>   #ifdef CONFIG_PARAVIRT_XXL
>   #include <asm/paravirt.h>
>   #else
> -#include <linux/errno.h>
>   
>   /* Short-circuit the paravirt MSR infrastructure when it is disabled: */
>   #define paravirt_read_msr	native_read_msr
> _


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v2 8/8] x86/msr: Remove duplicate #include

[Dave Hansen]

On 4/30/26 00:20, Jürgen Groß wrote:
> On 29.04.26 20:45, Dave Hansen wrote:
>> From: Dave Hansen <dave.hansen@linux.intel.com>
>>
>> errno.h is already included for C code at the top of the header.
> 
> I'm seeing only asm/errno.h being included.
> 
> I don't say linux/errno.h is needed, but the reasoning is not really
> convincing.

Yes, completely agree. I goofed that one was linux/errno.h and the other
was asm/errno.h. So the reasoning was bogus. But I do think it's still a
good idea for other reasons. How about:

	linux/errno.h was presumably being included here for some of
	the MSR function implementations inside the #fidef. But, even
	before those were moved out of the #ifdef, they were not using
	anything from errno.h. There does not appear to be any reason to
	include it here. Especially inside the (quite small now) #ifdef.

	Remove the #include.

?

Re: [PATCH v2 8/8] x86/msr: Remove duplicate #include

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 1007 bytes --]

On 30.04.26 19:42, Dave Hansen wrote:
> On 4/30/26 00:20, Jürgen Groß wrote:
>> On 29.04.26 20:45, Dave Hansen wrote:
>>> From: Dave Hansen <dave.hansen@linux.intel.com>
>>>
>>> errno.h is already included for C code at the top of the header.
>>
>> I'm seeing only asm/errno.h being included.
>>
>> I don't say linux/errno.h is needed, but the reasoning is not really
>> convincing.
> 
> Yes, completely agree. I goofed that one was linux/errno.h and the other
> was asm/errno.h. So the reasoning was bogus. But I do think it's still a
> good idea for other reasons. How about:
> 
> 	linux/errno.h was presumably being included here for some of
> 	the MSR function implementations inside the #fidef. But, even
> 	before those were moved out of the #ifdef, they were not using
> 	anything from errno.h. There does not appear to be any reason to
> 	include it here. Especially inside the (quite small now) #ifdef.
> 
> 	Remove the #include.
> 
> ?
> 

Fine with me.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH v2 0/8] x86/msr: Consolidate native/paravirt MSR functions

[Jürgen Groß]

[-- Attachment #1.1.1: Type: text/plain, Size: 1638 bytes --]

On 29.04.26 20:45, Dave Hansen wrote:
> Changes from v1:
>   - Remove "raw_" names. Just use "paravirt_" in the generic code.
> 
> I'm thinking I'll just apply this in the coming days if nobody
> screams too loudly.
> 
> --
> 
> This is old cruft, but it appears that having two copies of these
> MSR functions is enabling warnings to creep in[1].
> 
> I know there's also been some work to pare down the XXL code, but
> it's obviously not merged yet and this is a good baby step.
> 
> Create helpers that both paravirt and native can use in common code
> and remove the paravirt implementations of the helpers. This reduces
> the amount of logic that is duplicated in the paravirt code.
> 
> The wonky thing about this solution is that it has the common code
> always make literal "paravirt_" calls, even when paravirt is not in
> use for MSRs. In that case, the calls just go directly to the
> "native_" functions via #defines.
> 
> Conceptually:
>   -   native: The bare-metal implementation. Might not be usable under
> 	     paravirt XXL.
>   - paravirt: Call the native version directly if paravirt is compiled
>   	     out. Call into paravirt ops when available, which might
> 	     ultimately call a native implementation.
> 
> 1. https://lore.kernel.org/all/20260319152210.210854-1-aldocontelk@gmail.com/
> 
>   msr.h      |  124 +++++++++++++++++++++++++++++++------------------------------
>   paravirt.h |   44 ---------------------
>   2 files changed, 65 insertions(+), 103 deletions(-)

Apart from the comment for patch 8:

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]