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

[Michael Kelley <mhklinux@xxxxxxxxxxx>]

From: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> Sent: Tuesday, April 29, 
2025 2:46 AM
> 
> 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.

I've worked on the Hyper-V code in Linux for 8 years or so, first as
a Microsoft employee, and more recently as a retiree. :-) I'm not
aware of anything special w.r.t. MSR access for Hyper-V guests.
All the normal Linux code for accessing MSRs just works. Hyper-V
*does* provide a set of synthetic MSRs that are unique to
Hyper-V, but they are also accessed using normal Linux code. Of
course, at runtime the access to these synthetic MSRs always
traps to the hypervisor.

I'm planning to apply Xin Li's patch set and make sure nothing
breaks for Hyper-V guests, and particularly when running as an
SEV-SNP or TDX guest. Hopefully I can do that by early next week
at the latest.

Michael

> 
> >  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.