[PATCH v1 0/9] query & cache host-recommended CPU model

Notes: https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg02518.html - For information regarding the QEMU "static-recommended" (aka host-recommended) CPU model, please see the analogous QEMU patches: https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg02518.html - Patches include caps added to QEMU 8.1 test files. This is a stand-in for the interim and the appropriate caps files will be updated in the future pending QEMU acceptance. Description: Allow libvirt to query, cache, and report the "host-recommended" CPU model, which reports CPU features the hypervisor recommends older CPU models to disable in order to ensure migration to new-generation machines that will no longer support said features. The host-recommended CPU is a retrieved via a query to QEMU that is a combination of two parameters: the standard model name "host" and a new model type "static-recommended". The resulting CPU is similar to the host-model which now reports deprecated CPU features disabled (e.g. featurex=off). Once this model has been queried, the libvirt ABI will parse the feature on|off parameter and set the feature policy as appropriate. The resulting model is cached in the respective QEMU capabilities file. It can be reported via the virsh domcapabilities command Two "host models" are now represented: the original "host-model" that reports the hypervisor-supported features as-is, and the new "host-recommended" that reports the hypervisor-supported features with any deprecated features paired with the disabled policy. Additionally, hypervisor-cpu-baseline has been modified to output the feature policy and report disabled features. Baseline will also expand the resulting model with the host-recommended parameters if possible. The following s390x features are flagged as deprecated and will be dropped outright on future models: csske, te, cte, bpb. Examples: To enable this CPU model, define a guest with the following <cpu> element: <cpu mode='host-recommended' match='exact' check='partial'/> A guest will be generated with a CPU e.g.: <cpu mode='custom' match='exact' check='partial'> <model fallback='forbid'>gen16a-base</model> <feature policy='require' name='nnpa'/> <feature policy='require' name='aen'/> <feature policy='require' name='cmmnt'/> <feature policy='require' name='vxpdeh'/> <feature policy='require' name='aefsi'/> <feature policy='require' name='diag318'/> <feature policy='disable' name='csske'/> <feature policy='require' name='mepoch'/> <feature policy='require' name='msa9'/> <feature policy='require' name='msa8'/> <feature policy='require' name='msa7'/> <feature policy='require' name='msa6'/> <feature policy='require' name='msa5'/> <feature policy='require' name='msa4'/> <feature policy='require' name='msa3'/> <feature policy='require' name='msa2'/> <feature policy='require' name='msa1'/> <feature policy='require' name='sthyi'/> <feature policy='require' name='edat'/> <feature policy='require' name='ri'/> <feature policy='require' name='deflate'/> <feature policy='require' name='edat2'/> <feature policy='require' name='etoken'/> <feature policy='require' name='vx'/> <feature policy='require' name='ipter'/> <feature policy='require' name='pai'/> <feature policy='require' name='paie'/> <feature policy='require' name='mepochptff'/> <feature policy='require' name='ap'/> <feature policy='require' name='vxeh'/> <feature policy='require' name='vxpd'/> <feature policy='require' name='esop'/> <feature policy='require' name='msa9_pckmo'/> <feature policy='require' name='vxeh2'/> <feature policy='require' name='esort'/> <feature policy='require' name='apqi'/> <feature policy='require' name='apft'/> <feature policy='require' name='els'/> <feature policy='require' name='iep'/> <feature policy='require' name='apqci'/> <feature policy='disable' name='cte'/> <feature policy='require' name='ais'/> <feature policy='disable' name='bpb'/> <feature policy='require' name='gs'/> <feature policy='require' name='ppa15'/> <feature policy='require' name='zpci'/> <feature policy='require' name='rdp'/> <feature policy='require' name='sea_esop2'/> <feature policy='require' name='beareh'/> <feature policy='disable' name='te'/> <feature policy='require' name='cmm'/> <feature policy='require' name='vxpdeh2'/> </cpu> An example of the hypervisor-cpu-baseline command which will now invoke a CPU model expansion with the host-recommended parameters: # virsh hypervisor-cpu-baseline host-model.xml <cpu mode='custom' match='exact'> <model fallback='forbid'>z14.2-base</model> <feature policy='require' name='aen'/> <feature policy='require' name='cmmnt'/> <feature policy='require' name='aefsi'/> <feature policy='require' name='diag318'/> <feature policy='disable' name='csske'/> <feature policy='require' name='mepoch'/> <feature policy='require' name='msa8'/> <feature policy='require' name='msa7'/> <feature policy='require' name='msa6'/> <feature policy='require' name='msa5'/> <feature policy='require' name='msa4'/> <feature policy='require' name='msa3'/> <feature policy='require' name='msa2'/> <feature policy='require' name='msa1'/> <feature policy='require' name='sthyi'/> <feature policy='require' name='edat'/> <feature policy='require' name='ri'/> <feature policy='require' name='edat2'/> <feature policy='require' name='etoken'/> <feature policy='require' name='vx'/> <feature policy='require' name='ipter'/> <feature policy='require' name='mepochptff'/> <feature policy='require' name='ap'/> <feature policy='require' name='vxeh'/> <feature policy='require' name='vxpd'/> <feature policy='require' name='esop'/> <feature policy='require' name='apqi'/> <feature policy='require' name='apft'/> <feature policy='require' name='els'/> <feature policy='require' name='iep'/> <feature policy='require' name='apqci'/> <feature policy='disable' name='cte'/> <feature policy='require' name='ais'/> <feature policy='disable' name='bpb'/> <feature policy='require' name='gs'/> <feature policy='require' name='ppa15'/> <feature policy='require' name='zpci'/> <feature policy='require' name='sea_esop2'/> <feature policy='disable' name='te'/> <feature policy='require' name='cmm'/> </cpu> Collin Walling (9): qemu: monitor: enable query for static-recommended CPU model qemu: capabilities: add static-recommended capability qemu: capabilities: refactor virQEMUCapsLoadHostCPUModelInfo qemu: capabilities: query and cache host-recommended CPU model cpu: conf: add cpu mode for host-recommended domain: capabilities: report host-recommended CPU model qemu: process: allow guest to use host-recommended CPU model qemu_driver: report feature policy when baselining qemu_driver: expand baselined model to static_recommended for s390x src/conf/cpu_conf.c | 1 + src/conf/cpu_conf.h | 1 + src/conf/domain_capabilities.c | 16 ++ src/conf/domain_capabilities.h | 1 + src/conf/schemas/cputypes.rng | 1 + src/conf/schemas/domaincaps.rng | 24 +++ src/cpu/cpu.c | 2 + src/cpu/cpu_ppc64.c | 2 + src/qemu/qemu_capabilities.c | 144 +++++++++++++++--- src/qemu/qemu_capabilities.h | 4 + src/qemu/qemu_capspriv.h | 6 +- src/qemu/qemu_command.c | 5 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_driver.c | 18 ++- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 4 +- src/qemu/qemu_process.c | 9 +- src/qemu/qemu_validate.c | 8 + tests/cputest.c | 4 +- tests/domaincapsdata/qemu_8.1.0.s390x.xml | 55 +++++++ .../caps_8.1.0_s390x.replies | 80 +++++++++- .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 55 +++++++ ...c-cpu-kvm-ccw-virtio-8.1.s390x-latest.args | 32 ++++ .../s390-host-rec-cpu-kvm-ccw-virtio-8.1.xml | 17 +++ tests/qemuxml2argvtest.c | 2 + ...ec-cpu-kvm-ccw-virtio-8.1.s390x-latest.xml | 25 +++ tests/qemuxml2xmltest.c | 1 + 27 files changed, 486 insertions(+), 34 deletions(-) create mode 100644 tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.args create mode 100644 tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.xml create mode 100644 tests/qemuxml2xmloutdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.xml -- 2.41.0

