On 24.04.19 12:38, Christian Borntraeger wrote: > > > On 24.04.19 12:27, David Hildenbrand wrote: >> 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. > > Hmm, I think that should also be possible as an add-on patch series later on > (e.g. in 6 month or so). We would have instances of gen15 machines with > host-model > that start with csske but sooner or later they will be restarted and then > they no > longer have csske. > As we do not change anything for baselining this would allow us to separate > patch10 > from the other patches and deal with it later with a proper series on its on? > Need to think about that.
Makes sense, we already have forward migration issues to > gen15 with guests started in the past when dropping features. No need to rush with that. -- Thanks, David / dhildenb
