On 24.04.19 11:30, Daniel P. Berrangé wrote: > On Wed, Apr 24, 2019 at 11:03:03AM +0200, David Hildenbrand wrote: >> 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. > > I don't think that's the intended semantics of "max" vs "host". > > The "max" CPU model is supposed to enable all features that are possible to > enable. > > For KVM, thus "max" should be identical to "host".
There once was a mode used by x86-64 to simply pipe through cpuid features that were not even supported. (I remember something like passthorugh=true), do you remember if something like that still exists? > > For TCG, "max" should be everything that QEMU currently knows how to emulate. Yes, and on s390x. "max" contains more features than "qemu". > > Essentially think of "max" as a better name for "host", since "host" as > a name concept didn't make sense for TCG. I agree. The main question is, is it acceptable that "-cpu host" and "-cpu $expanded_host" produce different results, after expanding "host" via query-cpu-model-expansion? -- Thanks, David / dhildenb