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

[David Hildenbrand]

On 26.11.19 13:59, Christian Borntraeger wrote:
> re-adding ccs from the cover-letter
> 
>>>> On 25.11.19 18:20, David Hildenbrand wrote:
>>>>
>>>> As soon as dynamic feature groups are used, the CPU model becomes
>>>> migration-unsafe. Upper layers can expand these models to migration-safe
>>>> and static variants, allowing them to be migrated.
>>>
>>> I really dislike that. I am trying to get rid of the unsafe variants (e.g. 
>>> now
>>> defaulting to host-model instead of host-passthrough). I do not want to give
>>> users new ways of hurting themselves.
>>>
>>
>> Please note that this is just on the bare command line. Libvirt and friends 
>> will expand the model and have proper migration in place. What exactly is 
>> your concern in that regard?
> 
> What is then the value? libvirt can also use host-model or  baselining if 
> necessary.
> And for the command line this will just add more opportunity to shot yourself 
> in the
> foot, no?

I don't think so. It's in no way more dangerous than "-cpu host" or
"-cpu max". And it is in no way more dangerous than the discussed CPU
versions, where even a "-cpu z13" would no longer be migration-safe.

You could - in theory - baseline(z13, host), but it could suddenly
fallback to a, say, zEC12 - and that's not what you asked for. And you
should not simply mask of deprecated features when baselining. Sure, we
could eventually add config knobs for that , but ...

... I really do like the part where you can specify on the command line
to have specific CPU definition, but including all available/recommended
features (e.g., nested virtualization).

> 
> Let me put it this way, I might have misunderstood what you are trying to do 
> here,
> but if I do not get, then others (e.g. users) will also not get it.

I remember you participated in the relevant discussions. That's where we
also agreed that versioned CPU models on s390x don't make any sense. But
I can refine what's included in this patch description

"There is the demand from higher levels in the stack to "have the
best CPU model possible on a given accelerator, firmware and HW"" - the
best CPU model for a specific *CPU definition*.

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

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

-- 
Thanks,

David / dhildenb