[Qemu-devel] [PATCH 0/6] x86 CPU model versioning

Eduardo Habkost posted 6 patches 4 years, 9 months ago
Test docker-clang@ubuntu passed
Test s390x passed
Test asan passed
Test docker-mingw@fedora passed
Test FreeBSD passed
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190625050008.12789-1-ehabkost@redhat.com
Maintainers: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Markus Armbruster <armbru@redhat.com>, Richard Henderson <rth@twiddle.net>, Eric Blake <eblake@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
qapi/target.json                           |   9 +-
include/hw/i386/pc.h                       |   3 +
target/i386/cpu-qom.h                      |  10 +-
target/i386/cpu.h                          |  16 ++
hw/i386/pc.c                               |   3 +
hw/i386/pc_piix.c                          |   4 +
hw/i386/pc_q35.c                           |   4 +
target/i386/cpu.c                          | 188 +++++++++++++++++----
qemu-deprecated.texi                       |  19 +++
tests/acceptance/x86_cpu_model_versions.py | 173 +++++++++++++++++++
10 files changed, 388 insertions(+), 41 deletions(-)
create mode 100644 tests/acceptance/x86_cpu_model_versions.py
[Qemu-devel] [PATCH 0/6] x86 CPU model versioning
Posted by Eduardo Habkost 4 years, 9 months ago
This series implements basic infrastructure for CPU model
versioning, as discussed before[1][2][3].  This will finally
allow us to update CPU models in ways that introduce new software
or hardware requirements.

My original plan was to use "query-cpu-model-expansion
mode=static" to resolve aliases, but I dropped that plan because
it would increase complexity for management software a lot.
static CPU models are documented as not being affected by the
machine type and accelerator at all, which would make the
versioned CPU models very inconvenient to use in the command
line.  e.g.: users would be forced to replace:

  -cpu Haswell

with:

  -cpu Haswell-4.1,+2apic,+monitor,+kvmclock,+kvm-nopiodelay,+kvm-asyncpf,+kvm-steal-time,+kvm-pv-eoi,+kvmclock-stable-bit,+x2apic,-acpi,-monitor,-svm

In the end, making the versioned CPU models static is not a
requirement at all: what we really need is to drop the
runnability guarantees from unversioned CPU model names, and
require management software to resolve the unversioned alias
before saving the VM configuration.

Guest ABI compatibility and live migration guarantees are going
to be kept: unversioned CPU models will still be usable with live
migration.  Only runnability guarantees when updating the machine
type will be dropped.  This means unversioned CPU models are
still reported as migration-safe in query-cpu-definitions.

The last patch in the series demonstrates how the new feature can
be used to update a CPU model: it adds a Cascadelake-Server-4.1.1
CPU model, including "arch-capabilities=on" and "stepping=5".
Unfortunately we can't enable arch-capabilities in the -4.1
version of Cascadelake-Server because it would break our existing
runnability guarantees.

