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

Eduardo Habkost posted 9 patches 4 years, 10 months ago
Test checkpatch failed
Test s390x passed
Test asan passed
Test docker-mingw@fedora passed
Test FreeBSD passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190628002844.24894-1-ehabkost@redhat.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Markus Armbruster <armbru@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Richard Henderson <rth@twiddle.net>, Eduardo Habkost <ehabkost@redhat.com>, Eric Blake <eblake@redhat.com>
qapi/target.json                           |    9 +-
include/hw/i386/pc.h                       |    3 +
target/i386/cpu-qom.h                      |   10 +-
target/i386/cpu.h                          |   28 +
hw/i386/pc.c                               |    3 +
hw/i386/pc_piix.c                          |    4 +
hw/i386/pc_q35.c                           |    4 +
target/i386/cpu.c                          | 1028 +++++++++-----------
qemu-deprecated.texi                       |   19 +
tests/acceptance/x86_cpu_model_versions.py |  304 ++++++
10 files changed, 831 insertions(+), 581 deletions(-)
create mode 100644 tests/acceptance/x86_cpu_model_versions.py
[Qemu-devel] [PATCH v2 0/9] x86 CPU model versioning
Posted by Eduardo Habkost 4 years, 10 months ago
Changes v1 -> v2:

* Patch "i386: Infrastructure for versioned CPU models" was
  rewritten and split in two:
  * i386: Register versioned CPU models
  * i386: Make unversioned CPU models be aliases
* -IBRS, -noTSX, -IBPB CPU models are now aliases
* Enable rdctl-no, ibrs-all, skip-l1dfl-vmentry in
  Cascadelake-Server-v2
* New patch added:
  * i386: Get model-id from CPU object on "-cpu help"

---
Original description in v1:

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>
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: "Daniel P. Berrange" <berrange@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>

Eduardo Habkost (9):
  qmp: Add "alias-of" field to query-cpu-definitions
  i386: Add x-force-features option for testing
  i386: Get model-id from CPU object on "-cpu help"
  i386: Register versioned CPU models
  i386: Define -IBRS, -noTSX, -IBRS versions of CPU models
  i386: Replace -noTSX, -IBRS, -IBPB CPU models with aliases
  i386: Make unversioned CPU models be aliases
  docs: Deprecate CPU model runnability guarantees
  i386: Add Cascadelake-Server-v2 CPU model

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

-- 
2.18.0.rc1.1.g3f1ff2140


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



Hi,

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

Message-id: 20190628002844.24894-1-ehabkost@redhat.com
Type: series
Subject: [Qemu-devel] [PATCH v2 0/9] 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 ===

Switched to a new branch 'test'
6b27417 i386: Add Cascadelake-Server-v2 CPU model
87c5623 docs: Deprecate CPU model runnability guarantees
c62ec5c i386: Make unversioned CPU models be aliases
c3d6af9 i386: Replace -noTSX, -IBRS, -IBPB CPU models with aliases
0f8c078 i386: Define -IBRS, -noTSX, -IBRS versions of CPU models
07fa113 i386: Register versioned CPU models
0ccbf3d i386: Get model-id from CPU object on "-cpu help"
4d81fc5 i386: Add x-force-features option for testing
2d612b1 qmp: Add "alias-of" field to query-cpu-definitions

=== OUTPUT BEGIN ===
1/9 Checking commit 2d612b193c4b (qmp: Add "alias-of" field to query-cpu-definitions)
2/9 Checking commit 4d81fc55d3dd (i386: Add x-force-features option for testing)
3/9 Checking commit 0ccbf3dcd0a0 (i386: Get model-id from CPU object on "-cpu help")
4/9 Checking commit 07fa113322f8 (i386: Register versioned CPU models)
WARNING: line over 80 characters
#97: FILE: target/i386/cpu.c:1481:
+static const X86CPUVersionDefinition *x86_cpu_def_get_versions(X86CPUDefinition *def)

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

WARNING: Block comments use a leading /* on a separate line
#119: FILE: target/i386/cpu.c:1882:
+        .versions = (X86CPUVersionDefinition[]) { { /* end of list */ } },

WARNING: Block comments use a leading /* on a separate line
#131: FILE: target/i386/cpu.c:1941:
+        .versions = (X86CPUVersionDefinition[]) { { /* end of list */ } },

WARNING: Block comments use a leading /* on a separate line
#143: FILE: target/i386/cpu.c:2010:
+        .versions = (X86CPUVersionDefinition[]) { { /* end of list */ } },

