Re: [Qemu-devel] [PATCH 00/10] s390x: new guest features

["Daniel P. Berrangé" <berrange@redhat.com>]

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 :|