Querying "host" CPU model expansion only makes sense for KVM. QEMU 2.9.0
introduces a new "max" CPU model which can be used to ask QEMU what the
best CPU it can provide to a TCG domain is.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
Notes:
Version 2:
- no change
src/qemu/qemu_capabilities.c | 151 ++++++++++++-----
src/qemu/qemu_capabilities.h | 3 +-
src/qemu/qemu_capspriv.h | 3 +-
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_process.c | 5 +-
.../qemucapabilitiesdata/caps_2.8.0.s390x.replies | 8 +
tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 2 +-
.../qemucapabilitiesdata/caps_2.9.0.x86_64.replies | 179 +++++++++++++++++++++
tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 172 +++++++++++++++++++-
tests/qemuxml2argvtest.c | 3 +-
10 files changed, 480 insertions(+), 48 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 466852d13..2ba82456e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -400,13 +400,15 @@ struct _virQEMUCaps {
size_t ngicCapabilities;
virGICCapability *gicCapabilities;
- qemuMonitorCPUModelInfoPtr hostCPUModelInfo;
+ qemuMonitorCPUModelInfoPtr kvmCPUModelInfo;
+ qemuMonitorCPUModelInfoPtr tcgCPUModelInfo;
/* Anything below is not stored in the cache since the values are
* re-computed from the other fields or external data sources every
* time we probe QEMU or load the results from the cache.
*/
- virCPUDefPtr hostCPUModel;
+ virCPUDefPtr kvmCPUModel;
+ virCPUDefPtr tcgCPUModel;
};
struct virQEMUCapsSearchData {
@@ -2163,12 +2165,20 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
goto error;
}
- if (qemuCaps->hostCPUModel &&
- !(ret->hostCPUModel = virCPUDefCopy(qemuCaps->hostCPUModel)))
+ if (qemuCaps->kvmCPUModel &&
+ !(ret->kvmCPUModel = virCPUDefCopy(qemuCaps->kvmCPUModel)))
goto error;
- if (qemuCaps->hostCPUModelInfo &&
- !(ret->hostCPUModelInfo = qemuMonitorCPUModelInfoCopy(qemuCaps->hostCPUModelInfo)))
+ if (qemuCaps->tcgCPUModel &&
+ !(ret->tcgCPUModel = virCPUDefCopy(qemuCaps->tcgCPUModel)))
+ goto error;
+
+ if (qemuCaps->kvmCPUModelInfo &&
+ !(ret->kvmCPUModelInfo = qemuMonitorCPUModelInfoCopy(qemuCaps->kvmCPUModelInfo)))
+ goto error;
+
+ if (qemuCaps->tcgCPUModelInfo &&
+ !(ret->tcgCPUModelInfo = qemuMonitorCPUModelInfoCopy(qemuCaps->tcgCPUModelInfo)))
goto error;
if (VIR_ALLOC_N(ret->machineTypes, qemuCaps->nmachineTypes) < 0)
@@ -2217,8 +2227,10 @@ void virQEMUCapsDispose(void *obj)
VIR_FREE(qemuCaps->gicCapabilities);
- qemuMonitorCPUModelInfoFree(qemuCaps->hostCPUModelInfo);
- virCPUDefFree(qemuCaps->hostCPUModel);
+ qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo);
+ qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo);
+ virCPUDefFree(qemuCaps->kvmCPUModel);
+ virCPUDefFree(qemuCaps->tcgCPUModel);
}
void
@@ -2435,9 +2447,13 @@ virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
virCPUDefPtr
-virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps)
+virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
+ virDomainVirtType type)
{
- return qemuCaps->hostCPUModel;
+ if (type == VIR_DOMAIN_VIRT_KVM)
+ return qemuCaps->kvmCPUModel;
+ else
+ return qemuCaps->tcgCPUModel;
}
@@ -2455,7 +2471,7 @@ virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps,
virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch);
case VIR_CPU_MODE_HOST_MODEL:
- return !!qemuCaps->hostCPUModel;
+ return !!virQEMUCapsGetHostModel(qemuCaps, type);
case VIR_CPU_MODE_CUSTOM:
if (type == VIR_DOMAIN_VIRT_KVM)
@@ -2822,14 +2838,24 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps,
static int
virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
- qemuMonitorPtr mon)
+ qemuMonitorPtr mon,
+ bool tcg)
{
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION) ||
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
+ qemuMonitorCPUModelInfoPtr *modelInfo;
+ const char *model;
+
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION))
return 0;
- return qemuMonitorGetCPUModelExpansion(mon, "static", "host",
- &qemuCaps->hostCPUModelInfo);
+ if (tcg || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
+ modelInfo = &qemuCaps->tcgCPUModelInfo;
+ model = "max";
+ } else {
+ modelInfo = &qemuCaps->kvmCPUModelInfo;
+ model = "host";
+ }
+
+ return qemuMonitorGetCPUModelExpansion(mon, "static", model, modelInfo);
}
struct tpmTypeToCaps {
@@ -3053,12 +3079,16 @@ virQEMUCapsCPUFilterFeatures(const char *name,
*/
static int
virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
+ virDomainVirtType type,
virCPUDefPtr cpu)
{
- qemuMonitorCPUModelInfoPtr modelInfo = qemuCaps->hostCPUModelInfo;
+ qemuMonitorCPUModelInfoPtr modelInfo;
size_t i;
- if (!modelInfo) {
+ if (type != VIR_DOMAIN_VIRT_KVM)
+ return -1;
+
+ if (!(modelInfo = qemuCaps->kvmCPUModelInfo)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("missing host CPU model info from QEMU capabilities "
"for binary %s"),
@@ -3098,12 +3128,13 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
*/
static int
virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
+ virDomainVirtType type,
virCPUDefPtr cpu)
{
int ret = 1;
if (ARCH_IS_S390(qemuCaps->arch))
- ret = virQEMUCapsInitCPUModelS390(qemuCaps, cpu);
+ ret = virQEMUCapsInitCPUModelS390(qemuCaps, type, cpu);
if (ret == 0)
cpu->fallback = VIR_CPU_FALLBACK_FORBID;
@@ -3114,7 +3145,8 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
void
virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
- virCapsPtr caps)
+ virCapsPtr caps,
+ virDomainVirtType type)
{
virCPUDefPtr cpu = NULL;
int rc;
@@ -3130,7 +3162,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
cpu->match = VIR_CPU_MATCH_EXACT;
cpu->fallback = VIR_CPU_FALLBACK_ALLOW;
- if ((rc = virQEMUCapsInitCPUModel(qemuCaps, cpu)) < 0) {
+ if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, cpu)) < 0) {
goto error;
} else if (rc == 1) {
VIR_DEBUG("No host CPU model info from QEMU; using host capabilities");
@@ -3143,7 +3175,11 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
goto error;
}
- qemuCaps->hostCPUModel = cpu;
+ if (type == VIR_DOMAIN_VIRT_KVM)
+ qemuCaps->kvmCPUModel = cpu;
+ else
+ qemuCaps->tcgCPUModel = cpu;
+
return;
error:
@@ -3154,7 +3190,8 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
static int
virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
- xmlXPathContextPtr ctxt)
+ xmlXPathContextPtr ctxt,
+ virDomainVirtType type)
{
xmlNodePtr hostCPUNode;
xmlNodePtr *featureNodes = NULL;
@@ -3164,7 +3201,12 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
size_t i;
int n;
- if (!(hostCPUNode = virXPathNode("./hostCPU", ctxt))) {
+ if (type == VIR_DOMAIN_VIRT_KVM)
+ hostCPUNode = virXPathNode("./hostCPU[@type='kvm']", ctxt);
+ else
+ hostCPUNode = virXPathNode("./hostCPU[@type='tcg']", ctxt);
+
+ if (!hostCPUNode) {
ret = 0;
goto cleanup;
}
@@ -3232,7 +3274,10 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
}
}
- qemuCaps->hostCPUModelInfo = hostCPU;
+ if (type == VIR_DOMAIN_VIRT_KVM)
+ qemuCaps->kvmCPUModelInfo = hostCPU;
+ else
+ qemuCaps->tcgCPUModelInfo = hostCPU;
hostCPU = NULL;
ret = 0;
@@ -3438,7 +3483,8 @@ virQEMUCapsLoadCache(virCapsPtr caps,
}
VIR_FREE(str);
- if (virQEMUCapsLoadHostCPUModelInfo(qemuCaps, ctxt) < 0)
+ if (virQEMUCapsLoadHostCPUModelInfo(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0 ||
+ virQEMUCapsLoadHostCPUModelInfo(qemuCaps, ctxt, VIR_DOMAIN_VIRT_QEMU) < 0)
goto cleanup;
if (virQEMUCapsLoadCPUModels(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0 ||
@@ -3546,7 +3592,8 @@ virQEMUCapsLoadCache(virCapsPtr caps,
}
VIR_FREE(nodes);
- virQEMUCapsInitHostCPUModel(qemuCaps, caps);
+ virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_KVM);
+ virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_QEMU);
ret = 0;
cleanup:
@@ -3560,12 +3607,26 @@ virQEMUCapsLoadCache(virCapsPtr caps,
static void
virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
- virBufferPtr buf)
+ virBufferPtr buf,
+ virDomainVirtType type)
{
- qemuMonitorCPUModelInfoPtr model = qemuCaps->hostCPUModelInfo;
+ qemuMonitorCPUModelInfoPtr model;
+ const char *typeStr;
size_t i;
- virBufferAsprintf(buf, "<hostCPU model='%s'>\n", model->name);
+ if (type == VIR_DOMAIN_VIRT_KVM) {
+ typeStr = "kvm";
+ model = qemuCaps->kvmCPUModelInfo;
+ } else {
+ typeStr = "tcg";
+ model = qemuCaps->tcgCPUModelInfo;
+ }
+
+ if (!model)
+ return;
+
+ virBufferAsprintf(buf, "<hostCPU type='%s' model='%s'>\n",
+ typeStr, model->name);
virBufferAdjustIndent(buf, 2);
for (i = 0; i < model->nprops; i++) {
@@ -3670,8 +3731,8 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps,
virBufferAsprintf(&buf, "<arch>%s</arch>\n",
virArchToString(qemuCaps->arch));
- if (qemuCaps->hostCPUModelInfo)
- virQEMUCapsFormatHostCPUModelInfo(qemuCaps, &buf);
+ virQEMUCapsFormatHostCPUModelInfo(qemuCaps, &buf, VIR_DOMAIN_VIRT_KVM);
+ virQEMUCapsFormatHostCPUModelInfo(qemuCaps, &buf, VIR_DOMAIN_VIRT_QEMU);
virQEMUCapsFormatCPUModels(qemuCaps, &buf, VIR_DOMAIN_VIRT_KVM);
virQEMUCapsFormatCPUModels(qemuCaps, &buf, VIR_DOMAIN_VIRT_QEMU);
@@ -3806,11 +3867,15 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
VIR_FREE(qemuCaps->gicCapabilities);
qemuCaps->ngicCapabilities = 0;
- qemuMonitorCPUModelInfoFree(qemuCaps->hostCPUModelInfo);
- qemuCaps->hostCPUModelInfo = NULL;
+ qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo);
+ qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo);
+ qemuCaps->kvmCPUModelInfo = NULL;
+ qemuCaps->tcgCPUModelInfo = NULL;
- virCPUDefFree(qemuCaps->hostCPUModel);
- qemuCaps->hostCPUModel = NULL;
+ virCPUDefFree(qemuCaps->kvmCPUModel);
+ virCPUDefFree(qemuCaps->tcgCPUModel);
+ qemuCaps->kvmCPUModel = NULL;
+ qemuCaps->tcgCPUModel = NULL;
}
@@ -4368,7 +4433,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_QMP_SCHEMA) &&
virQEMUCapsProbeQMPSchemaCapabilities(qemuCaps, mon) < 0)
goto cleanup;
- if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon) < 0)
+ if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon, false) < 0)
goto cleanup;
/* 'intel-iommu' shows up as a device since 2.2.0, but can
@@ -4410,6 +4475,9 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED,
if (virQEMUCapsProbeQMPCPUDefinitions(qemuCaps, mon, true) < 0)
goto cleanup;
+ if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon, true) < 0)
+ goto cleanup;
+
ret = 0;
cleanup:
return ret;
@@ -4754,7 +4822,8 @@ virQEMUCapsNewForBinaryInternal(virCapsPtr caps,
virQEMUCapsRememberCached(qemuCaps, cacheDir) < 0)
goto error;
- virQEMUCapsInitHostCPUModel(qemuCaps, caps);
+ virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_KVM);
+ virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_QEMU);
}
cleanup:
@@ -5200,8 +5269,10 @@ virQEMUCapsFillDomainCPUCaps(virCapsPtr caps,
domCaps->cpu.hostPassthrough = true;
if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype,
- VIR_CPU_MODE_HOST_MODEL))
- domCaps->cpu.hostModel = virCPUDefCopy(qemuCaps->hostCPUModel);
+ VIR_CPU_MODE_HOST_MODEL)) {
+ virCPUDefPtr cpu = virQEMUCapsGetHostModel(qemuCaps, domCaps->virttype);
+ domCaps->cpu.hostModel = virCPUDefCopy(cpu);
+ }
if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype,
VIR_CPU_MODE_CUSTOM)) {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 95bb67d44..3331142ea 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -441,7 +441,8 @@ int virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
virDomainVirtType type,
char ***names,
size_t *count);
-virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps);
+virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
+ virDomainVirtType type);
bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps,
virCapsPtr caps,
virDomainVirtType type,
diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
index 38b971e0e..75499d462 100644
--- a/src/qemu/qemu_capspriv.h
+++ b/src/qemu/qemu_capspriv.h
@@ -71,5 +71,6 @@ virQEMUCapsSetArch(virQEMUCapsPtr qemuCaps,
void
virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
- virCapsPtr caps);
+ virCapsPtr caps,
+ virDomainVirtType type);
#endif
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c00a47a91..4e83dee37 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6782,7 +6782,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
if (def->cpu->mode == VIR_CPU_MODE_CUSTOM)
cpuDef = def->cpu;
else if (def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH)
- cpuDef = virQEMUCapsGetHostModel(qemuCaps);
+ cpuDef = virQEMUCapsGetHostModel(qemuCaps, def->virtType);
if (cpuDef) {
int svm = virCPUCheckFeature(def->os.arch, cpuDef, "svm");
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 92fa69b3c..b9df01da5 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5147,13 +5147,14 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def,
/* custom CPUs in TCG mode don't need to be compared to host CPU */
if (def->virtType != VIR_DOMAIN_VIRT_QEMU ||
def->cpu->mode != VIR_CPU_MODE_CUSTOM) {
- if (virCPUCompare(caps->host.arch, virQEMUCapsGetHostModel(qemuCaps),
+ if (virCPUCompare(caps->host.arch,
+ virQEMUCapsGetHostModel(qemuCaps, def->virtType),
def->cpu, true) < 0)
return -1;
}
if (virCPUUpdate(def->os.arch, def->cpu,
- virQEMUCapsGetHostModel(qemuCaps)) < 0)
+ virQEMUCapsGetHostModel(qemuCaps, def->virtType)) < 0)
goto cleanup;
if (virQEMUCapsGetCPUDefinitions(qemuCaps, def->virtType,
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies
index 0405d5d7b..c3cbeee0a 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies
@@ -13378,3 +13378,11 @@
],
"id": "libvirt-2"
}
+
+{
+ "id": "libvirt-3",
+ "error": {
+ "class": "GenericError",
+ "desc": "The CPU definition 'max' is unknown."
+ }
+}
diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
index 1f652bdc2..df7eb18f6 100644
--- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
+++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
@@ -133,7 +133,7 @@
<kvmVersion>0</kvmVersion>
<package></package>
<arch>s390x</arch>
- <hostCPU model='zEC12.2-base'>
+ <hostCPU type='kvm' model='zEC12.2-base'>
<property name='aefsi' boolean='yes'/>
<property name='msa5' boolean='yes'/>
<property name='msa4' boolean='yes'/>
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies
index 8d54788df..390f40f9f 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies
@@ -14708,3 +14708,182 @@
],
"id": "libvirt-2"
}
+
+{
+ "return": {
+ "model": {
+ "name": "base",
+ "props": {
+ "cmov": true,
+ "ia64": false,
+ "aes": true,
+ "mmx": true,
+ "rdpid": false,
+ "arat": true,
+ "pause-filter": false,
+ "xsavec": false,
+ "osxsave": false,
+ "kvm-asyncpf": false,
+ "perfctr-core": false,
+ "mpx": true,
+ "pbe": false,
+ "avx512cd": false,
+ "decodeassists": false,
+ "sse4.1": true,
+ "family": 6,
+ "avx512f": false,
+ "msr": true,
+ "mce": true,
+ "mca": true,
+ "xcrypt": false,
+ "min-level": 13,
+ "xgetbv1": true,
+ "cid": false,
+ "ds": false,
+ "fxsr": true,
+ "xsaveopt": true,
+ "xtpr": false,
+ "avx512vl": false,
+ "avx512-vpopcntdq": false,
+ "phe": false,
+ "extapic": false,
+ "3dnowprefetch": false,
+ "cr8legacy": true,
+ "xcrypt-en": false,
+ "pn": false,
+ "dca": false,
+ "vendor": "AuthenticAMD",
+ "pku": true,
+ "smx": false,
+ "cmp-legacy": false,
+ "avx512-4fmaps": false,
+ "vmcb-clean": false,
+ "hle": false,
+ "3dnowext": true,
+ "npt": false,
+ "clwb": true,
+ "lbrv": false,
+ "adx": true,
+ "ss": true,
+ "pni": true,
+ "svm-lock": false,
+ "smep": true,
+ "smap": true,
+ "pfthreshold": false,
+ "x2apic": false,
+ "avx512vbmi": false,
+ "flushbyasid": false,
+ "f16c": false,
+ "ace2-en": false,
+ "pae": true,
+ "pat": true,
+ "sse": true,
+ "phe-en": false,
+ "kvm-nopiodelay": false,
+ "tm": false,
+ "kvmclock-stable-bit": false,
+ "hypervisor": true,
+ "pcommit": true,
+ "syscall": true,
+ "avx512dq": false,
+ "svm": true,
+ "invtsc": false,
+ "sse2": true,
+ "est": false,
+ "avx512ifma": false,
+ "tm2": false,
+ "kvm-pv-eoi": false,
+ "cx8": true,
+ "kvm-mmu": false,
+ "sse4.2": true,
+ "pge": true,
+ "pdcm": false,
+ "model": 6,
+ "movbe": true,
+ "nrip-save": false,
+ "ssse3": true,
+ "sse4a": true,
+ "invpcid": false,
+ "pdpe1gb": true,
+ "tsc-deadline": false,
+ "fma": false,
+ "cx16": true,
+ "de": true,
+ "stepping": 3,
+ "xsave": true,
+ "clflush": true,
+ "skinit": false,
+ "tsc": true,
+ "tce": false,
+ "fpu": true,
+ "ds-cpl": false,
+ "ibs": false,
+ "fma4": false,
+ "la57": true,
+ "osvw": false,
+ "apic": true,
+ "pmm": false,
+ "tsc-adjust": false,
+ "kvm-steal-time": false,
+ "kvmclock": false,
+ "lwp": false,
+ "xop": false,
+ "avx": false,
+ "ospke": true,
+ "acpi": true,
+ "avx512bw": false,
+ "ace2": false,
+ "fsgsbase": true,
+ "ht": false,
+ "nx": true,
+ "pclmulqdq": true,
+ "mmxext": true,
+ "popcnt": true,
+ "xsaves": false,
+ "lm": true,
+ "umip": false,
+ "pse": true,
+ "avx2": false,
+ "sep": true,
+ "nodeid-msr": false,
+ "misalignsse": false,
+ "min-xlevel": 2147483658,
+ "bmi1": true,
+ "bmi2": true,
+ "kvm-pv-unhalt": false,
+ "tsc-scale": false,
+ "topoext": false,
+ "clflushopt": true,
+ "monitor": true,
+ "avx512er": false,
+ "pmm-en": false,
+ "pcid": false,
+ "3dnow": true,
+ "erms": true,
+ "lahf-lm": true,
+ "fxsr-opt": false,
+ "xstore": false,
+ "rtm": false,
+ "lmce": false,
+ "perfctr-nb": false,
+ "rdrand": false,
+ "rdseed": false,
+ "avx512-4vnniw": false,
+ "vme": false,
+ "vmx": false,
+ "dtes64": false,
+ "mtrr": true,
+ "rdtscp": true,
+ "pse36": true,
+ "tbm": false,
+ "wdt": false,
+ "model-id": "QEMU TCG CPU version 2.5+",
+ "sha-ni": false,
+ "abm": true,
+ "avx512pf": false,
+ "xstore-en": false
+ }
+ }
+ },
+ "id": "libvirt-3"
+}
diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
index 32368e648..520bf80f4 100644
--- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
@@ -205,7 +205,7 @@
<kvmVersion>0</kvmVersion>
<package> (v2.8.0-877-g38e4b757b4)</package>
<arch>x86_64</arch>
- <hostCPU model='base'>
+ <hostCPU type='kvm' model='base'>
<property name='cmov' boolean='yes'/>
<property name='ia64' boolean='no'/>
<property name='aes' boolean='yes'/>
@@ -375,6 +375,176 @@
<property name='avx512pf' boolean='no'/>
<property name='xstore-en' boolean='no'/>
</hostCPU>
+ <hostCPU type='tcg' model='base'>
+ <property name='cmov' boolean='yes'/>
+ <property name='ia64' boolean='no'/>
+ <property name='aes' boolean='yes'/>
+ <property name='mmx' boolean='yes'/>
+ <property name='rdpid' boolean='no'/>
+ <property name='arat' boolean='yes'/>
+ <property name='pause-filter' boolean='no'/>
+ <property name='xsavec' boolean='no'/>
+ <property name='osxsave' boolean='no'/>
+ <property name='kvm-asyncpf' boolean='no'/>
+ <property name='perfctr-core' boolean='no'/>
+ <property name='mpx' boolean='yes'/>
+ <property name='pbe' boolean='no'/>
+ <property name='avx512cd' boolean='no'/>
+ <property name='decodeassists' boolean='no'/>
+ <property name='sse4.1' boolean='yes'/>
+ <property name='family' ull='6'/>
+ <property name='avx512f' boolean='no'/>
+ <property name='msr' boolean='yes'/>
+ <property name='mce' boolean='yes'/>
+ <property name='mca' boolean='yes'/>
+ <property name='xcrypt' boolean='no'/>
+ <property name='min-level' ull='13'/>
+ <property name='xgetbv1' boolean='yes'/>
+ <property name='cid' boolean='no'/>
+ <property name='ds' boolean='no'/>
+ <property name='fxsr' boolean='yes'/>
+ <property name='xsaveopt' boolean='yes'/>
+ <property name='xtpr' boolean='no'/>
+ <property name='avx512vl' boolean='no'/>
+ <property name='avx512-vpopcntdq' boolean='no'/>
+ <property name='phe' boolean='no'/>
+ <property name='extapic' boolean='no'/>
+ <property name='3dnowprefetch' boolean='no'/>
+ <property name='cr8legacy' boolean='yes'/>
+ <property name='xcrypt-en' boolean='no'/>
+ <property name='pn' boolean='no'/>
+ <property name='dca' boolean='no'/>
+ <property name='vendor' string='AuthenticAMD'/>
+ <property name='pku' boolean='yes'/>
+ <property name='smx' boolean='no'/>
+ <property name='cmp-legacy' boolean='no'/>
+ <property name='avx512-4fmaps' boolean='no'/>
+ <property name='vmcb-clean' boolean='no'/>
+ <property name='hle' boolean='no'/>
+ <property name='3dnowext' boolean='yes'/>
+ <property name='npt' boolean='no'/>
+ <property name='clwb' boolean='yes'/>
+ <property name='lbrv' boolean='no'/>
+ <property name='adx' boolean='yes'/>
+ <property name='ss' boolean='yes'/>
+ <property name='pni' boolean='yes'/>
+ <property name='svm-lock' boolean='no'/>
+ <property name='smep' boolean='yes'/>
+ <property name='smap' boolean='yes'/>
+ <property name='pfthreshold' boolean='no'/>
+ <property name='x2apic' boolean='no'/>
+ <property name='avx512vbmi' boolean='no'/>
+ <property name='flushbyasid' boolean='no'/>
+ <property name='f16c' boolean='no'/>
+ <property name='ace2-en' boolean='no'/>
+ <property name='pae' boolean='yes'/>
+ <property name='pat' boolean='yes'/>
+ <property name='sse' boolean='yes'/>
+ <property name='phe-en' boolean='no'/>
+ <property name='kvm-nopiodelay' boolean='no'/>
+ <property name='tm' boolean='no'/>
+ <property name='kvmclock-stable-bit' boolean='no'/>
+ <property name='hypervisor' boolean='yes'/>
+ <property name='pcommit' boolean='yes'/>
+ <property name='syscall' boolean='yes'/>
+ <property name='avx512dq' boolean='no'/>
+ <property name='svm' boolean='yes'/>
+ <property name='invtsc' boolean='no'/>
+ <property name='sse2' boolean='yes'/>
+ <property name='est' boolean='no'/>
+ <property name='avx512ifma' boolean='no'/>
+ <property name='tm2' boolean='no'/>
+ <property name='kvm-pv-eoi' boolean='no'/>
+ <property name='cx8' boolean='yes'/>
+ <property name='kvm-mmu' boolean='no'/>
+ <property name='sse4.2' boolean='yes'/>
+ <property name='pge' boolean='yes'/>
+ <property name='pdcm' boolean='no'/>
+ <property name='model' ull='6'/>
+ <property name='movbe' boolean='yes'/>
+ <property name='nrip-save' boolean='no'/>
+ <property name='ssse3' boolean='yes'/>
+ <property name='sse4a' boolean='yes'/>
+ <property name='invpcid' boolean='no'/>
+ <property name='pdpe1gb' boolean='yes'/>
+ <property name='tsc-deadline' boolean='no'/>
+ <property name='fma' boolean='no'/>
+ <property name='cx16' boolean='yes'/>
+ <property name='de' boolean='yes'/>
+ <property name='stepping' ull='3'/>
+ <property name='xsave' boolean='yes'/>
+ <property name='clflush' boolean='yes'/>
+ <property name='skinit' boolean='no'/>
+ <property name='tsc' boolean='yes'/>
+ <property name='tce' boolean='no'/>
+ <property name='fpu' boolean='yes'/>
+ <property name='ds-cpl' boolean='no'/>
+ <property name='ibs' boolean='no'/>
+ <property name='fma4' boolean='no'/>
+ <property name='la57' boolean='yes'/>
+ <property name='osvw' boolean='no'/>
+ <property name='apic' boolean='yes'/>
+ <property name='pmm' boolean='no'/>
+ <property name='tsc-adjust' boolean='no'/>
+ <property name='kvm-steal-time' boolean='no'/>
+ <property name='kvmclock' boolean='no'/>
+ <property name='lwp' boolean='no'/>
+ <property name='xop' boolean='no'/>
+ <property name='avx' boolean='no'/>
+ <property name='ospke' boolean='yes'/>
+ <property name='acpi' boolean='yes'/>
+ <property name='avx512bw' boolean='no'/>
+ <property name='ace2' boolean='no'/>
+ <property name='fsgsbase' boolean='yes'/>
+ <property name='ht' boolean='no'/>
+ <property name='nx' boolean='yes'/>
+ <property name='pclmulqdq' boolean='yes'/>
+ <property name='mmxext' boolean='yes'/>
+ <property name='popcnt' boolean='yes'/>
+ <property name='xsaves' boolean='no'/>
+ <property name='lm' boolean='yes'/>
+ <property name='umip' boolean='no'/>
+ <property name='pse' boolean='yes'/>
+ <property name='avx2' boolean='no'/>
+ <property name='sep' boolean='yes'/>
+ <property name='nodeid-msr' boolean='no'/>
+ <property name='misalignsse' boolean='no'/>
+ <property name='min-xlevel' ull='2147483658'/>
+ <property name='bmi1' boolean='yes'/>
+ <property name='bmi2' boolean='yes'/>
+ <property name='kvm-pv-unhalt' boolean='no'/>
+ <property name='tsc-scale' boolean='no'/>
+ <property name='topoext' boolean='no'/>
+ <property name='clflushopt' boolean='yes'/>
+ <property name='monitor' boolean='yes'/>
+ <property name='avx512er' boolean='no'/>
+ <property name='pmm-en' boolean='no'/>
+ <property name='pcid' boolean='no'/>
+ <property name='3dnow' boolean='yes'/>
+ <property name='erms' boolean='yes'/>
+ <property name='lahf-lm' boolean='yes'/>
+ <property name='fxsr-opt' boolean='no'/>
+ <property name='xstore' boolean='no'/>
+ <property name='rtm' boolean='no'/>
+ <property name='lmce' boolean='no'/>
+ <property name='perfctr-nb' boolean='no'/>
+ <property name='rdrand' boolean='no'/>
+ <property name='rdseed' boolean='no'/>
+ <property name='avx512-4vnniw' boolean='no'/>
+ <property name='vme' boolean='no'/>
+ <property name='vmx' boolean='no'/>
+ <property name='dtes64' boolean='no'/>
+ <property name='mtrr' boolean='yes'/>
+ <property name='rdtscp' boolean='yes'/>
+ <property name='pse36' boolean='yes'/>
+ <property name='tbm' boolean='no'/>
+ <property name='wdt' boolean='no'/>
+ <property name='model-id' string='QEMU TCG CPU version 2.5+'/>
+ <property name='sha-ni' boolean='no'/>
+ <property name='abm' boolean='yes'/>
+ <property name='avx512pf' boolean='no'/>
+ <property name='xstore-en' boolean='no'/>
+ </hostCPU>
<cpu type='kvm' name='max' usable='yes'/>
<cpu type='kvm' name='host' usable='yes'/>
<cpu type='kvm' name='base' usable='yes'/>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index faa99c64c..5ba2eeab9 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -384,7 +384,8 @@ testUpdateQEMUCaps(const struct testInfo *info,
if (testAddCPUModels(info->qemuCaps, info->skipLegacyCPUs) < 0)
goto cleanup;
- virQEMUCapsInitHostCPUModel(info->qemuCaps, caps);
+ virQEMUCapsInitHostCPUModel(info->qemuCaps, caps, VIR_DOMAIN_VIRT_KVM);
+ virQEMUCapsInitHostCPUModel(info->qemuCaps, caps, VIR_DOMAIN_VIRT_QEMU);
virQEMUCapsFilterByMachineType(info->qemuCaps, vm->def->os.machine);
--
2.11.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 02/15/2017 11:44 AM, Jiri Denemark wrote:
> Querying "host" CPU model expansion only makes sense for KVM. QEMU 2.9.0
> introduces a new "max" CPU model which can be used to ask QEMU what the
> best CPU it can provide to a TCG domain is.
But how do we know "max" is available to be used for TCG? It seems to
me that virQEMUCapsProbeQMPHostCPU now is changed such that there's an
assumption that "max" is available, but the above makes it appear as if
it's a new paremter/attribute [1].
Every time I see TCG I think "Trusted Computing Group" and I certainly
don't think VIR_DOMAIN_TYPE_QEMU. What gets "confusing" eventually is
that virQEMUCapsInitHostCPUModel is called with VIR_DOMAIN_VIRT_KVM and
VIR_DOMAIN_VIRT_QEMU, but qemuCaps->tcgCPUModel is filled in for
VIR_DOMAIN_VIRT_QEMU.
It's not overtly obvious to me from just reading the commit message, but
it seems TCG now becomes a fallback position of sorts which I think is
fine, it's just not described.
>
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>
> Notes:
> Version 2:
> - no change
>
> src/qemu/qemu_capabilities.c | 151 ++++++++++++-----
> src/qemu/qemu_capabilities.h | 3 +-
> src/qemu/qemu_capspriv.h | 3 +-
> src/qemu/qemu_command.c | 2 +-
> src/qemu/qemu_process.c | 5 +-
> .../qemucapabilitiesdata/caps_2.8.0.s390x.replies | 8 +
> tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 2 +-
> .../qemucapabilitiesdata/caps_2.9.0.x86_64.replies | 179 +++++++++++++++++++++
> tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 172 +++++++++++++++++++-
> tests/qemuxml2argvtest.c | 3 +-
> 10 files changed, 480 insertions(+), 48 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 466852d13..2ba82456e 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -400,13 +400,15 @@ struct _virQEMUCaps {
> size_t ngicCapabilities;
> virGICCapability *gicCapabilities;
>
> - qemuMonitorCPUModelInfoPtr hostCPUModelInfo;
> + qemuMonitorCPUModelInfoPtr kvmCPUModelInfo;
> + qemuMonitorCPUModelInfoPtr tcgCPUModelInfo;
>
> /* Anything below is not stored in the cache since the values are
> * re-computed from the other fields or external data sources every
> * time we probe QEMU or load the results from the cache.
> */
> - virCPUDefPtr hostCPUModel;
> + virCPUDefPtr kvmCPUModel;
> + virCPUDefPtr tcgCPUModel;
> };
>
> struct virQEMUCapsSearchData {
> @@ -2163,12 +2165,20 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps)
> goto error;
> }
>
> - if (qemuCaps->hostCPUModel &&
> - !(ret->hostCPUModel = virCPUDefCopy(qemuCaps->hostCPUModel)))
> + if (qemuCaps->kvmCPUModel &&
> + !(ret->kvmCPUModel = virCPUDefCopy(qemuCaps->kvmCPUModel)))
> goto error;
>
> - if (qemuCaps->hostCPUModelInfo &&
> - !(ret->hostCPUModelInfo = qemuMonitorCPUModelInfoCopy(qemuCaps->hostCPUModelInfo)))
> + if (qemuCaps->tcgCPUModel &&
> + !(ret->tcgCPUModel = virCPUDefCopy(qemuCaps->tcgCPUModel)))
> + goto error;
> +
> + if (qemuCaps->kvmCPUModelInfo &&
> + !(ret->kvmCPUModelInfo = qemuMonitorCPUModelInfoCopy(qemuCaps->kvmCPUModelInfo)))
> + goto error;
> +
> + if (qemuCaps->tcgCPUModelInfo &&
> + !(ret->tcgCPUModelInfo = qemuMonitorCPUModelInfoCopy(qemuCaps->tcgCPUModelInfo)))
> goto error;
>
> if (VIR_ALLOC_N(ret->machineTypes, qemuCaps->nmachineTypes) < 0)
> @@ -2217,8 +2227,10 @@ void virQEMUCapsDispose(void *obj)
>
> VIR_FREE(qemuCaps->gicCapabilities);
>
> - qemuMonitorCPUModelInfoFree(qemuCaps->hostCPUModelInfo);
> - virCPUDefFree(qemuCaps->hostCPUModel);
> + qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo);
> + qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo);
> + virCPUDefFree(qemuCaps->kvmCPUModel);
> + virCPUDefFree(qemuCaps->tcgCPUModel);
> }
>
> void
> @@ -2435,9 +2447,13 @@ virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
>
>
> virCPUDefPtr
> -virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps)
> +virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
> + virDomainVirtType type)
> {
> - return qemuCaps->hostCPUModel;
> + if (type == VIR_DOMAIN_VIRT_KVM)
> + return qemuCaps->kvmCPUModel;
> + else
> + return qemuCaps->tcgCPUModel;
> }
>
>
> @@ -2455,7 +2471,7 @@ virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps,
> virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch);
>
> case VIR_CPU_MODE_HOST_MODEL:
> - return !!qemuCaps->hostCPUModel;
> + return !!virQEMUCapsGetHostModel(qemuCaps, type);
>
> case VIR_CPU_MODE_CUSTOM:
> if (type == VIR_DOMAIN_VIRT_KVM)
> @@ -2822,14 +2838,24 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps,
>
> static int
> virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
> - qemuMonitorPtr mon)
> + qemuMonitorPtr mon,
> + bool tcg)
> {
> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION) ||
> - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
> + qemuMonitorCPUModelInfoPtr *modelInfo;
> + const char *model;
> +
> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION))
> return 0;
So if no model expansion, then neither is filled in, but there are later
checks which seem to assume one or the other is filled in. I didn't
chase all the callers to check whether they would expect/require
something to be filled in (too many patches, too little time).
>
> - return qemuMonitorGetCPUModelExpansion(mon, "static", "host",
> - &qemuCaps->hostCPUModelInfo);
> + if (tcg || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
[1]
It's noot clear to the casual reviewer when tcg and in particular the
"max" parameter was supported. IOW: The first thought I had was why
isn't there a capability being checked for "max" being available. The
commit message seems to indicate that it's new for 2.9.0, but I don't
see a libvirt capability check for it being available before being used.
The logic changes now such that if !QEMU_CAPS_KVM that model expansion
is called with "max"; whereas, previously a 0 was returned. Thus if
!tcg and !QEMU_CAPS_KVM, then "max" is used and ModelExpansion is
called. If TCG becomes the "de facto" default -
Just making sure that's expected...
> + modelInfo = &qemuCaps->tcgCPUModelInfo;
> + model = "max";
> + } else {
> + modelInfo = &qemuCaps->kvmCPUModelInfo;
> + model = "host";
> + }
> +
> + return qemuMonitorGetCPUModelExpansion(mon, "static", model, modelInfo);
> }
>
> struct tpmTypeToCaps {
> @@ -3053,12 +3079,16 @@ virQEMUCapsCPUFilterFeatures(const char *name,
> */
> static int
> virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
> + virDomainVirtType type,
> virCPUDefPtr cpu)
> {
> - qemuMonitorCPUModelInfoPtr modelInfo = qemuCaps->hostCPUModelInfo;
> + qemuMonitorCPUModelInfoPtr modelInfo;
> size_t i;
>
> - if (!modelInfo) {
> + if (type != VIR_DOMAIN_VIRT_KVM)
> + return -1;
> +
> + if (!(modelInfo = qemuCaps->kvmCPUModelInfo)) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("missing host CPU model info from QEMU capabilities "
> "for binary %s"),
> @@ -3098,12 +3128,13 @@ virQEMUCapsInitCPUModelS390(virQEMUCapsPtr qemuCaps,
> */
> static int
> virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
> + virDomainVirtType type,
> virCPUDefPtr cpu)
> {
> int ret = 1;
>
> if (ARCH_IS_S390(qemuCaps->arch))
> - ret = virQEMUCapsInitCPUModelS390(qemuCaps, cpu);
> + ret = virQEMUCapsInitCPUModelS390(qemuCaps, type, cpu);
>
> if (ret == 0)
> cpu->fallback = VIR_CPU_FALLBACK_FORBID;
> @@ -3114,7 +3145,8 @@ virQEMUCapsInitCPUModel(virQEMUCapsPtr qemuCaps,
>
> void
> virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
> - virCapsPtr caps)
> + virCapsPtr caps,
> + virDomainVirtType type)
> {
> virCPUDefPtr cpu = NULL;
> int rc;
> @@ -3130,7 +3162,7 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
> cpu->match = VIR_CPU_MATCH_EXACT;
> cpu->fallback = VIR_CPU_FALLBACK_ALLOW;
>
> - if ((rc = virQEMUCapsInitCPUModel(qemuCaps, cpu)) < 0) {
> + if ((rc = virQEMUCapsInitCPUModel(qemuCaps, type, cpu)) < 0) {
> goto error;
> } else if (rc == 1) {
> VIR_DEBUG("No host CPU model info from QEMU; using host capabilities");
> @@ -3143,7 +3175,11 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
> goto error;
> }
>
> - qemuCaps->hostCPUModel = cpu;
> + if (type == VIR_DOMAIN_VIRT_KVM)
> + qemuCaps->kvmCPUModel = cpu;
> + else
> + qemuCaps->tcgCPUModel = cpu;
> +
> return;
>
> error:
> @@ -3154,7 +3190,8 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>
> static int
> virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
> - xmlXPathContextPtr ctxt)
> + xmlXPathContextPtr ctxt,
> + virDomainVirtType type)
> {
> xmlNodePtr hostCPUNode;
> xmlNodePtr *featureNodes = NULL;
> @@ -3164,7 +3201,12 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
> size_t i;
> int n;
>
> - if (!(hostCPUNode = virXPathNode("./hostCPU", ctxt))) {
> + if (type == VIR_DOMAIN_VIRT_KVM)
> + hostCPUNode = virXPathNode("./hostCPU[@type='kvm']", ctxt);
> + else
> + hostCPUNode = virXPathNode("./hostCPU[@type='tcg']", ctxt);
> +
> + if (!hostCPUNode) {
> ret = 0;
> goto cleanup;
> }
> @@ -3232,7 +3274,10 @@ virQEMUCapsLoadHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
> }
> }
>
> - qemuCaps->hostCPUModelInfo = hostCPU;
> + if (type == VIR_DOMAIN_VIRT_KVM)
> + qemuCaps->kvmCPUModelInfo = hostCPU;
> + else
> + qemuCaps->tcgCPUModelInfo = hostCPU;
> hostCPU = NULL;
> ret = 0;
>
> @@ -3438,7 +3483,8 @@ virQEMUCapsLoadCache(virCapsPtr caps,
> }
> VIR_FREE(str);
>
> - if (virQEMUCapsLoadHostCPUModelInfo(qemuCaps, ctxt) < 0)
> + if (virQEMUCapsLoadHostCPUModelInfo(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0 ||
> + virQEMUCapsLoadHostCPUModelInfo(qemuCaps, ctxt, VIR_DOMAIN_VIRT_QEMU) < 0)
> goto cleanup;
>
> if (virQEMUCapsLoadCPUModels(qemuCaps, ctxt, VIR_DOMAIN_VIRT_KVM) < 0 ||
> @@ -3546,7 +3592,8 @@ virQEMUCapsLoadCache(virCapsPtr caps,
> }
> VIR_FREE(nodes);
>
> - virQEMUCapsInitHostCPUModel(qemuCaps, caps);
> + virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_KVM);
> + virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_QEMU);
>
> ret = 0;
> cleanup:
> @@ -3560,12 +3607,26 @@ virQEMUCapsLoadCache(virCapsPtr caps,
>
> static void
> virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
> - virBufferPtr buf)
> + virBufferPtr buf,
> + virDomainVirtType type)
> {
> - qemuMonitorCPUModelInfoPtr model = qemuCaps->hostCPUModelInfo;
> + qemuMonitorCPUModelInfoPtr model;
> + const char *typeStr;
> size_t i;
>
> - virBufferAsprintf(buf, "<hostCPU model='%s'>\n", model->name);
> + if (type == VIR_DOMAIN_VIRT_KVM) {
> + typeStr = "kvm";
> + model = qemuCaps->kvmCPUModelInfo;
> + } else {
> + typeStr = "tcg";
> + model = qemuCaps->tcgCPUModelInfo;
> + }
> +
> + if (!model)
> + return;
if !model can happen, how does that affect other places which assume
that are making a similar check to decide which ModelInfo to use?
Also this is "similar" to virQEMUCapsGetHostModel at least for 'model'...
> +
> + virBufferAsprintf(buf, "<hostCPU type='%s' model='%s'>\n",
> + typeStr, model->name);
> virBufferAdjustIndent(buf, 2);
>
> for (i = 0; i < model->nprops; i++) {
> @@ -3670,8 +3731,8 @@ virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps,
> virBufferAsprintf(&buf, "<arch>%s</arch>\n",
> virArchToString(qemuCaps->arch));
>
> - if (qemuCaps->hostCPUModelInfo)
> - virQEMUCapsFormatHostCPUModelInfo(qemuCaps, &buf);
> + virQEMUCapsFormatHostCPUModelInfo(qemuCaps, &buf, VIR_DOMAIN_VIRT_KVM);
> + virQEMUCapsFormatHostCPUModelInfo(qemuCaps, &buf, VIR_DOMAIN_VIRT_QEMU);
>
> virQEMUCapsFormatCPUModels(qemuCaps, &buf, VIR_DOMAIN_VIRT_KVM);
> virQEMUCapsFormatCPUModels(qemuCaps, &buf, VIR_DOMAIN_VIRT_QEMU);
> @@ -3806,11 +3867,15 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
> VIR_FREE(qemuCaps->gicCapabilities);
> qemuCaps->ngicCapabilities = 0;
>
> - qemuMonitorCPUModelInfoFree(qemuCaps->hostCPUModelInfo);
> - qemuCaps->hostCPUModelInfo = NULL;
> + qemuMonitorCPUModelInfoFree(qemuCaps->kvmCPUModelInfo);
> + qemuMonitorCPUModelInfoFree(qemuCaps->tcgCPUModelInfo);
> + qemuCaps->kvmCPUModelInfo = NULL;
> + qemuCaps->tcgCPUModelInfo = NULL;
>
> - virCPUDefFree(qemuCaps->hostCPUModel);
> - qemuCaps->hostCPUModel = NULL;
> + virCPUDefFree(qemuCaps->kvmCPUModel);
> + virCPUDefFree(qemuCaps->tcgCPUModel);
> + qemuCaps->kvmCPUModel = NULL;
> + qemuCaps->tcgCPUModel = NULL;
> }
>
>
> @@ -4368,7 +4433,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
> if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_QMP_SCHEMA) &&
> virQEMUCapsProbeQMPSchemaCapabilities(qemuCaps, mon) < 0)
> goto cleanup;
> - if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon) < 0)
> + if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon, false) < 0)
> goto cleanup;
>
> /* 'intel-iommu' shows up as a device since 2.2.0, but can
> @@ -4410,6 +4475,9 @@ virQEMUCapsInitQMPMonitorTCG(virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED,
> if (virQEMUCapsProbeQMPCPUDefinitions(qemuCaps, mon, true) < 0)
> goto cleanup;
>
> + if (virQEMUCapsProbeQMPHostCPU(qemuCaps, mon, true) < 0)
> + goto cleanup;
> +
> ret = 0;
> cleanup:
> return ret;
> @@ -4754,7 +4822,8 @@ virQEMUCapsNewForBinaryInternal(virCapsPtr caps,
> virQEMUCapsRememberCached(qemuCaps, cacheDir) < 0)
> goto error;
>
> - virQEMUCapsInitHostCPUModel(qemuCaps, caps);
> + virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_KVM);
> + virQEMUCapsInitHostCPUModel(qemuCaps, caps, VIR_DOMAIN_VIRT_QEMU);
> }
>
> cleanup:
> @@ -5200,8 +5269,10 @@ virQEMUCapsFillDomainCPUCaps(virCapsPtr caps,
> domCaps->cpu.hostPassthrough = true;
>
> if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype,
> - VIR_CPU_MODE_HOST_MODEL))
> - domCaps->cpu.hostModel = virCPUDefCopy(qemuCaps->hostCPUModel);
> + VIR_CPU_MODE_HOST_MODEL)) {
> + virCPUDefPtr cpu = virQEMUCapsGetHostModel(qemuCaps, domCaps->virttype);
> + domCaps->cpu.hostModel = virCPUDefCopy(cpu);
> + }
>
> if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype,
> VIR_CPU_MODE_CUSTOM)) {
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 95bb67d44..3331142ea 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -441,7 +441,8 @@ int virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
> virDomainVirtType type,
> char ***names,
> size_t *count);
> -virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps);
> +virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps,
> + virDomainVirtType type);
> bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps,
> virCapsPtr caps,
> virDomainVirtType type,
> diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h
> index 38b971e0e..75499d462 100644
> --- a/src/qemu/qemu_capspriv.h
> +++ b/src/qemu/qemu_capspriv.h
> @@ -71,5 +71,6 @@ virQEMUCapsSetArch(virQEMUCapsPtr qemuCaps,
>
> void
> virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
> - virCapsPtr caps);
> + virCapsPtr caps,
> + virDomainVirtType type);
> #endif
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c00a47a91..4e83dee37 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6782,7 +6782,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
> if (def->cpu->mode == VIR_CPU_MODE_CUSTOM)
> cpuDef = def->cpu;
> else if (def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH)
> - cpuDef = virQEMUCapsGetHostModel(qemuCaps);
> + cpuDef = virQEMUCapsGetHostModel(qemuCaps, def->virtType);
>
> if (cpuDef) {
> int svm = virCPUCheckFeature(def->os.arch, cpuDef, "svm");
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 92fa69b3c..b9df01da5 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5147,13 +5147,14 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def,
> /* custom CPUs in TCG mode don't need to be compared to host CPU */
> if (def->virtType != VIR_DOMAIN_VIRT_QEMU ||
> def->cpu->mode != VIR_CPU_MODE_CUSTOM) {
> - if (virCPUCompare(caps->host.arch, virQEMUCapsGetHostModel(qemuCaps),
> + if (virCPUCompare(caps->host.arch,
> + virQEMUCapsGetHostModel(qemuCaps, def->virtType),
THis is about where I started wondering about a NULL return here and the
"default" of TCG now... Although looking at the code around it, this
would seemingly only be called for passthrough or host-model, true?
> def->cpu, true) < 0)
> return -1;
> }
>
> if (virCPUUpdate(def->os.arch, def->cpu,
> - virQEMUCapsGetHostModel(qemuCaps)) < 0)
> + virQEMUCapsGetHostModel(qemuCaps, def->virtType)) < 0)
> goto cleanup;
>
> if (virQEMUCapsGetCPUDefinitions(qemuCaps, def->virtType,
> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies
> index 0405d5d7b..c3cbeee0a 100644
> --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies
> +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies
> @@ -13378,3 +13378,11 @@
> ],
> "id": "libvirt-2"
> }
> +
> +{
> + "id": "libvirt-3",
> + "error": {
> + "class": "GenericError",
> + "desc": "The CPU definition 'max' is unknown."
> + }
> +}
And this seems to indicate my comments before in
virQEMUCapsProbeQMPHostCPU - if "max" isn't available, but tcg is true
or if !QEMU_CAPS_KVM, then "max" will be tried, which doesn't seem right.
Also does something similar need to be done in caps_2.8.0.x86_64 or does
it not really matter since if it doesn't exist, it wouldn't be defined
or usable.
John
> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
> index 1f652bdc2..df7eb18f6 100644
> --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
> +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml
> @@ -133,7 +133,7 @@
> <kvmVersion>0</kvmVersion>
> <package></package>
> <arch>s390x</arch>
> - <hostCPU model='zEC12.2-base'>
> + <hostCPU type='kvm' model='zEC12.2-base'>
> <property name='aefsi' boolean='yes'/>
> <property name='msa5' boolean='yes'/>
> <property name='msa4' boolean='yes'/>
> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies
> index 8d54788df..390f40f9f 100644
> --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies
> +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.replies
> @@ -14708,3 +14708,182 @@
> ],
> "id": "libvirt-2"
> }
> +
> +{
> + "return": {
> + "model": {
> + "name": "base",
> + "props": {
> + "cmov": true,
> + "ia64": false,
> + "aes": true,
> + "mmx": true,
> + "rdpid": false,
> + "arat": true,
> + "pause-filter": false,
> + "xsavec": false,
> + "osxsave": false,
> + "kvm-asyncpf": false,
> + "perfctr-core": false,
> + "mpx": true,
> + "pbe": false,
> + "avx512cd": false,
> + "decodeassists": false,
> + "sse4.1": true,
> + "family": 6,
> + "avx512f": false,
> + "msr": true,
> + "mce": true,
> + "mca": true,
> + "xcrypt": false,
> + "min-level": 13,
> + "xgetbv1": true,
> + "cid": false,
> + "ds": false,
> + "fxsr": true,
> + "xsaveopt": true,
> + "xtpr": false,
> + "avx512vl": false,
> + "avx512-vpopcntdq": false,
> + "phe": false,
> + "extapic": false,
> + "3dnowprefetch": false,
> + "cr8legacy": true,
> + "xcrypt-en": false,
> + "pn": false,
> + "dca": false,
> + "vendor": "AuthenticAMD",
> + "pku": true,
> + "smx": false,
> + "cmp-legacy": false,
> + "avx512-4fmaps": false,
> + "vmcb-clean": false,
> + "hle": false,
> + "3dnowext": true,
> + "npt": false,
> + "clwb": true,
> + "lbrv": false,
> + "adx": true,
> + "ss": true,
> + "pni": true,
> + "svm-lock": false,
> + "smep": true,
> + "smap": true,
> + "pfthreshold": false,
> + "x2apic": false,
> + "avx512vbmi": false,
> + "flushbyasid": false,
> + "f16c": false,
> + "ace2-en": false,
> + "pae": true,
> + "pat": true,
> + "sse": true,
> + "phe-en": false,
> + "kvm-nopiodelay": false,
> + "tm": false,
> + "kvmclock-stable-bit": false,
> + "hypervisor": true,
> + "pcommit": true,
> + "syscall": true,
> + "avx512dq": false,
> + "svm": true,
> + "invtsc": false,
> + "sse2": true,
> + "est": false,
> + "avx512ifma": false,
> + "tm2": false,
> + "kvm-pv-eoi": false,
> + "cx8": true,
> + "kvm-mmu": false,
> + "sse4.2": true,
> + "pge": true,
> + "pdcm": false,
> + "model": 6,
> + "movbe": true,
> + "nrip-save": false,
> + "ssse3": true,
> + "sse4a": true,
> + "invpcid": false,
> + "pdpe1gb": true,
> + "tsc-deadline": false,
> + "fma": false,
> + "cx16": true,
> + "de": true,
> + "stepping": 3,
> + "xsave": true,
> + "clflush": true,
> + "skinit": false,
> + "tsc": true,
> + "tce": false,
> + "fpu": true,
> + "ds-cpl": false,
> + "ibs": false,
> + "fma4": false,
> + "la57": true,
> + "osvw": false,
> + "apic": true,
> + "pmm": false,
> + "tsc-adjust": false,
> + "kvm-steal-time": false,
> + "kvmclock": false,
> + "lwp": false,
> + "xop": false,
> + "avx": false,
> + "ospke": true,
> + "acpi": true,
> + "avx512bw": false,
> + "ace2": false,
> + "fsgsbase": true,
> + "ht": false,
> + "nx": true,
> + "pclmulqdq": true,
> + "mmxext": true,
> + "popcnt": true,
> + "xsaves": false,
> + "lm": true,
> + "umip": false,
> + "pse": true,
> + "avx2": false,
> + "sep": true,
> + "nodeid-msr": false,
> + "misalignsse": false,
> + "min-xlevel": 2147483658,
> + "bmi1": true,
> + "bmi2": true,
> + "kvm-pv-unhalt": false,
> + "tsc-scale": false,
> + "topoext": false,
> + "clflushopt": true,
> + "monitor": true,
> + "avx512er": false,
> + "pmm-en": false,
> + "pcid": false,
> + "3dnow": true,
> + "erms": true,
> + "lahf-lm": true,
> + "fxsr-opt": false,
> + "xstore": false,
> + "rtm": false,
> + "lmce": false,
> + "perfctr-nb": false,
> + "rdrand": false,
> + "rdseed": false,
> + "avx512-4vnniw": false,
> + "vme": false,
> + "vmx": false,
> + "dtes64": false,
> + "mtrr": true,
> + "rdtscp": true,
> + "pse36": true,
> + "tbm": false,
> + "wdt": false,
> + "model-id": "QEMU TCG CPU version 2.5+",
> + "sha-ni": false,
> + "abm": true,
> + "avx512pf": false,
> + "xstore-en": false
> + }
> + }
> + },
> + "id": "libvirt-3"
> +}
> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
> index 32368e648..520bf80f4 100644
> --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
> +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml
> @@ -205,7 +205,7 @@
> <kvmVersion>0</kvmVersion>
> <package> (v2.8.0-877-g38e4b757b4)</package>
> <arch>x86_64</arch>
> - <hostCPU model='base'>
> + <hostCPU type='kvm' model='base'>
> <property name='cmov' boolean='yes'/>
> <property name='ia64' boolean='no'/>
> <property name='aes' boolean='yes'/>
> @@ -375,6 +375,176 @@
> <property name='avx512pf' boolean='no'/>
> <property name='xstore-en' boolean='no'/>
> </hostCPU>
> + <hostCPU type='tcg' model='base'>
> + <property name='cmov' boolean='yes'/>
> + <property name='ia64' boolean='no'/>
> + <property name='aes' boolean='yes'/>
> + <property name='mmx' boolean='yes'/>
> + <property name='rdpid' boolean='no'/>
> + <property name='arat' boolean='yes'/>
> + <property name='pause-filter' boolean='no'/>
> + <property name='xsavec' boolean='no'/>
> + <property name='osxsave' boolean='no'/>
> + <property name='kvm-asyncpf' boolean='no'/>
> + <property name='perfctr-core' boolean='no'/>
> + <property name='mpx' boolean='yes'/>
> + <property name='pbe' boolean='no'/>
> + <property name='avx512cd' boolean='no'/>
> + <property name='decodeassists' boolean='no'/>
> + <property name='sse4.1' boolean='yes'/>
> + <property name='family' ull='6'/>
> + <property name='avx512f' boolean='no'/>
> + <property name='msr' boolean='yes'/>
> + <property name='mce' boolean='yes'/>
> + <property name='mca' boolean='yes'/>
> + <property name='xcrypt' boolean='no'/>
> + <property name='min-level' ull='13'/>
> + <property name='xgetbv1' boolean='yes'/>
> + <property name='cid' boolean='no'/>
> + <property name='ds' boolean='no'/>
> + <property name='fxsr' boolean='yes'/>
> + <property name='xsaveopt' boolean='yes'/>
> + <property name='xtpr' boolean='no'/>
> + <property name='avx512vl' boolean='no'/>
> + <property name='avx512-vpopcntdq' boolean='no'/>
> + <property name='phe' boolean='no'/>
> + <property name='extapic' boolean='no'/>
> + <property name='3dnowprefetch' boolean='no'/>
> + <property name='cr8legacy' boolean='yes'/>
> + <property name='xcrypt-en' boolean='no'/>
> + <property name='pn' boolean='no'/>
> + <property name='dca' boolean='no'/>
> + <property name='vendor' string='AuthenticAMD'/>
> + <property name='pku' boolean='yes'/>
> + <property name='smx' boolean='no'/>
> + <property name='cmp-legacy' boolean='no'/>
> + <property name='avx512-4fmaps' boolean='no'/>
> + <property name='vmcb-clean' boolean='no'/>
> + <property name='hle' boolean='no'/>
> + <property name='3dnowext' boolean='yes'/>
> + <property name='npt' boolean='no'/>
> + <property name='clwb' boolean='yes'/>
> + <property name='lbrv' boolean='no'/>
> + <property name='adx' boolean='yes'/>
> + <property name='ss' boolean='yes'/>
> + <property name='pni' boolean='yes'/>
> + <property name='svm-lock' boolean='no'/>
> + <property name='smep' boolean='yes'/>
> + <property name='smap' boolean='yes'/>
> + <property name='pfthreshold' boolean='no'/>
> + <property name='x2apic' boolean='no'/>
> + <property name='avx512vbmi' boolean='no'/>
> + <property name='flushbyasid' boolean='no'/>
> + <property name='f16c' boolean='no'/>
> + <property name='ace2-en' boolean='no'/>
> + <property name='pae' boolean='yes'/>
> + <property name='pat' boolean='yes'/>
> + <property name='sse' boolean='yes'/>
> + <property name='phe-en' boolean='no'/>
> + <property name='kvm-nopiodelay' boolean='no'/>
> + <property name='tm' boolean='no'/>
> + <property name='kvmclock-stable-bit' boolean='no'/>
> + <property name='hypervisor' boolean='yes'/>
> + <property name='pcommit' boolean='yes'/>
> + <property name='syscall' boolean='yes'/>
> + <property name='avx512dq' boolean='no'/>
> + <property name='svm' boolean='yes'/>
> + <property name='invtsc' boolean='no'/>
> + <property name='sse2' boolean='yes'/>
> + <property name='est' boolean='no'/>
> + <property name='avx512ifma' boolean='no'/>
> + <property name='tm2' boolean='no'/>
> + <property name='kvm-pv-eoi' boolean='no'/>
> + <property name='cx8' boolean='yes'/>
> + <property name='kvm-mmu' boolean='no'/>
> + <property name='sse4.2' boolean='yes'/>
> + <property name='pge' boolean='yes'/>
> + <property name='pdcm' boolean='no'/>
> + <property name='model' ull='6'/>
> + <property name='movbe' boolean='yes'/>
> + <property name='nrip-save' boolean='no'/>
> + <property name='ssse3' boolean='yes'/>
> + <property name='sse4a' boolean='yes'/>
> + <property name='invpcid' boolean='no'/>
> + <property name='pdpe1gb' boolean='yes'/>
> + <property name='tsc-deadline' boolean='no'/>
> + <property name='fma' boolean='no'/>
> + <property name='cx16' boolean='yes'/>
> + <property name='de' boolean='yes'/>
> + <property name='stepping' ull='3'/>
> + <property name='xsave' boolean='yes'/>
> + <property name='clflush' boolean='yes'/>
> + <property name='skinit' boolean='no'/>
> + <property name='tsc' boolean='yes'/>
> + <property name='tce' boolean='no'/>
> + <property name='fpu' boolean='yes'/>
> + <property name='ds-cpl' boolean='no'/>
> + <property name='ibs' boolean='no'/>
> + <property name='fma4' boolean='no'/>
> + <property name='la57' boolean='yes'/>
> + <property name='osvw' boolean='no'/>
> + <property name='apic' boolean='yes'/>
> + <property name='pmm' boolean='no'/>
> + <property name='tsc-adjust' boolean='no'/>
> + <property name='kvm-steal-time' boolean='no'/>
> + <property name='kvmclock' boolean='no'/>
> + <property name='lwp' boolean='no'/>
> + <property name='xop' boolean='no'/>
> + <property name='avx' boolean='no'/>
> + <property name='ospke' boolean='yes'/>
> + <property name='acpi' boolean='yes'/>
> + <property name='avx512bw' boolean='no'/>
> + <property name='ace2' boolean='no'/>
> + <property name='fsgsbase' boolean='yes'/>
> + <property name='ht' boolean='no'/>
> + <property name='nx' boolean='yes'/>
> + <property name='pclmulqdq' boolean='yes'/>
> + <property name='mmxext' boolean='yes'/>
> + <property name='popcnt' boolean='yes'/>
> + <property name='xsaves' boolean='no'/>
> + <property name='lm' boolean='yes'/>
> + <property name='umip' boolean='no'/>
> + <property name='pse' boolean='yes'/>
> + <property name='avx2' boolean='no'/>
> + <property name='sep' boolean='yes'/>
> + <property name='nodeid-msr' boolean='no'/>
> + <property name='misalignsse' boolean='no'/>
> + <property name='min-xlevel' ull='2147483658'/>
> + <property name='bmi1' boolean='yes'/>
> + <property name='bmi2' boolean='yes'/>
> + <property name='kvm-pv-unhalt' boolean='no'/>
> + <property name='tsc-scale' boolean='no'/>
> + <property name='topoext' boolean='no'/>
> + <property name='clflushopt' boolean='yes'/>
> + <property name='monitor' boolean='yes'/>
> + <property name='avx512er' boolean='no'/>
> + <property name='pmm-en' boolean='no'/>
> + <property name='pcid' boolean='no'/>
> + <property name='3dnow' boolean='yes'/>
> + <property name='erms' boolean='yes'/>
> + <property name='lahf-lm' boolean='yes'/>
> + <property name='fxsr-opt' boolean='no'/>
> + <property name='xstore' boolean='no'/>
> + <property name='rtm' boolean='no'/>
> + <property name='lmce' boolean='no'/>
> + <property name='perfctr-nb' boolean='no'/>
> + <property name='rdrand' boolean='no'/>
> + <property name='rdseed' boolean='no'/>
> + <property name='avx512-4vnniw' boolean='no'/>
> + <property name='vme' boolean='no'/>
> + <property name='vmx' boolean='no'/>
> + <property name='dtes64' boolean='no'/>
> + <property name='mtrr' boolean='yes'/>
> + <property name='rdtscp' boolean='yes'/>
> + <property name='pse36' boolean='yes'/>
> + <property name='tbm' boolean='no'/>
> + <property name='wdt' boolean='no'/>
> + <property name='model-id' string='QEMU TCG CPU version 2.5+'/>
> + <property name='sha-ni' boolean='no'/>
> + <property name='abm' boolean='yes'/>
> + <property name='avx512pf' boolean='no'/>
> + <property name='xstore-en' boolean='no'/>
> + </hostCPU>
> <cpu type='kvm' name='max' usable='yes'/>
> <cpu type='kvm' name='host' usable='yes'/>
> <cpu type='kvm' name='base' usable='yes'/>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index faa99c64c..5ba2eeab9 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -384,7 +384,8 @@ testUpdateQEMUCaps(const struct testInfo *info,
> if (testAddCPUModels(info->qemuCaps, info->skipLegacyCPUs) < 0)
> goto cleanup;
>
> - virQEMUCapsInitHostCPUModel(info->qemuCaps, caps);
> + virQEMUCapsInitHostCPUModel(info->qemuCaps, caps, VIR_DOMAIN_VIRT_KVM);
> + virQEMUCapsInitHostCPUModel(info->qemuCaps, caps, VIR_DOMAIN_VIRT_QEMU);
>
> virQEMUCapsFilterByMachineType(info->qemuCaps, vm->def->os.machine);
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Feb 21, 2017 at 12:24:19 -0500, John Ferlan wrote:
>
>
> On 02/15/2017 11:44 AM, Jiri Denemark wrote:
> > Querying "host" CPU model expansion only makes sense for KVM. QEMU 2.9.0
> > introduces a new "max" CPU model which can be used to ask QEMU what the
> > best CPU it can provide to a TCG domain is.
>
> But how do we know "max" is available to be used for TCG? It seems to
> me that virQEMUCapsProbeQMPHostCPU now is changed such that there's an
> assumption that "max" is available, but the above makes it appear as if
> it's a new paremter/attribute [1].
We don't know if max is available, we just try to probe it. And if it
succeeds we know it's available and we can use the returned CPU model.
If max model is not available, the code does the same thing it did
before this patch series.
> Every time I see TCG I think "Trusted Computing Group" and I certainly
> don't think VIR_DOMAIN_TYPE_QEMU. What gets "confusing" eventually is
> that virQEMUCapsInitHostCPUModel is called with VIR_DOMAIN_VIRT_KVM and
> VIR_DOMAIN_VIRT_QEMU, but qemuCaps->tcgCPUModel is filled in for
> VIR_DOMAIN_VIRT_QEMU.
"tcg" and "kvm" is the QEMU's terminology while in libvirt we use
VIR_DOMAIN_TYPE_QEMU and VIR_DOMAIN_VIRT_KVM to distinguish between the
two types of domains our QEMU driver supports. Both are clear, but
VIR_DOMAIN_TYPE_QEMU is pretty long and shortening it to "qemu" and
"kvm" would be very confusing. That's why I use the QEMU's terminology
when referring to the types of domains.
> It's not overtly obvious to me from just reading the commit message, but
> it seems TCG now becomes a fallback position of sorts which I think is
> fine, it's just not described.
It's not described because it doesn't happen. Both the supported CPU
models and the host CPU model depend on the accelerator (tcg/kvm) so we
need to probe QEMU for both. We only probe kvm if it's available, but
tcg is always checked.
...
> > @@ -2822,14 +2838,24 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps,
> >
> > static int
> > virQEMUCapsProbeQMPHostCPU(virQEMUCapsPtr qemuCaps,
> > - qemuMonitorPtr mon)
> > + qemuMonitorPtr mon,
> > + bool tcg)
> > {
> > - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION) ||
> > - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
> > + qemuMonitorCPUModelInfoPtr *modelInfo;
> > + const char *model;
> > +
> > + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION))
> > return 0;
>
> So if no model expansion, then neither is filled in, but there are later
> checks which seem to assume one or the other is filled in. I didn't
> chase all the callers to check whether they would expect/require
> something to be filled in (too many patches, too little time).
Nope, none of them needs to be set. The code will fallback to using the
host CPU model from host capabilities probed directly by libvirt. The
only users of the {kvm,tcg}CPUModelInfo are virQEMUCapsInitCPUModel*
which transform the raw data from QEMU into virCPUDefPtr
{kvm,tcg}CPUModel.
> > - return qemuMonitorGetCPUModelExpansion(mon, "static", "host",
> > - &qemuCaps->hostCPUModelInfo);
> > + if (tcg || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) {
>
> [1]
> It's noot clear to the casual reviewer when tcg and in particular the
> "max" parameter was supported. IOW: The first thought I had was why
> isn't there a capability being checked for "max" being available. The
> commit message seems to indicate that it's new for 2.9.0, but I don't
> see a libvirt capability check for it being available before being used.
As already said, if max is not available, tcgCPUModelInfo will be NULL,
virQEMUCapsInitCPUModel will return 1, and virQEMUCapsInitHostCPUModel
will take the ``VIR_DEBUG("No host CPU model info from QEMU; using host
capabilities");'' branch.
> The logic changes now such that if !QEMU_CAPS_KVM that model expansion
> is called with "max"; whereas, previously a 0 was returned.
If "max" or query-cpu-model-expansion are not available,
qemuMonitorJSONGetCPUModelExpansion will return 0, which is exactly what
happened before. If both are available, we'll get the description of the
"max" CPU model in tcgCPUModelInfo.
> Thus if !tcg and !QEMU_CAPS_KVM, then "max" is used and ModelExpansion
> is called. If TCG becomes the "de facto" default -
virQEMUCapsProbeQMPHostCPU may be called twice. First time it is called
with tcg == false and it uses the QEMU_CAPS_KVM capability to see
whether it is probing QEMU using tcg or kvm accelerator. Now if kvm was
not available we have tcgCPUModelInfo filled in (if max is available, of
course) and we're done. If kvm was available, we have kvmCPUModelInfo
filled in (if model expansion is available) and
virQEMUCapsProbeQMPHostCPU will be called once more with tcg == true to
try to set tcgCPUModelInfo.
...
> > @@ -3560,12 +3607,26 @@ virQEMUCapsLoadCache(virCapsPtr caps,
> >
> > static void
> > virQEMUCapsFormatHostCPUModelInfo(virQEMUCapsPtr qemuCaps,
> > - virBufferPtr buf)
> > + virBufferPtr buf,
> > + virDomainVirtType type)
> > {
> > - qemuMonitorCPUModelInfoPtr model = qemuCaps->hostCPUModelInfo;
> > + qemuMonitorCPUModelInfoPtr model;
> > + const char *typeStr;
> > size_t i;
> >
> > - virBufferAsprintf(buf, "<hostCPU model='%s'>\n", model->name);
> > + if (type == VIR_DOMAIN_VIRT_KVM) {
> > + typeStr = "kvm";
> > + model = qemuCaps->kvmCPUModelInfo;
> > + } else {
> > + typeStr = "tcg";
> > + model = qemuCaps->tcgCPUModelInfo;
> > + }
> > +
> > + if (!model)
> > + return;
>
> if !model can happen, how does that affect other places which assume
> that are making a similar check to decide which ModelInfo to use?
!model means we cannot use the "host" or "max" CPU model data from QEMU
and thus the CPU model from host capabilities XML will be used instead.
> Also this is "similar" to virQEMUCapsGetHostModel at least for 'model'...
It's not. Are you confusing *CPUModelInfo with *CPUModel? The first one
is the raw data from QEMU while the second one is virCPUDefPtr. Only the
latter is ever used anywhere in the code. The raw data gets only stored
in the capabilities cache and used for creating the virCPUDefPtr.
...
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > index 92fa69b3c..b9df01da5 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
> > @@ -5147,13 +5147,14 @@ qemuProcessUpdateGuestCPU(virDomainDefPtr def,
> > /* custom CPUs in TCG mode don't need to be compared to host CPU */
> > if (def->virtType != VIR_DOMAIN_VIRT_QEMU ||
> > def->cpu->mode != VIR_CPU_MODE_CUSTOM) {
> > - if (virCPUCompare(caps->host.arch, virQEMUCapsGetHostModel(qemuCaps),
> > + if (virCPUCompare(caps->host.arch,
> > + virQEMUCapsGetHostModel(qemuCaps, def->virtType),
>
> THis is about where I started wondering about a NULL return here and the
> "default" of TCG now...
There's no "default" of TCG. According to the requested virtType we will
either use kvmCPUModel or tcgCPUModel. Either of them can either be just
a copy of the host CPU model from host capabilities or a model we got by
querying QEMU (i.e., asking for a model expansion of either "host" or
"max") or it can even be NULL. However, NULL means we cannot detect host
CPU model and thus host-model CPU mode is not supported. In any case
virCPUCompare will report an appropriate error if it founds host CPU
model is not set but required to do the job.
> Although looking at the code around it, this would seemingly only be
> called for passthrough or host-model, true?
Nope. It's never called for host-passthrough. It can only be called for
either host-model or custom CPUs.
...
> > diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies
> > index 0405d5d7b..c3cbeee0a 100644
> > --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies
> > +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.replies
> > @@ -13378,3 +13378,11 @@
> > ],
> > "id": "libvirt-2"
> > }
> > +
> > +{
> > + "id": "libvirt-3",
> > + "error": {
> > + "class": "GenericError",
> > + "desc": "The CPU definition 'max' is unknown."
> > + }
> > +}
>
> And this seems to indicate my comments before in
> virQEMUCapsProbeQMPHostCPU - if "max" isn't available, but tcg is true
> or if !QEMU_CAPS_KVM, then "max" will be tried, which doesn't seem right.
Yes, it will be tried and the error will be ignored.
> Also does something similar need to be done in caps_2.8.0.x86_64 or
> does it not really matter since if it doesn't exist, it wouldn't be
> defined or usable.
QEMU 2.8.0 on x86_64 does not have QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION
set and thus we will never ask it for any model expansion.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.