On Wed, Apr 24, 2019 at 12:26:26PM +0200, 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.
Funny that you mention deprecation with CPU features.... https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg03969.html So that makes 4 things we want to report deprecation for. Devices, machine types, cpu models and now cpu features. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
