Re: [Xen-devel] [PATCH 08/10] x86/cpuid: Introduce and use default CPUID policies

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.02.2020 11:55, Andrew Cooper wrote:
> On 27/02/2020 08:19, Jan Beulich wrote:
>> On 26.02.2020 21:22, Andrew Cooper wrote:
>>> For now, the default and max policies remain identical, but this will change
>>> in the future.  Write calculate_{pv,hvm}_def_policy() in a way which will 
>>> cope
>>> with simple feature differences for now.
>>>
>>> Update XEN_SYSCTL_get_cpu_policy and init_domain_cpuid_policy() to use the
>>> default policies.
>> For the sysctl the statement looks to be broader than reality,
>> as (of course) you don't touch XEN_SYSCTL_cpu_policy_*_max.
> 
> I'm afraid I don't understand what you mean here.  What would I need to
> touch in XEN_SYSCTL_cpu_policy_*_max at all?

Nothing, and hence my "too broad" response. Only part of
XEN_SYSCTL_get_cpu_policy gets updated. But yes, thinking about it
again, I think I see now how you mean this. So never mind.

>>> @@ -381,6 +386,23 @@ static void __init calculate_pv_max_policy(void)
>>>      p->extd.raw[0xa] = EMPTY_LEAF; /* No SVM for PV guests. */
>>>  }
>>>  
>>> +static void __init calculate_pv_def_policy(void)
>>> +{
>>> +    struct cpuid_policy *p = &pv_def_cpuid_policy;
>>> +    uint32_t pv_featureset[FSCAPINTS];
>>> +    unsigned int i;
>>> +
>>> +    *p = pv_max_cpuid_policy;
>>> +    cpuid_policy_to_featureset(p, pv_featureset);
>>> +
>>> +    for ( i = 0; i < ARRAY_SIZE(pv_featureset); ++i )
>>> +        pv_featureset[i] &= pv_def_featuremask[i];
>>> +
>>> +    sanitise_featureset(pv_featureset);
>>> +    cpuid_featureset_to_policy(pv_featureset, p);
>>> +    recalculate_xstate(p);
>>> +}
>> Is there a reason the call to guest_common_feature_adjustments()
>> is missing here?
> 
> Yes, for the same reason that other logic is dropped.  Inheriting from
> pv_max_cpuid_policy means that it has already been run on this object.
> 
> The host to *_max derivation is non-trivial.  Some features get added
> in, others are conditional on external factors.  The *_max to *_def
> derivation is much more simple in comparison.
> 
> Long term, I expect this logic to move into libx86 and further simplify
> cpuid.c
> 
> However, I'm not sure why guest_common_feature_adjustments() is special
> compared to the other removed logic, and why it should be called out.

Well, the oddity isn't with removed logic (and in fact in this patch I
can't see much of a removal of anything), but with the call being there
in calculate_hvm_def_policy(). This difference, if intentional, is what
I think needs calling out.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel