[PATCH v2 09/16] x86: Clean up asm/msr.h

Andrew Cooper posted 16 patches 2 months, 2 weeks ago
[PATCH v2 09/16] x86: Clean up asm/msr.h
Posted by Andrew Cooper 2 months, 2 weeks ago
Now that content has been split out, minimise the header files as msr.h is
included by many translation units.  A few more TUs were pulling dependencies
in transitively, so fix them up.

Give asm/time.h an SPDX tag, and strip trailing whitespace.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/acpi/cpufreq/acpi.c   |  2 ++
 xen/arch/x86/acpi/cpufreq/hwp.c    |  4 ++++
 xen/arch/x86/cpu/microcode/intel.c |  4 ++++
 xen/arch/x86/include/asm/msr.h     | 22 ++++++----------------
 xen/arch/x86/tsx.c                 |  2 ++
 5 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/xen/arch/x86/acpi/cpufreq/acpi.c b/xen/arch/x86/acpi/cpufreq/acpi.c
index 567c10dd0643..b02745941701 100644
--- a/xen/arch/x86/acpi/cpufreq/acpi.c
+++ b/xen/arch/x86/acpi/cpufreq/acpi.c
@@ -14,8 +14,10 @@
 #include <xen/delay.h>
 #include <xen/errno.h>
 #include <xen/param.h>
+#include <xen/smp.h>
 
 #include <asm/msr.h>
+#include <asm/processor.h>
 
 #include <acpi/acpi.h>
 #include <acpi/cpufreq/cpufreq.h>
diff --git a/xen/arch/x86/acpi/cpufreq/hwp.c b/xen/arch/x86/acpi/cpufreq/hwp.c
index 26dce9aaf89a..38037d8300cd 100644
--- a/xen/arch/x86/acpi/cpufreq/hwp.c
+++ b/xen/arch/x86/acpi/cpufreq/hwp.c
@@ -8,8 +8,12 @@
 #include <xen/cpumask.h>
 #include <xen/init.h>
 #include <xen/param.h>
+#include <xen/smp.h>
 #include <xen/xmalloc.h>
+
 #include <asm/msr.h>
+#include <asm/processor.h>
+
 #include <acpi/cpufreq/cpufreq.h>
 
 static bool __ro_after_init hwp_in_use;
diff --git a/xen/arch/x86/cpu/microcode/intel.c b/xen/arch/x86/cpu/microcode/intel.c
index 3f8e9ca63b55..281993e725cc 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -23,8 +23,12 @@
 
 #include <xen/err.h>
 #include <xen/init.h>
+#include <xen/lib.h>
+#include <xen/string.h>
+#include <xen/xmalloc.h>
 
 #include <asm/msr.h>
+#include <asm/processor.h>
 #include <asm/system.h>
 
 #include "private.h"
diff --git a/xen/arch/x86/include/asm/msr.h b/xen/arch/x86/include/asm/msr.h
index 4aeb06f6524d..c0d66562956d 100644
--- a/xen/arch/x86/include/asm/msr.h
+++ b/xen/arch/x86/include/asm/msr.h
@@ -1,18 +1,13 @@
-#ifndef __ASM_MSR_H
-#define __ASM_MSR_H
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef X86_MSR_H
+#define X86_MSR_H
 
-#include "msr-index.h"
-
-#include <xen/types.h>
 #include <xen/percpu.h>
 #include <xen/errno.h>
-#include <xen/kernel.h>
-
-#include <xen/lib/x86/cpu-policy.h>
 
+#include <asm/alternative.h>
 #include <asm/asm_defns.h>
-#include <asm/cpufeature.h>
-#include <asm/processor.h>
+#include <asm/msr-index.h>
 
 #define rdmsr(msr,val1,val2) \
      __asm__ __volatile__("rdmsr" \
@@ -113,11 +108,6 @@ static inline int wrmsr_safe(unsigned int msr, uint64_t val)
     return -EFAULT;
 }
 
-#define rdpmc(counter,low,high) \
-     __asm__ __volatile__("rdpmc" \
-			  : "=a" (low), "=d" (high) \
-			  : "c" (counter))
-
 DECLARE_PER_CPU(uint64_t, efer);
 static inline uint64_t read_efer(void)
 {
@@ -144,4 +134,4 @@ static inline void wrmsr_tsc_aux(uint32_t val)
     }
 }
 
