On 9/4/25 17:59, Hector Cao wrote:


On Fri, Sep 5, 2025 at 1:48 AM Jim Fehlig <[email protected] <mailto:[email protected]>> wrote:

    On 9/4/25 17:00, Hector Cao wrote:
     >
     >
     > On Thu, Sep 4, 2025 at 11:40 PM Jim Fehlig <[email protected]
    <mailto:[email protected]>
     > <mailto:[email protected] <mailto:[email protected]>>> wrote:
     >
     >     On 9/3/25 06:49, Hector Cao wrote:
     >      > On recent Intel CPUs, some of the CPU features (mostly
     >      > vmx-* subfeatures) 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 file for msr
     >      >    this is only enabled:
     >      >     - On x86 arch
     >      >     - If user asks for it explicitly via a build option:
     >      >       meson -Dmsr_module_load=true build
     >      >       By default, it is disabled.
     >
     >     Thanks for the build option, and having it disabled by default! Some
    distros or
     >     distro flavors may not want this functionality.
     >
     >
     > Hello Jim,
     > Thanks for the feedback,
     >
     > I realized that the commit description is wrong wrt to the actual code,
    my bad.

    Heh, I totally missed that. I'd much prefer if the code matched the original
    description :-).


Sorry about this again

     > I propose the let the discussions continue in this thread to gather all 
the
     > feedbacks but ultimately I will need to submit a v2
     > So here is the updated commit message:
     >
     > This commit introduces following changes:
     >
     >   - install modules-load file for msr
     >      this is only enabled:
     >       - On x86 arch
     >       - the file installation is enabled by default, it can be disabled
    via a
     > build option:
     >         meson -Dmsr_module_load=false build

    Let's hear what others say, but my vote is to have it disabled by default.


I hear and understand your preference.
If we enable the module installation only when qemu driver is enabled, do you think that
it is more "acceptable" ?

That would be a nice improvement to this patch IMO. The file could be packaged with the qemu driver, which reminds me you'll need a corresponding change to libvirt.spec.in.

Regards,
Jim

Reply via email to