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 !
> > 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.
> "-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.
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 :|