WARNING: Block comments use a leading /* on a separate line
#155: FILE: target/i386/cpu.c:2085:
+        .versions = (X86CPUVersionDefinition[]) { { /* end of list */ } },

WARNING: Block comments use a leading /* on a separate line
#167: FILE: target/i386/cpu.c:2126:
+        .versions = (X86CPUVersionDefinition[]) { { /* end of list */ } },

WARNING: Block comments use a leading /* on a separate line
#179: FILE: target/i386/cpu.c:2169:
+        .versions = (X86CPUVersionDefinition[]) { { /* end of list */ } },

WARNING: Block comments use a leading /* on a separate line
#191: FILE: target/i386/cpu.c:2250:
+        .versions = (X86CPUVersionDefinition[]) { { /* end of list */ } },

WARNING: Block comments use a leading /* on a separate line
#203: FILE: target/i386/cpu.c:2293:
+        .versions = (X86CPUVersionDefinition[]) { { /* end of list */ } },

WARNING: Block comments use a leading /* on a separate line
#215: FILE: target/i386/cpu.c:2338:
+        .versions = (X86CPUVersionDefinition[]) { { /* end of list */ } },

WARNING: Block comments use a leading /* on a separate line
#227: FILE: target/i386/cpu.c:2421:
+        .versions = (X86CPUVersionDefinition[]) { { /* end of list */ } },

WARNING: Block comments use a leading /* on a separate line
#239: FILE: target/i386/cpu.c:2518:
+        .versions = (X86CPUVersionDefinition[]) { { /* end of list */ } },

WARNING: Block comments use a leading /* on a separate line
#251: FILE: target/i386/cpu.c:2625:
+        .versions = (X86CPUVersionDefinition[]) { { /* end of list */ } },

WARNING: Block comments use a leading /* on a separate line
#263: FILE: target/i386/cpu.c:3067:
+        .versions = (X86CPUVersionDefinition[]) { { /* end of list */ } },

WARNING: line over 80 characters
#330: FILE: target/i386/cpu.c:4091:
+    for (vdef = x86_cpu_def_get_versions(model->cpudef); vdef->version; vdef++) {

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

ERROR: line over 90 characters
#549: FILE: tests/acceptance/x86_cpu_model_versions.py:39:
+                              '%s.alias-of (%s) is not a valid CPU model name' % (c['name'], c['alias-of']))

ERROR: line over 90 characters
#552: FILE: tests/acceptance/x86_cpu_model_versions.py:42:
+                                 '%s.alias-of (%s) points to another alias' % (c['name'], c['alias-of']))

WARNING: line over 80 characters
#588: FILE: tests/acceptance/x86_cpu_model_versions.py:78:
+        """Check if pc-*-4.0 unversioned CPU model won't be reported as aliases"""

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

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

total: 2 errors, 20 warnings, 550 lines checked

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

5/9 Checking commit 0f8c07835daf (i386: Define -IBRS, -noTSX, -IBRS versions of CPU models)
WARNING: Block comments use a leading /* on a separate line
#33: FILE: target/i386/cpu.c:1862:
+                    { /* end of list */ }

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

total: 0 errors, 22 warnings, 240 lines checked

Patch 5/9 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/9 Checking commit c3d6af98d417 (i386: Replace -noTSX, -IBRS, -IBPB CPU models with aliases)
7/9 Checking commit c62ec5c338fc (i386: Make unversioned CPU models be aliases)
WARNING: line over 80 characters
#264: FILE: tests/acceptance/x86_cpu_model_versions.py:108:
+        """Check if unversioned CPU model is an alias pointing to right version"""

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

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

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

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

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

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

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

ERROR: line over 90 characters
#375: FILE: tests/acceptance/x86_cpu_model_versions.py:219:
+        self.assertTrue(re.match('Cascadelake-Server-v[0-9]+', cpus['Cascadelake-Server']['alias-of']),

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

total: 2 errors, 8 warnings, 319 lines checked

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

8/9 Checking commit 87c5623c397d (docs: Deprecate CPU model runnability guarantees)
9/9 Checking commit 6b274179ee3f (i386: Add Cascadelake-Server-v2 CPU model)
WARNING: Block comments use a leading /* on a separate line
#42: FILE: target/i386/cpu.c:2354:
+                  { /* end of list */ }

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

total: 16 errors, 2 warnings, 102 lines checked

Patch 9/9 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/20190628002844.24894-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