Re: [Qemu-devel] [PATCH 00/10] s390x: new guest features

[David Hildenbrand]

On 24.04.19 10:40, Christian Borntraeger wrote:
> 
> 
> On 23.04.19 14:11, David Hildenbrand wrote:
>> On 18.04.19 13:31, Christian Borntraeger wrote:
>>> Adding generation 15.
>>>
>>> Some interesting aspects:
>>> - conditional SSKE and bpb are deprecated. This patch set addresses that
>>>   for csske.
>>> - no name yet for gen15, I suggest to use the assigned numbers and
>>>   provide an alias later on. (I have split out this into a separate
>>>   patch)
>>>
>>> Christian Borntraeger (10):
>>>   linux header sync
>>>   s390x/cpumodel: remove CSSKE from base model
>>>   s390x/cpumodel: Miscellaneous-Instruction-Extensions Facility 3
>>>   s390x/cpumodel: msa9 facility
>>>   s390x/cpumodel: vector enhancements
>>>   s390x/cpumodel: enhanced sort facility
>>>   s390x/cpumodel: deflate
>>>   s390x/cpumodel: add gen15 defintions
>>>   s390x/cpumodel: wire up 8561 and 8562 as gen15 machines
>>>   s390x/cpumodel: do not claim csske for expanded models in qmp
>>>
>>>  hw/s390x/s390-virtio-ccw.c      |  6 +++
>>>  linux-headers/asm-s390/kvm.h    |  5 +-
>>>  target/s390x/cpu_features.c     | 54 +++++++++++++++++++
>>>  target/s390x/cpu_features.h     |  3 ++
>>>  target/s390x/cpu_features_def.h | 49 +++++++++++++++++
>>>  target/s390x/cpu_models.c       | 35 ++++++++++++
>>>  target/s390x/cpu_models.h       |  1 +
>>>  target/s390x/gen-features.c     | 94 ++++++++++++++++++++++++++++++++-
>>>  target/s390x/kvm.c              | 18 +++++++
>>>  9 files changed, 263 insertions(+), 2 deletions(-)
>>>
>>
>> I guess to handle deprecation of CSSKE:
>>
>> 1. Remove it from the base + default model of the gen15, keep it in the
>> max model. This is completely done in target/s390x/gen-features.c.
>> Existing base models are not modified.
>>
>> 2. Add CSSKE to "ignored_base_feat", so fallback of gen15 to e.g. z14
>> will work. We can backport this to distros/stable.
> 
> Yes, I have already implemented that, still doing some testing and polishinh.
>>
>>
>> CPU model expansion:
>>
>> cpu_info_from_model() should already properly be based on the base
>> features. "gen15" vs. "gen15,csske=on" should be automatically generated
>> when expanding.
>>
>> CPU model baseline:
>>
>> s390_find_cpu_def() should make sure that CSSKE is basically ignored
>> when determining maximum model, however it will properly be indicated if
>> both models had the feature.
>>
>> CPU model comparison:
>>
>> Should work as expected. Availability of CSSKE will be considered when
>> calculating the result.
>>
>> gen14,csske=on and gen15,csske=off will result in
>> CPU_MODEL_COMPARE_RESULT_INCOMPATIBLE.
>>
>> gen14,csske=off and gen15,csske=off should result in
>> CPU_MODEL_COMPARE_RESULT_SUBSET
>>
>> gen14,csske=on and gen15,csske=on should result in
>> CPU_MODEL_COMPARE_RESULT_SUBSET
>>
>> Forward migration:
>>
>> Now, the only issue is when csske is actually turned of in future
>> machines. We would e.g. have
>>
>> gen15,csske=on and gen16,csske=off
>>
>> While baselining will work correctly (gen15,csske=off), forward
>> migration is broken (comparison will properly indicate
>> CPU_MODEL_COMPARE_RESULT_INCOMPATIBLE), which is expected when ripping
>> out features. Same applies to BPB.
>>
>>
>> Your patch "[PATCH 10/10] s390x/cpumodel: do not claim csske for
>> expanded models in qmp" tried to address this, however I am not really
>> happy with this approach. We should not play such tricks when expanding
>> the host model. "-cpu host" and "-cpu $expanded_host" would be
>> different,
> 
> We discussed this some time ago and I think we agreed that for host 
> passthrough
> it is ok to be different that host-model (e.g. passing through the cpuid, 
> passing
> through all non-hypervisor managed features etc).

I remember the plan was to use the "max" model to do such stuff. E.g.
-cpu max / no -cpu

Versus
-cpu host

We can have features in "host" we don't have in "max". But "-cpu host"
and it's expansion should look 100% the same.

> 
> 
>> that is really confusing, especially think about baselining
>> or comparing against "host", where your patch does not apply..
>>
>>
>> Whenever users would specify "-cpu gen15", everything would be fine in
>> regards to forward migration. Maybe clients really have to be taught
>> about upcoming forward migration issues. "Don't use the host model, make
>> sure bpa and csske are disabled".
> 
> 
> the thing is, I think the host-model is actually the preferred setting so I
> would like to have this forward-migrateable as well. I will focus on patches
> 1-9 first and lets have a side discussion on patch 10.
> 

Makes sense. This is also something that can be fixed on higher layers
(disable cpu feature for gen15 to prepare for migration).

-- 

Thanks,

David / dhildenb