Re: [PATCH v4 01/15] x86/msr: Add missing includes of

Xin Li
2025-04-29T17:28:04Z

On 4/29/2025 2:45 AM, Ilpo Järvinen wrote:
> 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@zytor.com>
>> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
>> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>> ---
>>
>> 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@linux.intel.com> # for pdx86

Thanks a lot!

>
> 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 am more than happy to send v4A to address all your comments.