1:38 p.m.
Doing so would normally break migration to older libvirt, but most of
the patches in this series prepare an infrastructure for removing
features from existing CPU models while maintaining backward
compatibility during migration.
See patch 7 for more details.
https://bugzilla.redhat.com/show_bug.cgi?id=1798004
Jiri Denemark (8):
conf: Rename virCPUDefUpdateFeatureInternal
conf: Use enum in virCPUDefAddFeatureInternal
conf: Add virCPUDefAddFeatureIfMissing
cpu: Run arch specific code for virCPUUpdate for all custom CPUs
cpu_x86: Change the flow in virCPUx86Update
cpu_x86: Add support for marking features as removed from a CPU model
cpu_x86: Make sure removed features are always mentioned in CPU def
cpu_map: Drop 'monitor' from modern x86 CPU models
src/conf/cpu_conf.c | 50 +++++--
src/conf/cpu_conf.h | 5 +
src/cpu/cpu.c | 52 +++----
src/cpu/cpu.h | 3 +-
src/cpu/cpu_arm.c | 5 +-
src/cpu/cpu_ppc64.c | 3 +-
src/cpu/cpu_s390.c | 6 +-
src/cpu/cpu_x86.c | 127 ++++++++++++++----
src/cpu_map/x86_Dhyana.xml | 2 +-
src/cpu_map/x86_EPYC-IBPB.xml | 2 +-
src/cpu_map/x86_EPYC.xml | 2 +-
src/cpu_map/x86_Opteron_G3.xml | 2 +-
src/libvirt_private.syms | 1 +
.../x86_64-cpuid-EPYC-7601-32-Core-guest.xml | 1 +
.../x86_64-cpuid-EPYC-7601-32-Core-host.xml | 1 +
..._64-cpuid-EPYC-7601-32-Core-ibpb-guest.xml | 1 +
...6_64-cpuid-EPYC-7601-32-Core-ibpb-host.xml | 1 +
...6_64-cpuid-EPYC-7601-32-Core-ibpb-json.xml | 2 +-
.../x86_64-cpuid-EPYC-7601-32-Core-json.xml | 2 +-
..._64-cpuid-Hygon-C86-7185-32-core-guest.xml | 1 +
...6_64-cpuid-Hygon-C86-7185-32-core-host.xml | 1 +
...6_64-cpuid-Hygon-C86-7185-32-core-json.xml | 2 +-
.../x86_64-cpuid-Opteron-1352-guest.xml | 1 +
.../x86_64-cpuid-Opteron-1352-host.xml | 1 +
.../x86_64-cpuid-Opteron-2350-guest.xml | 1 +
.../x86_64-cpuid-Opteron-2350-host.xml | 1 +
.../x86_64-cpuid-Opteron-2350-json.xml | 2 +-
.../x86_64-cpuid-Phenom-B95-guest.xml | 1 +
.../x86_64-cpuid-Phenom-B95-json.xml | 2 +-
...4-cpuid-Ryzen-7-1800X-Eight-Core-guest.xml | 1 +
...64-cpuid-Ryzen-7-1800X-Eight-Core-host.xml | 1 +
...64-cpuid-Ryzen-7-1800X-Eight-Core-json.xml | 2 +-
.../domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_3.0.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_3.1.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_4.0.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_4.1.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_4.2.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_5.0.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_5.2.0-tcg.x86_64.xml | 1 +
.../cpu-host-model-cmt.x86_64-4.0.0.args | 6 +-
43 files changed, 223 insertions(+), 78 deletions(-)
--
2.29.2
1:39 p.m.
New subject: [libvirt PATCH 1/8] conf: Rename virCPUDefUpdateFeatureInternal
The function is supposed to add a feature to a CPU definition, let's
name it virCPUDefAddFeatureInternal. The behavior in case the feature is
already present in the CPU def is configurable and we will soon add a
new option to not do anything in that case, which wouldn't really work
well with the current *Update* name.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/conf/cpu_conf.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 5cf7716b12..55a7925d8a 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -858,10 +858,10 @@ virCPUDefFormatBuf(virBufferPtr buf,
}
static int
-virCPUDefUpdateFeatureInternal(virCPUDefPtr def,
- const char *name,
- int policy,
- bool update)
+virCPUDefAddFeatureInternal(virCPUDefPtr def,
+ const char *name,
+ int policy,
+ bool update)
{
virCPUFeatureDefPtr feat;
@@ -898,7 +898,7 @@ virCPUDefUpdateFeature(virCPUDefPtr def,
const char *name,
int policy)
{
- return virCPUDefUpdateFeatureInternal(def, name, policy, true);
+ return virCPUDefAddFeatureInternal(def, name, policy, true);
}
int
@@ -906,7 +906,7 @@ virCPUDefAddFeature(virCPUDefPtr def,
const char *name,
int policy)
{
- return virCPUDefUpdateFeatureInternal(def, name, policy, false);
+ return virCPUDefAddFeatureInternal(def, name, policy, false);
}
--
2.29.2
1:39 p.m.
New subject: [libvirt PATCH 2/8] conf: Use enum in virCPUDefAddFeatureInternal
Replace the 'update' bool parameter with an enum so that we can have
more than two possible values.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/conf/cpu_conf.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 55a7925d8a..2367d36c32 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -857,11 +857,17 @@ virCPUDefFormatBuf(virBufferPtr buf,
return 0;
}
+
+typedef enum {
+ VIR_CPU_ADD_FEATURE_MODE_EXCLUSIVE, /* Fail if feature exists */
+ VIR_CPU_ADD_FEATURE_MODE_UPDATE, /* Add feature or update policy */
+} virCPUDefAddFeatureMode;
+
static int
virCPUDefAddFeatureInternal(virCPUDefPtr def,
const char *name,
int policy,
- bool update)
+ virCPUDefAddFeatureMode mode)
{
virCPUFeatureDefPtr feat;
@@ -869,16 +875,18 @@ virCPUDefAddFeatureInternal(virCPUDefPtr def,
policy = -1;
if ((feat = virCPUDefFindFeature(def, name))) {
- if (update) {
+ switch (mode) {
+ case VIR_CPU_ADD_FEATURE_MODE_UPDATE:
feat->policy = policy;
return 0;
- }
-
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("CPU feature '%s' specified more than once"),
- name);
- return -1;
+ case VIR_CPU_ADD_FEATURE_MODE_EXCLUSIVE:
+ default:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("CPU feature '%s' specified more than
once"),
+ name);
+ return -1;
+ }
}
if (VIR_RESIZE_N(def->features, def->nfeatures_max,
@@ -898,7 +906,8 @@ virCPUDefUpdateFeature(virCPUDefPtr def,
const char *name,
int policy)
{
- return virCPUDefAddFeatureInternal(def, name, policy, true);
+ return virCPUDefAddFeatureInternal(def, name, policy,
+ VIR_CPU_ADD_FEATURE_MODE_UPDATE);
}
int
@@ -906,7 +915,8 @@ virCPUDefAddFeature(virCPUDefPtr def,
const char *name,
int policy)
{
- return virCPUDefAddFeatureInternal(def, name, policy, false);
+ return virCPUDefAddFeatureInternal(def, name, policy,
+ VIR_CPU_ADD_FEATURE_MODE_EXCLUSIVE);
}
--
2.29.2
1:39 p.m.
New subject: [libvirt PATCH 3/8] conf: Add virCPUDefAddFeatureIfMissing
This new function adds a feature to a CPU definition only if it is not
present there yet.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/conf/cpu_conf.c | 14 ++++++++++++++
src/conf/cpu_conf.h | 5 +++++
src/libvirt_private.syms | 1 +
3 files changed, 20 insertions(+)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 2367d36c32..5ff87cb41c 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -861,6 +861,7 @@ virCPUDefFormatBuf(virBufferPtr buf,
typedef enum {
VIR_CPU_ADD_FEATURE_MODE_EXCLUSIVE, /* Fail if feature exists */
VIR_CPU_ADD_FEATURE_MODE_UPDATE, /* Add feature or update policy */
+ VIR_CPU_ADD_FEATURE_MODE_NEW, /* Add feature if it does not exist */
} virCPUDefAddFeatureMode;
static int
@@ -876,6 +877,9 @@ virCPUDefAddFeatureInternal(virCPUDefPtr def,
if ((feat = virCPUDefFindFeature(def, name))) {
switch (mode) {
+ case VIR_CPU_ADD_FEATURE_MODE_NEW:
+ return 0;
+
case VIR_CPU_ADD_FEATURE_MODE_UPDATE:
feat->policy = policy;
return 0;
@@ -920,6 +924,16 @@ virCPUDefAddFeature(virCPUDefPtr def,
}
+int
+virCPUDefAddFeatureIfMissing(virCPUDefPtr def,
+ const char *name,
+ int policy)
+{
+ return virCPUDefAddFeatureInternal(def, name, policy,
+ VIR_CPU_ADD_FEATURE_MODE_NEW);
+}
+
+
virCPUFeatureDefPtr
virCPUDefFindFeature(virCPUDefPtr def,
const char *name)
diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
index 3ef14b7932..b744b06c2d 100644
--- a/src/conf/cpu_conf.h
+++ b/src/conf/cpu_conf.h
@@ -229,6 +229,11 @@ virCPUDefUpdateFeature(virCPUDefPtr cpu,
const char *name,
int policy);
+int
+virCPUDefAddFeatureIfMissing(virCPUDefPtr def,
+ const char *name,
+ int policy);
+
virCPUFeatureDefPtr
virCPUDefFindFeature(virCPUDefPtr def,
const char *name);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 79a23f34cb..f5a8209efd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -96,6 +96,7 @@ virDomainCheckpointTypeToString;
virCPUCacheModeTypeFromString;
virCPUCacheModeTypeToString;
virCPUDefAddFeature;
+virCPUDefAddFeatureIfMissing;
virCPUDefCheckFeatures;
virCPUDefCopy;
virCPUDefCopyModel;
--
2.29.2
1:39 p.m.
New subject: [libvirt PATCH 4/8] cpu: Run arch specific code for virCPUUpdate for all custom CPUs
Until now, the function returned immediately when the guest CPU
definition did not use optional features or minimum match. Clearly,
there's nothing to be updated according to the host CPU in this case,
but the arch specific code may still want to do some compatibility
updates based on the model and features used in the guest CPU
definition.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu.c | 52 +++++++++++++++++++++++++--------------------
src/cpu/cpu.h | 3 ++-
src/cpu/cpu_arm.c | 5 +++--
src/cpu/cpu_ppc64.c | 3 ++-
src/cpu/cpu_s390.c | 6 +++++-
src/cpu/cpu_x86.c | 6 +++++-
6 files changed, 46 insertions(+), 29 deletions(-)
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index bf94811960..44094bd0df 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -538,12 +538,11 @@ virCPUBaseline(virArch arch,
* @guest: guest CPU definition to be updated
* @host: host CPU definition
*
- * Updates @guest CPU definition according to @host CPU. This is required to
- * support guest CPU definitions specified relatively to host CPU, such as
- * CPUs with VIR_CPU_MODE_CUSTOM and optional features or
- * VIR_CPU_MATCH_MINIMUM, or CPUs with VIR_CPU_MODE_HOST_MODEL.
- * When the guest CPU was not specified relatively, the function does nothing
- * and returns success.
+ * Updates @guest CPU definition possibly taking @host CPU into account. This
+ * is required for maintaining compatibility with older libvirt releases or to
+ * support guest CPU definitions specified relatively to host CPU, such as CPUs
+ * with VIR_CPU_MODE_CUSTOM and optional features or VIR_CPU_MATCH_MINIMUM, or
+ * CPUs with VIR_CPU_MODE_HOST_MODEL.
*
* Returns 0 on success, -1 on error.
*/
@@ -553,6 +552,7 @@ virCPUUpdate(virArch arch,
const virCPUDef *host)
{
struct cpuArchDriver *driver;
+ bool relative;
VIR_DEBUG("arch=%s, guest=%p mode=%s model=%s, host=%p model=%s",
virArchToString(arch), guest, virCPUModeTypeToString(guest->mode),
@@ -561,30 +561,36 @@ virCPUUpdate(virArch arch,
if (!(driver = cpuGetSubDriver(arch)))
return -1;
- if (guest->mode == VIR_CPU_MODE_HOST_PASSTHROUGH)
+ switch ((virCPUMode) guest->mode) {
+ case VIR_CPU_MODE_HOST_PASSTHROUGH:
return 0;
- if (guest->mode == VIR_CPU_MODE_CUSTOM &&
- guest->match != VIR_CPU_MATCH_MINIMUM) {
- size_t i;
- bool optional = false;
+ case VIR_CPU_MODE_HOST_MODEL:
+ relative = true;
+ break;
- for (i = 0; i < guest->nfeatures; i++) {
- if (guest->features[i].policy == VIR_CPU_FEATURE_OPTIONAL) {
- optional = true;
- break;
+ case VIR_CPU_MODE_CUSTOM:
+ if (guest->match == VIR_CPU_MATCH_MINIMUM) {
+ relative = true;
+ } else {
+ size_t i;
+
+ relative = false;
+ for (i = 0; i < guest->nfeatures; i++) {
+ if (guest->features[i].policy == VIR_CPU_FEATURE_OPTIONAL) {
+ relative = true;
+ break;
+ }
}
}
+ break;
- if (!optional)
- return 0;
+ case VIR_CPU_MODE_LAST:
+ default:
+ virReportEnumRangeError(virCPUMode, guest->mode);
+ return -1;
}
- /* We get here if guest CPU is either
- * - host-model
- * - custom with minimum match
- * - custom with optional features
- */
if (!driver->update) {
virReportError(VIR_ERR_NO_SUPPORT,
_("cannot update guest CPU for %s architecture"),
@@ -592,7 +598,7 @@ virCPUUpdate(virArch arch,
return -1;
}
- if (driver->update(guest, host) < 0)
+ if (driver->update(guest, host, relative) < 0)
return -1;
VIR_DEBUG("model=%s", NULLSTR(guest->model));
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index cc2d132275..ff4fb7e103 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -79,7 +79,8 @@ typedef virCPUDefPtr
typedef int
(*virCPUArchUpdate)(virCPUDefPtr guest,
- const virCPUDef *host);
+ const virCPUDef *host,
+ bool relative);
typedef int
(*virCPUArchUpdateLive)(virCPUDefPtr cpu,
diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index 31997b59cd..8a408a324a 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -415,11 +415,12 @@ virCPUarmGetMap(void)
static int
virCPUarmUpdate(virCPUDefPtr guest,
- const virCPUDef *host)
+ const virCPUDef *host,
+ bool relative)
{
g_autoptr(virCPUDef) updated = NULL;
- if (guest->mode != VIR_CPU_MODE_HOST_MODEL)
+ if (!relative || guest->mode != VIR_CPU_MODE_HOST_MODEL)
return 0;
if (!host) {
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
index 555eeecbe7..d71d147207 100644
--- a/src/cpu/cpu_ppc64.c
+++ b/src/cpu/cpu_ppc64.c
@@ -614,7 +614,8 @@ virCPUppc64GetHost(virCPUDefPtr cpu,
static int
virCPUppc64Update(virCPUDefPtr guest,
- const virCPUDef *host G_GNUC_UNUSED)
+ const virCPUDef *host G_GNUC_UNUSED,
+ bool relative G_GNUC_UNUSED)
{
/*
* - host-passthrough doesn't even get here
diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c
index c1c5686244..17321dc04a 100644
--- a/src/cpu/cpu_s390.c
+++ b/src/cpu/cpu_s390.c
@@ -43,11 +43,15 @@ virCPUs390Compare(virCPUDefPtr host G_GNUC_UNUSED,
static int
virCPUs390Update(virCPUDefPtr guest,
- const virCPUDef *host)
+ const virCPUDef *host,
+ bool relative)
{
g_autoptr(virCPUDef) updated = NULL;
size_t i;
+ if (!relative)
+ return 0;
+
if (guest->mode == VIR_CPU_MODE_CUSTOM) {
if (guest->match == VIR_CPU_MATCH_MINIMUM) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 0e533c62e1..72f17070e1 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2936,12 +2936,16 @@ x86UpdateHostModel(virCPUDefPtr guest,
static int
virCPUx86Update(virCPUDefPtr guest,
- const virCPUDef *host)
+ const virCPUDef *host,
+ bool relative)
{
g_autoptr(virCPUx86Model) model = NULL;
virCPUx86MapPtr map;
size_t i;
+ if (!relative)
+ return 0;
+
if (!host) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("unknown host CPU model"));
--
2.29.2
1:39 p.m.
New subject: [libvirt PATCH 5/8] cpu_x86: Change the flow in virCPUx86Update
This is just a preparation for adding new functionality to
virCPUx86Update.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu_x86.c | 49 ++++++++++++++++++++++++-----------------------
1 file changed, 25 insertions(+), 24 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 72f17070e1..a7ff095456 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -2943,38 +2943,39 @@ virCPUx86Update(virCPUDefPtr guest,
virCPUx86MapPtr map;
size_t i;
- if (!relative)
- return 0;
-
- if (!host) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("unknown host CPU model"));
- return -1;
- }
-
if (!(map = virCPUx86GetMap()))
return -1;
- if (!(model = x86ModelFromCPU(host, map, -1)))
- return -1;
+ if (relative) {
+ if (!host) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unknown host CPU model"));
+ return -1;
+ }
- for (i = 0; i < guest->nfeatures; i++) {
- if (guest->features[i].policy == VIR_CPU_FEATURE_OPTIONAL) {
- int supported = x86FeatureInData(guest->features[i].name,
- &model->data, map);
- if (supported < 0)
+ if (!(model = x86ModelFromCPU(host, map, -1)))
+ return -1;
+
+ for (i = 0; i < guest->nfeatures; i++) {
+ if (guest->features[i].policy == VIR_CPU_FEATURE_OPTIONAL) {
+ int supported = x86FeatureInData(guest->features[i].name,
+ &model->data, map);
+ if (supported < 0)
+ return -1;
+ else if (supported)
+ guest->features[i].policy = VIR_CPU_FEATURE_REQUIRE;
+ else
+ guest->features[i].policy = VIR_CPU_FEATURE_DISABLE;
+ }
+ }
+
+ if (guest->mode == VIR_CPU_MODE_HOST_MODEL ||
+ guest->match == VIR_CPU_MATCH_MINIMUM) {
+ if (x86UpdateHostModel(guest, host) < 0)
return -1;
- else if (supported)
- guest->features[i].policy = VIR_CPU_FEATURE_REQUIRE;
- else
- guest->features[i].policy = VIR_CPU_FEATURE_DISABLE;
}
}
- if (guest->mode == VIR_CPU_MODE_HOST_MODEL ||
- guest->match == VIR_CPU_MATCH_MINIMUM)
- return x86UpdateHostModel(guest, host);
-
return 0;
}
--
2.29.2
1:39 p.m.
New subject: [libvirt PATCH 6/8] cpu_x86: Add support for marking features as removed from a CPU model
The patch adds a new attribute for the 'feature' element in CPU model
specification to indicate that a given feature was removed from a CPU
model. In other words, older versions of libvirt would consider such
feature to be included in the CPU model.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu_x86.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index a7ff095456..40bf5b68d0 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -144,6 +144,7 @@ struct _virCPUx86Model {
virCPUx86VendorPtr vendor;
virCPUx86SignaturesPtr signatures;
virCPUx86Data data;
+ GStrv removedFeatures;
};
typedef struct _virCPUx86Map virCPUx86Map;
@@ -1241,6 +1242,7 @@ x86ModelFree(virCPUx86ModelPtr model)
g_free(model->name);
virCPUx86SignaturesFree(model->signatures);
virCPUx86DataClear(&model->data);
+ g_strfreev(model->removedFeatures);
g_free(model);
}
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUx86Model, x86ModelFree);
@@ -1255,6 +1257,7 @@ x86ModelCopy(virCPUx86ModelPtr model)
copy->name = g_strdup(model->name);
copy->signatures = virCPUx86SignaturesCopy(model->signatures);
x86DataCopy(©->data, &model->data);
+ copy->removedFeatures = g_strdupv(model->removedFeatures);
copy->vendor = model->vendor;
return g_steal_pointer(©);
@@ -1575,6 +1578,7 @@ x86ModelParseFeatures(virCPUx86ModelPtr model,
for (i = 0; i < n; i++) {
g_autofree char *ftname = NULL;
+ g_autofree char *removed = NULL;
virCPUx86FeaturePtr feature;
if (!(ftname = virXMLPropString(nodes[i], "name"))) {
@@ -1591,6 +1595,24 @@ x86ModelParseFeatures(virCPUx86ModelPtr model,
return -1;
}
+ if ((removed = virXMLPropString(nodes[i], "removed"))) {
+ int rem;
+
+ if ((rem = virTristateBoolTypeFromString(removed)) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid 'removed' attribute for feature %s
"
+ "in model %s"),
+ ftname, model->name);
+ return -1;
+ }
+
+ if (rem == VIR_TRISTATE_BOOL_YES) {
+ if (virStringListAdd(&model->removedFeatures, ftname) < 0)
+ return -1;
+ continue;
+ }
+ }
+
if (x86DataAdd(&model->data, &feature->data))
return -1;
}
--
2.29.2
1:39 p.m.
New subject: [libvirt PATCH 7/8] cpu_x86: Make sure removed features are always mentioned in CPU def
For backward compatibility with older versions of libvirt CPU models in
our CPU map are mostly immutable. We only changed them in a few specific
cases after showing it was safe. Sometimes QEMU developers realize a
specific feature should not be part of a particular (or any) CPU model
because it can never be enabled automatically without further
configuration. But we couldn't follow them because doing so would break
migration to older libvirt.
If QEMU drops feature F from CPU model M because F could not be enabled
automatically anyway, asking for M would never enable F. Even with older
QEMU versions. Naively removing F from libvirt's definition of M would
seem to work nicely on a single host. Libvirt would consider M to be
compatible with hosts CPU that do not support F. However, trying to
migrate domains using M without explicitly enabling or disabling F could
fail, because older libvirt would think F was enabled (it is part of M
there), but QEMU reports it as disabled once started.
Thus we can remove such feature from a libvirt's CPU model, but we have
to make sure any CPU definition using the affected model will always
explicitly mention the state of the removed feature.
https://bugzilla.redhat.com/show_bug.cgi?id=1798004
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu/cpu_x86.c | 58 ++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 55 insertions(+), 3 deletions(-)
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 40bf5b68d0..2422e258ec 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -804,11 +804,37 @@ x86DataAddSignature(virCPUx86Data *data,
}
+/*
+ * Disables features removed from the CPU @model unless they are already
+ * mentioned in @cpu to make sure these features will always be explicitly
+ * listed in the CPU definition.
+ */
+static int
+virCPUx86DisableRemovedFeatures(virCPUDefPtr cpu,
+ virCPUx86ModelPtr model)
+{
+ char **feat = model->removedFeatures;
+
+ if (!feat)
+ return 0;
+
+ while (*feat) {
+ if (virCPUDefAddFeatureIfMissing(cpu, *feat, VIR_CPU_FEATURE_DISABLE) < 0)
+ return -1;
+
+ feat++;
+ }
+
+ return 0;
+}
+
+
static virCPUDefPtr
x86DataToCPU(const virCPUx86Data *data,
virCPUx86ModelPtr model,
virCPUx86MapPtr map,
- virDomainCapsCPUModelPtr hvModel)
+ virDomainCapsCPUModelPtr hvModel,
+ virCPUType cpuType)
{
g_autoptr(virCPUDef) cpu = NULL;
g_auto(virCPUx86Data) copy = VIR_CPU_X86_DATA_INIT;
@@ -851,6 +877,13 @@ x86DataToCPU(const virCPUx86Data *data,
x86DataToCPUFeatures(cpu, VIR_CPU_FEATURE_DISABLE, &modelData, map))
return NULL;
+ if (cpuType == VIR_CPU_TYPE_GUEST) {
+ if (virCPUx86DisableRemovedFeatures(cpu, model) < 0)
+ return NULL;
+ }
+
+ cpu->type = cpuType;
+
return g_steal_pointer(&cpu);
}
@@ -2197,9 +2230,9 @@ x86Decode(virCPUDefPtr cpu,
continue;
}
- if (!(cpuCandidate = x86DataToCPU(&data, candidate, map, hvModel)))
+ if (!(cpuCandidate = x86DataToCPU(&data, candidate, map, hvModel,
+ cpu->type)))
return -1;
- cpuCandidate->type = cpu->type;
if ((rc = x86DecodeUseCandidate(model, cpuModel,
candidate, cpuCandidate,
@@ -2962,6 +2995,7 @@ virCPUx86Update(virCPUDefPtr guest,
bool relative)
{
g_autoptr(virCPUx86Model) model = NULL;
+ virCPUx86ModelPtr guestModel;
virCPUx86MapPtr map;
size_t i;
@@ -2998,6 +3032,15 @@ virCPUx86Update(virCPUDefPtr guest,
}
}
+ if (!(guestModel = x86ModelFind(map, guest->model))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unknown CPU model %s"), guest->model);
+ return -1;
+ }
+
+ if (virCPUx86DisableRemovedFeatures(guest, guestModel) < 0)
+ return -1;
+
return 0;
}
@@ -3067,6 +3110,9 @@ virCPUx86UpdateLive(virCPUDefPtr cpu,
}
}
+ if (virCPUx86DisableRemovedFeatures(cpu, model) < 0)
+ return -1;
+
virBufferTrim(&bufAdded, ",");
virBufferTrim(&bufRemoved, ",");
@@ -3190,6 +3236,7 @@ virCPUx86ExpandFeatures(virCPUDefPtr cpu)
}
model = x86ModelCopy(model);
+
if (x86DataToCPUFeatures(expanded, host ? -1 : VIR_CPU_FEATURE_REQUIRE,
&model->data, map) < 0)
return -1;
@@ -3206,6 +3253,11 @@ virCPUx86ExpandFeatures(virCPUDefPtr cpu)
return -1;
}
+ if (!host) {
+ if (virCPUx86DisableRemovedFeatures(expanded, model) < 0)
+ return -1;
+ }
+
virCPUDefFreeModel(cpu);
return virCPUDefCopyModel(cpu, expanded, false);
--
2.29.2
1:39 p.m.
New subject: [libvirt PATCH 8/8] cpu_map: Drop 'monitor' from modern x86 CPU models
The feature is never enabled by default on KVM and QEMU dropped it from
the models long ago.
https://bugzilla.redhat.com/show_bug.cgi?id=1798004
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/cpu_map/x86_Dhyana.xml | 2 +-
src/cpu_map/x86_EPYC-IBPB.xml | 2 +-
src/cpu_map/x86_EPYC.xml | 2 +-
src/cpu_map/x86_Opteron_G3.xml | 2 +-
tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-guest.xml | 1 +
tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-host.xml | 1 +
.../x86_64-cpuid-EPYC-7601-32-Core-ibpb-guest.xml | 1 +
.../x86_64-cpuid-EPYC-7601-32-Core-ibpb-host.xml | 1 +
.../x86_64-cpuid-EPYC-7601-32-Core-ibpb-json.xml | 2 +-
tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-json.xml | 2 +-
.../x86_64-cpuid-Hygon-C86-7185-32-core-guest.xml | 1 +
.../x86_64-cpuid-Hygon-C86-7185-32-core-host.xml | 1 +
.../x86_64-cpuid-Hygon-C86-7185-32-core-json.xml | 2 +-
tests/cputestdata/x86_64-cpuid-Opteron-1352-guest.xml | 1 +
tests/cputestdata/x86_64-cpuid-Opteron-1352-host.xml | 1 +
tests/cputestdata/x86_64-cpuid-Opteron-2350-guest.xml | 1 +
tests/cputestdata/x86_64-cpuid-Opteron-2350-host.xml | 1 +
tests/cputestdata/x86_64-cpuid-Opteron-2350-json.xml | 2 +-
tests/cputestdata/x86_64-cpuid-Phenom-B95-guest.xml | 1 +
tests/cputestdata/x86_64-cpuid-Phenom-B95-json.xml | 2 +-
.../x86_64-cpuid-Ryzen-7-1800X-Eight-Core-guest.xml | 1 +
.../x86_64-cpuid-Ryzen-7-1800X-Eight-Core-host.xml | 1 +
.../x86_64-cpuid-Ryzen-7-1800X-Eight-Core-json.xml | 2 +-
tests/domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 1 +
tests/domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 1 +
tests/domaincapsdata/qemu_3.0.0-tcg.x86_64.xml | 1 +
tests/domaincapsdata/qemu_3.1.0-tcg.x86_64.xml | 1 +
tests/domaincapsdata/qemu_4.0.0-tcg.x86_64.xml | 1 +
tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml | 1 +
tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml | 1 +
tests/domaincapsdata/qemu_5.0.0-tcg.x86_64.xml | 1 +
tests/domaincapsdata/qemu_5.1.0-tcg.x86_64.xml | 1 +
tests/domaincapsdata/qemu_5.2.0-tcg.x86_64.xml | 1 +
tests/qemuxml2argvdata/cpu-host-model-cmt.x86_64-4.0.0.args | 6 +++---
34 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/src/cpu_map/x86_Dhyana.xml b/src/cpu_map/x86_Dhyana.xml
index 689daf8649..cfde07f99f 100644
--- a/src/cpu_map/x86_Dhyana.xml
+++ b/src/cpu_map/x86_Dhyana.xml
@@ -33,7 +33,7 @@
<feature name='misalignsse'/>
<feature name='mmx'/>
<feature name='mmxext'/>
- <feature name='monitor'/>
+ <feature name='monitor' removed='yes'/>
<feature name='movbe'/>
<feature name='msr'/>
<feature name='mtrr'/>
diff --git a/src/cpu_map/x86_EPYC-IBPB.xml b/src/cpu_map/x86_EPYC-IBPB.xml
index 983c5f4445..fc5aadf52e 100644
--- a/src/cpu_map/x86_EPYC-IBPB.xml
+++ b/src/cpu_map/x86_EPYC-IBPB.xml
@@ -34,7 +34,7 @@
<feature name='misalignsse'/>
<feature name='mmx'/>
<feature name='mmxext'/>
- <feature name='monitor'/>
+ <feature name='monitor' removed='yes'/>
<feature name='movbe'/>
<feature name='msr'/>
<feature name='mtrr'/>
diff --git a/src/cpu_map/x86_EPYC.xml b/src/cpu_map/x86_EPYC.xml
index 3ebba9f4ed..3b406de37a 100644
--- a/src/cpu_map/x86_EPYC.xml
+++ b/src/cpu_map/x86_EPYC.xml
@@ -33,7 +33,7 @@
<feature name='misalignsse'/>
<feature name='mmx'/>
<feature name='mmxext'/>
- <feature name='monitor'/>
+ <feature name='monitor' removed='yes'/>
<feature name='movbe'/>
<feature name='msr'/>
<feature name='mtrr'/>
diff --git a/src/cpu_map/x86_Opteron_G3.xml b/src/cpu_map/x86_Opteron_G3.xml
index dab59d4f82..cf00af8698 100644
--- a/src/cpu_map/x86_Opteron_G3.xml
+++ b/src/cpu_map/x86_Opteron_G3.xml
@@ -18,7 +18,7 @@
<feature name='mce'/>
<feature name='misalignsse'/>
<feature name='mmx'/>
- <feature name='monitor'/>
+ <feature name='monitor' removed='yes'/>
<feature name='msr'/>
<feature name='mtrr'/>
<feature name='nx'/>
diff --git a/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-guest.xml
b/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-guest.xml
index 0053913327..9d14213a0b 100644
--- a/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-guest.xml
+++ b/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-guest.xml
@@ -2,6 +2,7 @@
<model fallback='forbid'>EPYC</model>
<vendor>AMD</vendor>
<feature policy='require' name='ht'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='osxsave'/>
<feature policy='require' name='xsaves'/>
<feature policy='require' name='cmp_legacy'/>
diff --git a/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-host.xml
b/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-host.xml
index 7acab0a999..8c2c975fdd 100644
--- a/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-host.xml
+++ b/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-host.xml
@@ -3,6 +3,7 @@
<model>EPYC</model>
<vendor>AMD</vendor>
<feature name='ht'/>
+ <feature name='monitor'/>
<feature name='osxsave'/>
<feature name='xsaves'/>
<feature name='cmp_legacy'/>
diff --git a/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-ibpb-guest.xml
b/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-ibpb-guest.xml
index 9164987bbd..1fee2fa9f8 100644
--- a/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-ibpb-guest.xml
+++ b/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-ibpb-guest.xml
@@ -2,6 +2,7 @@
<model fallback='forbid'>EPYC-IBPB</model>
<vendor>AMD</vendor>
<feature policy='require' name='ht'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='osxsave'/>
<feature policy='require' name='xsaves'/>
<feature policy='require' name='cmp_legacy'/>
diff --git a/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-ibpb-host.xml
b/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-ibpb-host.xml
index 2fa8861e44..c3bbf78396 100644
--- a/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-ibpb-host.xml
+++ b/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-ibpb-host.xml
@@ -3,6 +3,7 @@
<model>EPYC-IBPB</model>
<vendor>AMD</vendor>
<feature name='ht'/>
+ <feature name='monitor'/>
<feature name='osxsave'/>
<feature name='xsaves'/>
<feature name='cmp_legacy'/>
diff --git a/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-ibpb-json.xml
b/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-ibpb-json.xml
index af1e7f2f32..45f27a9a1f 100644
--- a/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-ibpb-json.xml
+++ b/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-ibpb-json.xml
@@ -8,7 +8,7 @@
<feature policy='require' name='cmp_legacy'/>
<feature policy='require' name='npt'/>
<feature policy='require' name='nrip-save'/>
- <feature policy='disable' name='monitor'/>
<feature policy='disable' name='rdtscp'/>
<feature policy='disable' name='svm'/>
+ <feature policy='disable' name='monitor'/>
</cpu>
diff --git a/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-json.xml
b/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-json.xml
index 4450a40f61..5af19749e7 100644
--- a/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-json.xml
+++ b/tests/cputestdata/x86_64-cpuid-EPYC-7601-32-Core-json.xml
@@ -8,6 +8,6 @@
<feature policy='require' name='cmp_legacy'/>
<feature policy='require' name='npt'/>
<feature policy='require' name='nrip-save'/>
- <feature policy='disable' name='monitor'/>
<feature policy='disable' name='svm'/>
+ <feature policy='disable' name='monitor'/>
</cpu>
diff --git a/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-guest.xml
b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-guest.xml
index 08c574255e..5df21521f6 100644
--- a/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-guest.xml
+++ b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-guest.xml
@@ -2,6 +2,7 @@
<model fallback='forbid'>Dhyana</model>
<vendor>Hygon</vendor>
<feature policy='require' name='ht'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='osxsave'/>
<feature policy='require' name='xsaves'/>
<feature policy='require' name='cmp_legacy'/>
diff --git a/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-host.xml
b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-host.xml
index f1cddb6a19..76c3753505 100644
--- a/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-host.xml
+++ b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-host.xml
@@ -3,6 +3,7 @@
<model>Dhyana</model>
<vendor>Hygon</vendor>
<feature name='ht'/>
+ <feature name='monitor'/>
<feature name='osxsave'/>
<feature name='xsaves'/>
<feature name='cmp_legacy'/>
diff --git a/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-json.xml
b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-json.xml
index 0fdd934c08..0408d51c10 100644
--- a/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-json.xml
+++ b/tests/cputestdata/x86_64-cpuid-Hygon-C86-7185-32-core-json.xml
@@ -9,6 +9,6 @@
<feature policy='require' name='virt-ssbd'/>
<feature policy='require' name='npt'/>
<feature policy='require' name='nrip-save'/>
- <feature policy='disable' name='monitor'/>
<feature policy='disable' name='svm'/>
+ <feature policy='disable' name='monitor'/>
</cpu>
diff --git a/tests/cputestdata/x86_64-cpuid-Opteron-1352-guest.xml
b/tests/cputestdata/x86_64-cpuid-Opteron-1352-guest.xml
index a52c4cd303..99c6420579 100644
--- a/tests/cputestdata/x86_64-cpuid-Opteron-1352-guest.xml
+++ b/tests/cputestdata/x86_64-cpuid-Opteron-1352-guest.xml
@@ -3,6 +3,7 @@
<vendor>AMD</vendor>
<feature policy='require' name='vme'/>
<feature policy='require' name='ht'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='mmxext'/>
<feature policy='require' name='fxsr_opt'/>
<feature policy='require' name='pdpe1gb'/>
diff --git a/tests/cputestdata/x86_64-cpuid-Opteron-1352-host.xml
b/tests/cputestdata/x86_64-cpuid-Opteron-1352-host.xml
index 800b092f14..ecd532167a 100644
--- a/tests/cputestdata/x86_64-cpuid-Opteron-1352-host.xml
+++ b/tests/cputestdata/x86_64-cpuid-Opteron-1352-host.xml
@@ -4,6 +4,7 @@
<vendor>AMD</vendor>
<feature name='vme'/>
<feature name='ht'/>
+ <feature name='monitor'/>
<feature name='mmxext'/>
<feature name='fxsr_opt'/>
<feature name='pdpe1gb'/>
diff --git a/tests/cputestdata/x86_64-cpuid-Opteron-2350-guest.xml
b/tests/cputestdata/x86_64-cpuid-Opteron-2350-guest.xml
index a52c4cd303..99c6420579 100644
--- a/tests/cputestdata/x86_64-cpuid-Opteron-2350-guest.xml
+++ b/tests/cputestdata/x86_64-cpuid-Opteron-2350-guest.xml
@@ -3,6 +3,7 @@
<vendor>AMD</vendor>
<feature policy='require' name='vme'/>
<feature policy='require' name='ht'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='mmxext'/>
<feature policy='require' name='fxsr_opt'/>
<feature policy='require' name='pdpe1gb'/>
diff --git a/tests/cputestdata/x86_64-cpuid-Opteron-2350-host.xml
b/tests/cputestdata/x86_64-cpuid-Opteron-2350-host.xml
index 800b092f14..ecd532167a 100644
--- a/tests/cputestdata/x86_64-cpuid-Opteron-2350-host.xml
+++ b/tests/cputestdata/x86_64-cpuid-Opteron-2350-host.xml
@@ -4,6 +4,7 @@
<vendor>AMD</vendor>
<feature name='vme'/>
<feature name='ht'/>
+ <feature name='monitor'/>
<feature name='mmxext'/>
<feature name='fxsr_opt'/>
<feature name='pdpe1gb'/>
diff --git a/tests/cputestdata/x86_64-cpuid-Opteron-2350-json.xml
b/tests/cputestdata/x86_64-cpuid-Opteron-2350-json.xml
index d128553c13..d874bfc049 100644
--- a/tests/cputestdata/x86_64-cpuid-Opteron-2350-json.xml
+++ b/tests/cputestdata/x86_64-cpuid-Opteron-2350-json.xml
@@ -15,6 +15,6 @@
<feature policy='require' name='3dnowprefetch'/>
<feature policy='require' name='osvw'/>
<feature policy='require' name='npt'/>
- <feature policy='disable' name='monitor'/>
<feature policy='disable' name='rdtscp'/>
+ <feature policy='disable' name='monitor'/>
</cpu>
diff --git a/tests/cputestdata/x86_64-cpuid-Phenom-B95-guest.xml
b/tests/cputestdata/x86_64-cpuid-Phenom-B95-guest.xml
index ab0e99f97d..f6106fdb1f 100644
--- a/tests/cputestdata/x86_64-cpuid-Phenom-B95-guest.xml
+++ b/tests/cputestdata/x86_64-cpuid-Phenom-B95-guest.xml
@@ -3,6 +3,7 @@
<vendor>AMD</vendor>
<feature policy='require' name='vme'/>
<feature policy='require' name='ht'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='mmxext'/>
<feature policy='require' name='fxsr_opt'/>
<feature policy='require' name='pdpe1gb'/>
diff --git a/tests/cputestdata/x86_64-cpuid-Phenom-B95-json.xml
b/tests/cputestdata/x86_64-cpuid-Phenom-B95-json.xml
index d161709981..b4198f66a5 100644
--- a/tests/cputestdata/x86_64-cpuid-Phenom-B95-json.xml
+++ b/tests/cputestdata/x86_64-cpuid-Phenom-B95-json.xml
@@ -15,8 +15,8 @@
<feature policy='require' name='osvw'/>
<feature policy='require' name='npt'/>
<feature policy='require' name='nrip-save'/>
- <feature policy='disable' name='monitor'/>
<feature policy='disable' name='nx'/>
<feature policy='disable' name='rdtscp'/>
<feature policy='disable' name='svm'/>
+ <feature policy='disable' name='monitor'/>
</cpu>
diff --git a/tests/cputestdata/x86_64-cpuid-Ryzen-7-1800X-Eight-Core-guest.xml
b/tests/cputestdata/x86_64-cpuid-Ryzen-7-1800X-Eight-Core-guest.xml
index 0053913327..9d14213a0b 100644
--- a/tests/cputestdata/x86_64-cpuid-Ryzen-7-1800X-Eight-Core-guest.xml
+++ b/tests/cputestdata/x86_64-cpuid-Ryzen-7-1800X-Eight-Core-guest.xml
@@ -2,6 +2,7 @@
<model fallback='forbid'>EPYC</model>
<vendor>AMD</vendor>
<feature policy='require' name='ht'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='osxsave'/>
<feature policy='require' name='xsaves'/>
<feature policy='require' name='cmp_legacy'/>
diff --git a/tests/cputestdata/x86_64-cpuid-Ryzen-7-1800X-Eight-Core-host.xml
b/tests/cputestdata/x86_64-cpuid-Ryzen-7-1800X-Eight-Core-host.xml
index 7acab0a999..8c2c975fdd 100644
--- a/tests/cputestdata/x86_64-cpuid-Ryzen-7-1800X-Eight-Core-host.xml
+++ b/tests/cputestdata/x86_64-cpuid-Ryzen-7-1800X-Eight-Core-host.xml
@@ -3,6 +3,7 @@
<model>EPYC</model>
<vendor>AMD</vendor>
<feature name='ht'/>
+ <feature name='monitor'/>
<feature name='osxsave'/>
<feature name='xsaves'/>
<feature name='cmp_legacy'/>
diff --git a/tests/cputestdata/x86_64-cpuid-Ryzen-7-1800X-Eight-Core-json.xml
b/tests/cputestdata/x86_64-cpuid-Ryzen-7-1800X-Eight-Core-json.xml
index aecc335c1e..9b9af2a6f7 100644
--- a/tests/cputestdata/x86_64-cpuid-Ryzen-7-1800X-Eight-Core-json.xml
+++ b/tests/cputestdata/x86_64-cpuid-Ryzen-7-1800X-Eight-Core-json.xml
@@ -8,6 +8,6 @@
<feature policy='require' name='cmp_legacy'/>
<feature policy='require' name='npt'/>
<feature policy='require' name='nrip-save'/>
- <feature policy='disable' name='monitor'/>
<feature policy='disable' name='sha-ni'/>
+ <feature policy='disable' name='monitor'/>
</cpu>
diff --git a/tests/domaincapsdata/qemu_2.11.0-tcg.x86_64.xml
b/tests/domaincapsdata/qemu_2.11.0-tcg.x86_64.xml
index 00056c7141..3a6971584e 100644
--- a/tests/domaincapsdata/qemu_2.11.0-tcg.x86_64.xml
+++ b/tests/domaincapsdata/qemu_2.11.0-tcg.x86_64.xml
@@ -34,6 +34,7 @@
<vendor>AMD</vendor>
<feature policy='require' name='acpi'/>
<feature policy='require' name='ss'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='hypervisor'/>
<feature policy='require' name='erms'/>
<feature policy='require' name='mpx'/>
diff --git a/tests/domaincapsdata/qemu_2.12.0-tcg.x86_64.xml
b/tests/domaincapsdata/qemu_2.12.0-tcg.x86_64.xml
index 4c867b18d0..049bc7e2e0 100644
--- a/tests/domaincapsdata/qemu_2.12.0-tcg.x86_64.xml
+++ b/tests/domaincapsdata/qemu_2.12.0-tcg.x86_64.xml
@@ -34,6 +34,7 @@
<vendor>AMD</vendor>
<feature policy='require' name='acpi'/>
<feature policy='require' name='ss'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='hypervisor'/>
<feature policy='require' name='erms'/>
<feature policy='require' name='mpx'/>
diff --git a/tests/domaincapsdata/qemu_3.0.0-tcg.x86_64.xml
b/tests/domaincapsdata/qemu_3.0.0-tcg.x86_64.xml
index 3a2fd5786a..1c650406c2 100644
--- a/tests/domaincapsdata/qemu_3.0.0-tcg.x86_64.xml
+++ b/tests/domaincapsdata/qemu_3.0.0-tcg.x86_64.xml
@@ -34,6 +34,7 @@
<vendor>AMD</vendor>
<feature policy='require' name='acpi'/>
<feature policy='require' name='ss'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='hypervisor'/>
<feature policy='require' name='erms'/>
<feature policy='require' name='mpx'/>
diff --git a/tests/domaincapsdata/qemu_3.1.0-tcg.x86_64.xml
b/tests/domaincapsdata/qemu_3.1.0-tcg.x86_64.xml
index e9e0cb9329..a7fde52d64 100644
--- a/tests/domaincapsdata/qemu_3.1.0-tcg.x86_64.xml
+++ b/tests/domaincapsdata/qemu_3.1.0-tcg.x86_64.xml
@@ -34,6 +34,7 @@
<vendor>AMD</vendor>
<feature policy='require' name='acpi'/>
<feature policy='require' name='ss'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='hypervisor'/>
<feature policy='require' name='erms'/>
<feature policy='require' name='mpx'/>
diff --git a/tests/domaincapsdata/qemu_4.0.0-tcg.x86_64.xml
b/tests/domaincapsdata/qemu_4.0.0-tcg.x86_64.xml
index 643cc4a631..cc19a61316 100644
--- a/tests/domaincapsdata/qemu_4.0.0-tcg.x86_64.xml
+++ b/tests/domaincapsdata/qemu_4.0.0-tcg.x86_64.xml
@@ -34,6 +34,7 @@
<vendor>AMD</vendor>
<feature policy='require' name='acpi'/>
<feature policy='require' name='ss'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='hypervisor'/>
<feature policy='require' name='erms'/>
<feature policy='require' name='mpx'/>
diff --git a/tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml
b/tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml
index bb1dacc14c..64bbef0a42 100644
--- a/tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml
+++ b/tests/domaincapsdata/qemu_4.1.0-tcg.x86_64.xml
@@ -34,6 +34,7 @@
<vendor>AMD</vendor>
<feature policy='require' name='acpi'/>
<feature policy='require' name='ss'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='hypervisor'/>
<feature policy='require' name='erms'/>
<feature policy='require' name='mpx'/>
diff --git a/tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml
b/tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml
index 622acc47a1..43d6d27e98 100644
--- a/tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml
+++ b/tests/domaincapsdata/qemu_4.2.0-tcg.x86_64.xml
@@ -34,6 +34,7 @@
<vendor>AMD</vendor>
<feature policy='require' name='acpi'/>
<feature policy='require' name='ss'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='hypervisor'/>
<feature policy='require' name='erms'/>
<feature policy='require' name='mpx'/>
diff --git a/tests/domaincapsdata/qemu_5.0.0-tcg.x86_64.xml
b/tests/domaincapsdata/qemu_5.0.0-tcg.x86_64.xml
index 40d691a62d..df562b5383 100644
--- a/tests/domaincapsdata/qemu_5.0.0-tcg.x86_64.xml
+++ b/tests/domaincapsdata/qemu_5.0.0-tcg.x86_64.xml
@@ -34,6 +34,7 @@
<vendor>AMD</vendor>
<feature policy='require' name='acpi'/>
<feature policy='require' name='ss'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='hypervisor'/>
<feature policy='require' name='erms'/>
<feature policy='require' name='mpx'/>
diff --git a/tests/domaincapsdata/qemu_5.1.0-tcg.x86_64.xml
b/tests/domaincapsdata/qemu_5.1.0-tcg.x86_64.xml
index 21db6a084a..0069c912dc 100644
--- a/tests/domaincapsdata/qemu_5.1.0-tcg.x86_64.xml
+++ b/tests/domaincapsdata/qemu_5.1.0-tcg.x86_64.xml
@@ -34,6 +34,7 @@
<vendor>AMD</vendor>
<feature policy='require' name='acpi'/>
<feature policy='require' name='ss'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='hypervisor'/>
<feature policy='require' name='erms'/>
<feature policy='require' name='mpx'/>
diff --git a/tests/domaincapsdata/qemu_5.2.0-tcg.x86_64.xml
b/tests/domaincapsdata/qemu_5.2.0-tcg.x86_64.xml
index ad10e7b8fd..1b9f986a95 100644
--- a/tests/domaincapsdata/qemu_5.2.0-tcg.x86_64.xml
+++ b/tests/domaincapsdata/qemu_5.2.0-tcg.x86_64.xml
@@ -34,6 +34,7 @@
<vendor>AMD</vendor>
<feature policy='require' name='acpi'/>
<feature policy='require' name='ss'/>
+ <feature policy='require' name='monitor'/>
<feature policy='require' name='hypervisor'/>
<feature policy='require' name='erms'/>
<feature policy='require' name='mpx'/>
diff --git a/tests/qemuxml2argvdata/cpu-host-model-cmt.x86_64-4.0.0.args
b/tests/qemuxml2argvdata/cpu-host-model-cmt.x86_64-4.0.0.args
index 6ee7bed18e..017e7a6c06 100644
--- a/tests/qemuxml2argvdata/cpu-host-model-cmt.x86_64-4.0.0.args
+++ b/tests/qemuxml2argvdata/cpu-host-model-cmt.x86_64-4.0.0.args
@@ -13,9 +13,9 @@ QEMU_AUDIO_DRV=none \
-object secret,id=masterKey0,format=raw,\
file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-machine pc-i440fx-4.0,accel=tcg,usb=off,dump-guest-core=off \
--cpu EPYC,acpi=on,ss=on,hypervisor=on,erms=on,mpx=on,pcommit=on,clwb=on,pku=on,\
-la57=on,3dnowext=on,3dnow=on,npt=on,vme=off,fma=off,avx=off,f16c=off,\
-rdrand=off,avx2=off,rdseed=off,sha-ni=off,xsavec=off,fxsr_opt=off,\
+-cpu EPYC,acpi=on,ss=on,monitor=on,hypervisor=on,erms=on,mpx=on,pcommit=on,\
+clwb=on,pku=on,la57=on,3dnowext=on,3dnow=on,npt=on,vme=off,fma=off,avx=off,\
+f16c=off,rdrand=off,avx2=off,rdseed=off,sha-ni=off,xsavec=off,fxsr_opt=off,\
misalignsse=off,3dnowprefetch=off,osvw=off,topoext=off,nrip-save=off \
-m 214 \
-overcommit mem-lock=off \
--
2.29.2
3:57 a.m.
On Fri, 2020-11-20 at 20:38 +0100, Jiri Denemark wrote:
Doing so would normally break migration to older libvirt, but most
of
the patches in this series prepare an infrastructure for removing
features from existing CPU models while maintaining backward
compatibility during migration.
See patch 7 for more details.
https://bugzilla.redhat.com/show_bug.cgi?id=1798004
Jiri Denemark (8):
conf: Rename virCPUDefUpdateFeatureInternal
conf: Use enum in virCPUDefAddFeatureInternal
conf: Add virCPUDefAddFeatureIfMissing
cpu: Run arch specific code for virCPUUpdate for all custom CPUs
cpu_x86: Change the flow in virCPUx86Update
cpu_x86: Add support for marking features as removed from a CPU
model
cpu_x86: Make sure removed features are always mentioned in CPU def
cpu_map: Drop 'monitor' from modern x86 CPU models
src/conf/cpu_conf.c | 50 +++++--
src/conf/cpu_conf.h | 5 +
src/cpu/cpu.c | 52 +++----
src/cpu/cpu.h | 3 +-
src/cpu/cpu_arm.c | 5 +-
src/cpu/cpu_ppc64.c | 3 +-
src/cpu/cpu_s390.c | 6 +-
src/cpu/cpu_x86.c | 127 ++++++++++++++
----
src/cpu_map/x86_Dhyana.xml | 2 +-
src/cpu_map/x86_EPYC-IBPB.xml | 2 +-
src/cpu_map/x86_EPYC.xml | 2 +-
src/cpu_map/x86_Opteron_G3.xml | 2 +-
src/libvirt_private.syms | 1 +
.../x86_64-cpuid-EPYC-7601-32-Core-guest.xml | 1 +
.../x86_64-cpuid-EPYC-7601-32-Core-host.xml | 1 +
..._64-cpuid-EPYC-7601-32-Core-ibpb-guest.xml | 1 +
...6_64-cpuid-EPYC-7601-32-Core-ibpb-host.xml | 1 +
...6_64-cpuid-EPYC-7601-32-Core-ibpb-json.xml | 2 +-
.../x86_64-cpuid-EPYC-7601-32-Core-json.xml | 2 +-
..._64-cpuid-Hygon-C86-7185-32-core-guest.xml | 1 +
...6_64-cpuid-Hygon-C86-7185-32-core-host.xml | 1 +
...6_64-cpuid-Hygon-C86-7185-32-core-json.xml | 2 +-
.../x86_64-cpuid-Opteron-1352-guest.xml | 1 +
.../x86_64-cpuid-Opteron-1352-host.xml | 1 +
.../x86_64-cpuid-Opteron-2350-guest.xml | 1 +
.../x86_64-cpuid-Opteron-2350-host.xml | 1 +
.../x86_64-cpuid-Opteron-2350-json.xml | 2 +-
.../x86_64-cpuid-Phenom-B95-guest.xml | 1 +
.../x86_64-cpuid-Phenom-B95-json.xml | 2 +-
...4-cpuid-Ryzen-7-1800X-Eight-Core-guest.xml | 1 +
...64-cpuid-Ryzen-7-1800X-Eight-Core-host.xml | 1 +
...64-cpuid-Ryzen-7-1800X-Eight-Core-json.xml | 2 +-
.../domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_3.0.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_3.1.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_4.0.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_4.1.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_4.2.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_5.0.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_5.2.0-tcg.x86_64.xml | 1 +
.../cpu-host-model-cmt.x86_64-4.0.0.args | 6 +-
43 files changed, 223 insertions(+), 78 deletions(-)
lgtm.
Do we want to have a schema for the cpu definitions in src/cpu_map/
somewhen, maybe as a "bite sized task"? I do understand it is kind of
superfluous, but if it *can* be tested automatically...
Reviewed-by: Tim Wiederhake <twiederh(a)redhat.com>
Cheers,
Tim
4:15 a.m.
On Fri, Nov 20, 2020 at 08:38:59PM +0100, Jiri Denemark wrote:
Doing so would normally break migration to older libvirt, but most
of
the patches in this series prepare an infrastructure for removing
features from existing CPU models while maintaining backward
compatibility during migration.
See patch 7 for more details.
QEMU introduced versioned CPU models to allow them to add/remove
arbitrary features within a named model. What's the timeframe for
supporting those in libvirt. IIUC, that should stop us needing to
introduce special workarounds like this series ?
https://bugzilla.redhat.com/show_bug.cgi?id=1798004
Jiri Denemark (8):
conf: Rename virCPUDefUpdateFeatureInternal
conf: Use enum in virCPUDefAddFeatureInternal
conf: Add virCPUDefAddFeatureIfMissing
cpu: Run arch specific code for virCPUUpdate for all custom CPUs
cpu_x86: Change the flow in virCPUx86Update
cpu_x86: Add support for marking features as removed from a CPU model
cpu_x86: Make sure removed features are always mentioned in CPU def
cpu_map: Drop 'monitor' from modern x86 CPU models
src/conf/cpu_conf.c | 50 +++++--
src/conf/cpu_conf.h | 5 +
src/cpu/cpu.c | 52 +++----
src/cpu/cpu.h | 3 +-
src/cpu/cpu_arm.c | 5 +-
src/cpu/cpu_ppc64.c | 3 +-
src/cpu/cpu_s390.c | 6 +-
src/cpu/cpu_x86.c | 127 ++++++++++++++----
src/cpu_map/x86_Dhyana.xml | 2 +-
src/cpu_map/x86_EPYC-IBPB.xml | 2 +-
src/cpu_map/x86_EPYC.xml | 2 +-
src/cpu_map/x86_Opteron_G3.xml | 2 +-
src/libvirt_private.syms | 1 +
.../x86_64-cpuid-EPYC-7601-32-Core-guest.xml | 1 +
.../x86_64-cpuid-EPYC-7601-32-Core-host.xml | 1 +
..._64-cpuid-EPYC-7601-32-Core-ibpb-guest.xml | 1 +
...6_64-cpuid-EPYC-7601-32-Core-ibpb-host.xml | 1 +
...6_64-cpuid-EPYC-7601-32-Core-ibpb-json.xml | 2 +-
.../x86_64-cpuid-EPYC-7601-32-Core-json.xml | 2 +-
..._64-cpuid-Hygon-C86-7185-32-core-guest.xml | 1 +
...6_64-cpuid-Hygon-C86-7185-32-core-host.xml | 1 +
...6_64-cpuid-Hygon-C86-7185-32-core-json.xml | 2 +-
.../x86_64-cpuid-Opteron-1352-guest.xml | 1 +
.../x86_64-cpuid-Opteron-1352-host.xml | 1 +
.../x86_64-cpuid-Opteron-2350-guest.xml | 1 +
.../x86_64-cpuid-Opteron-2350-host.xml | 1 +
.../x86_64-cpuid-Opteron-2350-json.xml | 2 +-
.../x86_64-cpuid-Phenom-B95-guest.xml | 1 +
.../x86_64-cpuid-Phenom-B95-json.xml | 2 +-
...4-cpuid-Ryzen-7-1800X-Eight-Core-guest.xml | 1 +
...64-cpuid-Ryzen-7-1800X-Eight-Core-host.xml | 1 +
...64-cpuid-Ryzen-7-1800X-Eight-Core-json.xml | 2 +-
.../domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_3.0.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_3.1.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_4.0.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_4.1.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_4.2.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_5.0.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_5.1.0-tcg.x86_64.xml | 1 +
.../domaincapsdata/qemu_5.2.0-tcg.x86_64.xml | 1 +
.../cpu-host-model-cmt.x86_64-4.0.0.args | 6 +-
43 files changed, 223 insertions(+), 78 deletions(-)
--
2.29.2
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 :|
7:08 a.m.
On Mon, Nov 23, 2020 at 10:15:11 +0000, Daniel P. Berrangé wrote:
On Fri, Nov 20, 2020 at 08:38:59PM +0100, Jiri Denemark wrote:
> Doing so would normally break migration to older libvirt, but most of
> the patches in this series prepare an infrastructure for removing
> features from existing CPU models while maintaining backward
> compatibility during migration.
>
> See patch 7 for more details.
QEMU introduced versioned CPU models to allow them to add/remove
arbitrary features within a named model. What's the timeframe for
supporting those in libvirt.
No idea, to be honest. QEMU currently does not provide all data we need
to start using it. I've been (occasionally, though) asking for QMP
extension for some time.
IIUC, that should stop us needing to introduce special workarounds
like this series ?
However, versioned CPU models would not save us in this case anyway as
QEMU did not introduce a new versions of models when removing "monitor".
This workaround is targeted at features which were included in CPU
models mostly accident and when they were not in fact enabled even
though the definitions of the CPU models contained them.
Jirka
1496
days inactive
1499
days old
11 comments
3 participants
participants (3)
-
Daniel P. Berrangé -
Jiri Denemark -
Tim Wiederhake