Re: [PATCH v2 2/2] s390x/cpumodel: Introduce dynamic feature groups

[David Hildenbrand]

>> Say the user has the option to select a model (zEC12, z13, z14), upper
>> layers always want to have a model that includes all backported security
>> features. While the host model can do that, CPU definitions can't. You
>> can't change default models within a QEMU release, or for older releases
>> (e.g., a z13).
>>
> 
> This is a good description of the main use case we're worried
> about in x86 too, and the main reason we have added versioned CPU
> models.
> 
> I remember I was planning to use `query-cpu-model-expansion` for
> "please give me the best configuration for this specific CPU
> model" (which would be very similar to the approach used in this
> series).  Now, I need to refresh my memory and try to remember
> why I concluded this approach wouldn't work for x86.

I would be interested in that - I don't really think exposing CPU
versions to the user is necessary here.

E.g., you can maintain the versions internally and enable the stored
features of the fitting one with "recommended-features=on...".

> 
> 
>>>
>>> Maybe its just the interface or the name. But I find this very non-intuitive
>>
>> I'm open for suggestions.
>>
>>>
>>> e.g. you wrote
>>>
>>>     Get the maximum possible feature set (e.g., including deprecated
>>>     features) for a CPU definition in the configuration ("everything that
>>>     could be enabled"):
>>>         -cpu z14,all-features=off,available-features=on
>>>
>>>     Get all valid features for a CPU definition:
>>>         -cpu z14,all-features=on
>>>
>>> What is the point of this? It is either the same as the one before, or it 
>>> wont
>>> be able to start. 
>>
>> valid != available, all != available. Yes, the model won't run unless
>> you are on pretty good HW :)
>>
>> Maybe I should just have dropped the last example, as it seems to
>> confuse people - it's mostly only relevant for introspection via CPU
>> model expansion.
>>
>> I am open for better names. e.g. all-features -> valid-features.
> 
> "all" is not a meaningful name to me.  It surely doesn't mean
> "all features in the universe", so it means a more specific set
> of features.  How is that set defined?
> 
> "valid" seems clearer, but we still need a description of what
> "valid" means exactly.
> 

So, we have

+static S390DynFeatGroupDef s390_dyn_feature_groups[] = {
+    /* "all" corresponds to our "full" definitions */
+    DYN_FEAT_GROUP_INIT("all-features", ALL, "Features valid for a CPU
definition"),
[...]
+};

it includes features that are not available - all features that could
theoretically be enabled for that CPU definition.

(e.g., "vx" was introduced with z13 and cannot be enabled for the z12.
It's part of the full model of a z13, but not of a z12)

-- 
Thanks,

David / dhildenb