Future S390 hardware generations may decide to drop CPU features completely, thus rendering guests running with such features incapable of migrating to newer machines. This results in a problem whereas the user must manually disable features ahead of time to prepare a guest for migration in a mixed environment. To circumvent this issue, a "host-recommended" CPU model has been introduced, which is the host-model with delta changes that disable features that have been flagged as deprecated. Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 6c590933aa..d3f63e0703 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1144,6 +1144,7 @@ struct _qemuMonitorCPUModelInfo { typedef enum { QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC, QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL, + QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_REC, QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL, } qemuMonitorCPUModelExpansionType; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5b9edadcf7..7b2aee7a42 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5062,7 +5062,9 @@ qemuMonitorJSONQueryCPUModelExpansionOne(qemuMonitor *mon, case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL: typeStr = "static"; break; - + case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_REC: + typeStr = "static-recommended"; + break; case QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL: typeStr = "full"; break; -- 2.41.0

Check for the QEMU capability to query for a static-recommended CPU model via CPU model expansion. Cache this capability for later. Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies | 6 +++++- tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 87412dd4ec..4ace8eea4a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -697,6 +697,7 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 450 */ "run-with.async-teardown", /* QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN */ + "query-cpu-model-expansion.static-recommended", /* QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_STATIC_RECOMMENDED */ ); @@ -1557,6 +1558,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "object-add/arg-type/+iothread/thread-pool-max", QEMU_CAPS_IOTHREAD_THREAD_POOL_MAX }, { "query-migrate/ret-type/blocked-reasons", QEMU_CAPS_MIGRATION_BLOCKED_REASONS }, { "screendump/arg-type/format/^png", QEMU_CAPS_SCREENSHOT_FORMAT_PNG }, + { "query-cpu-model-expansion/arg-type/type/^static-recommended", QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_STATIC_RECOMMENDED }, }; typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index e51d3fffdc..4fa64b6435 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -676,6 +676,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 450 */ QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN, /* asynchronous teardown -run-with async-teardown=on|off */ + QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_STATIC_RECOMMENDED, /* query-cpu-model-expansion supports type static-recommended*/ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies index 57ce64e88e..8cd7312bea 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies +++ b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies @@ -15398,12 +15398,16 @@ }, { "name": "full" + }, + { + "name": "static-recommended" } ], "meta-type": "enum", "values": [ "static", - "full" + "full", + "static-recommended" ] }, { diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml index 427ee9d5c7..0bb4233383 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml +++ b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml @@ -112,6 +112,7 @@ <flag name='rbd-encryption-layering'/> <flag name='rbd-encryption-luks-any'/> <flag name='run-with.async-teardown'/> + <flag name='query-cpu-model-expansion.static-recommended'/> <version>8000050</version> <microcodeVersion>39100245</microcodeVersion> <package>v8.0.0-1270-g1c12355b</package> -- 2.41.0

On Mon, Sep 11, 2023 at 17:07:09 -0400, Collin Walling wrote:
Check for the QEMU capability to query for a static-recommended CPU model via CPU model expansion. Cache this capability for later.
Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies | 6 +++++- tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + 4 files changed, 9 insertions(+), 1 deletion(-)
[...]
diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies index 57ce64e88e..8cd7312bea 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies +++ b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies @@ -15398,12 +15398,16 @@ }, { "name": "full" + }, + { + "name": "static-recommended" } ], "meta-type": "enum", "values": [ "static", - "full" + "full", + "static-recommended"
This is suspicious. We usually don't allow manual modification of the file because it can be overwritten by a subsequent update of the file. Could you please properly update the capability dump, by running tests/qemucapsprobe /path/to/qemu-system-s390x > /path/to/libvirt.git/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies and commiting that to a separate commit. Ideally do this with the released qemu-8.1, so that we have the most recent dump.

On 9/12/23 03:13, Peter Krempa wrote:
On Mon, Sep 11, 2023 at 17:07:09 -0400, Collin Walling wrote:
Check for the QEMU capability to query for a static-recommended CPU model via CPU model expansion. Cache this capability for later.
Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies | 6 +++++- tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + 4 files changed, 9 insertions(+), 1 deletion(-)
[...]
diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies index 57ce64e88e..8cd7312bea 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies +++ b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies @@ -15398,12 +15398,16 @@ }, { "name": "full" + }, + { + "name": "static-recommended" } ], "meta-type": "enum", "values": [ "static", - "full" + "full", + "static-recommended"
This is suspicious. We usually don't allow manual modification of the file because it can be overwritten by a subsequent update of the file.
Could you please properly update the capability dump, by running tests/qemucapsprobe /path/to/qemu-system-s390x > /path/to/libvirt.git/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies
and commiting that to a separate commit. Ideally do this with the released qemu-8.1, so that we have the most recent dump.
Understood. Thank you for the advice on how to generate the caps. I will keep this noted for the future on next version and any future contributions. -- Regards, Collin

This will be used to load the host recommended CPU model, introduced in a subsequent patch. Also move ctxt auto restore to end of variable declarations to avoid a compiler warning. Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/qemu/qemu_capabilities.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4ace8eea4a..40cdcffbfe 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3894,19 +3894,18 @@ virQEMUCapsSetCPUModelInfo(virQEMUCaps *qemuCaps, static int -virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, - xmlXPathContextPtr ctxt, - const char *typeStr) +virQEMUCapsLoadCPUModelInfo(qemuMonitorCPUModelInfo **modelInfo, + xmlXPathContextPtr ctxt, + char *xpath) { xmlNodePtr hostCPUNode; g_autofree xmlNodePtr *nodes = NULL; - VIR_XPATH_NODE_AUTORESTORE(ctxt) g_autoptr(qemuMonitorCPUModelInfo) hostCPU = NULL; - g_autofree char *xpath = g_strdup_printf("./hostCPU[@type='%s']", typeStr); size_t i; int n; virTristateBool migratability; int val; + VIR_XPATH_NODE_AUTORESTORE(ctxt) if (!(hostCPUNode = virXPathNode(xpath, ctxt))) { return 0; @@ -3991,11 +3990,22 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, } } - caps->hostCPU.info = g_steal_pointer(&hostCPU); + *modelInfo = g_steal_pointer(&hostCPU); return 0; } +static int +virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, + xmlXPathContextPtr ctxt, + const char *typeStr) +{ + g_autofree char *xpath = g_strdup_printf("./hostCPU[@type='%s']", typeStr); + + return virQEMUCapsLoadCPUModelInfo(&caps->hostCPU.info, ctxt, xpath); +} + + static int virQEMUCapsLoadCPUModels(virArch arch, virQEMUCapsAccel *caps, -- 2.41.0

Query the host-recommended CPU model from QEMU, if it is supported. This model will be stored in the QEMU capabilities file underneath the closing </hostCPU> tag, labeled <hostRecCPU>. An example follows, (shortened as to not overwhelm the commit message): <hostRecCPU type='kvm' model='gen16a-base' migratability='no'> <property name='aen' type='boolean' value='true'/> <property name='cmmnt' type='boolean' value='true'/> <property name='aefsi' type='boolean' value='true'/> <property name='diag318' type='boolean' value='true'/> <property name='csske' type='boolean' value='false'/> <property name='mepoch' type='boolean' value='true'/> <property name='msa8' type='boolean' value='true'/> ... <property name='te' type='boolean' value='false'/> <property name='cmm' type='boolean' value='true'/> </hostRecCPU> Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/conf/schemas/cputypes.rng | 1 + src/qemu/qemu_capabilities.c | 108 +++++++++++++++--- src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_capspriv.h | 6 +- tests/cputest.c | 4 +- .../caps_8.1.0_s390x.replies | 74 ++++++++++++ .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 54 +++++++++ 7 files changed, 231 insertions(+), 19 deletions(-) diff --git a/src/conf/schemas/cputypes.rng b/src/conf/schemas/cputypes.rng index db1aa57158..5e89d67c8e 100644 --- a/src/conf/schemas/cputypes.rng +++ b/src/conf/schemas/cputypes.rng @@ -10,6 +10,7 @@ <value>host-model</value> <value>host-passthrough</value> <value>maximum</value> + <value>host-recommended</value> </choice> </attribute> </define> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 40cdcffbfe..59403808ee 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -724,6 +724,11 @@ struct _virQEMUCapsHostCPUData { qemuMonitorCPUModelInfo *info; /* Physical address size of the host CPU or 0 if unknown or not applicable. */ unsigned int physAddrSize; + /* Host CPU model with delta changes reported by the hypervisor to ensure + * migration compatibility with newer machines that may exclude certain + * features available on older machines. + */ + qemuMonitorCPUModelInfo *hostRec; /* Host CPU definition reported in domain capabilities. */ virCPUDef *reported; /* Migratable host CPU definition used for updating guest CPU. */ @@ -732,6 +737,8 @@ struct _virQEMUCapsHostCPUData { * combined with features reported by QEMU. This is used for backward * compatible comparison between a guest CPU and a host CPU. */ virCPUDef *full; + /* CPU definition converted from hostRec model info */ + virCPUDef *recommended; }; typedef struct _virQEMUCapsAccel virQEMUCapsAccel; @@ -1834,6 +1841,12 @@ virQEMUCapsHostCPUDataCopy(virQEMUCapsHostCPUData *dst, if (src->full) dst->full = virCPUDefCopy(src->full); + + if (src->hostRec) + dst->hostRec = qemuMonitorCPUModelInfoCopy(src->hostRec); + + if (src->recommended) + dst->recommended = virCPUDefCopy(src->recommended); } @@ -1841,9 +1854,11 @@ static void virQEMUCapsHostCPUDataClear(virQEMUCapsHostCPUData *cpuData) { qemuMonitorCPUModelInfoFree(cpuData->info); + qemuMonitorCPUModelInfoFree(cpuData->hostRec); virCPUDefFree(cpuData->reported); virCPUDefFree(cpuData->migratable); virCPUDefFree(cpuData->full); + virCPUDefFree(cpuData->recommended); memset(cpuData, 0, sizeof(*cpuData)); } @@ -2190,6 +2205,9 @@ virQEMUCapsGetHostModel(virQEMUCaps *qemuCaps, /* 'full' is non-NULL only if we have data from both QEMU and * virCPUGetHost */ return cpuData->full ? cpuData->full : cpuData->reported; + + case VIR_QEMU_CAPS_HOST_CPU_RECOMMENDED: + return cpuData->recommended; } return NULL; @@ -2202,7 +2220,8 @@ virQEMUCapsSetHostModel(virQEMUCaps *qemuCaps, unsigned int physAddrSize, virCPUDef *reported, virCPUDef *migratable, - virCPUDef *full) + virCPUDef *full, + virCPUDef *recommended) { virQEMUCapsHostCPUData *cpuData; @@ -2211,6 +2230,7 @@ virQEMUCapsSetHostModel(virQEMUCaps *qemuCaps, cpuData->reported = reported; cpuData->migratable = migratable; cpuData->full = full; + cpuData->recommended = recommended; } @@ -3048,6 +3068,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCaps *qemuCaps, { const char *model = virQEMUCapsTypeIsAccelerated(virtType) ? "host" : "max"; g_autoptr(qemuMonitorCPUModelInfo) modelInfo = NULL; + g_autoptr(qemuMonitorCPUModelInfo) hostRecInfo = NULL; g_autoptr(qemuMonitorCPUModelInfo) nonMigratable = NULL; g_autoptr(virCPUDef) cpu = NULL; qemuMonitorCPUModelExpansionType type; @@ -3133,6 +3154,19 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCaps *qemuCaps, return -1; } + /* Retrieve the hypervisor recommended host CPU */ + if (virQEMUCapsTypeIsAccelerated(virtType) && + ARCH_IS_S390(qemuCaps->arch) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_STATIC_RECOMMENDED)) { + type = QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_REC; + + if (qemuMonitorGetCPUModelExpansion(mon, type, cpu, true, false, fail_no_props, + &hostRecInfo) < 0) + return -1; + + accel->hostCPU.hostRec = g_steal_pointer(&hostRecInfo); + } + accel->hostCPU.info = g_steal_pointer(&modelInfo); return 0; } @@ -3157,7 +3191,7 @@ virQEMUCapsGetCPUFeatures(virQEMUCaps *qemuCaps, size_t n; *features = NULL; - modelInfo = virQEMUCapsGetCPUModelInfo(qemuCaps, virtType); + modelInfo = virQEMUCapsGetCPUModelInfo(qemuCaps, virtType, false); if (!modelInfo) return 0; @@ -3725,14 +3759,19 @@ int virQEMUCapsInitCPUModel(virQEMUCaps *qemuCaps, virDomainVirtType type, virCPUDef *cpu, - bool migratable) + bool migratable, + bool recommended) { - qemuMonitorCPUModelInfo *modelInfo = virQEMUCapsGetCPUModelInfo(qemuCaps, type); + qemuMonitorCPUModelInfo *modelInfo = virQEMUCapsGetCPUModelInfo(qemuCaps, type, + recommended); int ret = 1; if (migratable && modelInfo && !modelInfo->migratability) return 1; + if (recommended && !modelInfo) + return 2; + if (ARCH_IS_S390(qemuCaps->arch)) { ret = virQEMUCapsInitCPUModelS390(qemuCaps, type, modelInfo, cpu, migratable); @@ -3775,6 +3814,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCaps *qemuCaps, virCPUDef *hostCPU = NULL; virCPUDef *fullCPU = NULL; unsigned int physAddrSize = 0; + virCPUDef *recCPU = NULL; size_t i; int rc; @@ -3784,7 +3824,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCaps *qemuCaps, if (!(cpu = virQEMUCapsNewHostCPUModel())) goto error; - if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, cpu, false)) < 0) { + if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, cpu, false, false)) < 0) { goto error; } else if (rc == 1) { g_autoptr(virDomainCapsCPUModels) cpuModels = NULL; @@ -3823,7 +3863,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCaps *qemuCaps, if (!(migCPU = virQEMUCapsNewHostCPUModel())) goto error; - if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, migCPU, true)) < 0) { + if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, migCPU, true, false)) < 0) { goto error; } else if (rc == 1) { VIR_DEBUG("CPU migratability not provided by QEMU"); @@ -3833,6 +3873,17 @@ virQEMUCapsInitHostCPUModel(virQEMUCaps *qemuCaps, goto error; } + if (!(recCPU = virQEMUCapsNewHostCPUModel())) + goto error; + + if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, recCPU, false, true)) < 0) { + goto error; + } else if (rc == 2) { + VIR_DEBUG("CPU reccomendation not provided by QEMU"); + virCPUDefFree(recCPU); + recCPU = NULL; + } + if (ARCH_IS_X86(qemuCaps->arch) && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_UNAVAILABLE_FEATURES)) { if (cpu && @@ -3851,7 +3902,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCaps *qemuCaps, if (virQEMUCapsTypeIsAccelerated(type)) virHostCPUGetPhysAddrSize(hostArch, &physAddrSize); - virQEMUCapsSetHostModel(qemuCaps, type, physAddrSize, cpu, migCPU, fullCPU); + virQEMUCapsSetHostModel(qemuCaps, type, physAddrSize, cpu, migCPU, fullCPU, recCPU); cleanup: virCPUDefFree(cpuExpanded); @@ -3862,6 +3913,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCaps *qemuCaps, virCPUDefFree(cpu); virCPUDefFree(migCPU); virCPUDefFree(fullCPU); + virCPUDefFree(recCPU); virResetLastError(); goto cleanup; } @@ -3878,8 +3930,12 @@ virQEMUCapsUpdateHostCPUModel(virQEMUCaps *qemuCaps, qemuMonitorCPUModelInfo * virQEMUCapsGetCPUModelInfo(virQEMUCaps *qemuCaps, - virDomainVirtType type) + virDomainVirtType type, + bool recommended) { + if (recommended) + return virQEMUCapsGetAccel(qemuCaps, type)->hostCPU.hostRec; + return virQEMUCapsGetAccel(qemuCaps, type)->hostCPU.info; } @@ -4006,6 +4062,17 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsAccel *caps, } +static int +virQEMUCapsLoadHostRecCPUModelInfo(virQEMUCapsAccel *caps, + xmlXPathContextPtr ctxt, + const char *typeStr) +{ + g_autofree char *xpath = g_strdup_printf("./hostRecCPU[@type='%s']", typeStr); + + return virQEMUCapsLoadCPUModelInfo(&caps->hostCPU.hostRec, ctxt, xpath); +} + + static int virQEMUCapsLoadCPUModels(virArch arch, virQEMUCapsAccel *caps, @@ -4169,6 +4236,10 @@ virQEMUCapsLoadAccel(virQEMUCaps *qemuCaps, if (virQEMUCapsLoadHostCPUModelInfo(caps, ctxt, typeStr) < 0) return -1; + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_STATIC_RECOMMENDED) && + virQEMUCapsLoadHostRecCPUModelInfo(caps, ctxt, typeStr) < 0) + return -1; + if (virQEMUCapsLoadCPUModels(qemuCaps->arch, caps, ctxt, typeStr) < 0) return -1; @@ -4705,18 +4776,24 @@ virQEMUCapsLoadCache(virArch hostArch, static void virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsAccel *caps, virBuffer *buf, - const char *typeStr) + const char *typeStr, + bool recommended) { - qemuMonitorCPUModelInfo *model = caps->hostCPU.info; + qemuMonitorCPUModelInfo *model; size_t i; + if (recommended) + model = caps->hostCPU.hostRec; + else + model = caps->hostCPU.info; + if (!model) return; virBufferAsprintf(buf, - "<hostCPU type='%s' model='%s' migratability='%s'>\n", - typeStr, model->name, - model->migratability ? "yes" : "no"); + "<%s type='%s' model='%s' migratability='%s'>\n", + recommended ? "hostRecCPU" : "hostCPU", typeStr, + model->name, model->migratability ? "yes" : "no"); virBufferAdjustIndent(buf, 2); for (i = 0; i < model->nprops; i++) { @@ -4752,7 +4829,7 @@ virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsAccel *caps, } virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</hostCPU>\n"); + virBufferAsprintf(buf, "</%s>\n", recommended ? "hostRecCPU" : "hostCPU"); } @@ -4846,7 +4923,8 @@ virQEMUCapsFormatAccel(virQEMUCaps *qemuCaps, virQEMUCapsAccel *caps = virQEMUCapsGetAccel(qemuCaps, type); const char *typeStr = virQEMUCapsAccelStr(type); - virQEMUCapsFormatHostCPUModelInfo(caps, buf, typeStr); + virQEMUCapsFormatHostCPUModelInfo(caps, buf, typeStr, false); + virQEMUCapsFormatHostCPUModelInfo(caps, buf, typeStr, true); virQEMUCapsFormatCPUModels(qemuCaps->arch, caps, buf, typeStr); virQEMUCapsFormatMachines(caps, buf, typeStr); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4fa64b6435..bb68b74328 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -726,6 +726,9 @@ typedef enum { * combined with features reported by QEMU. This is used for backward * compatible comparison between a guest CPU and a host CPU. */ VIR_QEMU_CAPS_HOST_CPU_FULL, + /* Host CPU definition with a modified feature set based on dropped + * features on newer CPU models. */ + VIR_QEMU_CAPS_HOST_CPU_RECOMMENDED, } virQEMUCapsHostCPUType; virCPUDef *virQEMUCapsGetHostModel(virQEMUCaps *qemuCaps, diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index 06b36d2eb8..a97710eecf 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -64,11 +64,13 @@ int virQEMUCapsInitCPUModel(virQEMUCaps *qemuCaps, virDomainVirtType type, virCPUDef *cpu, - bool migratable); + bool migratable, + bool recommended); qemuMonitorCPUModelInfo * virQEMUCapsGetCPUModelInfo(virQEMUCaps *qemuCaps, - virDomainVirtType type); + virDomainVirtType type, + bool recommended); void virQEMUCapsSetCPUModelInfo(virQEMUCaps *qemuCaps, diff --git a/tests/cputest.c b/tests/cputest.c index b3253e3116..8d3a0dd4d9 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -857,7 +857,7 @@ cpuTestJSONCPUID(const void *arg) cpu->match = VIR_CPU_MATCH_EXACT; cpu->fallback = VIR_CPU_FALLBACK_FORBID; - if (virQEMUCapsInitCPUModel(qemuCaps, VIR_DOMAIN_VIRT_KVM, cpu, false) != 0) + if (virQEMUCapsInitCPUModel(qemuCaps, VIR_DOMAIN_VIRT_KVM, cpu, false, false) != 0) return -1; return cpuTestCompareXML(data->arch, cpu, result); @@ -875,7 +875,7 @@ cpuTestJSONSignature(const void *arg) if (!(qemuCaps = cpuTestMakeQEMUCaps(data))) return -1; - modelInfo = virQEMUCapsGetCPUModelInfo(qemuCaps, VIR_DOMAIN_VIRT_KVM); + modelInfo = virQEMUCapsGetCPUModelInfo(qemuCaps, VIR_DOMAIN_VIRT_KVM, false); if (!(hostData = virQEMUCapsGetCPUModelX86Data(qemuCaps, modelInfo, false))) return -1; diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies index 8cd7312bea..2720408726 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies +++ b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.replies @@ -29649,6 +29649,80 @@ "id": "libvirt-37" } +{ + "execute": "query-cpu-model-expansion", + "arguments": { + "type": "static-recommended", + "model": { + "name": "host" + } + }, + "id": "libvirt-38" +} + +{ + "return": { + "model": { + "name": "gen16a-base", + "props": { + "nnpa": true, + "aen": true, + "cmmnt": true, + "vxpdeh": true, + "aefsi": true, + "diag318": true, + "csske": false, + "mepoch": true, + "msa9": true, + "msa8": true, + "msa7": true, + "msa6": true, + "msa5": true, + "msa4": true, + "msa3": true, + "msa2": true, + "msa1": true, + "sthyi": true, + "edat": true, + "ri": true, + "deflate": true, + "edat2": true, + "etoken": true, + "vx": true, + "ipter": true, + "pai": true, + "paie": true, + "mepochptff": true, + "ap": true, + "vxeh": true, + "vxpd": true, + "esop": true, + "msa9_pckmo": true, + "vxeh2": true, + "esort": true, + "apqi": true, + "apft": true, + "els": true, + "iep": true, + "apqci": true, + "cte": false, + "ais": true, + "bpb": false, + "gs": true, + "ppa15": true, + "zpci": true, + "rdp": true, + "sea_esop2": true, + "beareh": true, + "te": false, + "cmm": true, + "vxpdeh2": true + } + } + }, + "id": "libvirt-38" +} + { "execute": "qmp_capabilities", "id": "libvirt-1" diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml index 0bb4233383..c580cbb5b0 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml +++ b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml @@ -171,6 +171,60 @@ <property name='cmm' type='boolean' value='true'/> <property name='vxpdeh2' type='boolean' value='true'/> </hostCPU> + <hostRecCPU type='kvm' model='gen16a-base' migratability='no'> + <property name='nnpa' type='boolean' value='true'/> + <property name='aen' type='boolean' value='true'/> + <property name='cmmnt' type='boolean' value='true'/> + <property name='vxpdeh' type='boolean' value='true'/> + <property name='aefsi' type='boolean' value='true'/> + <property name='diag318' type='boolean' value='true'/> + <property name='csske' type='boolean' value='false'/> + <property name='mepoch' type='boolean' value='true'/> + <property name='msa9' type='boolean' value='true'/> + <property name='msa8' type='boolean' value='true'/> + <property name='msa7' type='boolean' value='true'/> + <property name='msa6' type='boolean' value='true'/> + <property name='msa5' type='boolean' value='true'/> + <property name='msa4' type='boolean' value='true'/> + <property name='msa3' type='boolean' value='true'/> + <property name='msa2' type='boolean' value='true'/> + <property name='msa1' type='boolean' value='true'/> + <property name='sthyi' type='boolean' value='true'/> + <property name='edat' type='boolean' value='true'/> + <property name='ri' type='boolean' value='true'/> + <property name='deflate' type='boolean' value='true'/> + <property name='edat2' type='boolean' value='true'/> + <property name='etoken' type='boolean' value='true'/> + <property name='vx' type='boolean' value='true'/> + <property name='ipter' type='boolean' value='true'/> + <property name='pai' type='boolean' value='true'/> + <property name='paie' type='boolean' value='true'/> + <property name='mepochptff' type='boolean' value='true'/> + <property name='ap' type='boolean' value='true'/> + <property name='vxeh' type='boolean' value='true'/> + <property name='vxpd' type='boolean' value='true'/> + <property name='esop' type='boolean' value='true'/> + <property name='msa9_pckmo' type='boolean' value='true'/> + <property name='vxeh2' type='boolean' value='true'/> + <property name='esort' type='boolean' value='true'/> + <property name='apqi' type='boolean' value='true'/> + <property name='apft' type='boolean' value='true'/> + <property name='els' type='boolean' value='true'/> + <property name='iep' type='boolean' value='true'/> + <property name='apqci' type='boolean' value='true'/> + <property name='cte' type='boolean' value='false'/> + <property name='ais' type='boolean' value='true'/> + <property name='bpb' type='boolean' value='false'/> + <property name='gs' type='boolean' value='true'/> + <property name='ppa15' type='boolean' value='true'/> + <property name='zpci' type='boolean' value='true'/> + <property name='rdp' type='boolean' value='true'/> + <property name='sea_esop2' type='boolean' value='true'/> + <property name='beareh' type='boolean' value='true'/> + <property name='te' type='boolean' value='false'/> + <property name='cmm' type='boolean' value='true'/> + <property name='vxpdeh2' type='boolean' value='true'/> + </hostRecCPU> <cpu type='kvm' name='gen16a-base' typename='gen16a-base-s390x-cpu' usable='yes'/> <cpu type='kvm' name='gen16a' typename='gen16a-s390x-cpu' usable='yes'/> <cpu type='kvm' name='z800-base' typename='z800-base-s390x-cpu' usable='yes'/> -- 2.41.0

