All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: "Xin Li (Intel)" <xin@zytor.com>, mingo@redhat.com
Cc: LKML <linux-kernel@vger.kernel.org>,
	kvm@vger.kernel.org,  linux-perf-users@vger.kernel.org,
	linux-hyperv@vger.kernel.org,  virtualization@lists.linux.dev,
	linux-pm@vger.kernel.org,  linux-edac@vger.kernel.org,
	xen-devel@lists.xenproject.org,  linux-acpi@vger.kernel.org,
	linux-hwmon@vger.kernel.org,  Netdev <netdev@vger.kernel.org>,
	platform-driver-x86@vger.kernel.org,  tglx@linutronix.de,
	bp@alien8.de, dave.hansen@linux.intel.com,  x86@kernel.org,
	hpa@zytor.com, acme@kernel.org, jgross@suse.com,
	 andrew.cooper3@citrix.com, peterz@infradead.org,
	namhyung@kernel.org,  mark.rutland@arm.com,
	alexander.shishkin@linux.intel.com, jolsa@kernel.org,
	 irogers@google.com, adrian.hunter@intel.com,
	kan.liang@linux.intel.com,  wei.liu@kernel.org,
	ajay.kaher@broadcom.com,  bcm-kernel-feedback-list@broadcom.com,
	tony.luck@intel.com,  pbonzini@redhat.com, vkuznets@redhat.com,
	seanjc@google.com,  luto@kernel.org, boris.ostrovsky@oracle.com,
	kys@microsoft.com,  haiyangz@microsoft.com, decui@microsoft.com,
	dapeng1.mi@linux.intel.com
Subject: Re: [PATCH v4A 01/15] x86/msr: Add missing includes of <asm/msr.h>
Date: Fri, 2 May 2025 16:13:03 +0300 (EEST)	[thread overview]
Message-ID: <a34d7955-aa31-7bef-52cf-65dc4bb7a5c1@linux.intel.com> (raw)
In-Reply-To: <20250501054241.1245648-1-xin@zytor.com>