[1] https://www.mail-archive.com/libvir-list@redhat.com/msg167342.html
[2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg590034.html
[3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg611244.html

---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Pavel Hrdina <phrdina@redhat.com>
Cc: Jiri Denemark <jdenemar@redhat.com>
Cc: "Hu, Robert" <robert.hu@intel.com>
Cc: Tao Xu <tao3.xu@intel.com>
Cc: Richard Henderson <rth@twiddle.net>

Eduardo Habkost (6):
  i386: Add x-force-features option for testing
  i386: Remove unused host_cpudef variable
  qmp: Add "alias-of" field to query-cpu-definitions
  i386: Infrastructure for versioned CPU models
  docs: Deprecate CPU model runnability guarantees
  i386: Add Cascadelake-Server-4.1.1 CPU model

 qapi/target.json                           |   9 +-
 include/hw/i386/pc.h                       |   3 +
 target/i386/cpu-qom.h                      |  10 +-
 target/i386/cpu.h                          |  16 ++
 hw/i386/pc.c                               |   3 +
 hw/i386/pc_piix.c                          |   4 +
 hw/i386/pc_q35.c                           |   4 +
 target/i386/cpu.c                          | 188 +++++++++++++++++----
 qemu-deprecated.texi                       |  19 +++
 tests/acceptance/x86_cpu_model_versions.py | 173 +++++++++++++++++++
 10 files changed, 388 insertions(+), 41 deletions(-)
 create mode 100644 tests/acceptance/x86_cpu_model_versions.py

-- 
2.18.0.rc1.1.g3f1ff2140


Re: [Qemu-devel] [PATCH 0/6] x86 CPU model versioning
Posted by no-reply@patchew.org 4 years, 9 months ago
Patchew URL: https://patchew.org/QEMU/20190625050008.12789-1-ehabkost@redhat.com/



Hi,

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

Message-id: 20190625050008.12789-1-ehabkost@redhat.com
Type: series
Subject: [Qemu-devel] [PATCH 0/6] x86 CPU model versioning

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

From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20190625050008.12789-1-ehabkost@redhat.com -> patchew/20190625050008.12789-1-ehabkost@redhat.com
Switched to a new branch 'test'
fa60fbbfe4 i386: Add Cascadelake-Server-4.1.1 CPU model
41aac3f41c docs: Deprecate CPU model runnability guarantees
7702d3e8ca i386: Infrastructure for versioned CPU models
3fe33acf1b qmp: Add "alias-of" field to query-cpu-definitions
582f02c5f5 i386: Remove unused host_cpudef variable
be6cf76099 i386: Add x-force-features option for testing

=== OUTPUT BEGIN ===
1/6 Checking commit be6cf760996c (i386: Add x-force-features option for testing)
2/6 Checking commit 582f02c5f580 (i386: Remove unused host_cpudef variable)
3/6 Checking commit 3fe33acf1b30 (qmp: Add "alias-of" field to query-cpu-definitions)
4/6 Checking commit 7702d3e8ca63 (i386: Infrastructure for versioned CPU models)
WARNING: Block comments use a leading /* on a separate line
#212: FILE: target/i386/cpu.c:3075:
+    { /* end of list */ },

WARNING: line over 80 characters
#261: FILE: target/i386/cpu.c:3987:
+static const X86CPUVersionDefinition *x86_cpu_def_get_versions(X86CPUDefinition *def)

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#457: 
new file mode 100644

WARNING: line over 80 characters
#507: FILE: tests/acceptance/x86_cpu_model_versions.py:46:
+        cpus = dict((m['name'], m) for m in self.vm.command('query-cpu-definitions'))

WARNING: line over 80 characters
#510: FILE: tests/acceptance/x86_cpu_model_versions.py:49:
+                         'unversioned Cascadelake-Server CPU model must not be static')

WARNING: line over 80 characters
#527: FILE: tests/acceptance/x86_cpu_model_versions.py:66:
+        cpus = dict((m['name'], m) for m in self.vm.command('query-cpu-definitions'))

WARNING: line over 80 characters
#530: FILE: tests/acceptance/x86_cpu_model_versions.py:69:
+                         'unversioned Cascadelake-Server CPU model must not be static')

ERROR: line over 90 characters
#531: FILE: tests/acceptance/x86_cpu_model_versions.py:70:
+        self.assertEquals(cpus['Cascadelake-Server'].get('alias-of'), 'Cascadelake-Server-4.1',

WARNING: line over 80 characters
#532: FILE: tests/acceptance/x86_cpu_model_versions.py:71:
+                          'Cascadelake-Server must be an alias of Cascadelake-Server-4.1')

WARNING: line over 80 characters
#549: FILE: tests/acceptance/x86_cpu_model_versions.py:88:
+        cpus = dict((m['name'], m) for m in self.vm.command('query-cpu-definitions'))

WARNING: line over 80 characters
#552: FILE: tests/acceptance/x86_cpu_model_versions.py:91:
+                         'unversioned Cascadelake-Server CPU model must not be static')

ERROR: line over 90 characters
#553: FILE: tests/acceptance/x86_cpu_model_versions.py:92:
+        self.assertTrue(cpus['Cascadelake-Server']['alias-of'].startswith('Cascadelake-Server-'),

WARNING: line over 80 characters
#554: FILE: tests/acceptance/x86_cpu_model_versions.py:93:
+                          'Cascadelake-Server must be an alias of versioned CPU model')