-#endif /* __ASM_MSR_H */
+#endif /* X86_MSR_H */
diff --git a/xen/arch/x86/tsx.c b/xen/arch/x86/tsx.c
index fbdd05971c8b..2a0c7c08a2ba 100644
--- a/xen/arch/x86/tsx.c
+++ b/xen/arch/x86/tsx.c
@@ -1,7 +1,9 @@
 #include <xen/init.h>
 #include <xen/param.h>
+
 #include <asm/microcode.h>
 #include <asm/msr.h>
+#include <asm/processor.h>
 
 /*
  * Valid values:
-- 
2.39.5


Re: [PATCH v2 09/16] x86: Clean up asm/msr.h
Posted by Jan Beulich 2 months, 1 week ago
On 15.08.2025 22:41, Andrew Cooper wrote:
> Now that content has been split out, minimise the header files as msr.h is
> included by many translation units.  A few more TUs were pulling dependencies
> in transitively, so fix them up.
> 
> Give asm/time.h an SPDX tag, and strip trailing whitespace.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

I think though that ...

> --- a/xen/arch/x86/include/asm/msr.h
> +++ b/xen/arch/x86/include/asm/msr.h
> @@ -1,18 +1,13 @@
> -#ifndef __ASM_MSR_H
> -#define __ASM_MSR_H
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef X86_MSR_H
> +#define X86_MSR_H
>  
> -#include "msr-index.h"
> -
> -#include <xen/types.h>
>  #include <xen/percpu.h>
>  #include <xen/errno.h>
> -#include <xen/kernel.h>
> -
> -#include <xen/lib/x86/cpu-policy.h>
>  
> +#include <asm/alternative.h>
>  #include <asm/asm_defns.h>
> -#include <asm/cpufeature.h>
> -#include <asm/processor.h>
> +#include <asm/msr-index.h>
>  
>  #define rdmsr(msr,val1,val2) \
>       __asm__ __volatile__("rdmsr" \
> @@ -113,11 +108,6 @@ static inline int wrmsr_safe(unsigned int msr, uint64_t val)
>      return -EFAULT;
>  }
>  
> -#define rdpmc(counter,low,high) \
> -     __asm__ __volatile__("rdpmc" \
> -			  : "=a" (low), "=d" (high) \
> -			  : "c" (counter))
> -

... this removal wants mentioning in the description. I'm actually surprised this
is unused - how does vPMU get away?

Jan
Re: [PATCH v2 09/16] x86: Clean up asm/msr.h
Posted by Andrew Cooper 2 months, 1 week ago
On 19/08/2025 11:42 am, Jan Beulich wrote:
> On 15.08.2025 22:41, Andrew Cooper wrote:
>> Now that content has been split out, minimise the header files as msr.h is
>> included by many translation units.  A few more TUs were pulling dependencies
>> in transitively, so fix them up.
>>
>> Give asm/time.h an SPDX tag, and strip trailing whitespace.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>

Thanks.

>
> I think though that ...
>
>> --- a/xen/arch/x86/include/asm/msr.h
>> +++ b/xen/arch/x86/include/asm/msr.h
>> @@ -1,18 +1,13 @@
>> -#ifndef __ASM_MSR_H
>> -#define __ASM_MSR_H
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +#ifndef X86_MSR_H
>> +#define X86_MSR_H
>>  
>> -#include "msr-index.h"
>> -
>> -#include <xen/types.h>
>>  #include <xen/percpu.h>
>>  #include <xen/errno.h>
>> -#include <xen/kernel.h>
>> -
>> -#include <xen/lib/x86/cpu-policy.h>
>>  
>> +#include <asm/alternative.h>
>>  #include <asm/asm_defns.h>
>> -#include <asm/cpufeature.h>
>> -#include <asm/processor.h>
>> +#include <asm/msr-index.h>
>>  
>>  #define rdmsr(msr,val1,val2) \
>>       __asm__ __volatile__("rdmsr" \
>> @@ -113,11 +108,6 @@ static inline int wrmsr_safe(unsigned int msr, uint64_t val)
>>      return -EFAULT;
>>  }
>>  
>> -#define rdpmc(counter,low,high) \
>> -     __asm__ __volatile__("rdpmc" \
>> -			  : "=a" (low), "=d" (high) \
>> -			  : "c" (counter))
>> -
> ... this removal wants mentioning in the description. I'm actually surprised this
> is unused - how does vPMU get away?

Kernel software can't really use RDPMC.  Everything else about perf
counters needs model specific knowledge, and given that model specific
knowledge, transforming the indices into another address space is just a
waste.

~Andrew