[-- Attachment #1: Type: text/plain, Size: 12224 bytes --]

On Wed, 30 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>
> Acked-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> 
> Change in v4A:
> *) Use "git grep -l -e $PATTERN | grep -v -f <(git grep -l -e 'asm/msr\.h')"
>    to ensure ALL required *direct* inclusion of <asm/msr.h> (Ilpo Järvinen).
> 
> 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/coco/sev/core.c                                    | 1 +
>  arch/x86/events/amd/core.c                                  | 1 +
>  arch/x86/events/amd/ibs.c                                   | 1 +
>  arch/x86/events/amd/iommu.c                                 | 2 ++
>  arch/x86/events/amd/lbr.c                                   | 1 +
>  arch/x86/events/amd/power.c                                 | 1 +
>  arch/x86/events/core.c                                      | 1 +
>  arch/x86/events/intel/bts.c                                 | 1 +
>  arch/x86/events/intel/core.c                                | 1 +
>  arch/x86/events/intel/cstate.c                              | 1 +
>  arch/x86/events/intel/ds.c                                  | 1 +
>  arch/x86/events/intel/knc.c                                 | 1 +
>  arch/x86/events/intel/p4.c                                  | 1 +
>  arch/x86/events/intel/p6.c                                  | 1 +
>  arch/x86/events/intel/pt.c                                  | 1 +
>  arch/x86/events/intel/uncore.c                              | 1 +
>  arch/x86/events/intel/uncore_discovery.c                    | 1 +
>  arch/x86/events/intel/uncore_nhmex.c                        | 1 +
>  arch/x86/events/intel/uncore_snb.c                          | 1 +
>  arch/x86/events/intel/uncore_snbep.c                        | 1 +
>  arch/x86/events/msr.c                                       | 2 ++
>  arch/x86/events/perf_event.h                                | 1 +
>  arch/x86/events/probe.c                                     | 2 ++
>  arch/x86/events/rapl.c                                      | 1 +
>  arch/x86/events/utils.c                                     | 1 +
>  arch/x86/events/zhaoxin/core.c                              | 1 +
>  arch/x86/hyperv/hv_apic.c                                   | 1 +
>  arch/x86/hyperv/hv_init.c                                   | 1 +
>  arch/x86/hyperv/hv_spinlock.c                               | 1 +
>  arch/x86/hyperv/hv_vtl.c                                    | 1 +
>  arch/x86/hyperv/ivm.c                                       | 1 +
>  arch/x86/include/asm/fred.h                                 | 1 +
>  arch/x86/include/asm/kvm_host.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/resctrl.h                              | 2 ++
>  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/sleep.c                                | 1 +
>  arch/x86/kernel/amd_nb.c                                    | 1 +
>  arch/x86/kernel/apic/apic.c                                 | 1 +
>  arch/x86/kernel/apic/apic_numachip.c                        | 1 +
>  arch/x86/kernel/cet.c                                       | 1 +
>  arch/x86/kernel/cpu/amd.c                                   | 1 +
>  arch/x86/kernel/cpu/aperfmperf.c                            | 1 +
>  arch/x86/kernel/cpu/bus_lock.c                              | 1 +
>  arch/x86/kernel/cpu/feat_ctl.c                              | 1 +
>  arch/x86/kernel/cpu/hygon.c                                 | 1 +
>  arch/x86/kernel/cpu/mce/inject.c                            | 1 +
>  arch/x86/kernel/cpu/microcode/core.c                        | 1 +
>  arch/x86/kernel/cpu/mshyperv.c                              | 1 +
>  arch/x86/kernel/cpu/resctrl/core.c                          | 1 +
>  arch/x86/kernel/cpu/resctrl/monitor.c                       | 1 +
>  arch/x86/kernel/cpu/resctrl/pseudo_lock.c                   | 1 +
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c                      | 1 +
>  arch/x86/kernel/cpu/sgx/main.c                              | 1 +
>  arch/x86/kernel/cpu/topology.c                              | 1 +
>  arch/x86/kernel/cpu/topology_amd.c                          | 1 +
>  arch/x86/kernel/cpu/tsx.c                                   | 1 +
>  arch/x86/kernel/cpu/zhaoxin.c                               | 1 +
>  arch/x86/kernel/fpu/core.c                                  | 1 +
>  arch/x86/kernel/fpu/xstate.c                                | 1 +
>  arch/x86/kernel/fpu/xstate.h                                | 1 +
>  arch/x86/kernel/fred.c                                      | 1 +
>  arch/x86/kernel/hpet.c                                      | 1 +
>  arch/x86/kernel/kvm.c                                       | 1 +
>  arch/x86/kernel/paravirt.c                                  | 1 +
>  arch/x86/kernel/process.c                                   | 1 +
>  arch/x86/kernel/process_64.c                                | 1 +
>  arch/x86/kernel/trace_clock.c                               | 2 +-
>  arch/x86/kernel/traps.c                                     | 1 +
>  arch/x86/kernel/tsc.c                                       | 1 +
>  arch/x86/kernel/tsc_sync.c                                  | 1 +
>  arch/x86/kvm/svm/avic.c                                     | 1 +
>  arch/x86/kvm/svm/sev.c                                      | 1 +
>  arch/x86/kvm/svm/svm.c                                      | 1 +
>  arch/x86/kvm/vmx/nested.c                                   | 1 +
>  arch/x86/kvm/vmx/pmu_intel.c                                | 1 +
>  arch/x86/kvm/vmx/sgx.c                                      | 1 +
>  arch/x86/kvm/vmx/vmx.c                                      | 1 +
>  arch/x86/lib/insn-eval.c                                    | 1 +
>  arch/x86/lib/kaslr.c                                        | 2 +-
>  arch/x86/mm/mem_encrypt_identity.c                          | 1 +
>  arch/x86/mm/tlb.c                                           | 1 +
>  arch/x86/pci/amd_bus.c                                      | 1 +
>  arch/x86/pci/mmconfig-shared.c                              | 3 ++-
>  arch/x86/power/cpu.c                                        | 1 +
>  arch/x86/realmode/init.c                                    | 1 +
>  arch/x86/virt/svm/sev.c                                     | 1 +
>  arch/x86/xen/enlighten_pv.c                                 | 1 +
>  arch/x86/xen/pmu.c                                          | 1 +
>  arch/x86/xen/suspend.c                                      | 1 +
>  drivers/accel/habanalabs/common/habanalabs_ioctl.c          | 2 --
>  drivers/acpi/acpi_extlog.c                                  | 1 +
>  drivers/acpi/processor_perflib.c                            | 1 +
>  drivers/acpi/processor_throttling.c                         | 6 +++++-
>  drivers/char/agp/nvidia-agp.c                               | 1 +
>  drivers/cpufreq/amd-pstate-ut.c                             | 2 ++
>  drivers/cpufreq/elanfreq.c                                  | 1 -
>  drivers/cpufreq/sc520_freq.c                                | 1 -
>  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 +
>  .../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 +
>  .../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 +
>  127 files changed, 142 insertions(+), 8 deletions(-)
> 