total: 2 errors, 11 warnings, 477 lines checked

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

5/6 Checking commit 41aac3f41c03 (docs: Deprecate CPU model runnability guarantees)
6/6 Checking commit fa60fbbfe411 (i386: Add Cascadelake-Server-4.1.1 CPU model)
WARNING: Block comments use a leading /* on a separate line
#43: FILE: target/i386/cpu.c:2620:
+                  { /* end of list */ },

WARNING: Block comments use a leading /* on a separate line
#46: FILE: target/i386/cpu.c:2623:
+            { /* end of list */ },

ERROR: line over 90 characters
#74: FILE: tests/acceptance/x86_cpu_model_versions.py:111:
+        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')

ERROR: line over 90 characters
#77: FILE: tests/acceptance/x86_cpu_model_versions.py:114:
+                         'pc-i440fx-4.1 + Cascadelake-Server should not have arch-capabilities')

ERROR: line over 90 characters
#82: FILE: tests/acceptance/x86_cpu_model_versions.py:119:
+        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off')

ERROR: line over 90 characters
#85: FILE: tests/acceptance/x86_cpu_model_versions.py:122:
+                         'pc-i440fx-4.0 + Cascadelake-Server should not have arch-capabilities')

ERROR: line over 90 characters
#91: FILE: tests/acceptance/x86_cpu_model_versions.py:128:
+        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')

ERROR: line over 90 characters
#94: FILE: tests/acceptance/x86_cpu_model_versions.py:131:
+                        'pc-i440fx-4.0 + Cascadelake-Server,+arch-capabilities should have arch-capabilities')

ERROR: line over 90 characters
#99: FILE: tests/acceptance/x86_cpu_model_versions.py:136:
+        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,-arch-capabilities')

ERROR: line over 90 characters
#102: FILE: tests/acceptance/x86_cpu_model_versions.py:139:
+                         'pc-i440fx-4.1 + Cascadelake-Server,-arch-capabilities should not have arch-capabilities')

ERROR: line over 90 characters
#108: FILE: tests/acceptance/x86_cpu_model_versions.py:145:
+        vm.add_args('-cpu', 'Cascadelake-Server-4.1,x-force-features=on,check=off,enforce=off')

ERROR: line over 90 characters
#111: FILE: tests/acceptance/x86_cpu_model_versions.py:148:
+                         'pc-i440fx-4.1 + Cascadelake-Server-4.1 should not have arch-capabilities')

ERROR: line over 90 characters
#116: FILE: tests/acceptance/x86_cpu_model_versions.py:153:
+        vm.add_args('-cpu', 'Cascadelake-Server-4.1.1,x-force-features=on,check=off,enforce=off')

ERROR: line over 90 characters
#119: FILE: tests/acceptance/x86_cpu_model_versions.py:156:
+                         'pc-i440fx-4.1 + Cascadelake-Server-4.1 should have arch-capabilities')

ERROR: line over 90 characters
#125: FILE: tests/acceptance/x86_cpu_model_versions.py:162:
+        vm.add_args('-cpu', 'Cascadelake-Server,x-force-features=on,check=off,enforce=off,+arch-capabilities')

ERROR: line over 90 characters
#128: FILE: tests/acceptance/x86_cpu_model_versions.py:165:
+                         'pc-i440fx-4.0 + Cascadelake-Server-4.1,+arch-capabilities should have arch-capabilities')

ERROR: line over 90 characters
#133: FILE: tests/acceptance/x86_cpu_model_versions.py:170:
+        vm.add_args('-cpu', 'Cascadelake-Server-4.1.1,x-force-features=on,check=off,enforce=off,-arch-capabilities')

ERROR: line over 90 characters
#136: FILE: tests/acceptance/x86_cpu_model_versions.py:173:
+                         'pc-i440fx-4.1 + Cascadelake-Server-4.1.1,-arch-capabilities should not have arch-capabilities')

total: 16 errors, 2 warnings, 101 lines checked

