It may come handy to be able to tweak TCG options, in this specific case the size of translation block cache size (tb-size). Since we can expect more knobs to tweak let's put them under common element, like this: <domain> <features> <tcg> <tb-cache unit='MiB'>128</tb-cache> </tcg> </features> </domain> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/formatdomain.rst | 11 +++ docs/schemas/domaincommon.rng | 15 +++- src/conf/domain_conf.c | 90 +++++++++++++++++++ src/conf/domain_conf.h | 7 ++ src/qemu/qemu_validate.c | 11 +++ .../x86_64-default-cpu-tcg-features.xml | 67 ++++++++++++++ ...default-cpu-tcg-features.x86_64-latest.xml | 1 + tests/qemuxml2xmltest.c | 1 + 8 files changed, 202 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/x86_64-default-cpu-tcg-features.xml create mode 120000 tests/qemuxml2xmloutdata/x86_64-default-cpu-tcg-features.x86_64-latest.xml diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 0651975c88..d7f4f093fb 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1864,6 +1864,9 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. <cfpc value='workaround'/> <sbbc value='workaround'/> <ibs value='fixed-na'/> + <tcg> + <tb-cache unit='MiB'>128</tb-cache> + </tcg> </features> ... @@ -2065,6 +2068,14 @@ are: ``fixed-na (fixed in hardware - no longer applicable)``. If the attribute is not defined, the hypervisor default will be used. :since:`Since 6.3.0` (QEMU/KVM only) +``tcg`` + Various features to change the behavior of the TCG accelerator. + + =========== ============================================== =================================================== ============== + Feature Description Value Since + =========== ============================================== =================================================== ============== + tb-cache The size of translation block cache size an integer :since:`7.10.0` + =========== ============================================== =================================================== ============== :anchor:`<a id="elementsTime"/>` diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 67df13d90d..73401e6b75 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6062,7 +6062,7 @@ </element> </define> <!-- - A set of optional features: PAE, APIC, ACPI, GIC, + A set of optional features: PAE, APIC, ACPI, GIC, TCG, HyperV Enlightenment, KVM features, paravirtual spinlocks and HAP support --> <define name="features"> @@ -6202,6 +6202,9 @@ <optional> <ref name="ibs"/> </optional> + <optional> + <ref name="tcgfeatures"/> + </optional> </interleave> </element> </optional> @@ -6490,6 +6493,16 @@ </element> </define> + <define name="tcgfeatures"> + <element name="tcg"> + <optional> + <element name="tb-cache"> + <ref name="scaledInteger"/> + </element> + </optional> + </element> + </define> + <define name="address"> <element name="address"> <choice> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4644d18120..2db013cab4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -172,6 +172,7 @@ VIR_ENUM_IMPL(virDomainFeature, "cfpc", "sbbc", "ibs", + "tcg", ); VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, @@ -17650,6 +17651,30 @@ virDomainFeaturesCapabilitiesDefParse(virDomainDef *def, } +static int +virDomainFeaturesTCGDefParse(virDomainDef *def, + xmlXPathContextPtr ctxt, + xmlNodePtr node) +{ + g_autofree virDomainFeatureTCG *tcg = NULL; + VIR_XPATH_NODE_AUTORESTORE(ctxt); + + tcg = g_new0(virDomainFeatureTCG, 1); + ctxt->node = node; + + if (virDomainParseMemory("./tb-cache", "./tb-cache/@unit", + ctxt, &tcg->tb_cache, false, false) < 0) + return -1; + + if (tcg->tb_cache == 0) + return 0; + + def->features[VIR_DOMAIN_FEATURE_TCG] = VIR_TRISTATE_SWITCH_ON; + def->tcg_features = g_steal_pointer(&tcg); + return 0; +} + + static int virDomainFeaturesDefParse(virDomainDef *def, xmlXPathContextPtr ctxt) @@ -17858,6 +17883,11 @@ virDomainFeaturesDefParse(virDomainDef *def, break; } + case VIR_DOMAIN_FEATURE_TCG: + if (virDomainFeaturesTCGDefParse(def, ctxt, nodes[i]) < 0) + return -1; + break; + case VIR_DOMAIN_FEATURE_LAST: break; } @@ -21526,6 +21556,39 @@ virDomainRedirFilterDefCheckABIStability(virDomainRedirFilterDef *src, } +static bool +virDomainFeatureTCGCheckABIStability(const virDomainDef *src, + const virDomainDef *dst) +{ + const int srcF = src->features[VIR_DOMAIN_FEATURE_TCG]; + const int dstF = dst->features[VIR_DOMAIN_FEATURE_TCG]; + + if (srcF != dstF || + !!src->tcg_features != !!dst->tcg_features) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("State of feature '%s' differs: " + "source: '%s', destination: '%s'"), + virDomainFeatureTypeToString(VIR_DOMAIN_FEATURE_TCG), + virTristateSwitchTypeToString(srcF), + virTristateSwitchTypeToString(dstF)); + return false; + } + + if (!src->tcg_features && !dst->tcg_features) + return true; + + if (src->tcg_features->tb_cache != dst->tcg_features->tb_cache) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("TCG tb-cache mismatch: source %llu, destination: %llu"), + src->tcg_features->tb_cache, + dst->tcg_features->tb_cache); + return false; + } + + return true; +} + + static bool virDomainDefFeaturesCheckABIStability(virDomainDef *src, virDomainDef *dst) @@ -21676,6 +21739,11 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_FEATURE_MSRS: break; + case VIR_DOMAIN_FEATURE_TCG: + if (!virDomainFeatureTCGCheckABIStability(src, dst)) + return false; + break; + case VIR_DOMAIN_FEATURE_LAST: break; } @@ -27645,6 +27713,24 @@ virDomainDefFormatBlkiotune(virBuffer *buf, } +static void +virDomainFeatureTCGFormat(virBuffer *buf, + const virDomainDef *def) +{ + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + + if (!def->tcg_features || + def->features[VIR_DOMAIN_FEATURE_TCG] != VIR_TRISTATE_SWITCH_ON) + return; + + virBufferAsprintf(&childBuf, + "<tb-cache unit='KiB'>%lld</tb-cache>\n", + def->tcg_features->tb_cache); + + virXMLFormatElement(buf, "tcg", NULL, &childBuf); +} + + static int virDomainDefFormatFeatures(virBuffer *buf, virDomainDef *def) @@ -27965,6 +28051,10 @@ virDomainDefFormatFeatures(virBuffer *buf, virDomainIBSTypeToString(def->features[i])); break; + case VIR_DOMAIN_FEATURE_TCG: + virDomainFeatureTCGFormat(&childBuf, def); + break; + case VIR_DOMAIN_FEATURE_LAST: break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cb6d8975b8..e2287b3be1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2044,6 +2044,7 @@ typedef enum { VIR_DOMAIN_FEATURE_CFPC, VIR_DOMAIN_FEATURE_SBBC, VIR_DOMAIN_FEATURE_IBS, + VIR_DOMAIN_FEATURE_TCG, VIR_DOMAIN_FEATURE_LAST } virDomainFeature; @@ -2252,6 +2253,11 @@ typedef enum { VIR_ENUM_DECL(virDomainIBS); +typedef struct _virDomainFeatureTCG virDomainFeatureTCG; +struct _virDomainFeatureTCG { + unsigned long long tb_cache; /* Stored in KiB */ +}; + /* Operating system configuration data & machine / arch */ struct _virDomainOSEnv { char *name; @@ -2814,6 +2820,7 @@ struct _virDomainDef { unsigned long long hpt_maxpagesize; /* Stored in KiB */ char *hyperv_vendor_id; virTristateSwitch apic_eoi; + virDomainFeatureTCG *tcg_features; bool tseg_specified; unsigned long long tseg_size; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index c4384dbe8b..5ee4d3ffcb 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -294,6 +294,17 @@ qemuValidateDomainDefFeatures(const virDomainDef *def, } break; + case VIR_DOMAIN_FEATURE_TCG: + if (def->features[i] == VIR_TRISTATE_SWITCH_ON) { + if (def->virtType != VIR_DOMAIN_VIRT_QEMU) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("TCG features are incompatible with domain type '%s'"), + virDomainVirtTypeToString(def->virtType)); + return -1; + } + } + break; + case VIR_DOMAIN_FEATURE_SMM: case VIR_DOMAIN_FEATURE_KVM: case VIR_DOMAIN_FEATURE_XEN: diff --git a/tests/qemuxml2argvdata/x86_64-default-cpu-tcg-features.xml b/tests/qemuxml2argvdata/x86_64-default-cpu-tcg-features.xml new file mode 100644 index 0000000000..e2058487b2 --- /dev/null +++ b/tests/qemuxml2argvdata/x86_64-default-cpu-tcg-features.xml @@ -0,0 +1,67 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>4194304</memory> + <currentMemory unit='KiB'>4194304</currentMemory> + <vcpu placement='static'>4</vcpu> + <os> + <type arch='x86_64' machine='pc-q35-6.2'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <tcg> + <tb-cache unit='KiB'>102400</tb-cache> + </tcg> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/guest.qcow2'/> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> + </disk> + <controller type='usb' index='0' model='qemu-xhci'> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='sata' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> + </controller> + <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> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='3' port='0xa'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='4' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='4' port='0xb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x3'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/x86_64-default-cpu-tcg-features.x86_64-latest.xml b/tests/qemuxml2xmloutdata/x86_64-default-cpu-tcg-features.x86_64-latest.xml new file mode 120000 index 0000000000..8226a54027 --- /dev/null +++ b/tests/qemuxml2xmloutdata/x86_64-default-cpu-tcg-features.x86_64-latest.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/x86_64-default-cpu-tcg-features.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index a066c35db0..8240982470 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1405,6 +1405,7 @@ mymain(void) 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"); DO_TEST_CAPS_ARCH_LATEST("x86_64-default-cpu-tcg-q35-4.2", "x86_64"); + DO_TEST_CAPS_ARCH_LATEST("x86_64-default-cpu-tcg-features", "x86_64"); DO_TEST_CAPS_LATEST("virtio-9p-multidevs"); DO_TEST_CAPS_LATEST("virtio-9p-createmode"); -- 2.32.0

On Thu, Nov 04, 2021 at 01:22:53PM +0100, Michal Privoznik wrote: The "-machine accel=nnn" syntax was the original way to configure accelerator. The "-accel nnn,$opts" syntax was added in QEMU 2.9.0 with commit 8d4e9146b3568022ea5730d92841345d41275d66 Author: KONRAD Frederic <fred.konrad(a)greensocs.com&gt; Date: Thu Feb 23 18:29:08 2017 +0000 tcg: add options for enabling MTTCG with the accelerator name being sugar for "-machine accel=nnn". Then in 5.0.0, this was reversed so that '-machine accel=nnn' is now sugar for '-accel nnn' commit 6f6e1698a68ceb49e57676528612f22eaf2c16c3 Author: Paolo Bonzini <pbonzini(a)redhat.com&gt; Date: Wed Nov 13 10:10:47 2019 +0100 vl: configure accelerators from -accel options IOW, long term we want to be using -accel exclusively. Given our minimum QEMU 2.11.0, we can in fact use -accel exclusively already AFAICT. That would simplify this patch so that we're not supporting distinct syntax with tb_cache. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|