> diff --git a/arch/x86/kernel/trace_clock.c b/arch/x86/kernel/trace_clock.c
> index b8e7abe00b06..708d61743d15 100644
> --- a/arch/x86/kernel/trace_clock.c
> +++ b/arch/x86/kernel/trace_clock.c
> @@ -4,7 +4,7 @@
>   */
>  #include <asm/trace_clock.h>
>  #include <asm/barrier.h>
> -#include <asm/msr.h>
> +#include <asm/tsc.h>

Does this change belong to this patch?

It might even cause a build failure until the second patch which moves 
the tsc related things to the other file (unless there's indirect include 
path to asm/msr.h).

> diff --git a/arch/x86/lib/kaslr.c b/arch/x86/lib/kaslr.c
> index a58f451a7dd3..b5893928d55c 100644
> --- a/arch/x86/lib/kaslr.c
> +++ b/arch/x86/lib/kaslr.c
> @@ -8,7 +8,7 @@
>   */
>  #include <asm/asm.h>
>  #include <asm/kaslr.h>
> -#include <asm/msr.h>
> +#include <asm/tsc.h>

Same thing here.

>  #include <asm/archrandom.h>
>  #include <asm/e820/api.h>
>  #include <asm/shared/io.h>

> diff --git a/drivers/accel/habanalabs/common/habanalabs_ioctl.c b/drivers/accel/habanalabs/common/habanalabs_ioctl.c
> index 8729a0c57d78..dc80ca921d90 100644
> --- a/drivers/accel/habanalabs/common/habanalabs_ioctl.c
> +++ b/drivers/accel/habanalabs/common/habanalabs_ioctl.c
> @@ -17,8 +17,6 @@
>  #include <linux/uaccess.h>
>  #include <linux/vmalloc.h>
>  
> -#include <asm/msr.h>
> -

I suggested making a separate patch out of these removals. Currently you 
do them without any clear warning in the changelog which only talks about 
adding asm/msr.h.

> diff --git a/drivers/acpi/processor_throttling.c b/drivers/acpi/processor_throttling.c
> index 00d045e5f524..ecd7fe256153 100644
> --- a/drivers/acpi/processor_throttling.c
> +++ b/drivers/acpi/processor_throttling.c
> @@ -18,9 +18,13 @@
>  #include <linux/sched.h>
>  #include <linux/cpufreq.h>
>  #include <linux/acpi.h>
> +#include <linux/uaccess.h>
>  #include <acpi/processor.h>
>  #include <asm/io.h>
> -#include <linux/uaccess.h>
> +#include <asm/asm.h>

???

> +#ifdef CONFIG_X86
> +#include <asm/msr.h>
> +#endif


I really appreciate you took the effort to do this change the correct
way! :-)

--
 i.

  parent reply	other threads:[~2025-05-02 13:13 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-27  9:20 [PATCH v4 00/15] MSR code cleanup part one Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 01/15] x86/msr: Add missing includes of <asm/msr.h> Xin Li (Intel)