Patch 6/6 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/20190625050008.12789-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
Re: [Qemu-devel] [PATCH 0/6] x86 CPU model versioning
Posted by Daniel P. Berrangé 4 years, 9 months ago
On Tue, Jun 25, 2019 at 02:00:02AM -0300, Eduardo Habkost wrote:
> This series implements basic infrastructure for CPU model
> versioning, as discussed before[1][2][3].  This will finally
> allow us to update CPU models in ways that introduce new software
> or hardware requirements.
> 
> My original plan was to use "query-cpu-model-expansion
> mode=static" to resolve aliases, but I dropped that plan because
> it would increase complexity for management software a lot.
> static CPU models are documented as not being affected by the
> machine type and accelerator at all, which would make the
> versioned CPU models very inconvenient to use in the command
> line.  e.g.: users would be forced to replace:
> 
>   -cpu Haswell
> 
> with:
> 
>   -cpu Haswell-4.1,+2apic,+monitor,+kvmclock,+kvm-nopiodelay,+kvm-asyncpf,+kvm-steal-time,+kvm-pv-eoi,+kvmclock-stable-bit,+x2apic,-acpi,-monitor,-svm
> 
> In the end, making the versioned CPU models static is not a
> requirement at all: what we really need is to drop the
> runnability guarantees from unversioned CPU model names, and
> require management software to resolve the unversioned alias
> before saving the VM configuration.
> 
> Guest ABI compatibility and live migration guarantees are going
> to be kept: unversioned CPU models will still be usable with live
> migration.  Only runnability guarantees when updating the machine
> type will be dropped.  This means unversioned CPU models are
> still reported as migration-safe in query-cpu-definitions.

I'm having a little bit of a hard time parsing the overall behaviour
from the above description. Let me outline the examples so you can
confirm if I've got it right.

Lets assume there is a VM configured using "Haswell"

Today a mgmt app would pass the CPU model name in to QEMU as is,
and thus we get

  $QEMU -machine pc-i440fx-4.0 -cpu Haswell ...more args...

The semantics of "Haswell" here is going to vary according to
what the machine type pc-i440fx-4.0 customizes wrt Haswell.

If the config later has the machine type changed to pc-i440fx-4.1
the semantics of Haswell might change in an arbitrary way. It
might even become unrunnable on the current hardware.

With the new versioned machine types, the VM config of "Haswell"
would be resolved into some arbitrary versioned machine type
"Haswell-NNN" and thus the mgmt app would launch

  $QEMU -machine pc-i440fx-4.0 -cpu Haswell-NNN ...more args...

The semantics of "Haswell-NNN" will never change no matter what
the machine type does.

The end user has the option to explicitly give "Haswell-NNN" to
the mgmt app if they know they want that particular variant, and
in this case the mgmt app won't need to resolve anything (or at
least the process of trying to resolve it won't change it).


