[libvirt PATCH 0/8] Drop 'monitor' from modern x86 CPU models
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
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@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
Replace the 'update' bool parameter with an enum so that we can have more than two possible values. Signed-off-by: Jiri Denemark <jdenemar@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
This new function adds a feature to a CPU definition only if it is not present there yet. Signed-off-by: Jiri Denemark <jdenemar@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
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@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
This is just a preparation for adding new functionality to virCPUx86Update. Signed-off-by: Jiri Denemark <jdenemar@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
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@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
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@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
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@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
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@redhat.com> Cheers, Tim
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 :|
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
participants (3)
-
Daniel P. Berrangé -
Jiri Denemark -
Tim Wiederhake