[PATCH v2 0/1] x86: install modules-load.d file to load msr module
This submission is a follow-up (v2) of the discussion (RFC) that was happening at: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/ADJ4A... On x86, libvirt makes use of the API exposed by the msr kernel module to read the MSR (Model Specific Registers) values. On some Linux distros, this module is not loaded by default so libvirt might fail to read the MSR values and falls back to /dev/kvm to obtain these values. KVM might be behind in term of exposing all the values of the MSRs so using /dev/kvm is not ideal. This submission tries to make libvirt deploy the modules-load.d configuration so that linux loads the msr module at next boot. One caveat, libvirt installs only the file and does not trigger the load that is only done at next reboot, it is up to each distro to trigger immediately the load of the module without a reboot. v2: - only install the file when qemu driver is enabled Hector Cao (1): x86: install modules-load.d file to load msr module meson_options.txt | 1 + src/util/meson.build | 19 +++++++++++++++++++ src/util/modules-load.d/msr.conf | 1 + tools/virt-host-validate-qemu.c | 7 +++++++ 4 files changed, 28 insertions(+) create mode 100644 src/util/modules-load.d/msr.conf -- 2.45.2
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. Signed-off-by: Hector Cao <hector.cao@canonical.com> --- meson_options.txt | 1 + src/util/meson.build | 19 +++++++++++++++++++ src/util/modules-load.d/msr.conf | 1 + tools/virt-host-validate-qemu.c | 7 +++++++ 4 files changed, 28 insertions(+) create mode 100644 src/util/modules-load.d/msr.conf diff --git a/meson_options.txt b/meson_options.txt index 3dc3e8667b..5a9e067407 100644 --- a/meson_options.txt +++ 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') diff --git a/src/util/meson.build b/src/util/meson.build index 69ef49139a..024c05f0dd 100644 --- a/src/util/meson.build +++ 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 + (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 \ No newline at end of file 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 diff --git a/tools/virt-host-validate-qemu.c b/tools/virt-host-validate-qemu.c index 833bb1b914..f582e1146e 100644 --- a/tools/virt-host-validate-qemu.c +++ 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; + } + if (virHostValidateDeviceExists("QEMU", "/dev/vhost-net", VIR_VALIDATE_WARN, _("Load the 'vhost_net' module to improve performance of virtio networking")) < 0) -- 2.45.2
Hello, A gentle ping for this thread, Can someone please take a look please ! Best Hector
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
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/devel@lists.libvirt.org/message/G5TA... [2] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/VEO2... -- Andrea Bolognani / Red Hat / Virtualization
Thanks Andreas, Really appreciate your feedback and suggestions, I took a look at your revised patch, it looks great, I have however a question: +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 Do we need the second "if get_option('msr_module_load').enabled()" since we already check it in the outer if ?
On Mon, Jan 12, 2026 at 10:56:54AM +0000, Hector CAO via Devel wrote:
Thanks Andreas,
Really appreciate your feedback and suggestions,
I took a look at your revised patch, it looks great, I have however a question:
+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
Do we need the second "if get_option('msr_module_load').enabled()" since we already check it in the outer if ?
Yes. The outer check is there to skip the entire block if the user explicitly disabled the feature, while the inner ones are to ensure that we error out if the user explicitly asked for it to be enabled but it's not possible to do so (wrong OS or architecture). But I have actually gotten it wrong, because as it is it doesn't ensure that installation of the file is automatically disabled if the OS is not Linux or the architecture is not x86. This is what it should look like instead: if not get_option('msr_module_load').disabled() msr_module_load = true if host_machine.system() != 'linux' if get_option('msr_module_load').enabled() error('msr module loading can be enabled on Linux only') else msr_module_load = false 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') else msr_module_load = false endif endif if msr_module_load conf.set('WITH_MSR_MODULE_LOAD', 1) endif endif I believe it's correct this time around O:-) -- Andrea Bolognani / Red Hat / Virtualization
Hi Andreas, Now, I get the reason of explicitly specific .disabled() and .enabled() Indeed, the returned option is a "feature" and the value range is not boolean. With your updated logic, I still however have a question: Do we really need the ... else msr_module_load = false endif ... and per consequence the msr_module_load variable, I think we can count on error(...) to stop the build to continue, does that make sense ? Hector
Sorry, I took a look again and think that the updated snippet is fine, Please ignore my previous question, LGTM, let me update the v2 patch to take into account your various proposed changes. Thanks,
participants (3)
-
Andrea Bolognani -
Hector CAO -
Hector Cao