Re: [Xen-devel] [PATCH 04/10] x86/gen-cpuid: Create max and default variations of INIT_*_FEATURES

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.02.2020 11:29, Andrew Cooper wrote:
> On 27/02/2020 08:02, Jan Beulich wrote:
>> On 26.02.2020 21:22, Andrew Cooper wrote:
>>> For now, write the same content for both.  Update the users of the
>>> initialisers to use the new name, and extend xen-cpuid to dump both default
>>> and max featuresets.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Hypervisor and libxc parts
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Which other bit are you concerned with?  xen-cpuid.c is explicitly under
> x86 maintainership.
> 
>>
>>> --- a/tools/misc/xen-cpuid.c
>>> +++ b/tools/misc/xen-cpuid.c
>>> @@ -283,12 +283,18 @@ static void dump_info(xc_interface *xch, bool detail)
>>>                        nr_features, "Known", detail);
>>>      
>>> decode_featureset(xc_get_static_cpu_featuremask(XC_FEATUREMASK_SPECIAL),
>>>                        nr_features, "Special", detail);
>>> -    decode_featureset(xc_get_static_cpu_featuremask(XC_FEATUREMASK_PV),
>>> -                      nr_features, "PV Mask", detail);
>>> -    
>>> decode_featureset(xc_get_static_cpu_featuremask(XC_FEATUREMASK_HVM_SHADOW),
>>> -                      nr_features, "HVM Shadow Mask", detail);
>>> -    
>>> decode_featureset(xc_get_static_cpu_featuremask(XC_FEATUREMASK_HVM_HAP),
>>> -                      nr_features, "HVM Hap Mask", detail);
>>> +    decode_featureset(xc_get_static_cpu_featuremask(XC_FEATUREMASK_PV_MAX),
>>> +                      nr_features, "PV Max", detail);
>>> +    decode_featureset(xc_get_static_cpu_featuremask(XC_FEATUREMASK_PV_DEF),
>>> +                      nr_features, "PV Default", detail);
>>> +    
>>> decode_featureset(xc_get_static_cpu_featuremask(XC_FEATUREMASK_HVM_SHADOW_MAX),
>>> +                      nr_features, "HVM Shadow Max", detail);
>>> +    
>>> decode_featureset(xc_get_static_cpu_featuremask(XC_FEATUREMASK_HVM_SHADOW_DEF),
>>> +                      nr_features, "HVM Shadow Default", detail);
>>> +    
>>> decode_featureset(xc_get_static_cpu_featuremask(XC_FEATUREMASK_HVM_HAP_MAX),
>>> +                      nr_features, "HVM Hap Max", detail);
>>> +    
>>> decode_featureset(xc_get_static_cpu_featuremask(XC_FEATUREMASK_HVM_HAP_DEF),
>>> +                      nr_features, "HVM Hap Default", detail);
>> Spotting differences between max and default this way is, I assume,
>> going to be quite difficult / error prone.
> 
> Not any more or less than between other similar sets (most obviously,
> hap and shadow, but raw and host also tend to fairly similar).

Well, yes, but it may matter more for the (def,max) pairs.

>> Wouldn't it be better to
>> produce the default set in full, and then list just the extra items
>> in max?
> 
> I don't see how that would work.  The sets are either rendered as a hex
> bitmap (so spotting a different is fairly easy), or tabulated with
> feature names subdivided by word.

For the hex printing I agree it's fine this way. For the tabulated
printing, why not simply mask out all "default" bits from "max"
before producing the output, adjusting the headline accordingly?

>> Aiui max is always going to be a superset of def.
> 
> It is.  I did consider distinguishing using lower and upper case, which
> is about the only way I can think of sensibly merging the two sets together.
> 
> However, this is a pain to do in C, and it would result in the set being
> rendered differently depending on whether it was a static set, or a
> user-provided one.  It would also result in the case being inverted
> compared to the annotation character.
> 
> For now, I'm honestly not sure that it matters too much.  I'm probably
> going to give xen-cpuid an overhaul anyway (perhaps into python) to be a
> more useful calculator for policy settings.

Oh, okay. In that case perhaps indeed not worth to spend too much
effort here anymore.

Jan

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