2025-04-29  9:45   ` Ilpo Järvinen
2025-04-29 17:27     ` Xin Li
2025-04-30  4:20     ` Michael Kelley
2025-04-30  9:12       ` Xin Li
2025-04-30  8:41     ` Xin Li
2025-04-30  9:17       ` Ilpo Järvinen
2025-04-30 17:34         ` Xin Li
2025-05-01  5:42     ` [PATCH v4A " Xin Li (Intel)
2025-05-02  9:04       ` [tip: x86/merge] x86/msr: Add explicit " tip-bot2 for Xin Li (Intel)
2025-05-02 13:13       ` Ilpo Järvinen [this message]
2025-05-02 17:50         ` [PATCH v4A 01/15] x86/msr: Add missing " Xin Li
2025-04-27  9:20 ` [PATCH v4 02/15] x86/msr: Move rdtsc{,_ordered}() to <asm/tsc.h> Xin Li (Intel)
2025-04-27 22:39   ` kernel test robot
2025-05-02  8:02   ` Ingo Molnar
2025-05-02 18:09     ` Xin Li
2025-05-02  8:18   ` Ingo Molnar
2025-05-02 18:01     ` Xin Li
2025-05-02  8:52   ` Ingo Molnar
2025-05-02 18:00     ` Xin Li
2025-05-02  9:04   ` [tip: x86/merge] " tip-bot2 for Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 03/15] x86/msr: Remove rdpmc() Xin Li (Intel)
2025-05-02  9:04   ` [tip: x86/merge] x86/msr: Remove the unused rdpmc() method tip-bot2 for Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 04/15] x86/msr: Rename rdpmcl() to rdpmc() Xin Li (Intel)
2025-05-02  9:04   ` [tip: x86/merge] " tip-bot2 for Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 05/15] x86/msr: Convert the rdpmc() macro into an always inline function Xin Li (Intel)
2025-05-02  9:04   ` [tip: x86/merge] x86/msr: Convert the rdpmc() macro to an __always_inline function tip-bot2 for Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 06/15] x86/xen/msr: Return u64 consistently in Xen PMC read functions Xin Li (Intel)
2025-05-02  9:04   ` [tip: x86/merge] x86/xen/msr: Return u64 consistently in Xen PMC xen_*_read functions tip-bot2 for Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 07/15] x86/msr: Convert __wrmsr() uses to native_wrmsr{,q}() uses Xin Li (Intel)
2025-05-02  9:04   ` [tip: x86/merge] " tip-bot2 for Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 08/15] x86/msr: Add the native_rdmsrq() helper Xin Li (Intel)
2025-05-02  9:04   ` [tip: x86/merge] " tip-bot2 for Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 09/15] x86/msr: Convert __rdmsr() uses to native_rdmsrq() uses Xin Li (Intel)
2025-05-02  9:04   ` [tip: x86/merge] " tip-bot2 for Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 10/15] x86/xen/msr: Remove calling native_{read,write}_msr{,_safe}() in pmu_msr_{read,write}() Xin Li (Intel)
2025-05-02  8:30   ` Ingo Molnar
2025-05-02  9:04   ` [tip: x86/merge] " tip-bot2 for Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 11/15] x86/xen/msr: Remove pmu_msr_{read,write}() Xin Li (Intel)
2025-05-02  9:04   ` [tip: x86/merge] " tip-bot2 for Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 12/15] x86/xen/msr: Remove the error pointer argument from set_seg() Xin Li (Intel)
2025-05-02  9:04   ` [tip: x86/merge] " tip-bot2 for Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 13/15] x86/pvops/msr: refactor pv_cpu_ops.write_msr{,_safe}() Xin Li (Intel)
2025-05-02  9:04   ` [tip: x86/merge] x86/pvops/msr: Refactor pv_cpu_ops.write_msr{,_safe}() tip-bot2 for Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 14/15] x86/msr: Replace wrmsr(msr, low, 0) with wrmsrq(msr, low) Xin Li (Intel)
2025-05-02  9:04   ` [tip: x86/merge] " tip-bot2 for Xin Li (Intel)
2025-04-27  9:20 ` [PATCH v4 15/15] x86/msr: Change the function type of native_read_msr_safe() Xin Li (Intel)
2025-05-02  9:04   ` [tip: x86/merge] " tip-bot2 for Xin Li (Intel)
2025-05-02 14:13 ` [PATCH v4 00/15] MSR code cleanup part one Michael Kelley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a34d7955-aa31-7bef-52cf-65dc4bb7a5c1@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=ajay.kaher@broadcom.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=bp@alien8.de \
    --cc=dapeng1.mi@linux.intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=decui@microsoft.com \
    --cc=haiyangz@microsoft.com \
    --cc=hpa@zytor.com \
    --cc=irogers@google.com \
    --cc=jgross@suse.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=kys@microsoft.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=virtualization@lists.linux.dev \
    --cc=vkuznets@redhat.com \
    --cc=wei.liu@kernel.org \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    --cc=xin@zytor.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.