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

[David Hildenbrand]

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.


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

-- 

Thanks,

David / dhildenb