On Thu, Sep 18, 2025 at 02:13:57AM +0200, Hector Cao wrote:
> On recent Intel CPUs, some of the CPU features (e.g.
> vmx-* sub-features) are listed and controlled via the
> MSRs (Model Specific Registers) instead of the traditional
> CPUID instruction method.
>
> To be able to read the MSR's values, the kernel module msr
> has to be loaded and the values can be read via /dev/cpu/*/msr.
>
> This commit introduces following changes:
>
> - install modules-load.d file for msr for x86 and when qemu
> driver is enabled.
> by default, this option is enabled but user can disable it
> via a build option:
> meson ... -Dmsr_module_load=false ... build
>
> - modify virt-host-validate to check of /dev/cpu/*/msr is
> available.
Hint: if you need to list the things that your patch does in the
commit message, that's usually a pretty good indication that you
should have split it into separate patches instead ;)
> +++ b/meson_options.txt
> @@ -135,3 +135,4 @@ option('sysctl_config', type: 'feature', value: 'auto',
> description: 'Whether to
> # dep:sysctl_config
> option('userfaultfd_sysctl', type: 'feature', value: 'auto', description:
> 'Whether to install sysctl config for enabling unprivileged userfaultfd')
> option('tls_priority', type: 'string', value: 'NORMAL', description: 'set
> the default TLS session priority string')
> +option('msr_module_load', type: 'boolean', value: true, description:
> 'Install modules-load.d file for msr module on x86')
I think this should be a feature rather than a boolean, with default
value "auto", i.e. "do the right thing".
> diff --git a/src/util/modules-load.d/msr.conf
> b/src/util/modules-load.d/msr.conf
> new file mode 100644
> index 0000000000..3e5ee7fa15
> --- /dev/null
> +++ b/src/util/modules-load.d/msr.conf
> @@ -0,0 +1 @@
> +msr
The name of the installed file should probably be
/etc/modules-load.d/libvirt-msr.conf
to make its origin self-explanatory to the admin. Relatedly, the name
of the source file should probably be
src/util/libvirt-msr.modules-load.conf
to follow the existing pattern: see for example
src/qemu/libvirt-qemu.sysusers.conf
src/remote/libvirtd.sysctl
src/remote/libvirtd.policy.in
and so on.
> +++ b/src/util/meson.build
> @@ -225,3 +225,22 @@ if conf.has('WITH_LIBVIRTD')
> endif
>
> util_inc_dir = include_directories('.')
> +
> +# Install the modules-load.d file to load the MSR module
> +# NB: For the file to taken into account and the module load to happen,
> users must trigger
> +# the modules-load.d reload (by reboot or restart the systemd-modules-load
> service).
> +msr_module_load = get_option('msr_module_load')
> +if (msr_module_load and
> + get_option('driver_qemu').enabled() and
I don't think this should be conditional on whether the QEMU driver
is enabled. The function that needs the kernel module to be loaded is
in the generic utility code and it's invoked by the x86 CPU driver,
so we should install the file unconditionally.
> + (host_machine.cpu_family() == 'x86' or host_machine.cpu_family() ==
> 'x86_64'))
> + message('msr_module_load: Enabling installation of modules-load.d/msr.conf
> for x86')
> + install_data(
> + 'modules-load.d/msr.conf',
> + install_dir: join_paths(sysconfdir, 'modules-load.d')
> + )
> +elif msr_module_load
> + message('msr_module_load: option enabled, but not applicable : skipping')
> +else
> + message('msr_module_load: option disabled: skipping')
> +endif
We don't really use message() in libvirt so let's not start doing
that now. On the other hand, we *do* include information about
whether optional files have been installed in the meson summary, so
please make sure that happens. See for example how the sysctl_config
option is handled.
We also usually try to perform fairly strict validation for meson
arguments, erroring out when unreasonable things are being requested.
That's considered preferrable to silently failing to comply with the
user's explicit requests.
Putting it all together, this is what the check should look like:
if not get_option('msr_module_load').disabled()
if host_machine.system() != 'linux'
if get_option('msr_module_load').enabled()
error('msr module loading can be enabled on Linux only')
endif
endif
if host_machine.cpu_family() not in [ 'x86', 'x86_64' ]
if get_option('msr_module_load').enabled()
error('msr module loading can be enabled on x86 arches only')
endif
endif
conf.set('WITH_MSR_MODULE_LOAD', 1)
endif
And the installation part:
if conf.has('WITH_MSR_MODULE_LOAD')
install_data(
'libvirt-msr.modules-load.conf',
install_dir: sysconfdir / 'modules-load.d',
rename: 'libvirt-msr.conf',
)
endif
Note the use of '/' instead of join_paths() and the trailing comma
for all lines, including the last one. Again, this follows the
established patterns instead of diverging from them.
> +++ b/tools/virt-host-validate-qemu.c
> @@ -95,6 +95,13 @@ int virHostValidateQEMU(void)
> virValidatePass();
> }
>
> + if (arch == VIR_ARCH_X86_64) {
> + if (virHostValidateDeviceExists("QEMU", "/dev/cpu/0/msr",
> + VIR_HOST_VALIDATE_WARN,
> + _("Load the 'msr' module for better
> x86 CPU features detection")) < 0)
> + ret = -1;
> + }
On the meson side, the file gets installed for both i386 and x86_64,
so it seems to me that we should run the check on the /dev node for
both architectures here as well. You can use the
ARCH_IS_X86(arch)
helper. Also it's just VIR_VALIDATE_WARN now. And as hinted at the
very start this should be a separate patch.
Finally, there's the issue of the spec file not having been updated
after adding this file. Currently that results in the following
failure when building the RPM:
Installed (but unpackaged) file(s) found:
/etc/modules-load.d/msr.conf
which is quite expected. We could take two roads here.
One is to ensure that we explicitly pass -Dmsr_module_load=enabled
when building on x86 arches, -Dmsr_module_load=disabled otherwise
(including MinGW builds), then make the file installation
conditional.
The other option is to rely on the fact that Fedora and RHEL have the
msr module as built-in and always pass -Dmsr_module_load=disabled,
avoiding a bunch of complexity.
I'm not 100% sure which one I like better, to be honest. The attached
patch implements the former, as well as the other review comments.
--
Andrea Bolognani / Red Hat / Virtualization
From 031201dc6adb4bff784140768e549c8bd6c9096e Mon Sep 17 00:00:00 2001
From: Andrea Bolognani <[email protected]>
Date: Sun, 11 Jan 2026 20:35:10 +0100
Subject: [PATCH] msr_module_load
---
libvirt.spec.in | 18 ++++++++++++++++++
meson.build | 14 ++++++++++++++
meson_options.txt | 1 +
src/util/libvirt-msr.modules-load.conf | 1 +
src/util/meson.build | 8 ++++++++
tools/virt-host-validate-qemu.c | 7 +++++++
6 files changed, 49 insertions(+)
create mode 100644 src/util/libvirt-msr.modules-load.conf
diff --git a/libvirt.spec.in b/libvirt.spec.in
index ccfe75135b..305ddc0e5f 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -211,6 +211,13 @@
%define with_dmidecode 0%{!?_without_dmidecode:1}
%endif
+%define with_msr_module_load 0
+%if 0%{?fedora} || 0%{?rhel}
+ %ifarch %{arches_x86}
+ %define with_msr_module_load 0%{!?_without_msr_module_load:1}
+ %endif
+%endif
+
%define with_modular_daemons 0
%if 0%{?fedora} || 0%{?rhel}
%define with_modular_daemons 1
@@ -1338,6 +1345,12 @@ exit 1
%define arg_remote_mode -Dremote_default_mode=legacy
%endif
+%if %{with_msr_module_load}
+ %define arg_msr_module_load -Dmsr_module_load=enabled
+%else
+ %define arg_msr_module_load -Dmsr_module_load=disabled
+%endif
+
%define when %(date +"%%F-%%T")
%define where %(hostname)
%define who %{?packager}%{!?packager:Unknown}
@@ -1422,6 +1435,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/libvirt.spec)
-Dtls_priority=%{tls_priority} \
-Dsysctl_config=enabled \
-Dssh_proxy=enabled \
+ %{?arg_msr_module_load} \
%{?enable_werror} \
-Dexpensive_tests=enabled \
-Dinit_script=systemd \
@@ -1509,6 +1523,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/libvirt.spec)
-Dstorage_zfs=disabled \
-Dsysctl_config=disabled \
-Dssh_proxy=disabled \
+ -Dmsr_module_load=disabled \
-Dtests=disabled \
-Dudev=disabled \
-Dwireshark_dissector=disabled \
@@ -2512,6 +2527,9 @@ exit 0
%endif
%dir %{_datadir}/libvirt/cpu_map
%{_datadir}/libvirt/cpu_map/*.xml
+ %if %{with_msr_module_load}
+%{_sysconfdir}/modules-load.d/libvirt-msr.conf
+ %endif
%if %{with_wireshark}
%files wireshark
diff --git a/meson.build b/meson.build
index 2f807bc3a2..7bcdb35512 100644
--- a/meson.build
+++ b/meson.build
@@ -2042,6 +2042,19 @@ if prio == 'auto'
endif
conf.set_quoted('TLS_PRIORITY', prio)
+if not get_option('msr_module_load').disabled()
+ if host_machine.system() != 'linux'
+ if get_option('msr_module_load').enabled()
+ error('msr module loading can be enabled on Linux only')
+ endif
+ endif
+ if host_machine.cpu_family() not in [ 'x86', 'x86_64' ]
+ if get_option('msr_module_load').enabled()
+ error('msr module loading can be enabled on x86 only')
+ endif
+ endif
+ conf.set('WITH_MSR_MODULE_LOAD', 1)
+endif
# test options
@@ -2337,6 +2350,7 @@ misc_summary = {
'pm_utils': conf.has('WITH_PM_UTILS'),
'SSH proxy': conf.has('WITH_SSH_PROXY'),
'sysctl config': conf.has('WITH_SYSCTL'),
+ 'msr module load': conf.has('WITH_MSR_MODULE_LOAD'),
'tests': tests_enabled,
'TLS priority': conf.get_unquoted('TLS_PRIORITY'),
'virt-host-validate': conf.has('WITH_HOST_VALIDATE'),
diff --git a/meson_options.txt b/meson_options.txt
index e12ace4e11..4b6c030e92 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -133,3 +133,4 @@ option('pm_utils', type: 'feature', value: 'auto',
description: 'use pm-utils fo
option('ssh_proxy', type: 'feature', value: 'auto', description: 'Build
ssh-proxy for ssh over vsock')
option('sysctl_config', type: 'feature', value: 'auto', description: 'Whether
to install sysctl configs')
option('tls_priority', type: 'string', value: 'auto', description: 'set the
default TLS session priority string')
+option('msr_module_load', type: 'feature', value: 'auto', description:
'Install modules-load.d file for msr module')
diff --git a/src/util/libvirt-msr.modules-load.conf
b/src/util/libvirt-msr.modules-load.conf
new file mode 100644
index 0000000000..3e5ee7fa15
--- /dev/null
+++ b/src/util/libvirt-msr.modules-load.conf
@@ -0,0 +1 @@
+msr
diff --git a/src/util/meson.build b/src/util/meson.build
index 4950a795cc..3d2dbe199d 100644
--- a/src/util/meson.build
+++ b/src/util/meson.build
@@ -223,4 +223,12 @@ if conf.has('WITH_LIBVIRTD')
}
endif
+if conf.has('WITH_MSR_MODULE_LOAD')
+ install_data(
+ 'libvirt-msr.modules-load.conf',
+ install_dir: sysconfdir / 'modules-load.d',
+ rename: 'libvirt-msr.conf',
+ )
+endif
+
util_inc_dir = include_directories('.')
diff --git a/tools/virt-host-validate-qemu.c b/tools/virt-host-validate-qemu.c
index c5c1a0ab7e..68e5d037c8 100644
--- a/tools/virt-host-validate-qemu.c
+++ b/tools/virt-host-validate-qemu.c
@@ -104,6 +104,13 @@ int virHostValidateQEMU(void)
virValidatePass();
}
+ if (ARCH_IS_X86(arch)) {
+ if (virHostValidateDeviceExists("QEMU", "/dev/cpu/0/msr",
+ VIR_VALIDATE_WARN,
+ _("Load the 'msr' module for better
x86 CPU features detection")) < 0)
+ ret = -1;
+ }
+
if (virHostValidateDeviceExists("QEMU", "/dev/vhost-net",
VIR_VALIDATE_WARN,
_("Load the 'vhost_net' module to improve
performance of virtio networking")) < 0)
--
2.52.0
