Re: [PATCH v4 01/15] x86/msr: Add missing includes of <asm/msr.h>

[Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>]

On Sun, 27 Apr 2025, Xin Li (Intel) wrote:

> For some reason, there are some TSC-related functions in the MSR
> header even though there is a tsc.h header.
> 
> To facilitate the relocation of rdtsc{,_ordered}() from <asm/msr.h>
> to <asm/tsc.h> and to eventually eliminate the inclusion of
> <asm/msr.h> in <asm/tsc.h>, add <asm/msr.h> to the source files that
> reference definitions from <asm/msr.h>.
> 
> Signed-off-by: Xin Li (Intel) <xin@xxxxxxxxx>
> Acked-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Acked-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> ---
> 
> Change in v4:
> *) Add missing includes in a different patch (Ilpo Järvinen).
> *) Add all necessary direct inclusions for msr.h (Ilpo Järvinen).
> 
> Change in v3:
> * Add a problem statement to the changelog (Dave Hansen).
> ---
>  arch/x86/events/msr.c                                         | 3 +++
>  arch/x86/events/perf_event.h                                  | 1 +
>  arch/x86/events/probe.c                                       | 2 ++

Under arch/x86/events/ a few files seem to be missing the include?

>  arch/x86/hyperv/ivm.c                                         | 1 +

Also under hyperv/ not all files are covered but I'm a bit hesitant to 
suggest a change there since I'm not sure if they (hypervisors) do 
something special w.r.t. msr.

>  arch/x86/include/asm/fred.h                                   | 1 +
>  arch/x86/include/asm/microcode.h                              | 2 ++
>  arch/x86/include/asm/mshyperv.h                               | 1 +
>  arch/x86/include/asm/msr.h                                    | 1 +
>  arch/x86/include/asm/suspend_32.h                             | 1 +
>  arch/x86/include/asm/suspend_64.h                             | 1 +
>  arch/x86/include/asm/switch_to.h                              | 2 ++

arch/x86/kernel/acpi/ ?
acrh/x86/kernel/cet.c ?
...

There seem to be quite many under arch/x86/ that still don't have it, I 
didn't list them all as there were so many after this point.

But that's up to x86 maintainers how throughout they want you to be.

This command may be helpful to exclude the files which already have the 
include so you can focus on the ones that may still be missing it:

git grep -l -e rdmsr -e wrmsr | grep -v -f <(git grep -l -e 'asm/msr\.h')

>  arch/x86/kernel/cpu/resctrl/pseudo_lock.c                     | 1 +
>  arch/x86/kernel/fpu/xstate.h                                  | 1 +
>  arch/x86/kernel/hpet.c                                        | 1 +
>  arch/x86/kernel/process_64.c                                  | 1 +
>  arch/x86/kernel/trace_clock.c                                 | 2 +-
>  arch/x86/kernel/tsc_sync.c                                    | 1 +
>  arch/x86/lib/kaslr.c                                          | 2 +-
>  arch/x86/mm/mem_encrypt_identity.c                            | 1 +
>  arch/x86/realmode/init.c                                      | 1 +
>  drivers/acpi/acpi_extlog.c                                    | 1 +
>  drivers/acpi/processor_perflib.c                              | 1 +
>  drivers/acpi/processor_throttling.c                           | 3 ++-
>  drivers/char/agp/nvidia-agp.c                                 | 1 +
>  drivers/cpufreq/amd-pstate-ut.c                               | 2 ++
>  drivers/crypto/ccp/sev-dev.c                                  | 1 +
>  drivers/edac/amd64_edac.c                                     | 1 +
>  drivers/edac/ie31200_edac.c                                   | 1 +
>  drivers/edac/mce_amd.c                                        | 1 +
>  drivers/hwmon/hwmon-vid.c                                     | 4 ++++
>  drivers/idle/intel_idle.c                                     | 1 +
>  drivers/misc/cs5535-mfgpt.c                                   | 1 +
>  drivers/net/vmxnet3/vmxnet3_drv.c                             | 4 ++++
>  drivers/platform/x86/intel/ifs/core.c                         | 1 +
>  drivers/platform/x86/intel/ifs/load.c                         | 1 +
>  drivers/platform/x86/intel/ifs/runtest.c                      | 1 +
>  drivers/platform/x86/intel/pmc/cnp.c                          | 1 +
>  drivers/platform/x86/intel/speed_select_if/isst_if_common.c   | 1 +
>  drivers/platform/x86/intel/speed_select_if/isst_if_mbox_msr.c | 1 +
>  drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c   | 1 +
>  drivers/platform/x86/intel/turbo_max_3.c                      | 1 +
>  .../platform/x86/intel/uncore-frequency/uncore-frequency.c    | 1 +
>  drivers/powercap/intel_rapl_common.c                          | 1 +
>  drivers/powercap/intel_rapl_msr.c                             | 1 +
>  .../thermal/intel/int340x_thermal/processor_thermal_device.c  | 1 +
>  drivers/thermal/intel/intel_tcc_cooling.c                     | 1 +
>  drivers/thermal/intel/x86_pkg_temp_thermal.c                  | 1 +
>  drivers/video/fbdev/geode/display_gx.c                        | 1 +
>  drivers/video/fbdev/geode/gxfb_core.c                         | 1 +
>  drivers/video/fbdev/geode/lxfb_ops.c                          | 1 +

Under drivers/ this looked pretty complete. Nice work.

Acked-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> # for pdx86

I also noticed these files might not need to include msr.h:

drivers/cpufreq/elanfreq.c
drivers/cpufreq/sc520_freq.c
drivers/accel/habanalabs/common/habanalabs_ioctl.c

...so if you want, you may consider optionally adding a cleanup patch to 
remove the include from them.

> --- a/drivers/video/fbdev/geode/gxfb_core.c
> +++ b/drivers/video/fbdev/geode/gxfb_core.c
> @@ -30,6 +30,7 @@
>  #include <linux/cs5535.h>
>  
>  #include <asm/olpc.h>
> +#include <asm/msr.h>

In wrong order.
>  
>  #include "gxfb.h"

--
 i.