Re: [PATCH v3 3/3] Add support for RAPL MSRs in KVM/Qemu

[Anthony Harivel]

Daniel P. Berrangé, Mar 04, 2024 at 15:48:
> On Mon, Mar 04, 2024 at 03:41:02PM +0100, Anthony Harivel wrote:
> > 
> > Hi Daniel,
> > 
> > > > +        if (s->msr_energy.enable == true) {
> > >
> > > This looks to be where we need to check that both the host CPU
> > > vendor is intel, and the guest CPU vendor is intel, and that
> > > the host CPU has the RAPL feature we're using.
> >
> > Checking for the host cpu and RAPL enable is fine and done. 
> > 
> > But checking for guest CPU is confusing me. 
> > The RAPL feature is enable only with KVM enable. 
> > This means "-cpu" can only be "host" or its derivative that essentially 
> > copy the host CPU definition, no?
>
> KVM can use any named CPU.
>
> > That means if we are already checking the host cpu we don't need to do 
> > anything for the guest, do we ?
>
> When I first wrote this I though it would be as simple as checknig a
> CPUID feature flag. That appears to not be the case, however, as Linux
> is just checking for various CPU models directly. With that in mind
> perhaps we should just check of the guest CPU model vendor
> == CPUID_VENDOR_INTEL and leave it at that.
>
> eg, create an error if running an AMD CPU such as $QEMU -cpu EPYC

The idea looks good to me. Now the hiccups of this solution is that 
I cannot find a way to reach CPUArchState at this level of code (i.e 
kvm_arch_init() ) with only the MachineState or the KVMState. 
I can only reach the topology with x86_possible_cpu_arch_ids().

CPUArchState struct is holding the cpuid_vendor variables where we can 
use IS_INTEL_CPU() for checking.

Maybe you know the trick that I miss ? 

Regards,
Anthony

>
> With regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|