On 24.04.19 12:26, Christian Borntraeger wrote: > > > On 24.04.19 11:56, David Hildenbrand wrote: >> On 24.04.19 11:41, Daniel P. Berrangé wrote: >>> On Wed, Apr 24, 2019 at 11:35:40AM +0200, David Hildenbrand wrote: >>>> 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? >>> >>> I don't recall such a feature existing even in the past ! >> >> Maybe my mind is tricking me, or maybe that has long been removed :) >> >>> >>>>> 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 >>> >>> Hmm, maybe I misinterpreted when you wrote >>> >>> We can have features in "host" we don't have in "max" >>> >>> I read that as meaning that "-cpu host" and "-cpu max" would be >>> different. >> >> No you didn't misinterpret it, I agreed after you spelled it out :) >> >>> >>>> "-cpu host" and "-cpu $expanded_host" produce different results, after >>>> expanding "host" via query-cpu-model-expansion? >>> >>> That has always been the case on x86-64, since it is not possible to set >>> the level, xlevel, vendor, family, model & stepping properties via -cpu, >>> only the feature flags. >> >> Fair enough, but the question is if we should mess with feature flags we >> can indicate on that level. >> >> It would mean that on a specific host e.g. >> >> "-cpu gen15,csske=on" and "-cpu gen15,csske=off" >> >> Would work. However, "host" model expansion would give us >> >> "-cpu gen15,csske=off" >> >> So trying to e.g. do a query-cpu-model-comparison or >> query-cpu-model-baseline against "host" and against the expanded host >> model will produce different results. >> >> Libvirt could detect "-cpu gen14,csske=on" as not runnable on the host, >> because comparing "-cpu gen14,csske=on" vs. "-cpu gen15,csske=off" would >> be "incompatible". But running "-cpu gen14,csske=on" on the host would >> work perfectly fine. > > I would like to avoid special knowledge in libvirt (since we moved to have > everything in qemu). > > A more complex idea would be to extend the qmp query with a list of deprecated > features and libvirt could then disable that for expansion but allow it for > baselining.
That would fit in nicely into query-cpu-model-expansion. Nice idea. -- Thanks, David / dhildenb
