[PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions

Eduardo Habkost posted 7 patches 4 years, 6 months ago
Test asan passed
Test checkpatch failed
Test FreeBSD passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191025022553.25298-1-ehabkost@redhat.com
Maintainers: Cornelia Huck <cohuck@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Richard Henderson <rth@twiddle.net>, Eduardo Habkost <ehabkost@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Markus Armbruster <armbru@redhat.com>, Aleksandar Markovic <amarkovic@wavecomp.com>, David Gibson <david@gibson.dropbear.id.au>, Eric Blake <eblake@redhat.com>, Christian Borntraeger <borntraeger@de.ibm.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Aurelien Jarno <aurelien@aurel32.net>, Aleksandar Rikalo <arikalo@wavecomp.com>, Halil Pasic <pasic@linux.ibm.com>, David Hildenbrand <david@redhat.com>
There is a newer version of this series
qapi/machine-target.json                   | 14 +++-
include/hw/boards.h                        |  1 +
include/hw/i386/pc.h                       |  5 +-
target/i386/cpu.h                          |  6 --
hw/core/machine.c                          | 16 ++++
hw/i386/pc.c                               |  3 -
target/arm/helper.c                        |  4 +-
target/i386/cpu.c                          | 93 +++++++++++++++-------
target/mips/helper.c                       |  4 +-
target/ppc/translate_init.inc.c            |  4 +-
target/s390x/cpu_models.c                  |  4 +-
vl.c                                       | 17 +---
tests/acceptance/x86_cpu_model_versions.py | 42 ++++++++++
13 files changed, 154 insertions(+), 59 deletions(-)
[PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
Posted by Eduardo Habkost 4 years, 6 months ago
We had introduced versioned CPU models in QEMU 4.1, including a
method for querying for CPU model versions using
query-cpu-definitions.  This only has one problem: fetching CPU
alias information for multiple machine types required restarting
QEMU for each machine being queried.

This series adds a new `machine` parameter to
query-cpu-definitions, that can be used to query CPU model alias
information for multiple machines without restarting QEMU.

Eduardo Habkost (7):
  i386: Use g_autofree at x86_cpu_list_entry()
  i386: Add default_version parameter to CPU version functions
  i386: Don't use default_cpu_version at "-cpu help"
  machine: machine_find_class() function
  i386: Remove x86_cpu_set_default_version() function
  i386: Don't use default_cpu_version() inside query-cpu-definitions
  cpu: Add `machine` parameter to query-cpu-definitions

 qapi/machine-target.json                   | 14 +++-
 include/hw/boards.h                        |  1 +
 include/hw/i386/pc.h                       |  5 +-
 target/i386/cpu.h                          |  6 --
 hw/core/machine.c                          | 16 ++++
 hw/i386/pc.c                               |  3 -
 target/arm/helper.c                        |  4 +-
 target/i386/cpu.c                          | 93 +++++++++++++++-------
 target/mips/helper.c                       |  4 +-
 target/ppc/translate_init.inc.c            |  4 +-
 target/s390x/cpu_models.c                  |  4 +-
 vl.c                                       | 17 +---
 tests/acceptance/x86_cpu_model_versions.py | 42 ++++++++++
 13 files changed, 154 insertions(+), 59 deletions(-)

-- 
2.21.0


Re: [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
Posted by David Hildenbrand 4 years, 6 months ago
On 25.10.19 04:25, Eduardo Habkost wrote:
> We had introduced versioned CPU models in QEMU 4.1, including a
> method for querying for CPU model versions using

Interesting, I was not aware of that.

On s390x, we somewhat have versioned CPU models, but we decided against 
giving them explicit names (e.g., z13-v1 or z13-4.1.0), because it 
didn't really seem to be necessary. (and we often implement/add features 
for older CPU models, there is a lot of fluctuation) Actually, you would 
have had to add "z13-z/VM-x.x.x" or "z13-LPAR-x.x.x" or "z13-KVM-x.x.x" 
to model the features you actually see in all the different virtual 
environments ("what a CPU looks like"). Not to talk about QEMU versions 
in addition to that. So we decided to always model what you would see 
under LPAR and are able to specify for a KVM guest. So you can use "z13" 
in an up-to-date LPAR environment, but not in a z/VM environment (you 
would have to disable features).

Each (!base) CPU model has a specific feature set per machine. Libvirt 
uses query-cpu-model-expansion() to convert this model+machine to a 
machine-independent representation. That representation is sufficient 
for all use cases we were aware of (esp. "virsh domcapabilities", the 
host CPU model, migration).

While s390x has versioned CPU models, we have no explicit way of 
specifying them for older machines, besides going over 
query-cpu-model-expansion and using expanded "base model + features".

I can see that this might make sense on x86-64, where you only have 
maybe 3 versions of a CPU (e.g., the one Intel messed up first - 
Haswell, the one Intel messed up next - Haswell-noTSX, and the one that 
Intel eventually did right - Haswell-noTSX-IBRS) and you might want to 
specify "Haswell" vs. "Haswell-IBRS" vs. "Haswell-noTSX-IBRS". But 
actually, you will always want to go for "Haswell-noTSX-IBRS", because 
you can expect to run in updated environments if I am not wrong, 
everything else are corner cases.

Of course, versioned CPU model are neat to avoid "base model + list of 
features", but at least for expanding the host model on s390x, it is not 
really helpful. When migrating, the model expansion does the trick.

I haven't looked into details of "how to specify or model versions". 
Maybe IBM wants to look into creating versions for all the old models we 
had. But again, not sure if that is of any help for s390x. CCing IBM.

> query-cpu-definitions.  This only has one problem: fetching CPU
> alias information for multiple machine types required restarting
> QEMU for each machine being queried.
> 
> This series adds a new `machine` parameter to
> query-cpu-definitions, that can be used to query CPU model alias
> information for multiple machines without restarting QEMU.
> 
> Eduardo Habkost (7):
>    i386: Use g_autofree at x86_cpu_list_entry()
>    i386: Add default_version parameter to CPU version functions
>    i386: Don't use default_cpu_version at "-cpu help"
>    machine: machine_find_class() function
>    i386: Remove x86_cpu_set_default_version() function
>    i386: Don't use default_cpu_version() inside query-cpu-definitions
>    cpu: Add `machine` parameter to query-cpu-definitions
> 
>   qapi/machine-target.json                   | 14 +++-
>   include/hw/boards.h                        |  1 +
>   include/hw/i386/pc.h                       |  5 +-
>   target/i386/cpu.h                          |  6 --
>   hw/core/machine.c                          | 16 ++++
>   hw/i386/pc.c                               |  3 -
>   target/arm/helper.c                        |  4 +-
>   target/i386/cpu.c                          | 93 +++++++++++++++-------
>   target/mips/helper.c                       |  4 +-
>   target/ppc/translate_init.inc.c            |  4 +-
>   target/s390x/cpu_models.c                  |  4 +-
>   vl.c                                       | 17 +---
>   tests/acceptance/x86_cpu_model_versions.py | 42 ++++++++++
>   13 files changed, 154 insertions(+), 59 deletions(-)
> 


-- 

Thanks,

David / dhildenb


Re: [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
Posted by Christian Borntraeger 4 years, 6 months ago

On 25.10.19 09:17, David Hildenbrand wrote:
> On 25.10.19 04:25, Eduardo Habkost wrote:
>> We had introduced versioned CPU models in QEMU 4.1, including a
>> method for querying for CPU model versions using
> 
> Interesting, I was not aware of that.
> 
> On s390x, we somewhat have versioned CPU models, but we decided against giving them explicit names (e.g., z13-v1 or z13-4.1.0), because it didn't really seem to be necessary. (and we often implement/add features for older CPU models, there is a lot of fluctuation) Actually, you would have had to add "z13-z/VM-x.x.x" or "z13-LPAR-x.x.x" or "z13-KVM-x.x.x" to model the features you actually see in all the different virtual environments ("what a CPU looks like"). Not to talk about QEMU versions in addition to that. So we decided to always model what you would see under LPAR and are able to specify for a KVM guest. So you can use "z13" in an up-to-date LPAR environment, but not in a z/VM environment (you would have to disable features).
> 
> Each (!base) CPU model has a specific feature set per machine. Libvirt uses query-cpu-model-expansion() to convert this model+machine to a machine-independent representation. That representation is sufficient for all use cases we were aware of (esp. "virsh domcapabilities", the host CPU model, migration).
> 
> While s390x has versioned CPU models, we have no explicit way of specifying them for older machines, besides going over query-cpu-model-expansion and using expanded "base model + features".
> 
> I can see that this might make sense on x86-64, where you only have maybe 3 versions of a CPU (e.g., the one Intel messed up first - Haswell, the one Intel messed up next - Haswell-noTSX, and the one that Intel eventually did right - Haswell-noTSX-IBRS) and you might want to specify "Haswell" vs. "Haswell-IBRS" vs. "Haswell-noTSX-IBRS". But actually, you will always want to go for "Haswell-noTSX-IBRS", because you can expect to run in updated environments if I am not wrong, everything else are corner cases.
> 
> Of course, versioned CPU model are neat to avoid "base model + list of features", but at least for expanding the host model on s390x, it is not really helpful. When migrating, the model expansion does the trick.
> 
> I haven't looked into details of "how to specify or model versions". Maybe IBM wants to look into creating versions for all the old models we had. But again, not sure if that is of any help for s390x. CCing IBM.

I agree that this does not look very helpful. 
Especially as several things depend on the kernel version a QEMU version is
not sufficient to be guarantee construction success.
So we would need something like z14-qemu4.0-kernel-5.2-suse-flavour-onLPAR

Instead we do check if we can construct an equivalent model on the migration target.
And that model is precise. We do even have versions.
Right now with QEMU on s390  our models are versioned in a way that we fence of
facilities for old machine versions.

For example
-machine s390-virtio-ccw-3.1 -cpu z14 will not have the multiple epoch facility
and 
-machine s390-virtio-ccw-4.0 -cpu z14 will have the multiple epoch facility.
As migration does always require the tuple of machine and cpu this is save. I fail
to see what the benefit of an explicit z14-3.1 would be.


Re: [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
Posted by David Hildenbrand 4 years, 6 months ago
On 25.10.19 09:55, Christian Borntraeger wrote:
> 
> 
> On 25.10.19 09:17, David Hildenbrand wrote:
>> On 25.10.19 04:25, Eduardo Habkost wrote:
>>> We had introduced versioned CPU models in QEMU 4.1, including a
>>> method for querying for CPU model versions using
>>
>> Interesting, I was not aware of that.
>>
>> On s390x, we somewhat have versioned CPU models, but we decided against giving them explicit names (e.g., z13-v1 or z13-4.1.0), because it didn't really seem to be necessary. (and we often implement/add features for older CPU models, there is a lot of fluctuation) Actually, you would have had to add "z13-z/VM-x.x.x" or "z13-LPAR-x.x.x" or "z13-KVM-x.x.x" to model the features you actually see in all the different virtual environments ("what a CPU looks like"). Not to talk about QEMU versions in addition to that. So we decided to always model what you would see under LPAR and are able to specify for a KVM guest. So you can use "z13" in an up-to-date LPAR environment, but not in a z/VM environment (you would have to disable features).
>>
>> Each (!base) CPU model has a specific feature set per machine. Libvirt uses query-cpu-model-expansion() to convert this model+machine to a machine-independent representation. That representation is sufficient for all use cases we were aware of (esp. "virsh domcapabilities", the host CPU model, migration).
>>
>> While s390x has versioned CPU models, we have no explicit way of specifying them for older machines, besides going over query-cpu-model-expansion and using expanded "base model + features".
>>
>> I can see that this might make sense on x86-64, where you only have maybe 3 versions of a CPU (e.g., the one Intel messed up first - Haswell, the one Intel messed up next - Haswell-noTSX, and the one that Intel eventually did right - Haswell-noTSX-IBRS) and you might want to specify "Haswell" vs. "Haswell-IBRS" vs. "Haswell-noTSX-IBRS". But actually, you will always want to go for "Haswell-noTSX-IBRS", because you can expect to run in updated environments if I am not wrong, everything else are corner cases.
>>
>> Of course, versioned CPU model are neat to avoid "base model + list of features", but at least for expanding the host model on s390x, it is not really helpful. When migrating, the model expansion does the trick.
>>
>> I haven't looked into details of "how to specify or model versions". Maybe IBM wants to look into creating versions for all the old models we had. But again, not sure if that is of any help for s390x. CCing IBM.
> 
> I agree that this does not look very helpful.
> Especially as several things depend on the kernel version a QEMU version is
> not sufficient to be guarantee construction success.
> So we would need something like z14-qemu4.0-kernel-5.2-suse-flavour-onLPAR
> 
> Instead we do check if we can construct an equivalent model on the migration target.
> And that model is precise. We do even have versions.
> Right now with QEMU on s390  our models are versioned in a way that we fence of
> facilities for old machine versions.
> 
> For example
> -machine s390-virtio-ccw-3.1 -cpu z14 will not have the multiple epoch facility
> and
> -machine s390-virtio-ccw-4.0 -cpu z14 will have the multiple epoch facility.
> As migration does always require the tuple of machine and cpu this is save. I fail
> to see what the benefit of an explicit z14-3.1 would be.
> 

AFAIKS the only real benefit of versioned CPU models is that you can add 
new CPU model versions without new QEMU version.

Then you can specify "-cpu z13-vX" or "-cpu z13 -cpuv X" (no idea how 
versioned CPU model were implemented) on any QEMU machine. Which is the 
same as telling your customer "please use z13,featX=on" in case you have 
a good reason to not use the host model (along with baselining) but use 
an explicit model.

If you can change the default model of QEMU machines, you can automate 
this process. I am pretty sure this is a corner case, though (e.g., 
IBRS). Usually you have a new QEMU machine and can simply enable the new 
feature from that point on.

-- 

Thanks,

David / dhildenb


Re: [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
Posted by Daniel P. Berrangé 4 years, 6 months ago
On Fri, Oct 25, 2019 at 10:02:29AM +0200, David Hildenbrand wrote:
> On 25.10.19 09:55, Christian Borntraeger wrote:
> > 
> > 
> > On 25.10.19 09:17, David Hildenbrand wrote:
> > > On 25.10.19 04:25, Eduardo Habkost wrote:
> > > > We had introduced versioned CPU models in QEMU 4.1, including a
> > > > method for querying for CPU model versions using
> > > 
> > > Interesting, I was not aware of that.
> > > 
> > > On s390x, we somewhat have versioned CPU models, but we decided against giving them explicit names (e.g., z13-v1 or z13-4.1.0), because it didn't really seem to be necessary. (and we often implement/add features for older CPU models, there is a lot of fluctuation) Actually, you would have had to add "z13-z/VM-x.x.x" or "z13-LPAR-x.x.x" or "z13-KVM-x.x.x" to model the features you actually see in all the different virtual environments ("what a CPU looks like"). Not to talk about QEMU versions in addition to that. So we decided to always model what you would see under LPAR and are able to specify for a KVM guest. So you can use "z13" in an up-to-date LPAR environment, but not in a z/VM environment (you would have to disable features).
> > > 
> > > Each (!base) CPU model has a specific feature set per machine. Libvirt uses query-cpu-model-expansion() to convert this model+machine to a machine-independent representation. That representation is sufficient for all use cases we were aware of (esp. "virsh domcapabilities", the host CPU model, migration).
> > > 
> > > While s390x has versioned CPU models, we have no explicit way of specifying them for older machines, besides going over query-cpu-model-expansion and using expanded "base model + features".
> > > 
> > > I can see that this might make sense on x86-64, where you only have maybe 3 versions of a CPU (e.g., the one Intel messed up first - Haswell, the one Intel messed up next - Haswell-noTSX, and the one that Intel eventually did right - Haswell-noTSX-IBRS) and you might want to specify "Haswell" vs. "Haswell-IBRS" vs. "Haswell-noTSX-IBRS". But actually, you will always want to go for "Haswell-noTSX-IBRS", because you can expect to run in updated environments if I am not wrong, everything else are corner cases.
> > > 
> > > Of course, versioned CPU model are neat to avoid "base model + list of features", but at least for expanding the host model on s390x, it is not really helpful. When migrating, the model expansion does the trick.
> > > 
> > > I haven't looked into details of "how to specify or model versions". Maybe IBM wants to look into creating versions for all the old models we had. But again, not sure if that is of any help for s390x. CCing IBM.
> > 
> > I agree that this does not look very helpful.
> > Especially as several things depend on the kernel version a QEMU version is
> > not sufficient to be guarantee construction success.
> > So we would need something like z14-qemu4.0-kernel-5.2-suse-flavour-onLPAR
> > 
> > Instead we do check if we can construct an equivalent model on the migration target.
> > And that model is precise. We do even have versions.
> > Right now with QEMU on s390  our models are versioned in a way that we fence of
> > facilities for old machine versions.
> > 
> > For example
> > -machine s390-virtio-ccw-3.1 -cpu z14 will not have the multiple epoch facility
> > and
> > -machine s390-virtio-ccw-4.0 -cpu z14 will have the multiple epoch facility.
> > As migration does always require the tuple of machine and cpu this is save. I fail
> > to see what the benefit of an explicit z14-3.1 would be.
> > 
> 
> AFAIKS the only real benefit of versioned CPU models is that you can add new
> CPU model versions without new QEMU version.

This is very important for backporting CPU security fixes to existing QEMU
releases. 

> 
> Then you can specify "-cpu z13-vX" or "-cpu z13 -cpuv X" (no idea how
> versioned CPU model were implemented) on any QEMU machine. Which is the same
> as telling your customer "please use z13,featX=on" in case you have a good
> reason to not use the host model (along with baselining) but use an explicit
> model.
> 
> If you can change the default model of QEMU machines, you can automate this
> process. I am pretty sure this is a corner case, though (e.g., IBRS).
> Usually you have a new QEMU machine and can simply enable the new feature
> from that point on.

There are now 4 Haswell variants, only some of which are runnable
on any given host, depending on what microcode the user has installed
or what particular Haswell silicon SKU the user purchased. Given the
frequency of new CPU flaws arrived since the first Meltdown/Spectre,
this isn't a corner case, at least for the x86 world & Intel in
particular. Other arches/vendors haven't been quite so badly affected
in this way.

If we tied each new Haswell variant to a machine type, then users would
be blocked from consuming a new machine type depending on runnability of
the CPU model. This is not at all desirable, as mgmt apps now have complex
rules on what machine type they can use.

When dealing with backporting patches for new CPU hardware flaws, the
new CPU features are backported to many old QEMU versions. The new
machine types are not backportable.

Both these called for making CPU versioning independant of machine
type versioning.

Essentially the goal with CPU versioning is that the user can request
a bare "Haswell" and libvirt (or the mgmt app) will automatically
expand this to the best Haswell version that the host is able to
support with its CPUs / microcode / BIOS config combination.



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


Re: [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
Posted by David Hildenbrand 4 years, 6 months ago
>>> For example
>>> -machine s390-virtio-ccw-3.1 -cpu z14 will not have the multiple epoch facility
>>> and
>>> -machine s390-virtio-ccw-4.0 -cpu z14 will have the multiple epoch facility.
>>> As migration does always require the tuple of machine and cpu this is save. I fail
>>> to see what the benefit of an explicit z14-3.1 would be.
>>>
>>
>> AFAIKS the only real benefit of versioned CPU models is that you can add new
>> CPU model versions without new QEMU version.
> 
> This is very important for backporting CPU security fixes to existing QEMU
> releases.

I'd say it's not really relevant for backporting per se. It's relevant 
for automatically enabling security fixes when not using the host model. 
That part I understand. Less likely to make mistakes when explicitly 
specifying CPU models.

I once was told that if a user actually specified an explicit CPU model 
in the libvirt XML ("haswell-whatever"), you should not go ahead and 
make any later changes to that model (guest ABI should not change when 
you update/restart the guest ...). So this only applies when creating 
new guests? Or will you change existing model definitions implicitly?

> 
>>
>> Then you can specify "-cpu z13-vX" or "-cpu z13 -cpuv X" (no idea how
>> versioned CPU model were implemented) on any QEMU machine. Which is the same
>> as telling your customer "please use z13,featX=on" in case you have a good
>> reason to not use the host model (along with baselining) but use an explicit
>> model.
>>
>> If you can change the default model of QEMU machines, you can automate this
>> process. I am pretty sure this is a corner case, though (e.g., IBRS).
>> Usually you have a new QEMU machine and can simply enable the new feature
>> from that point on.
> 
> There are now 4 Haswell variants, only some of which are runnable
> on any given host, depending on what microcode the user has installed
> or what particular Haswell silicon SKU the user purchased. Given the
> frequency of new CPU flaws arrived since the first Meltdown/Spectre,
> this isn't a corner case, at least for the x86 world & Intel in
> particular. Other arches/vendors haven't been quite so badly affected
> in this way.

On s390x you can assume that such firmware/microcode updates will be on 
any machine after some time. That is a big difference to x86-64 AFAIK.

> 
> If we tied each new Haswell variant to a machine type, then users would
> be blocked from consuming a new machine type depending on runnability of
> the CPU model. This is not at all desirable, as mgmt apps now have complex
> rules on what machine type they can use.

So you actually want different CPU variants, which you have already, 
just in a different form. (e.g., "haswell" will be mapped to 
"haswell-whatever", just differently via versions)

> 
> When dealing with backporting patches for new CPU hardware flaws, the
> new CPU features are backported to many old QEMU versions. The new
> machine types are not backportable.

That part I understand.

> 
> Both these called for making CPU versioning independant of machine
> type versioning.
> 
> Essentially the goal with CPU versioning is that the user can request
> a bare "Haswell" and libvirt (or the mgmt app) will automatically
> expand this to the best Haswell version that the host is able to
> support with its CPUs / microcode / BIOS config combination.


So if I do a "-cpu haswell -M whatever-machine", as far as I understood 
reading this,  I get the "default CPU model alias for that QEMU machine" 
and *not* the "best Haswell version that the host is able to support".

Or does the default actually also depend on the current host?

-- 

Thanks,

David / dhildenb


Re: [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
Posted by Daniel P. Berrangé 4 years, 6 months ago
On Fri, Oct 25, 2019 at 04:23:59PM +0200, David Hildenbrand wrote:
> > > > For example
> > > > -machine s390-virtio-ccw-3.1 -cpu z14 will not have the multiple epoch facility
> > > > and
> > > > -machine s390-virtio-ccw-4.0 -cpu z14 will have the multiple epoch facility.
> > > > As migration does always require the tuple of machine and cpu this is save. I fail
> > > > to see what the benefit of an explicit z14-3.1 would be.
> > > > 
> > > 
> > > AFAIKS the only real benefit of versioned CPU models is that you can add new
> > > CPU model versions without new QEMU version.
> > 
> > This is very important for backporting CPU security fixes to existing QEMU
> > releases.
> 
> I'd say it's not really relevant for backporting per se. It's relevant for
> automatically enabling security fixes when not using the host model. That
> part I understand. Less likely to make mistakes when explicitly specifying
> CPU models.
> 
> I once was told that if a user actually specified an explicit CPU model in
> the libvirt XML ("haswell-whatever"), you should not go ahead and make any
> later changes to that model (guest ABI should not change when you
> update/restart the guest ...). So this only applies when creating new
> guests? Or will you change existing model definitions implicitly?

Libvirt will only ever expand a bare CPU model at time it first parses
the XML. So if a mgmt app defines a new persistent guest in libvirt, the
CPU is expanded them and remains unchanged thereafter, in order to preserve
ABI compat.

If using transient guests its different as libvirt doesn't store the config
in disk when the guest isn't running. So mgmt apps using transient guests
are responsible  for picking a explicit versioned model themselves if they
need stable ABI.

> > > Then you can specify "-cpu z13-vX" or "-cpu z13 -cpuv X" (no idea how
> > > versioned CPU model were implemented) on any QEMU machine. Which is the same
> > > as telling your customer "please use z13,featX=on" in case you have a good
> > > reason to not use the host model (along with baselining) but use an explicit
> > > model.
> > > 
> > > If you can change the default model of QEMU machines, you can automate this
> > > process. I am pretty sure this is a corner case, though (e.g., IBRS).
> > > Usually you have a new QEMU machine and can simply enable the new feature
> > > from that point on.
> > 
> > There are now 4 Haswell variants, only some of which are runnable
> > on any given host, depending on what microcode the user has installed
> > or what particular Haswell silicon SKU the user purchased. Given the
> > frequency of new CPU flaws arrived since the first Meltdown/Spectre,
> > this isn't a corner case, at least for the x86 world & Intel in
> > particular. Other arches/vendors haven't been quite so badly affected
> > in this way.
> 
> On s390x you can assume that such firmware/microcode updates will be on any
> machine after some time. That is a big difference to x86-64 AFAIK.

I don't know s390x much, but can we really assume that users promptly
install firmware updates, any better than users do for x86 or other
arch. IME corporate beaurcracy can drag out time to update arbitrarily
long.

> > If we tied each new Haswell variant to a machine type, then users would
> > be blocked from consuming a new machine type depending on runnability of
> > the CPU model. This is not at all desirable, as mgmt apps now have complex
> > rules on what machine type they can use.
> 
> So you actually want different CPU variants, which you have already, just in
> a different form. (e.g., "haswell" will be mapped to "haswell-whatever",
> just differently via versions)

Yes, you can think of "Haswell", "Haswell-noTSX", "Haswell-noTSX-IBRS"
as all being versions of the same thing. There was never any explicit
association or naming though. So what's changing is that we're defining
a sane naming scheme for the variants of each model so we don't end
up with   "Haswell-noTSX-IBRS-SSBD-MDS-WHATEVER-NEXT-INTEL-FLAW-IS",
and we declaring that a bare "Haswell" will expand to some "best"
version depending on machine type (but also selectable by mgmt app
above).

> > Both these called for making CPU versioning independant of machine
> > type versioning.
> > 
> > Essentially the goal with CPU versioning is that the user can request
> > a bare "Haswell" and libvirt (or the mgmt app) will automatically
> > expand this to the best Haswell version that the host is able to
> > support with its CPUs / microcode / BIOS config combination.
> 
> 
> So if I do a "-cpu haswell -M whatever-machine", as far as I understood
> reading this,  I get the "default CPU model alias for that QEMU machine" and
> *not* the "best Haswell version that the host is able to support".
> 
> Or does the default actually also depend on the current host?

At the QEMU level "haswell" will expand to a particular CPU version
per machine type. So yes, at the QEMU level machine types might have
a dependancy on the host.

Above QEMU though, libvirt/mgmt apps can be more dynamic in how they
expand a bare "haswell" to take account of what the host supports.

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


Re: [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
Posted by David Hildenbrand 4 years, 6 months ago
>> I once was told that if a user actually specified an explicit CPU model in
>> the libvirt XML ("haswell-whatever"), you should not go ahead and make any
>> later changes to that model (guest ABI should not change when you
>> update/restart the guest ...). So this only applies when creating new
>> guests? Or will you change existing model definitions implicitly?
> 
> Libvirt will only ever expand a bare CPU model at time it first parses
> the XML. So if a mgmt app defines a new persistent guest in libvirt, the
> CPU is expanded them and remains unchanged thereafter, in order to preserve
> ABI compat.

Okay, perfect.

> 
> If using transient guests its different as libvirt doesn't store the config
> in disk when the guest isn't running. So mgmt apps using transient guests
> are responsible  for picking a explicit versioned model themselves if they
> need stable ABI.

That makes sense.

> 
>>>> Then you can specify "-cpu z13-vX" or "-cpu z13 -cpuv X" (no idea how
>>>> versioned CPU model were implemented) on any QEMU machine. Which is the same
>>>> as telling your customer "please use z13,featX=on" in case you have a good
>>>> reason to not use the host model (along with baselining) but use an explicit
>>>> model.
>>>>
>>>> If you can change the default model of QEMU machines, you can automate this
>>>> process. I am pretty sure this is a corner case, though (e.g., IBRS).
>>>> Usually you have a new QEMU machine and can simply enable the new feature
>>>> from that point on.
>>>
>>> There are now 4 Haswell variants, only some of which are runnable
>>> on any given host, depending on what microcode the user has installed
>>> or what particular Haswell silicon SKU the user purchased. Given the
>>> frequency of new CPU flaws arrived since the first Meltdown/Spectre,
>>> this isn't a corner case, at least for the x86 world & Intel in
>>> particular. Other arches/vendors haven't been quite so badly affected
>>> in this way.
>>
>> On s390x you can assume that such firmware/microcode updates will be on any
>> machine after some time. That is a big difference to x86-64 AFAIK.
> 
> I don't know s390x much, but can we really assume that users promptly
> install firmware updates, any better than users do for x86 or other
> arch. IME corporate beaurcracy can drag out time to update arbitrarily
> long.

That's what you get when you pay premium prices for premium support :D

The real issue when it comes to CPU models on s390x is the variance of 
features of a specific model across environments (especially different 
hypervisors).

>>> If we tied each new Haswell variant to a machine type, then users would
>>> be blocked from consuming a new machine type depending on runnability of
>>> the CPU model. This is not at all desirable, as mgmt apps now have complex
>>> rules on what machine type they can use.
>>
>> So you actually want different CPU variants, which you have already, just in
>> a different form. (e.g., "haswell" will be mapped to "haswell-whatever",
>> just differently via versions)
> 
> Yes, you can think of "Haswell", "Haswell-noTSX", "Haswell-noTSX-IBRS"
> as all being versions of the same thing. There was never any explicit
> association or naming though. So what's changing is that we're defining
> a sane naming scheme for the variants of each model so we don't end
> up with   "Haswell-noTSX-IBRS-SSBD-MDS-WHATEVER-NEXT-INTEL-FLAW-IS",
> and we declaring that a bare "Haswell" will expand to some "best"
> version depending on machine type (but also selectable by mgmt app
> above).

I mean, all you really want is a way to specify "give me the best 
haswell you can do with this accelerator", which *could* map to "-cpu 
haswell,tsx=off,ibrs=on,ssbf=on" ... but also something else on the HW.

I really don't see why we need versioned CPU models for that, all you 
want to do is apply delta updates to the initial model if possible on 
the current accelerator. Just like HW does. See below for a simpler 
approach.

> 
>>> Both these called for making CPU versioning independant of machine
>>> type versioning.
>>>
>>> Essentially the goal with CPU versioning is that the user can request
>>> a bare "Haswell" and libvirt (or the mgmt app) will automatically
>>> expand this to the best Haswell version that the host is able to
>>> support with its CPUs / microcode / BIOS config combination.
>>
>>
>> So if I do a "-cpu haswell -M whatever-machine", as far as I understood
>> reading this,  I get the "default CPU model alias for that QEMU machine" and
>> *not* the "best Haswell version that the host is able to support".
>>
>> Or does the default actually also depend on the current host?
> 
> At the QEMU level "haswell" will expand to a particular CPU version
> per machine type. So yes, at the QEMU level machine types might have
> a dependancy on the host.
> 
> Above QEMU though, libvirt/mgmt apps can be more dynamic in how they
> expand a bare "haswell" to take account of what the host supports.

Let me propose something *much* simpler which would work just fine on 
s390x, and I assume on x86-64 as well.

On s390x we e.g. have the two models:
- "z14-base"
  -> minimum feature set we expect to have in every environment
  -> QEMU version/machine independent, will never change
- "z14"
  -> "default model", can change between QEMU machines
  -> migration-safe

Now, internally we have in addition something that matches:
- "z14-max"
  -> all possible CPU features valid for this model
  -> Includes e.g., nested virt features not in the "z14" model
  -> Can and will change between QEMU versions

Of course we also have:
- "max"
  -> "all features supported by the accelerator in the current host"

What we really want is:
- "z14-best"
  -> "best features for this model supported by the accelerator in the
      current host"

The trick is that *usually* :
	"z14-best" = baseline("z14-max", "max")

Minor exceptions can be easily added (e.g., always disable a certain 
feature). So, what I would be proposing for s390x (and also x86-64, but 
I know that they have more legacy handling) is simply implementing and 
exposing all -best models.


Why is this good in general?

1. No semantic changes of existing models. What was migration safe
    remains migration safe.
2. No CPU versions, less complexity.
3. No changes in the tool stack. Implement it in QEMU and it will be
    supported on every layer. Simply specify/expand "z14-best" and you
    get something that will run and make use of the best (on s390x
    usually all) features available that are valid for this model.

Why is this good for s390x?

1. As discussed, versioning all the different flavors we have is
    not feasible, nor practicable.

2. Features that are typically not around (especially, nested virt
    features) will be enabled similar to the host model when around.


I think I can hack a prototype of that in a couple of hours.

-- 

Thanks,

David / dhildenb


Re: [libvirt] [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
Posted by Eduardo Habkost 4 years, 6 months ago
CCing mskrivanek, danpb, libvir-list.

On Fri, Oct 25, 2019 at 10:02:29AM +0200, David Hildenbrand wrote:
> On 25.10.19 09:55, Christian Borntraeger wrote:
> > 
> > 
> > On 25.10.19 09:17, David Hildenbrand wrote:
> > > On 25.10.19 04:25, Eduardo Habkost wrote:
> > > > We had introduced versioned CPU models in QEMU 4.1, including a
> > > > method for querying for CPU model versions using
> > > 
> > > Interesting, I was not aware of that.
> > > 
> > > On s390x, we somewhat have versioned CPU models, but we decided against giving them explicit names (e.g., z13-v1 or z13-4.1.0), because it didn't really seem to be necessary. (and we often implement/add features for older CPU models, there is a lot of fluctuation) Actually, you would have had to add "z13-z/VM-x.x.x" or "z13-LPAR-x.x.x" or "z13-KVM-x.x.x" to model the features you actually see in all the different virtual environments ("what a CPU looks like"). Not to talk about QEMU versions in addition to that. So we decided to always model what you would see under LPAR and are able to specify for a KVM guest. So you can use "z13" in an up-to-date LPAR environment, but not in a z/VM environment (you would have to disable features).
> > > 
> > > Each (!base) CPU model has a specific feature set per machine. Libvirt uses query-cpu-model-expansion() to convert this model+machine to a machine-independent representation. That representation is sufficient for all use cases we were aware of (esp. "virsh domcapabilities", the host CPU model, migration).
> > > 
> > > While s390x has versioned CPU models, we have no explicit way of specifying them for older machines, besides going over query-cpu-model-expansion and using expanded "base model + features".
> > > 
> > > I can see that this might make sense on x86-64, where you only have maybe 3 versions of a CPU (e.g., the one Intel messed up first - Haswell, the one Intel messed up next - Haswell-noTSX, and the one that Intel eventually did right - Haswell-noTSX-IBRS) and you might want to specify "Haswell" vs. "Haswell-IBRS" vs. "Haswell-noTSX-IBRS". But actually, you will always want to go for "Haswell-noTSX-IBRS", because you can expect to run in updated environments if I am not wrong, everything else are corner cases.
> > > 
> > > Of course, versioned CPU model are neat to avoid "base model + list of features", but at least for expanding the host model on s390x, it is not really helpful. When migrating, the model expansion does the trick.
> > > 
> > > I haven't looked into details of "how to specify or model versions". Maybe IBM wants to look into creating versions for all the old models we had. But again, not sure if that is of any help for s390x. CCing IBM.
> > 
> > I agree that this does not look very helpful.
> > Especially as several things depend on the kernel version a QEMU version is
> > not sufficient to be guarantee construction success.
> > So we would need something like z14-qemu4.0-kernel-5.2-suse-flavour-onLPAR
> > 
> > Instead we do check if we can construct an equivalent model on the migration target.
> > And that model is precise. We do even have versions.
> > Right now with QEMU on s390  our models are versioned in a way that we fence of
> > facilities for old machine versions.
> > 
> > For example
> > -machine s390-virtio-ccw-3.1 -cpu z14 will not have the multiple epoch facility
> > and
> > -machine s390-virtio-ccw-4.0 -cpu z14 will have the multiple epoch facility.
> > As migration does always require the tuple of machine and cpu this is save. I fail
> > to see what the benefit of an explicit z14-3.1 would be.
> > 
> 
> AFAIKS the only real benefit of versioned CPU models is that you can add new
> CPU model versions without new QEMU version.
> 
> Then you can specify "-cpu z13-vX" or "-cpu z13 -cpuv X" (no idea how
> versioned CPU model were implemented) on any QEMU machine. Which is the same
> as telling your customer "please use z13,featX=on" in case you have a good
> reason to not use the host model (along with baselining) but use an explicit
> model.

Exactly.  oVirt, specifically, don't use host-model.

> 
> If you can change the default model of QEMU machines, you can automate this
> process. I am pretty sure this is a corner case, though (e.g., IBRS).
> Usually you have a new QEMU machine and can simply enable the new feature
> from that point on.

When -noTSX happened, we thought it was a corner case.  Then we
had -IBRS & -IBPB.  Then we had SSBD (with no new CPU models).
Then we had MD_CLEAR (with no new CPU models).  It's now very
easy to get an insecure VM created if you are not using
host-model.

-- 
Eduardo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
Posted by Eduardo Habkost 4 years, 6 months ago
CCing danpb, libvir-list, mskrivanek.

On Fri, Oct 25, 2019 at 09:17:46AM +0200, David Hildenbrand wrote:
> On 25.10.19 04:25, Eduardo Habkost wrote:
> > We had introduced versioned CPU models in QEMU 4.1, including a
> > method for querying for CPU model versions using
> 
> Interesting, I was not aware of that.
> 
> On s390x, we somewhat have versioned CPU models, but we decided against
> giving them explicit names (e.g., z13-v1 or z13-4.1.0), because it didn't
> really seem to be necessary. (and we often implement/add features for older
> CPU models, there is a lot of fluctuation) Actually, you would have had to
> add "z13-z/VM-x.x.x" or "z13-LPAR-x.x.x" or "z13-KVM-x.x.x" to model the
> features you actually see in all the different virtual environments ("what a
> CPU looks like"). Not to talk about QEMU versions in addition to that. So we
> decided to always model what you would see under LPAR and are able to
> specify for a KVM guest. So you can use "z13" in an up-to-date LPAR
> environment, but not in a z/VM environment (you would have to disable
> features).
> 
> Each (!base) CPU model has a specific feature set per machine. Libvirt uses
> query-cpu-model-expansion() to convert this model+machine to a
> machine-independent representation. That representation is sufficient for
> all use cases we were aware of (esp. "virsh domcapabilities", the host CPU
> model, migration).
> 
> While s390x has versioned CPU models, we have no explicit way of specifying
> them for older machines, besides going over query-cpu-model-expansion and
> using expanded "base model + features".
> 
> I can see that this might make sense on x86-64, where you only have maybe 3
> versions of a CPU (e.g., the one Intel messed up first - Haswell, the one
> Intel messed up next - Haswell-noTSX, and the one that Intel eventually did
> right - Haswell-noTSX-IBRS) and you might want to specify "Haswell" vs.
> "Haswell-IBRS" vs. "Haswell-noTSX-IBRS". But actually, you will always want
> to go for "Haswell-noTSX-IBRS", because you can expect to run in updated
> environments if I am not wrong, everything else are corner cases.
> 
> Of course, versioned CPU model are neat to avoid "base model + list of
> features", but at least for expanding the host model on s390x, it is not
> really helpful. When migrating, the model expansion does the trick.
> 
> I haven't looked into details of "how to specify or model versions". Maybe
> IBM wants to look into creating versions for all the old models we had. But
> again, not sure if that is of any help for s390x. CCing IBM.

I'm not sure yet if there are the x86-specific goals and
constraints that would make it difficult to follow the same
approach followed by s390x.  I have the impression we do,
but I need to think more carefully about it.

Let's discuss that during KVM Forum?

The two main goals of versioned CPU models in x86 are:
1) Decoupling CPU model updates from machine-types (users should be
   able to update a CPU model definition without changing machine
   type).
2) Letting management software automate CPU model updates.
   Normally this is necessary when bare metal microcode or
   software updates add/remove features from CPUs.  Sometimes the
   Virtual CPU model update needs to be performed before the host
   updates are applied (if features are being removed).

The main constraint that makes it difficult to address the above
without a new API is:
A) Every CPU model in x86 except "host" is already expected to
   be migration-safe (I don't know how this compares to s390x).


> 
> > query-cpu-definitions.  This only has one problem: fetching CPU
> > alias information for multiple machine types required restarting
> > QEMU for each machine being queried.
> > 
> > This series adds a new `machine` parameter to
> > query-cpu-definitions, that can be used to query CPU model alias
> > information for multiple machines without restarting QEMU.
> > 
> > Eduardo Habkost (7):
> >    i386: Use g_autofree at x86_cpu_list_entry()
> >    i386: Add default_version parameter to CPU version functions
> >    i386: Don't use default_cpu_version at "-cpu help"
> >    machine: machine_find_class() function
> >    i386: Remove x86_cpu_set_default_version() function
> >    i386: Don't use default_cpu_version() inside query-cpu-definitions
> >    cpu: Add `machine` parameter to query-cpu-definitions
> > 
> >   qapi/machine-target.json                   | 14 +++-
> >   include/hw/boards.h                        |  1 +
> >   include/hw/i386/pc.h                       |  5 +-
> >   target/i386/cpu.h                          |  6 --
> >   hw/core/machine.c                          | 16 ++++
> >   hw/i386/pc.c                               |  3 -
> >   target/arm/helper.c                        |  4 +-
> >   target/i386/cpu.c                          | 93 +++++++++++++++-------
> >   target/mips/helper.c                       |  4 +-
> >   target/ppc/translate_init.inc.c            |  4 +-
> >   target/s390x/cpu_models.c                  |  4 +-
> >   vl.c                                       | 17 +---
> >   tests/acceptance/x86_cpu_model_versions.py | 42 ++++++++++
> >   13 files changed, 154 insertions(+), 59 deletions(-)
> > 
> 
> 
> -- 
> 
> Thanks,
> 
> David / dhildenb

-- 
Eduardo


Re: [libvirt] [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
Posted by David Hildenbrand 4 years, 6 months ago
On 25.10.19 15:38, Eduardo Habkost wrote:
> CCing danpb, libvir-list, mskrivanek.
> 
> On Fri, Oct 25, 2019 at 09:17:46AM +0200, David Hildenbrand wrote:
>> On 25.10.19 04:25, Eduardo Habkost wrote:
>>> We had introduced versioned CPU models in QEMU 4.1, including a
>>> method for querying for CPU model versions using
>>
>> Interesting, I was not aware of that.
>>
>> On s390x, we somewhat have versioned CPU models, but we decided against
>> giving them explicit names (e.g., z13-v1 or z13-4.1.0), because it didn't
>> really seem to be necessary. (and we often implement/add features for older
>> CPU models, there is a lot of fluctuation) Actually, you would have had to
>> add "z13-z/VM-x.x.x" or "z13-LPAR-x.x.x" or "z13-KVM-x.x.x" to model the
>> features you actually see in all the different virtual environments ("what a
>> CPU looks like"). Not to talk about QEMU versions in addition to that. So we
>> decided to always model what you would see under LPAR and are able to
>> specify for a KVM guest. So you can use "z13" in an up-to-date LPAR
>> environment, but not in a z/VM environment (you would have to disable
>> features).
>>
>> Each (!base) CPU model has a specific feature set per machine. Libvirt uses
>> query-cpu-model-expansion() to convert this model+machine to a
>> machine-independent representation. That representation is sufficient for
>> all use cases we were aware of (esp. "virsh domcapabilities", the host CPU
>> model, migration).
>>
>> While s390x has versioned CPU models, we have no explicit way of specifying
>> them for older machines, besides going over query-cpu-model-expansion and
>> using expanded "base model + features".
>>
>> I can see that this might make sense on x86-64, where you only have maybe 3
>> versions of a CPU (e.g., the one Intel messed up first - Haswell, the one
>> Intel messed up next - Haswell-noTSX, and the one that Intel eventually did
>> right - Haswell-noTSX-IBRS) and you might want to specify "Haswell" vs.
>> "Haswell-IBRS" vs. "Haswell-noTSX-IBRS". But actually, you will always want
>> to go for "Haswell-noTSX-IBRS", because you can expect to run in updated
>> environments if I am not wrong, everything else are corner cases.
>>
>> Of course, versioned CPU model are neat to avoid "base model + list of
>> features", but at least for expanding the host model on s390x, it is not
>> really helpful. When migrating, the model expansion does the trick.
>>
>> I haven't looked into details of "how to specify or model versions". Maybe
>> IBM wants to look into creating versions for all the old models we had. But
>> again, not sure if that is of any help for s390x. CCing IBM.
> 
> I'm not sure yet if there are the x86-specific goals and
> constraints that would make it difficult to follow the same
> approach followed by s390x.  I have the impression we do,
> but I need to think more carefully about it.
> 
> Let's discuss that during KVM Forum?

I won't be attending KVM Forum this year, but Christian should be around.

> 
> The two main goals of versioned CPU models in x86 are:
> 1) Decoupling CPU model updates from machine-types (users should be
>     able to update a CPU model definition without changing machine
>     type).
> 2) Letting management software automate CPU model updates.
>     Normally this is necessary when bare metal microcode or
>     software updates add/remove features from CPUs.  Sometimes the
>     Virtual CPU model update needs to be performed before the host
>     updates are applied (if features are being removed)

We have 2) on s390x after a QEMU machine update. You need a QEMU update 
usually either way to support the new CPU feature. But I can see how 
decoupling the CPU model definition from the machine makes this easier. 
It does introduce complexity. It applies only when running older QEMU 
machines, or if we have to update a CPU model before we get a new QEMU 
machine.

But we do have versioned CPU models already, so the discussion is 
already over :)

> 
> The main constraint that makes it difficult to address the above
> without a new API is:
> A) Every CPU model in x86 except "host" is already expected to
>     be migration-safe (I don't know how this compares to s390x).

I would describe that not a bug but a feature :) It does come with the 
price that only using the newest QEMU machine results in the newest CPU 
model, that part I understand.

-- 

Thanks,

David / dhildenb

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
Posted by no-reply@patchew.org 4 years, 6 months ago
Patchew URL: https://patchew.org/QEMU/20191025022553.25298-1-ehabkost@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH 0/7] i386: Add `machine` parameter to query-cpu-definitions
Type: series
Message-id: 20191025022553.25298-1-ehabkost@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
80774f3 cpu: Add `machine` parameter to query-cpu-definitions
a94469e i386: Don't use default_cpu_version() inside query-cpu-definitions
9c82004 i386: Remove x86_cpu_set_default_version() function
c529624 machine: machine_find_class() function
f79edfc i386: Don't use default_cpu_version at "-cpu help"
0106983 i386: Add default_version parameter to CPU version functions
d6a172b i386: Use g_autofree at x86_cpu_list_entry()

=== OUTPUT BEGIN ===
1/7 Checking commit d6a172b05619 (i386: Use g_autofree at x86_cpu_list_entry())
2/7 Checking commit 0106983c7b3c (i386: Add default_version parameter to CPU version functions)
WARNING: line over 80 characters
#28: FILE: target/i386/cpu.c:3191:
+                                                   X86CPUVersion default_version)

WARNING: line over 80 characters
#60: FILE: target/i386/cpu.c:3983:
+    g_autofree char *alias_of = x86_cpu_class_get_alias_of(cc, default_cpu_version);

WARNING: line over 80 characters
#78: FILE: target/i386/cpu.c:4121:
+    X86CPUVersion version = x86_cpu_model_resolve_version(model, default_cpu_version);

total: 0 errors, 3 warnings, 55 lines checked

Patch 2/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/7 Checking commit f79edfcd6195 (i386: Don't use default_cpu_version at "-cpu help")
4/7 Checking commit c529624d287b (machine: machine_find_class() function)
5/7 Checking commit 9c820045c733 (i386: Remove x86_cpu_set_default_version() function)
WARNING: line over 80 characters
#81: FILE: target/i386/cpu.c:3178:
+        (PCMachineClass *)object_class_dynamic_cast(OBJECT_CLASS(mc), TYPE_PC_MACHINE);

WARNING: line over 80 characters
#87: FILE: target/i386/cpu.c:3184:
+    return default_cpu_version_for_machine(MACHINE_GET_CLASS(qdev_get_machine()));

WARNING: line over 80 characters
#110: FILE: target/i386/cpu.c:4134:
+    X86CPUVersion version = x86_cpu_model_resolve_version(model, default_cpu_version());

total: 0 errors, 3 warnings, 88 lines checked

Patch 5/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/7 Checking commit a94469ea9b83 (i386: Don't use default_cpu_version() inside query-cpu-definitions)
7/7 Checking commit 80774f3866be (cpu: Add `machine` parameter to query-cpu-definitions)
WARNING: line over 80 characters
#147: FILE: tests/acceptance/x86_cpu_model_versions.py:238:
+        """Check if unversioned CPU model is an alias pointing to right version"""

ERROR: line over 90 characters
#152: FILE: tests/acceptance/x86_cpu_model_versions.py:243:
+        cpus1 = dict((m['name'], m.get('alias-of')) for m in vm1.command('query-cpu-definitions', machine='none'))

ERROR: line over 90 characters
#159: FILE: tests/acceptance/x86_cpu_model_versions.py:250:
+        cpus2 = dict((m['name'], m.get('alias-of')) for m in vm2.command('query-cpu-definitions'))

WARNING: line over 80 characters
#165: FILE: tests/acceptance/x86_cpu_model_versions.py:256:
+        """Check if unversioned CPU model is an alias pointing to right version"""

ERROR: line over 90 characters
#170: FILE: tests/acceptance/x86_cpu_model_versions.py:261:
+        cpus1 = dict((m['name'], m.get('alias-of')) for m in vm1.command('query-cpu-definitions', machine='pc-i440fx-4.1'))

ERROR: line over 90 characters
#177: FILE: tests/acceptance/x86_cpu_model_versions.py:268:
+        cpus2 = dict((m['name'], m.get('alias-of')) for m in vm2.command('query-cpu-definitions'))

total: 4 errors, 2 warnings, 141 lines checked

Patch 7/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20191025022553.25298-1-ehabkost@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com