On Sun, Jan 11, 2026 at 11:40:18AM -0800, Andrea Bolognani wrote:
> 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.
Looking back at previous conversations about the topic (should
probably have done that before replying O:-) it seems that Daniel
would rather not have the file show up at all on distros that don't
compile msr as a module[1] while Jim would like the default to be
disabled.
I think a good compromise is to keep the upstream default at the
meson level to be enabled, and disable it explicitly in the spec
file. Other distros will make their own choices about it, with Debian
obviously wanting to enable it. Anyone installing from source will
get the outcome that, while possibly unnecessary, has the most chance
of working correctly. Revised patch attached.
[1]
https://lists.libvirt.org/archives/list/[email protected]/message/G5TAPXMV6SG6PNA2RFFOSN6VZJBQ3ARE/
[2]
https://lists.libvirt.org/archives/list/[email protected]/message/VEO2RYMD2W24Y56COMBBPQ3VE4HHS2XH/
--
Andrea Bolognani / Red Hat / Virtualization
From d96b74b725473e1ecee688c2aee7bb9f83bf2678 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 | 2 ++
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, 33 insertions(+)
create mode 100644 src/util/libvirt-msr.modules-load.conf
diff --git a/libvirt.spec.in b/libvirt.spec.in
index ccfe75135b..b031c657d6 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1422,6 +1422,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/libvirt.spec)
-Dtls_priority=%{tls_priority} \
-Dsysctl_config=enabled \
-Dssh_proxy=enabled \
+ -Dmsr_module_load=disabled \
%{?enable_werror} \
-Dexpensive_tests=enabled \
-Dinit_script=systemd \
@@ -1509,6 +1510,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 \
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
