[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7df3c16b-9353-4f30-a13b-1f0eb8563e69@zytor.com>
Date: Tue, 29 Apr 2025 10:27:09 -0700
From: Xin Li <xin@...or.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: LKML <linux-kernel@...r.kernel.org>, kvm@...r.kernel.org,
linux-perf-users@...r.kernel.org, linux-hyperv@...r.kernel.org,
virtualization@...ts.linux.dev, linux-pm@...r.kernel.org,
linux-edac@...r.kernel.org, xen-devel@...ts.xenproject.org,
linux-acpi@...r.kernel.org, linux-hwmon@...r.kernel.org,
Netdev <netdev@...r.kernel.org>, platform-driver-x86@...r.kernel.org,
tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
acme@...nel.org, jgross@...e.com, andrew.cooper3@...rix.com,
peterz@...radead.org, namhyung@...nel.org, mark.rutland@....com,
alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
irogers@...gle.com, adrian.hunter@...el.com, kan.liang@...ux.intel.com,
wei.liu@...nel.org, ajay.kaher@...adcom.com,
bcm-kernel-feedback-list@...adcom.com, tony.luck@...el.com,
pbonzini@...hat.com, vkuznets@...hat.com, seanjc@...gle.com,
luto@...nel.org, boris.ostrovsky@...cle.com, kys@...rosoft.com,
haiyangz@...rosoft.com, decui@...rosoft.com,
dapeng1.mi@...ux.intel.com
Subject: Re: [PATCH v4 01/15] x86/msr: Add missing includes of <asm/msr.h>
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@...or.com>
>> Acked-by: Dave Hansen <dave.hansen@...ux.intel.com>
>> Acked-by: Peter Zijlstra (Intel) <peterz@...radead.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@...ux.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.
Powered by blists - more mailing lists