Add VIR_CPU_MODE_HOST_RECOMMENDED for host-recommended CPU models and satisfy all switch cases. For the most part, host-recommended will follow similar paths as host-model, aside from touching any architecture specifics. Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/conf/cpu_conf.c | 1 + src/conf/cpu_conf.h | 1 + src/cpu/cpu.c | 1 + src/cpu/cpu_ppc64.c | 2 ++ src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 2 ++ src/qemu/qemu_validate.c | 1 + 8 files changed, 13 insertions(+) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 7abe489733..c0116808d8 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -43,6 +43,7 @@ VIR_ENUM_IMPL(virCPUMode, "host-model", "host-passthrough", "maximum", + "host-recommended", ); VIR_ENUM_IMPL(virCPUMatch, diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 3e4c53675c..f73d852e69 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -44,6 +44,7 @@ typedef enum { VIR_CPU_MODE_HOST_MODEL, VIR_CPU_MODE_HOST_PASSTHROUGH, VIR_CPU_MODE_MAXIMUM, + VIR_CPU_MODE_HOST_RECOMMENDED, VIR_CPU_MODE_LAST } virCPUMode; diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index bb5737e938..805aff1bf5 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -587,6 +587,7 @@ virCPUUpdate(virArch arch, return 0; case VIR_CPU_MODE_HOST_MODEL: + case VIR_CPU_MODE_HOST_RECOMMENDED: relative = true; break; diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index e13cdbdf6b..dc42e869e7 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -483,6 +483,8 @@ ppc64Compute(virCPUDef *host, * look up guest CPU information */ guest_model = ppc64ModelFromCPU(cpu, map); break; + case VIR_CPU_MODE_HOST_RECOMMENDED: + break; } } else { /* Other host CPU information */ diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 59403808ee..d7096a08c2 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2340,6 +2340,10 @@ virQEMUCapsIsCPUModeSupported(virQEMUCaps *qemuCaps, case VIR_CPU_MODE_MAXIMUM: return virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_MAX); + case VIR_CPU_MODE_HOST_RECOMMENDED: + return !!virQEMUCapsGetHostModel(qemuCaps, type, + VIR_QEMU_CAPS_HOST_CPU_RECOMMENDED); + case VIR_CPU_MODE_LAST: break; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a912ed064f..a45e2fbb95 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6293,6 +6293,7 @@ qemuBuildCpuModelArgStr(virQEMUDriver *driver, virBufferAdd(buf, cpu->model, -1); break; + case VIR_CPU_MODE_HOST_RECOMMENDED: case VIR_CPU_MODE_LAST: break; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c7d64e1b5c..f7493431ac 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4753,6 +4753,7 @@ qemuDomainDefCPUPostParse(virDomainDef *def, break; case VIR_CPU_MODE_HOST_MODEL: + case VIR_CPU_MODE_HOST_RECOMMENDED: def->cpu->check = VIR_CPU_CHECK_PARTIAL; break; @@ -6995,6 +6996,7 @@ qemuDomainObjCheckCPUTaint(virQEMUDriver *driver, } break; case VIR_CPU_MODE_HOST_MODEL: + case VIR_CPU_MODE_HOST_RECOMMENDED: case VIR_CPU_MODE_LAST: default: break; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 1346bbfb44..387c2f1fa8 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -372,6 +372,7 @@ qemuValidateDomainDefCpu(virQEMUDriver *driver, } break; + case VIR_CPU_MODE_HOST_RECOMMENDED: case VIR_CPU_MODE_CUSTOM: case VIR_CPU_MODE_LAST: break; -- 2.41.0

Report the host-recommended CPU definition in the domaincapabilities. Currently, only s390x supports this model, but the formatting remains open if other archs decide to support this as well. Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/conf/domain_capabilities.c | 16 +++++++ src/conf/domain_capabilities.h | 1 + src/conf/schemas/domaincaps.rng | 24 ++++++++++ src/qemu/qemu_capabilities.c | 8 ++++ tests/domaincapsdata/qemu_8.1.0.s390x.xml | 55 +++++++++++++++++++++++ 5 files changed, 104 insertions(+) diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 2fa5756184..4b9019f252 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -450,6 +450,22 @@ virDomainCapsCPUFormat(virBuffer *buf, virBufferAddLit(buf, "supported='no'/>\n"); } + if (cpu->hostRecommended) { + virBufferAsprintf(buf, "<mode name='%s' ", + virCPUModeTypeToString(VIR_CPU_MODE_HOST_RECOMMENDED)); + if (cpu->hostRecommended) { + virBufferAddLit(buf, "supported='yes'>\n"); + virBufferAdjustIndent(buf, 2); + + virCPUDefFormatBuf(buf, cpu->hostRecommended); + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</mode>\n"); + } else { + virBufferAddLit(buf, "supported='no'/>\n"); + } + } + virBufferAsprintf(buf, "<mode name='%s' ", virCPUModeTypeToString(VIR_CPU_MODE_CUSTOM)); if (cpu->custom && cpu->custom->nmodels) { diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index 01bcfa2e39..893101ee97 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -201,6 +201,7 @@ struct _virDomainCapsCPU { bool maximum; virDomainCapsEnum maximumMigratable; virCPUDef *hostModel; + virCPUDef *hostRecommended; virDomainCapsCPUModels *custom; }; diff --git a/src/conf/schemas/domaincaps.rng b/src/conf/schemas/domaincaps.rng index 99ef148d44..91657555dd 100644 --- a/src/conf/schemas/domaincaps.rng +++ b/src/conf/schemas/domaincaps.rng @@ -93,6 +93,9 @@ <ref name="cpuHost"/> <ref name="cpuMax"/> <ref name="cpuHostModel"/> + <optional> + <ref name="cpuHostRecModel"/> + </optional> <ref name="cpuCustom"/> </element> </define> @@ -142,6 +145,27 @@ </element> </define> + <define name="cpuHostRecModel"> + <element name="mode"> + <attribute name="name"> + <value>host-recommended</value> + </attribute> + <ref name="supported"/> + <optional> + <ref name="cpuModel"/> + <optional> + <ref name="cpuVendor"/> + </optional> + <optional> + <ref name="cpuMaxPhysAddr"/> + </optional> + <zeroOrMore> + <ref name="cpuFeature"/> + </zeroOrMore> + </optional> + </element> + </define> + <define name="cpuCustom"> <element name="mode"> <attribute name="name"> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d7096a08c2..6ac8aaf248 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -6250,6 +6250,14 @@ virQEMUCapsFillDomainCPUCaps(virQEMUCaps *qemuCaps, domCaps->cpu.custom = NULL; } } + + if (virQEMUCapsIsCPUModeSupported(qemuCaps, hostarch, domCaps->virttype, + VIR_CPU_MODE_HOST_RECOMMENDED, + domCaps->machine)) { + virCPUDef *cpu = virQEMUCapsGetHostModel(qemuCaps, domCaps->virttype, + VIR_QEMU_CAPS_HOST_CPU_RECOMMENDED); + domCaps->cpu.hostRecommended = virCPUDefCopy(cpu); + } } diff --git a/tests/domaincapsdata/qemu_8.1.0.s390x.xml b/tests/domaincapsdata/qemu_8.1.0.s390x.xml index 37c7c3b8bf..68a456645c 100644 --- a/tests/domaincapsdata/qemu_8.1.0.s390x.xml +++ b/tests/domaincapsdata/qemu_8.1.0.s390x.xml @@ -90,6 +90,61 @@ <feature policy='require' name='cmm'/> <feature policy='require' name='vxpdeh2'/> </mode> + <mode name='host-recommended' supported='yes'> + <model fallback='forbid'>gen16a-base</model> + <feature policy='require' name='nnpa'/> + <feature policy='require' name='aen'/> + <feature policy='require' name='cmmnt'/> + <feature policy='require' name='vxpdeh'/> + <feature policy='require' name='aefsi'/> + <feature policy='require' name='diag318'/> + <feature policy='disable' name='csske'/> + <feature policy='require' name='mepoch'/> + <feature policy='require' name='msa9'/> + <feature policy='require' name='msa8'/> + <feature policy='require' name='msa7'/> + <feature policy='require' name='msa6'/> + <feature policy='require' name='msa5'/> + <feature policy='require' name='msa4'/> + <feature policy='require' name='msa3'/> + <feature policy='require' name='msa2'/> + <feature policy='require' name='msa1'/> + <feature policy='require' name='sthyi'/> + <feature policy='require' name='edat'/> + <feature policy='require' name='ri'/> + <feature policy='require' name='deflate'/> + <feature policy='require' name='edat2'/> + <feature policy='require' name='etoken'/> + <feature policy='require' name='vx'/> + <feature policy='require' name='ipter'/> + <feature policy='require' name='pai'/> + <feature policy='require' name='paie'/> + <feature policy='require' name='mepochptff'/> + <feature policy='require' name='ap'/> + <feature policy='require' name='vxeh'/> + <feature policy='require' name='vxpd'/> + <feature policy='require' name='esop'/> + <feature policy='require' name='msa9_pckmo'/> + <feature policy='require' name='vxeh2'/> + <feature policy='require' name='esort'/> + <feature policy='require' name='apqi'/> + <feature policy='require' name='apft'/> + <feature policy='require' name='els'/> + <feature policy='require' name='iep'/> + <feature policy='require' name='apqci'/> + <feature policy='disable' name='cte'/> + <feature policy='require' name='ais'/> + <feature policy='disable' name='bpb'/> + <feature policy='require' name='gs'/> + <feature policy='require' name='ppa15'/> + <feature policy='require' name='zpci'/> + <feature policy='require' name='rdp'/> + <feature policy='require' name='sea_esop2'/> + <feature policy='require' name='beareh'/> + <feature policy='disable' name='te'/> + <feature policy='require' name='cmm'/> + <feature policy='require' name='vxpdeh2'/> + </mode> <mode name='custom' supported='yes'> <model usable='yes' vendor='IBM'>gen16a-base</model> <model usable='yes' vendor='IBM'>gen16a</model> -- 2.41.0

A guest may enable the host-recommended CPU model via the following domain XML: <cpu mode='host-recommended' check='partial'/> Once the guest is running, the deprecated features will nest under the <cpu> tag paired with the "disable" policy, e.g.: <cpu mode='custom' match='exact' check='partial'> <model fallback='forbid'>gen16a-base</model> <feature policy='require' name='aen'/> <feature policy='require' name='cmmnt'/> <feature policy='require' name='aefsi'/> <feature policy='require' name='diag318'/> <feature policy='disable' name='csske'/> <feature policy='require' name='mepoch'/> <feature policy='require' name='msa8'/> ... <feature policy='disable' name='te'/> <feature policy='require' name='cmm'/> </cpu> Though existing guests must restart for the CPU model to update with these changes, this at least removes a requirement of the user to manually stay up-to-date on which features are recommended to be disabled as future hardware updates come into play -- so long as the hypervisor maintains the most up-to-date CPU model definitions. Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/cpu/cpu.c | 1 + src/qemu/qemu_command.c | 4 +++ src/qemu/qemu_process.c | 9 ++++-- src/qemu/qemu_validate.c | 7 ++++ ...c-cpu-kvm-ccw-virtio-8.1.s390x-latest.args | 32 +++++++++++++++++++ .../s390-host-rec-cpu-kvm-ccw-virtio-8.1.xml | 17 ++++++++++ tests/qemuxml2argvtest.c | 2 ++ ...ec-cpu-kvm-ccw-virtio-8.1.s390x-latest.xml | 25 +++++++++++++++ tests/qemuxml2xmltest.c | 1 + 9 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.args create mode 100644 tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.xml create mode 100644 tests/qemuxml2xmloutdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.xml diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 805aff1bf5..4ac34a5efb 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -981,6 +981,7 @@ virCPUTranslate(virArch arch, if (cpu->mode == VIR_CPU_MODE_HOST_MODEL || cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH || + cpu->mode == VIR_CPU_MODE_HOST_RECOMMENDED || cpu->mode == VIR_CPU_MODE_MAXIMUM) return 0; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a45e2fbb95..ace9deb246 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6294,6 +6294,10 @@ qemuBuildCpuModelArgStr(virQEMUDriver *driver, break; case VIR_CPU_MODE_HOST_RECOMMENDED: + if (ARCH_IS_S390(def->os.arch)) + virBufferAddLit(buf, "host-recommended"); + break; + case VIR_CPU_MODE_LAST: break; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7a1cdb0302..4ff8c1c87a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6303,6 +6303,7 @@ qemuProcessUpdateGuestCPU(virDomainDef *def, if (def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH && def->cpu->mode != VIR_CPU_MODE_MAXIMUM) { g_autoptr(virDomainCapsCPUModels) cpuModels = NULL; + virQEMUCapsHostCPUType cpuType; if (def->cpu->check == VIR_CPU_CHECK_PARTIAL && virCPUCompare(hostarch, @@ -6311,9 +6312,13 @@ qemuProcessUpdateGuestCPU(virDomainDef *def, def->cpu, true) < 0) return -1; + if (def->cpu->mode == VIR_CPU_MODE_HOST_RECOMMENDED) + cpuType = VIR_QEMU_CAPS_HOST_CPU_RECOMMENDED; + else + cpuType = VIR_QEMU_CAPS_HOST_CPU_MIGRATABLE; + if (virCPUUpdate(def->os.arch, def->cpu, - virQEMUCapsGetHostModel(qemuCaps, def->virtType, - VIR_QEMU_CAPS_HOST_CPU_MIGRATABLE)) < 0) + virQEMUCapsGetHostModel(qemuCaps, def->virtType, cpuType)) < 0) return -1; cpuModels = virQEMUCapsGetCPUModels(qemuCaps, def->virtType, NULL, NULL); diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 387c2f1fa8..a60445f686 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -373,6 +373,13 @@ qemuValidateDomainDefCpu(virQEMUDriver *driver, break; case VIR_CPU_MODE_HOST_RECOMMENDED: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION_STATIC_RECOMMENDED)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("host-recommended CPU is not supported by QEMU binary")); + return -1; + } + break; + case VIR_CPU_MODE_CUSTOM: case VIR_CPU_MODE_LAST: break; diff --git a/tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.args b/tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.args new file mode 100644 index 0000000000..78546380d7 --- /dev/null +++ b/tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.args @@ -0,0 +1,32 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-test \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-test/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-test/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-test/.config \ +/usr/bin/qemu-system-s390x \ +-name guest=test,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-test/master-key.aes"}' \ +-machine s390-ccw-virtio-8.1,usb=off,dump-guest-core=off,memory-backend=s390.ram \ +-accel kvm \ +-cpu gen16a-base,nnpa=on,aen=on,cmmnt=on,vxpdeh=on,aefsi=on,diag318=on,csske=off,mepoch=on,msa9=on,msa8=on,msa7=on,msa6=on,msa5=on,msa4=on,msa3=on,msa2=on,msa1=on,sthyi=on,edat=on,ri=on,deflate=on,edat2=on,etoken=on,vx=on,ipter=on,pai=on,paie=on,mepochptff=on,ap=on,vxeh=on,vxpd=on,esop=on,msa9_pckmo=on,vxeh2=on,esort=on,apqi=on,apft=on,els=on,iep=on,apqci=on,cte=off,ais=on,bpb=off,gs=on,ppa15=on,zpci=on,rdp=on,sea_esop2=on,beareh=on,te=off,cmm=on,vxpdeh2=on \ +-m size=262144k \ +-object '{"qom-type":"memory-backend-ram","id":"s390.ram","size":268435456}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 9aa4b45c-b9dd-45ef-91fe-862b27b4231f \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"virtio-balloon-ccw","id":"balloon0","devno":"fe.0.0000"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.xml b/tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.xml new file mode 100644 index 0000000000..cacc1933dd --- /dev/null +++ b/tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.xml @@ -0,0 +1,17 @@ +<domain type='kvm'> + <name>test</name> + <uuid>9aa4b45c-b9dd-45ef-91fe-862b27b4231f</uuid> + <memory>262144</memory> + <currentMemory>262144</currentMemory> + <os> + <type arch='s390x' machine='s390-ccw-virtio-8.1'>hvm</type> + </os> + <cpu mode='host-recommended'/> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2d06e2a0d1..4c3b698b83 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2295,6 +2295,8 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("s390-async-teardown-disabled", "s390x"); DO_TEST_CAPS_ARCH_VER("s390-async-teardown-disabled", "s390x", "6.0.0"); + DO_TEST_CAPS_ARCH_LATEST("s390-host-rec-cpu-kvm-ccw-virtio-8.1", "s390x"); + qemuTestDriverFree(&driver); virFileWrapperClearPrefixes(); diff --git a/tests/qemuxml2xmloutdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.xml b/tests/qemuxml2xmloutdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.xml new file mode 100644 index 0000000000..177694afe8 --- /dev/null +++ b/tests/qemuxml2xmloutdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.xml @@ -0,0 +1,25 @@ +<domain type='kvm'> + <name>test</name> + <uuid>9aa4b45c-b9dd-45ef-91fe-862b27b4231f</uuid> + <memory unit='KiB'>262144</memory> + <currentMemory unit='KiB'>262144</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio-8.1'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='host-recommended' check='partial'/> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <controller type='pci' index='0' model='pci-root'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> + </memballoon> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 72f976358f..1038fa8f07 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -941,6 +941,7 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("s390-default-cpu-tcg-ccw-virtio-2.7", "s390x"); DO_TEST_CAPS_ARCH_LATEST("s390-default-cpu-kvm-ccw-virtio-4.2", "s390x"); DO_TEST_CAPS_ARCH_LATEST("s390-default-cpu-tcg-ccw-virtio-4.2", "s390x"); + DO_TEST_CAPS_ARCH_LATEST("s390-host-rec-cpu-kvm-ccw-virtio-8.1", "s390x"); DO_TEST_CAPS_ARCH_LATEST("x86_64-default-cpu-kvm-pc-4.2", "x86_64"); DO_TEST_CAPS_ARCH_LATEST("x86_64-default-cpu-tcg-pc-4.2", "x86_64"); DO_TEST_CAPS_ARCH_LATEST("x86_64-default-cpu-kvm-q35-4.2", "x86_64"); -- 2.41.0

Some baselined models may require that certain features are disabled to ensure compatability with all computed models. When converting the model info to a CPU definition, check the model info feature value for false and set the CPU def policy to disabled. Additionally, set the CPU def type to VIR_CPU_TYPE_GUEST so that the XML formatter will consider the feature policies. Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/qemu/qemu_driver.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0d4da937b0..1c61685489 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11659,15 +11659,19 @@ qemuConnectStealCPUModelFromInfo(virCPUDef *dst, info = g_steal_pointer(src); dst->model = g_steal_pointer(&info->name); + dst->type = VIR_CPU_TYPE_GUEST; for (i = 0; i < info->nprops; i++) { char *name = info->props[i].name; + virCPUFeaturePolicy policy; - if (info->props[i].type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN || - !info->props[i].value.boolean) + if (info->props[i].type != QEMU_MONITOR_CPU_PROPERTY_BOOLEAN) continue; - if (virCPUDefAddFeature(dst, name, VIR_CPU_FEATURE_REQUIRE) < 0) + policy = info->props[i].value.boolean ? VIR_CPU_FEATURE_REQUIRE + : VIR_CPU_FEATURE_DISABLE; + + if (virCPUDefAddFeature(dst, name, policy) < 0) return -1; } -- 2.41.0

In order for the CPU model expansion command to consider any deprecated CPU features, s390x will need to use the special static recommended expansion type after baselining. Signed-off-by: Collin Walling <walling@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> --- src/qemu/qemu_driver.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1c61685489..c4428c066f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11687,7 +11687,8 @@ qemuConnectCPUModelBaseline(virQEMUCaps *qemuCaps, bool expand_features, virCPUDef **cpus, int ncpus, - virDomainCapsCPUModels *cpuModels) + virDomainCapsCPUModels *cpuModels, + virArch arch) { g_autoptr(qemuProcessQMP) proc = NULL; g_autoptr(virCPUDef) baseline = NULL; @@ -11739,6 +11740,9 @@ qemuConnectCPUModelBaseline(virQEMUCaps *qemuCaps, expansion_type = expand_features ? QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL : QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC; + if (ARCH_IS_S390(arch) && expansion_type == QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC) + expansion_type = QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_REC; + if (qemuMonitorGetCPUModelExpansion(proc->mon, expansion_type, baseline, true, false, false, &result) < 0) @@ -11827,7 +11831,7 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, if (!(cpu = qemuConnectCPUModelBaseline(qemuCaps, cfg->libDir, cfg->user, cfg->group, expand_features, cpus, ncpus, - cpuModels))) + cpuModels, arch))) goto cleanup; } else { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, -- 2.41.0

On Mon, Sep 11, 2023 at 17:07:07 -0400, Collin Walling wrote:
Notes: https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg02518.html - For information regarding the QEMU "static-recommended" (aka host-recommended) CPU model, please see the analogous QEMU patches:
https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg02518.html
- Patches include caps added to QEMU 8.1 test files. This is a stand-in for the interim and the appropriate caps files will be updated in the future pending QEMU acceptance.
I went looking for the feature in qemu and couldn't find it, but you explain it here. Note that it's acceptable only as a RFC to gather feedback, but must not be comitted in this form. The capability dumps must represent real qemu capabilities, and thus hacks where you "backport" something are not acceptable. Once the qemu patches get commited upstream, it's okay if you create a capability dump from the in-progress development cycle. We do that also for x86_64.

On 9/12/23 03:58, Peter Krempa wrote:
On Mon, Sep 11, 2023 at 17:07:07 -0400, Collin Walling wrote:
Notes: https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg02518.html - For information regarding the QEMU "static-recommended" (aka host-recommended) CPU model, please see the analogous QEMU patches:
https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg02518.html
- Patches include caps added to QEMU 8.1 test files. This is a stand-in for the interim and the appropriate caps files will be updated in the future pending QEMU acceptance.
I went looking for the feature in qemu and couldn't find it, but you explain it here.
Note that it's acceptable only as a RFC to gather feedback, but must not be comitted in this form. The capability dumps must represent real qemu capabilities, and thus hacks where you "backport" something are not acceptable.
Once the qemu patches get commited upstream, it's okay if you create a capability dump from the in-progress development cycle. We do that also for x86_64.
Absolutely makes sense. I agree, should have posted as an RFC. Will keep this noted for the future. -- Regards, Collin
participants (2)
-
Collin Walling
-
Peter Krempa