lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