> 
> The last patch in the series demonstrates how the new feature can
> be used to update a CPU model: it adds a Cascadelake-Server-4.1.1
> CPU model, including "arch-capabilities=on" and "stepping=5".
> Unfortunately we can't enable arch-capabilities in the -4.1
> version of Cascadelake-Server because it would break our existing
> runnability guarantees.
> 
> [1] https://www.mail-archive.com/libvir-list@redhat.com/msg167342.html
> [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg590034.html
> [3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg611244.html
> 
> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Pavel Hrdina <phrdina@redhat.com>
> Cc: Jiri Denemark <jdenemar@redhat.com>
> Cc: "Hu, Robert" <robert.hu@intel.com>
> Cc: Tao Xu <tao3.xu@intel.com>
> Cc: Richard Henderson <rth@twiddle.net>
> 
> Eduardo Habkost (6):
>   i386: Add x-force-features option for testing
>   i386: Remove unused host_cpudef variable
>   qmp: Add "alias-of" field to query-cpu-definitions
>   i386: Infrastructure for versioned CPU models
>   docs: Deprecate CPU model runnability guarantees
>   i386: Add Cascadelake-Server-4.1.1 CPU model
> 
>  qapi/target.json                           |   9 +-
>  include/hw/i386/pc.h                       |   3 +
>  target/i386/cpu-qom.h                      |  10 +-
>  target/i386/cpu.h                          |  16 ++
>  hw/i386/pc.c                               |   3 +
>  hw/i386/pc_piix.c                          |   4 +
>  hw/i386/pc_q35.c                           |   4 +
>  target/i386/cpu.c                          | 188 +++++++++++++++++----
>  qemu-deprecated.texi                       |  19 +++
>  tests/acceptance/x86_cpu_model_versions.py | 173 +++++++++++++++++++
>  10 files changed, 388 insertions(+), 41 deletions(-)
>  create mode 100644 tests/acceptance/x86_cpu_model_versions.py
> 
> -- 
> 2.18.0.rc1.1.g3f1ff2140
> 

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: [Qemu-devel] [PATCH 0/6] x86 CPU model versioning
Posted by Eduardo Habkost 4 years, 9 months ago
On Tue, Jun 25, 2019 at 05:13:57PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jun 25, 2019 at 02:00:02AM -0300, Eduardo Habkost wrote:
> > This series implements basic infrastructure for CPU model
> > versioning, as discussed before[1][2][3].  This will finally
> > allow us to update CPU models in ways that introduce new software
> > or hardware requirements.
> > 
> > My original plan was to use "query-cpu-model-expansion
> > mode=static" to resolve aliases, but I dropped that plan because
> > it would increase complexity for management software a lot.
> > static CPU models are documented as not being affected by the
> > machine type and accelerator at all, which would make the
> > versioned CPU models very inconvenient to use in the command
> > line.  e.g.: users would be forced to replace:
> > 
> >   -cpu Haswell
> > 
> > with:
> > 
> >   -cpu Haswell-4.1,+2apic,+monitor,+kvmclock,+kvm-nopiodelay,+kvm-asyncpf,+kvm-steal-time,+kvm-pv-eoi,+kvmclock-stable-bit,+x2apic,-acpi,-monitor,-svm
> > 
> > In the end, making the versioned CPU models static is not a
> > requirement at all: what we really need is to drop the
> > runnability guarantees from unversioned CPU model names, and
> > require management software to resolve the unversioned alias
> > before saving the VM configuration.
> > 
> > Guest ABI compatibility and live migration guarantees are going
> > to be kept: unversioned CPU models will still be usable with live
> > migration.  Only runnability guarantees when updating the machine
> > type will be dropped.  This means unversioned CPU models are
> > still reported as migration-safe in query-cpu-definitions.
> 
> I'm having a little bit of a hard time parsing the overall behaviour
> from the above description. Let me outline the examples so you can
> confirm if I've got it right.
> 
> Lets assume there is a VM configured using "Haswell"
> 
> Today a mgmt app would pass the CPU model name in to QEMU as is,
> and thus we get
> 
>   $QEMU -machine pc-i440fx-4.0 -cpu Haswell ...more args...
> 
> The semantics of "Haswell" here is going to vary according to
> what the machine type pc-i440fx-4.0 customizes wrt Haswell.
> 
> If the config later has the machine type changed to pc-i440fx-4.1
> the semantics of Haswell might change in an arbitrary way. It
> might even become unrunnable on the current hardware.

If we follow our deprecation policy, this shouldn't happen until
pc-*-4.3.  On pc-*-4.1 and pc-*-4.2, I expect "-cpu Haswell" to
stay runnable if it's already runnable with pc-*-4.0.  This will
give libvirt and other management software some time to adapt.

CPU model changes that don't affect runnability, though, are
already allowed and will still be allowed.


> 
> With the new versioned machine types, the VM config of "Haswell"
> would be resolved into some arbitrary versioned machine type
> "Haswell-NNN" and thus the mgmt app would launch
> 
>   $QEMU -machine pc-i440fx-4.0 -cpu Haswell-NNN ...more args...
> 
> The semantics of "Haswell-NNN" will never change no matter what
> the machine type does.

Exactly.

> 
> The end user has the option to explicitly give "Haswell-NNN" to
> the mgmt app if they know they want that particular variant, and
> in this case the mgmt app won't need to resolve anything (or at
> least the process of trying to resolve it won't change it).

Yes.  Management software should see that "Haswell-NNN" has no
"alias-of" field, so it doesn't need to be changed.

-- 
Eduardo