Re: [Xen-devel] [PATCH v2 2/3] x86/vpmu: Disable VPMU if guest's CPUID indicates no PMU support

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/02/17 15:02, Boris Ostrovsky wrote:
> On 02/22/2017 09:38 AM, Jan Beulich wrote:
>>>>> On 22.02.17 at 15:15, <boris.ostrovsky@xxxxxxxxxx> wrote:
>>> On 02/22/2017 04:55 AM, Jan Beulich wrote:
>>>>>>> On 17.02.17 at 18:40, <boris.ostrovsky@xxxxxxxxxx> wrote:
>>>>> --- a/xen/arch/x86/cpu/vpmu_intel.c
>>>>> +++ b/xen/arch/x86/cpu/vpmu_intel.c
>>>>> @@ -884,6 +884,10 @@ int vmx_vpmu_initialise(struct vcpu *v)
>>>>>      if ( vpmu_mode == XENPMU_MODE_OFF )
>>>>>          return 0;
>>>>>  
>>>>> +    if ( MASK_EXTR(v->domain->arch.cpuid->basic.raw[0xa].a, 
>>>>> +                   PMU_VERSION_MASK) == 0 )
>>>>> +        return -EINVAL;
>>>> How about other unsupported (too large) values?
>>> Yes, we can check here for version >=5 as well.
>>>
>>> (I don't think we should make this additional test in
>>> update_domain_cpuid_info())
>> ... because of ... ? After all it's the purpose of this patch to not
>> expose the vPMU in such cases, which imo ought to be done
>> consistently in both places.
> Because I felt that having zero as a version is an indication of
> explicit admin's desire to disable VPMU. Too high a version is more
> likely misconfiguration and can be taken care of during VPMU initialization.

I would expect the 0 case to be disabled.  5 will be a real PMU version
number sometime (probably).

I think the code as-is is ok, although it would be nice to extend the
basic{} union to have a named uint8_t for pmu_version.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel