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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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).

> 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.

> 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.

~Andrew

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