[RFC PATCH v2 5/5] Support 'value' attribute on CPU features

From: Sandesh Patel <sandesh.patel@nutanix.com> ARM CPU features exposed by QEMU are not pure booleans: each property may take one of several QEMU-defined values (e.g. "1.0", "1.1_base", "off"). To let users request a specific value, extend the guest CPU XML <feature> element with an optional 'value' attribute: <feature policy='require' name='feat_RAS' value='1.0'/> Signed-off-by: Sandesh Patel <sandesh.patel@nutanix.com> --- src/conf/cpu_conf.c | 21 ++- src/conf/cpu_conf.h | 1 + src/conf/schemas/cputypes.rng | 5 + src/cpu_map/arm_features.xml | 171 ++++++++++++++++++ src/qemu/qemu_command.c | 13 ++ ...res-values.aarch64-latest+armcpuprops.args | 34 ++++ ...ures-values.aarch64-latest+armcpuprops.xml | 46 +++++ .../aarch64-cpu-features-values.xml | 30 +++ tests/qemuxmlconftest.c | 2 + tests/testutilshostcpus.h | 74 ++++---- 10 files changed, 357 insertions(+), 40 deletions(-) create mode 100644 tests/qemuxmlconfdata/aarch64-cpu-features-values.aarch64-latest+armcpuprops.args create mode 100644 tests/qemuxmlconfdata/aarch64-cpu-features-values.aarch64-latest+armcpuprops.xml create mode 100644 tests/qemuxmlconfdata/aarch64-cpu-features-values.xml diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index f5a2004ee6..bd23a1cf47 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -101,8 +101,10 @@ virCPUDefFreeFeatures(virCPUDef *def) { size_t i; - for (i = 0; i < def->nfeatures; i++) + for (i = 0; i < def->nfeatures; i++) { VIR_FREE(def->features[i].name); + VIR_FREE(def->features[i].value); + } VIR_FREE(def->features); def->nfeatures = def->nfeatures_max = 0; @@ -184,6 +186,7 @@ virCPUDefCopyModelFilter(virCPUDef *dst, } dst->features[n].name = g_strdup(src->features[i].name); + dst->features[n].value = g_strdup(src->features[i].value); } } @@ -652,6 +655,7 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt, def->features[i].name = g_steal_pointer(&name); def->features[i].policy = policy; + def->features[i].value = virXMLPropString(nodes[i], "value"); } if ((ncacheNodes = virXPathNodeSet("./cache", ctxt, &cacheNodes)) > 0) { @@ -894,12 +898,17 @@ virCPUDefFormatBuf(virBuffer *buf, feature->policy); return -1; } - virBufferAsprintf(buf, "<feature policy='%s' name='%s'/>\n", + virBufferAsprintf(buf, "<feature policy='%s' name='%s'", policy, feature->name); } else { - virBufferAsprintf(buf, "<feature name='%s'/>\n", + virBufferAsprintf(buf, "<feature name='%s'", feature->name); } + + if (feature->value) + virBufferEscapeString(buf, " value='%s'", feature->value); + + virBufferAddLit(buf, "/>\n"); } return 0; @@ -1194,6 +1203,12 @@ virCPUDefIsEqual(virCPUDef *src, virCPUFeaturePolicyTypeToString(src->features[i].policy)); return false; } + + if (STRNEQ_NULLABLE(src->features[i].value, dst->features[i].value)) { + MISMATCH(_("Target CPU feature value %1$s does not match source %2$s"), + NULLSTR(dst->features[i].value), NULLSTR(src->features[i].value)); + return false; + } } if ((src->cache && !dst->cache) || diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 0cac1a1489..c0bab0765f 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -96,6 +96,7 @@ typedef struct _virCPUFeatureDef virCPUFeatureDef; struct _virCPUFeatureDef { char *name; int policy; /* enum virCPUFeaturePolicy */ + char *value; /* optional property value; currently only used by ARM CPU features */ }; diff --git a/src/conf/schemas/cputypes.rng b/src/conf/schemas/cputypes.rng index 8edf1d14e3..e50fb2c385 100644 --- a/src/conf/schemas/cputypes.rng +++ b/src/conf/schemas/cputypes.rng @@ -78,6 +78,11 @@ <attribute name="name"> <ref name="featureName"/> </attribute> + <optional> + <attribute name="value"> + <text/> + </attribute> + </optional> <empty/> </element> </define> diff --git a/src/cpu_map/arm_features.xml b/src/cpu_map/arm_features.xml index fc80a3b5bd..bfeaa244dd 100644 --- a/src/cpu_map/arm_features.xml +++ b/src/cpu_map/arm_features.xml @@ -22,4 +22,175 @@ <!-- Pointer authentication --> <feature name='pauth'/> + <!-- + ARM CPU properties exposed by QEMU. Names match the QMP property + names returned by query-arm-cpu-props-info + --> + <feature name='feat_AES'/> + <feature name='feat_SHA1'/> + <feature name='feat_SHA2'/> + <feature name='feat_CRC32'/> + <feature name='feat_ATOMIC'/> + <feature name='feat_RDM'/> + <feature name='feat_SHA3'/> + <feature name='feat_SM3'/> + <feature name='feat_SM4'/> + <feature name='feat_DP'/> + <feature name='feat_FHM'/> + <feature name='feat_TS'/> + <feature name='feat_TLB'/> + <feature name='feat_RNDR'/> + <feature name='feat_DPB'/> + <feature name='hw_prop_APA'/> + <feature name='hw_prop_API'/> + <feature name='feat_JSCVT'/> + <feature name='feat_FCMA'/> + <feature name='feat_LRCPC'/> + <feature name='hw_prop_GPA'/> + <feature name='hw_prop_GPI'/> + <feature name='feat_FRINTTS'/> + <feature name='feat_SB'/> + <feature name='feat_SPECRES'/> + <feature name='feat_BF16'/> + <feature name='feat_DGH'/> + <feature name='feat_I8MM'/> + <feature name='feat_XS'/> + <feature name='feat_LS64'/> + <feature name='feat_WFXT'/> + <feature name='feat_RPRES'/> + <feature name='hw_prop_GPA3'/> + <feature name='hw_prop_APA3'/> + <feature name='feat_MOPS'/> + <feature name='feat_BC'/> + <feature name='feat_CLRBHB'/> + <feature name='feat_RPRFM'/> + <feature name='feat_CSSC'/> + <feature name='feat_LUT'/> + <feature name='feat_FAMINMAX'/> + <feature name='feat_LSFE'/> + <feature name='feat_FPRCVT'/> + <feature name='el0_mode'/> + <feature name='el1_mode'/> + <feature name='el2_mode'/> + <feature name='el3_mode'/> + <feature name='feat_FP'/> + <feature name='feat_AdvSIMD'/> + <feature name='feat_SVE'/> + <feature name='feat_SEL2'/> + <feature name='feat_AMU'/> + <feature name='feat_DIT'/> + <feature name='feat_CSV3'/> + <feature name='feat_BT'/> + <feature name='feat_SSBS'/> + <feature name='feat_MTE'/> + <feature name='feat_NMI'/> + <feature name='feat_MTE_FRAC'/> + <feature name='feat_GCS'/> + <feature name='feat_DF2'/> + <feature name='feat_MTESTOREONLY'/> + <feature name='feat_MTEFAR'/> + <feature name='feat_FPMR'/> + <feature name='hw_prop_PARANGE'/> + <feature name='hw_prop_ASIDBITS'/> + <feature name='feat_BIGEND'/> + <feature name='feat_SNSMEM'/> + <feature name='feat_BIGENDEL0'/> + <feature name='hw_prop_TGRAN16'/> + <feature name='hw_prop_TGRAN64'/> + <feature name='hw_prop_TGRAN4'/> + <feature name='hw_prop_TGRAN16_2'/> + <feature name='hw_prop_TGRAN64_2'/> + <feature name='hw_prop_TGRAN4_2'/> + <feature name='feat_EXS'/> + <feature name='feat_FGT'/> + <feature name='feat_ECV'/> + <feature name='feat_HAFDBS'/> + <feature name='hw_prop_VMIDBITS'/> + <feature name='feat_VH'/> + <feature name='feat_HPDS'/> + <feature name='feat_LO'/> + <feature name='feat_PAN'/> + <feature name='feat_SpecSEI'/> + <feature name='feat_XNX'/> + <feature name='feat_TWED'/> + <feature name='feat_ETS'/> + <feature name='feat_HCX'/> + <feature name='feat_AFP'/> + <feature name='feat_TIDCP1'/> + <feature name='feat_ECBHB'/> + <feature name='feat_CNP'/> + <feature name='feat_UAO'/> + <feature name='feat_LSM'/> + <feature name='feat_IESB'/> + <feature name='hw_prop_VARANGE'/> + <feature name='hw_prop_CCIDX'/> + <feature name='hw_prop_ST'/> + <feature name='feat_AT'/> + <feature name='hw_prop_IDS'/> + <feature name='hw_prop_FWB'/> + <feature name='feat_TTL'/> + <feature name='feat_BBM'/> + <feature name='feat_EVT'/> + <feature name='feat_E0PD'/> + <feature name='feat_TCRX'/> + <feature name='feat_SCTLRX'/> + <feature name='feat_S1PIE'/> + <feature name='feat_S1POE'/> + <feature name='feat_E2H0'/> + <feature name='feat_DBG'/> + <feature name='feat_PMU'/> + <feature name='hw_prop_BRPS'/> + <feature name='hw_prop_WRPs'/> + <feature name='hw_prop_CTX_CMPs'/> + <feature name='hw_prop_PMSVer'/> + <feature name='feat_DoubleLock'/> + <feature name='hw_prop_SVEVer'/> + <feature name='feat_SVE_AES'/> + <feature name='feat_SVE_ELTPERM'/> + <feature name='feat_SVE_BITPERM'/> + <feature name='feat_SVE_BFLOAT16'/> + <feature name='feat_SVE_B16B16'/> + <feature name='feat_SVE_SHA3'/> + <feature name='feat_SVE_SM4'/> + <feature name='feat_SVE_I8MM'/> + <feature name='feat_SVE_F16MM'/> + <feature name='feat_SVE_F32MM'/> + <feature name='feat_SVE_F64MM'/> + <feature name='feat_F8E5M2'/> + <feature name='feat_F8E4M3'/> + <feature name='feat_F8MM4'/> + <feature name='feat_F8MM8'/> + <feature name='feat_F8DP2'/> + <feature name='feat_F8DP4'/> + <feature name='feat_F8FMA'/> + <feature name='feat_F8CVT'/> + <feature name='hw_prop_IMInline'/> + <feature name='hw_prop_L1IP'/> + <feature name='hw_prop_DMInline'/> + <feature name='hw_prop_ERG'/> + <feature name='hw_prop_CWG'/> + <feature name='hw_prop_IDC'/> + <feature name='hw_prop_DIC'/> + <feature name='hw_prop_BS'/> + <feature name='hw_prop_DZP'/> + <feature name='cpu_revision'/> + <feature name='cpu_partnum'/> + <feature name='cpu_architecture'/> + <feature name='cpu_variant'/> + <feature name='cpu_implementer'/> + <feature name='cpu_revidr'/> + <feature name='cpu_aidr'/> + <feature name='cache_lvl1_type'/> + <feature name='cache_lvl2_type'/> + <feature name='cache_lvl3_type'/> + <feature name='cache_louis'/> + <feature name='cache_loc'/> + <feature name='cache_louu'/> + <feature name='cache_icb_type'/> + <feature name='cache_ttypen'/> + <feature name='feat_CSV2'/> + <feature name='feat_MPAM'/> + <feature name='feat_RAS'/> + <feature name='feat_NV'/> + </cpus> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e726dc661c..77f925ee2b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6516,6 +6516,19 @@ qemuBuildCpuModelArgStr(virQEMUDriver *driver, const char *featname = virQEMUCapsCPUFeatureToQEMU(def->os.arch, cpu->features[i].name); + /* If an explicit value is provided, policy must be 'force' or 'require'. */ + if (cpu->features[i].value) { + if (cpu->features[i].policy != VIR_CPU_FEATURE_FORCE + && cpu->features[i].policy != VIR_CPU_FEATURE_REQUIRE) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Feature %1$s with explicit values must have policy 'force' or 'require'"), + cpu->features[i].name); + return -1; + } + virBufferAsprintf(buf, ",%s=%s", featname, cpu->features[i].value); + continue; + } + switch ((virCPUFeaturePolicy) cpu->features[i].policy) { case VIR_CPU_FEATURE_FORCE: case VIR_CPU_FEATURE_REQUIRE: diff --git a/tests/qemuxmlconfdata/aarch64-cpu-features-values.aarch64-latest+armcpuprops.args b/tests/qemuxmlconfdata/aarch64-cpu-features-values.aarch64-latest+armcpuprops.args new file mode 100644 index 0000000000..05efb141c7 --- /dev/null +++ b/tests/qemuxmlconfdata/aarch64-cpu-features-values.aarch64-latest+armcpuprops.args @@ -0,0 +1,34 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-aarch64test \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.config \ +/usr/bin/qemu-system-aarch64 \ +-name guest=aarch64test,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-aarch64test/master-key.aes"}' \ +-machine virt,usb=off,gic-version=3,dump-guest-core=off,memory-backend=mach-virt.ram,acpi=off \ +-accel kvm \ +-cpu host,feat_RAS=1.0,hw_prop_API=off,hw_prop_WRPs=2 \ +-m size=1048576k \ +-object '{"qom-type":"memory-backend-ram","id":"mach-virt.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 496d7ea8-9739-544b-4ebd-ef08be936e8b \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=@mon-fd@,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"pcie-root-port","port":8,"chassis":1,"id":"pci.1","bus":"pcie.0","multifunction":true,"addr":"0x1"}' \ +-device '{"driver":"pcie-root-port","port":9,"chassis":2,"id":"pci.2","bus":"pcie.0","addr":"0x1.0x1"}' \ +-blockdev '{"driver":"file","filename":"/aarch64.raw","node-name":"libvirt-1-storage","read-only":false}' \ +-device '{"driver":"virtio-blk-pci","bus":"pci.1","addr":"0x0","drive":"libvirt-1-storage","id":"virtio-disk0","bootindex":1}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/aarch64-cpu-features-values.aarch64-latest+armcpuprops.xml b/tests/qemuxmlconfdata/aarch64-cpu-features-values.aarch64-latest+armcpuprops.xml new file mode 100644 index 0000000000..55b24fb771 --- /dev/null +++ b/tests/qemuxmlconfdata/aarch64-cpu-features-values.aarch64-latest+armcpuprops.xml @@ -0,0 +1,46 @@ +<domain type='kvm'> + <name>aarch64test</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <apic/> + <pae/> + <gic version='3'/> + </features> + <cpu mode='host-passthrough' check='none'> + <feature policy='require' name='feat_RAS' value='1.0'/> + <feature policy='require' name='hw_prop_API' value='off'/> + <feature policy='require' name='hw_prop_WRPs' value='2'/> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='raw'/> + <source file='/aarch64.raw'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </disk> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='2' port='0x9'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <audio id='1' type='none'/> + </devices> +</domain> diff --git a/tests/qemuxmlconfdata/aarch64-cpu-features-values.xml b/tests/qemuxmlconfdata/aarch64-cpu-features-values.xml new file mode 100644 index 0000000000..b3f77b273e --- /dev/null +++ b/tests/qemuxmlconfdata/aarch64-cpu-features-values.xml @@ -0,0 +1,30 @@ +<domain type="kvm"> + <name>aarch64test</name> + <uuid>496d7ea8-9739-544b-4ebd-ef08be936e8b</uuid> + <memory>1048576</memory> + <currentMemory>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch="aarch64" machine="virt">hvm</type> + </os> + <features> + <apic/> + <pae/> + </features> + <cpu mode='host-passthrough'> + <feature policy='require' name='feat_RAS' value='1.0'/> + <feature policy='require' name='hw_prop_API' value='off'/> + <feature policy='require' name='hw_prop_WRPs' value='2'/> + </cpu> + <clock offset="utc"/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <disk type='file' device='disk'> + <source file='/aarch64.raw'/> + <target dev='vda' bus='virtio'/> + </disk> + </devices> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index a1389775b9..eee8e47c0b 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2826,6 +2826,8 @@ mymain(void) DO_TEST_CAPS_ARCH_LATEST("aarch64-video-default", "aarch64"); DO_TEST_CAPS_ARCH_LATEST("aarch64-virt-default-nic", "aarch64"); DO_TEST_CAPS_ARCH_LATEST("aarch64-cpu-passthrough", "aarch64"); + DO_TEST_CAPS_ARCH_LATEST_FULL("aarch64-cpu-features-values", "aarch64", + ARG_CAPS_VARIANT, "+armcpuprops", ARG_END); DO_TEST_GIC("aarch64-gic-none", "latest", GIC_NONE); DO_TEST_GIC("aarch64-gic-none-v2", "latest", GIC_V2); DO_TEST_GIC("aarch64-gic-none-v3", "latest", GIC_V3); diff --git a/tests/testutilshostcpus.h b/tests/testutilshostcpus.h index 36118d6acb..3dd38a585d 100644 --- a/tests/testutilshostcpus.h +++ b/tests/testutilshostcpus.h @@ -21,19 +21,19 @@ #include "util/virarch.h" static virCPUFeatureDef cpuDefaultFeatures[] = { - { (char *) "ds", -1 }, - { (char *) "acpi", -1 }, - { (char *) "ss", -1 }, - { (char *) "ht", -1 }, - { (char *) "tm", -1 }, - { (char *) "pbe", -1 }, - { (char *) "ds_cpl", -1 }, - { (char *) "vmx", -1 }, - { (char *) "est", -1 }, - { (char *) "tm2", -1 }, - { (char *) "cx16", -1 }, - { (char *) "xtpr", -1 }, - { (char *) "lahf_lm", -1 }, + { (char *) "ds", -1, NULL }, + { (char *) "acpi", -1, NULL }, + { (char *) "ss", -1, NULL }, + { (char *) "ht", -1, NULL }, + { (char *) "tm", -1, NULL }, + { (char *) "pbe", -1, NULL }, + { (char *) "ds_cpl", -1, NULL }, + { (char *) "vmx", -1, NULL }, + { (char *) "est", -1, NULL }, + { (char *) "tm2", -1, NULL }, + { (char *) "cx16", -1, NULL }, + { (char *) "xtpr", -1, NULL }, + { (char *) "lahf_lm", -1, NULL }, }; static virCPUDef cpuDefaultData = { .type = VIR_CPU_TYPE_HOST, @@ -49,30 +49,30 @@ static virCPUDef cpuDefaultData = { }; static virCPUFeatureDef cpuHaswellFeatures[] = { - { (char *) "vme", -1 }, - { (char *) "ds", -1 }, - { (char *) "acpi", -1 }, - { (char *) "ss", -1 }, - { (char *) "ht", -1 }, - { (char *) "tm", -1 }, - { (char *) "pbe", -1 }, - { (char *) "dtes64", -1 }, - { (char *) "monitor", -1 }, - { (char *) "ds_cpl", -1 }, - { (char *) "vmx", -1 }, - { (char *) "smx", -1 }, - { (char *) "est", -1 }, - { (char *) "tm2", -1 }, - { (char *) "xtpr", -1 }, - { (char *) "pdcm", -1 }, - { (char *) "osxsave", -1 }, - { (char *) "f16c", -1 }, - { (char *) "rdrand", -1 }, - { (char *) "cmt", -1 }, - { (char *) "pdpe1gb", -1 }, - { (char *) "abm", -1 }, - { (char *) "invtsc", -1 }, - { (char *) "lahf_lm", -1 }, + { (char *) "vme", -1, NULL }, + { (char *) "ds", -1, NULL }, + { (char *) "acpi", -1, NULL }, + { (char *) "ss", -1, NULL }, + { (char *) "ht", -1, NULL }, + { (char *) "tm", -1, NULL }, + { (char *) "pbe", -1, NULL }, + { (char *) "dtes64", -1, NULL }, + { (char *) "monitor", -1, NULL }, + { (char *) "ds_cpl", -1, NULL }, + { (char *) "vmx", -1, NULL }, + { (char *) "smx", -1, NULL }, + { (char *) "est", -1, NULL }, + { (char *) "tm2", -1, NULL }, + { (char *) "xtpr", -1, NULL }, + { (char *) "pdcm", -1, NULL }, + { (char *) "osxsave", -1, NULL }, + { (char *) "f16c", -1, NULL }, + { (char *) "rdrand", -1, NULL }, + { (char *) "cmt", -1, NULL }, + { (char *) "pdpe1gb", -1, NULL }, + { (char *) "abm", -1, NULL }, + { (char *) "invtsc", -1, NULL }, + { (char *) "lahf_lm", -1, NULL }, }; static virCPUDef cpuHaswellData = { .type = VIR_CPU_TYPE_HOST, -- 2.43.7