[PATCH 0/4] Introduce ARM MTE feature
*** BLURB HERE *** Michal Prívozník (4): conf: Introduce MTE domain feature qemu:: Introduce QEMU_CAPS_MACHINE_VIRT_MTE capability qemu: Validate MTE feature qemu: Generate command line for MTE feature docs/formatdomain.rst | 7 +++++ src/conf/domain_conf.c | 6 ++++- src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 5 ++++ src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 6 +++++ src/qemu/qemu_validate.c | 26 +++++++++++++------ .../caps_5.2.0_aarch64.xml | 1 + .../caps_6.0.0_aarch64.xml | 1 + .../caps_6.2.0_aarch64.xml | 1 + .../caps_7.0.0_aarch64+hvf.xml | 1 + .../caps_7.0.0_aarch64.xml | 1 + tests/qemuxml2argvdata/aarch64-gic-v3.args | 2 +- tests/qemuxml2argvdata/aarch64-gic-v3.xml | 1 + .../aarch64-gic-v3.aarch64-latest.xml | 1 + 16 files changed, 53 insertions(+), 10 deletions(-) -- 2.39.3
The Memory Tagging Extensions are hardware acceleration present in some ARM processors that allow memory error detection [1]. Introduce a domain XML knob that turns them on or off. 1: https://www.arm.com/blogs/blueprint/memory-safety-arm-memory-tagging-extensi... Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.rst | 7 +++++++ src/conf/domain_conf.c | 6 +++++- src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 5 +++++ src/qemu/qemu_validate.c | 1 + tests/qemuxml2argvdata/aarch64-gic-v3.xml | 1 + tests/qemuxml2xmloutdata/aarch64-gic-v3.aarch64-latest.xml | 1 + 7 files changed, 21 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 99383e725c..1f52f58d37 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2000,6 +2000,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. <tcg> <tb-cache unit='MiB'>128</tb-cache> </tcg> + <mte state='on'/> </features> ... @@ -2230,6 +2231,12 @@ are: tb-cache The size of translation block cache size an integer (a multiple of MiB) :since:`8.0.0` =========== ============================================== =================================================== ============== +``mte`` + Configure Memory Tagging Extensions for ARM guests. Possible values for the + ``state`` attribute are ``on`` and ``off``. If the attribute is not + defined, the hypervisor default will be used. :since:`Since 9.4.0` (QEMU/KVM + only) + Time keeping ------------ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6a864a8db9..047a4c97bf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -181,6 +181,7 @@ VIR_ENUM_IMPL(virDomainFeature, "sbbc", "ibs", "tcg", + "mte", ); VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, @@ -16645,7 +16646,8 @@ virDomainFeaturesDefParse(virDomainDef *def, case VIR_DOMAIN_FEATURE_HTM: case VIR_DOMAIN_FEATURE_NESTED_HV: - case VIR_DOMAIN_FEATURE_CCF_ASSIST: { + case VIR_DOMAIN_FEATURE_CCF_ASSIST: + case VIR_DOMAIN_FEATURE_MTE: { virTristateSwitch state; if (virXMLPropTristateSwitch(nodes[i], "state", @@ -20486,6 +20488,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_FEATURE_HTM: case VIR_DOMAIN_FEATURE_NESTED_HV: case VIR_DOMAIN_FEATURE_CCF_ASSIST: + case VIR_DOMAIN_FEATURE_MTE: if (src->features[i] != dst->features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of feature '%1$s' differs: source: '%2$s', destination: '%3$s'"), @@ -27005,6 +27008,7 @@ virDomainDefFormatFeatures(virBuffer *buf, case VIR_DOMAIN_FEATURE_HTM: case VIR_DOMAIN_FEATURE_NESTED_HV: case VIR_DOMAIN_FEATURE_CCF_ASSIST: + case VIR_DOMAIN_FEATURE_MTE: switch ((virTristateSwitch) def->features[i]) { case VIR_TRISTATE_SWITCH_LAST: case VIR_TRISTATE_SWITCH_ABSENT: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c1cb2ed69d..3f8d6e81c0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2169,6 +2169,7 @@ typedef enum { VIR_DOMAIN_FEATURE_SBBC, VIR_DOMAIN_FEATURE_IBS, VIR_DOMAIN_FEATURE_TCG, + VIR_DOMAIN_FEATURE_MTE, VIR_DOMAIN_FEATURE_LAST } virDomainFeature; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index f8c7b6a648..37e350ac2c 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -6653,6 +6653,11 @@ <optional> <ref name="tcgfeatures"/> </optional> + <optional> + <element name="mte"> + <ref name="featurestate"/> + </element> + </optional> </interleave> </element> </optional> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index da4b9a3b35..99c7775e9b 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -123,6 +123,7 @@ qemuValidateDomainDefFeatures(const virDomainDef *def, break; case VIR_DOMAIN_FEATURE_GIC: + case VIR_DOMAIN_FEATURE_MTE: if (def->features[i] == VIR_TRISTATE_SWITCH_ON && !qemuDomainIsARMVirt(def)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/tests/qemuxml2argvdata/aarch64-gic-v3.xml b/tests/qemuxml2argvdata/aarch64-gic-v3.xml index 247d6025f7..b9317284b0 100644 --- a/tests/qemuxml2argvdata/aarch64-gic-v3.xml +++ b/tests/qemuxml2argvdata/aarch64-gic-v3.xml @@ -10,6 +10,7 @@ </os> <features> <gic version='3'/> + <mte state='on'/> </features> <cpu mode='host-passthrough' check='none'/> <clock offset='utc'/> diff --git a/tests/qemuxml2xmloutdata/aarch64-gic-v3.aarch64-latest.xml b/tests/qemuxml2xmloutdata/aarch64-gic-v3.aarch64-latest.xml index 5b2fb7df75..1a74903aaa 100644 --- a/tests/qemuxml2xmloutdata/aarch64-gic-v3.aarch64-latest.xml +++ b/tests/qemuxml2xmloutdata/aarch64-gic-v3.aarch64-latest.xml @@ -10,6 +10,7 @@ </os> <features> <gic version='3'/> + <mte state='on'/> </features> <cpu mode='host-passthrough' check='none'/> <clock offset='utc'/> -- 2.39.3
The MTE feature (introduced in QEMU commit of v5.1.0-rc1~8^2~11) is detectable via 'qom-list-properties' for 'virt' machine type. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_5.2.0_aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml | 1 + 7 files changed, 8 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cf85d42198..a88c4070ab 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -693,6 +693,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "virtio-gpu.blob", /* QEMU_CAPS_VIRTIO_GPU_BLOB */ "rbd-encryption-layering", /* QEMU_CAPS_RBD_ENCRYPTION_LAYERING */ "rbd-encryption-luks-any", /* QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY */ + "machine.virt.mte", /* QEMU_CAPS_MACHINE_VIRT_MTE */ ); @@ -1720,6 +1721,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsMachinePropsPSeries[] = { static struct virQEMUCapsStringFlags virQEMUCapsMachinePropsVirt[] = { { "iommu", QEMU_CAPS_MACHINE_VIRT_IOMMU }, + { "mte", QEMU_CAPS_MACHINE_VIRT_MTE }, }; static struct virQEMUCapsStringFlags virQEMUCapsMachinePropsGeneric[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3b55aed07a..ffece17877 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -672,6 +672,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_VIRTIO_GPU_BLOB, /* -device virtio-gpu-*.blob= */ QEMU_CAPS_RBD_ENCRYPTION_LAYERING, /* layered encryption support for Ceph RBD */ QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY, /* luks-any (LUKS and LUKS2) encryption format for Ceph RBD */ + QEMU_CAPS_MACHINE_VIRT_MTE, /* -machine virt,mte=* for ARM guests */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_5.2.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_5.2.0_aarch64.xml index b1c5c21abb..4843b32a45 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0_aarch64.xml @@ -130,6 +130,7 @@ <flag name='virtio-net.rss'/> <flag name='usb-host.guest-resets-all'/> <flag name='virtio-crypto'/> + <flag name='machine.virt.mte'/> <version>5002000</version> <microcodeVersion>61700243</microcodeVersion> <package>v5.2.0</package> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml index 6faf407a97..78e297c7f4 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml @@ -140,6 +140,7 @@ <flag name='migration.blocked-reasons'/> <flag name='virtio-crypto'/> <flag name='pvpanic-pci'/> + <flag name='machine.virt.mte'/> <version>6000000</version> <microcodeVersion>61700242</microcodeVersion> <package>v6.0.0</package> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml index e312801b89..f37b84d399 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml @@ -153,6 +153,7 @@ <flag name='virtio-crypto'/> <flag name='pvpanic-pci'/> <flag name='virtio-gpu.blob'/> + <flag name='machine.virt.mte'/> <version>6001050</version> <microcodeVersion>61700244</microcodeVersion> <package></package> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml index 3517e81d15..d0f1815e85 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml @@ -161,6 +161,7 @@ <flag name='virtio-crypto'/> <flag name='pvpanic-pci'/> <flag name='virtio-gpu.blob'/> + <flag name='machine.virt.mte'/> <version>6002092</version> <microcodeVersion>61700243</microcodeVersion> <package>v7.0.0-rc2</package> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml index 58db75d1d7..5ca98e19be 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml @@ -161,6 +161,7 @@ <flag name='virtio-crypto'/> <flag name='pvpanic-pci'/> <flag name='virtio-gpu.blob'/> + <flag name='machine.virt.mte'/> <version>6002092</version> <microcodeVersion>61700243</microcodeVersion> <package>v7.0.0-rc2</package> -- 2.39.3
The MTE feature is not supported by all QEMUs, only those with QEMU_CAPS_MACHINE_VIRT_MTE capability. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_validate.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 99c7775e9b..aec1801672 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -124,14 +124,23 @@ qemuValidateDomainDefFeatures(const virDomainDef *def, case VIR_DOMAIN_FEATURE_GIC: case VIR_DOMAIN_FEATURE_MTE: - if (def->features[i] == VIR_TRISTATE_SWITCH_ON && - !qemuDomainIsARMVirt(def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("The '%1$s' feature is not supported for architecture '%2$s' or machine type '%3$s'"), - featureName, - virArchToString(def->os.arch), - def->os.machine); - return -1; + if (def->features[i] == VIR_TRISTATE_SWITCH_ON) { + if (!qemuDomainIsARMVirt(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The '%1$s' feature is not supported for architecture '%2$s' or machine type '%3$s'"), + featureName, + virArchToString(def->os.arch), + def->os.machine); + return -1; + } + + if (i == VIR_DOMAIN_FEATURE_MTE && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_VIRT_MTE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The '%1$s' feature is not supported with this QEMU binary"), + featureName); + return -1; + } } break; -- 2.39.3
On Tue, May 16, 2023 at 12:54:15PM +0200, Michal Privoznik wrote:
The MTE feature is not supported by all QEMUs, only those with QEMU_CAPS_MACHINE_VIRT_MTE capability.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_validate.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 99c7775e9b..aec1801672 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -124,14 +124,23 @@ qemuValidateDomainDefFeatures(const virDomainDef *def,
case VIR_DOMAIN_FEATURE_GIC: case VIR_DOMAIN_FEATURE_MTE: - if (def->features[i] == VIR_TRISTATE_SWITCH_ON && - !qemuDomainIsARMVirt(def)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("The '%1$s' feature is not supported for architecture '%2$s' or machine type '%3$s'"), - featureName, - virArchToString(def->os.arch), - def->os.machine); - return -1; + if (def->features[i] == VIR_TRISTATE_SWITCH_ON) {
This is not a big deal, but this should be '!= absent'. Not because there could be a "default on" in qemu, but because formatting mte='off' for a qemu (or machine type) that does not support it would not work. This is also pre-existing for the GIC feature. I think it'd be nicer to have that checked properly.
+ if (!qemuDomainIsARMVirt(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The '%1$s' feature is not supported for architecture '%2$s' or machine type '%3$s'"), + featureName, + virArchToString(def->os.arch), + def->os.machine); + return -1; + } + + if (i == VIR_DOMAIN_FEATURE_MTE && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_VIRT_MTE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The '%1$s' feature is not supported with this QEMU binary"), + featureName); + return -1; + } } break;
-- 2.39.3
This is pretty trivia, just append "mte=on/off" to -machine arguments. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 6 ++++++ tests/qemuxml2argvdata/aarch64-gic-v3.args | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2a6d9408f6..9b993c3aad 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6797,6 +6797,12 @@ qemuAppendDomainFeaturesMachineParam(virBuffer *buf, virBufferAsprintf(buf, ",cap-ibs=%s", str); } + if (def->features[VIR_DOMAIN_FEATURE_MTE] != VIR_TRISTATE_SWITCH_ABSENT) { + const char *str; + str = virTristateSwitchTypeToString(def->features[VIR_DOMAIN_FEATURE_MTE]); + virBufferAsprintf(buf, ",mte=%s", str); + } + return 0; } diff --git a/tests/qemuxml2argvdata/aarch64-gic-v3.args b/tests/qemuxml2argvdata/aarch64-gic-v3.args index 0d7a1c259a..0244951d87 100644 --- a/tests/qemuxml2argvdata/aarch64-gic-v3.args +++ b/tests/qemuxml2argvdata/aarch64-gic-v3.args @@ -10,7 +10,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.config \ -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 \ +-machine virt,usb=off,gic-version=3,mte=on,dump-guest-core=off,memory-backend=mach-virt.ram \ -accel kvm \ -cpu host \ -m 1024 \ -- 2.39.3
On Tue, May 16, 2023 at 12:54:16PM +0200, Michal Privoznik wrote:
This is pretty trivia, just append "mte=on/off" to -machine
*trivial
arguments.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 6 ++++++ tests/qemuxml2argvdata/aarch64-gic-v3.args | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2a6d9408f6..9b993c3aad 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6797,6 +6797,12 @@ qemuAppendDomainFeaturesMachineParam(virBuffer *buf, virBufferAsprintf(buf, ",cap-ibs=%s", str); }
+ if (def->features[VIR_DOMAIN_FEATURE_MTE] != VIR_TRISTATE_SWITCH_ABSENT) { + const char *str; + str = virTristateSwitchTypeToString(def->features[VIR_DOMAIN_FEATURE_MTE]); + virBufferAsprintf(buf, ",mte=%s", str); + } + return 0; }
diff --git a/tests/qemuxml2argvdata/aarch64-gic-v3.args b/tests/qemuxml2argvdata/aarch64-gic-v3.args index 0d7a1c259a..0244951d87 100644 --- a/tests/qemuxml2argvdata/aarch64-gic-v3.args +++ b/tests/qemuxml2argvdata/aarch64-gic-v3.args @@ -10,7 +10,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.config \ -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 \ +-machine virt,usb=off,gic-version=3,mte=on,dump-guest-core=off,memory-backend=mach-virt.ram \ -accel kvm \ -cpu host \ -m 1024 \ -- 2.39.3
On Tue, May 16, 2023 at 12:54:12PM +0200, Michal Privoznik wrote:
*** BLURB HERE ***
Michal Prívozník (4): conf: Introduce MTE domain feature qemu:: Introduce QEMU_CAPS_MACHINE_VIRT_MTE capability qemu: Validate MTE feature qemu: Generate command line for MTE feature
With the two points pointed out in 3/4 and 4/4 fixed Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
docs/formatdomain.rst | 7 +++++ src/conf/domain_conf.c | 6 ++++- src/conf/domain_conf.h | 1 + src/conf/schemas/domaincommon.rng | 5 ++++ src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 6 +++++ src/qemu/qemu_validate.c | 26 +++++++++++++------ .../caps_5.2.0_aarch64.xml | 1 + .../caps_6.0.0_aarch64.xml | 1 + .../caps_6.2.0_aarch64.xml | 1 + .../caps_7.0.0_aarch64+hvf.xml | 1 + .../caps_7.0.0_aarch64.xml | 1 + tests/qemuxml2argvdata/aarch64-gic-v3.args | 2 +- tests/qemuxml2argvdata/aarch64-gic-v3.xml | 1 + .../aarch64-gic-v3.aarch64-latest.xml | 1 + 16 files changed, 53 insertions(+), 10 deletions(-)
-- 2.39.3
On Tue, May 16, 2023 at 12:54:12PM +0200, Michal Privoznik wrote:
Michal Prívozník (4): conf: Introduce MTE domain feature qemu:: Introduce QEMU_CAPS_MACHINE_VIRT_MTE capability qemu: Validate MTE feature qemu: Generate command line for MTE feature
I wish I'd managed to see this before it got reviewed and merged :/ For context, I have been following the development of the MTE feature in QEMU for a while, and was planning to work on the libvirt part later down the line. The main reason why I have not done so yet is that there are still some open questions about the interface. Specifically, MTE is not just a single thing: there are at least two versions that I'm aware of, MTE and MTE3. Right now, mte=on gives you MTE3 with TCG and whatever the host supports on KVM. Of course the latter is problematic when it comes to guaranteeing a stable guest ABI... I think a reasonable interface would be similar to what we have for GIC, with a 'version' attribute used to explicitly choose between MTE and MTE3, and some logic to fill in a reasonable value for the host by default. But there's also the question of whether MTE should be a machine property in the first place, rather than a CPU feature? Committing to any specific interface in libvirt at this point in time feels premature, as it's pretty much guaranteed that it will no longer fit once the questions above have been answered. Last but not least, the way detection has been implemented is not accurate: as of today, QEMU does *not* support enabling MTE with KVM. Patches adding this feature have been posted[1] and are going to be merged soon, but even then just looking at the machine type property is not going to be enough to determine whether MTE can actually be used. CC'ing Connie so that she can point out any mistakes I might have made above :) [1] https://lists.gnu.org/archive/html/qemu-devel/2023-04/msg05452.html -- Andrea Bolognani / Red Hat / Virtualization
On 5/16/23 18:32, Andrea Bolognani wrote:
On Tue, May 16, 2023 at 12:54:12PM +0200, Michal Privoznik wrote:
Michal Prívozník (4): conf: Introduce MTE domain feature qemu:: Introduce QEMU_CAPS_MACHINE_VIRT_MTE capability qemu: Validate MTE feature qemu: Generate command line for MTE feature
I wish I'd managed to see this before it got reviewed and merged :/
We can still revert the patches, if needed.
For context, I have been following the development of the MTE feature in QEMU for a while, and was planning to work on the libvirt part later down the line. The main reason why I have not done so yet is that there are still some open questions about the interface.
Specifically, MTE is not just a single thing: there are at least two versions that I'm aware of, MTE and MTE3.
Right now, mte=on gives you MTE3 with TCG and whatever the host supports on KVM. Of course the latter is problematic when it comes to guaranteeing a stable guest ABI... I think a reasonable interface would be similar to what we have for GIC, with a 'version' attribute used to explicitly choose between MTE and MTE3, and some logic to fill in a reasonable value for the host by default.
But there's also the question of whether MTE should be a machine property in the first place, rather than a CPU feature?
I admit that my QEMU code base understanding is limited, but the patch you've linked doesn't really expose any CPU feature that libvirt could set. Instead, it enables MTE for KVM under the same API, i.e. -machine virt,mte=on.
Committing to any specific interface in libvirt at this point in time feels premature, as it's pretty much guaranteed that it will no longer fit once the questions above have been answered.
Fair enough, feel free to revert my patches.
Last but not least, the way detection has been implemented is not accurate: as of today, QEMU does *not* support enabling MTE with KVM. Patches adding this feature have been posted[1] and are going to be merged soon, but even then just looking at the machine type property is not going to be enough to determine whether MTE can actually be used.
Then it's a matter of: diff --git i/hw/arm/virt.c w/hw/arm/virt.c index b99ae18501..ead3555dfa 100644 --- i/hw/arm/virt.c +++ w/hw/arm/virt.c @@ -3111,11 +3111,13 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) "Set on/off to enable/disable reporting host memory errors " "to a KVM guest using ACPI and guest external abort exceptions"); - object_class_property_add_bool(oc, "mte", virt_get_mte, virt_set_mte); - object_class_property_set_description(oc, "mte", - "Set on/off to enable/disable emulating a " - "guest CPU which implements the ARM " - "Memory Tagging Extension"); + if (kvm_arm_mte_supported()) { + object_class_property_add_bool(oc, "mte", virt_get_mte, virt_set_mte); + object_class_property_set_description(oc, "mte", + "Set on/off to enable/disable emulating a " + "guest CPU which implements the ARM " + "Memory Tagging Extension"); + } object_class_property_add_bool(oc, "its", virt_get_its, virt_set_its); Or querying KVM extensions in libvirt (we already do that for some features). Michal
On Wed, May 17 2023, Michal Prívozník <mprivozn@redhat.com> wrote:
On 5/16/23 18:32, Andrea Bolognani wrote:
On Tue, May 16, 2023 at 12:54:12PM +0200, Michal Privoznik wrote:
Michal Prívozník (4): conf: Introduce MTE domain feature qemu:: Introduce QEMU_CAPS_MACHINE_VIRT_MTE capability qemu: Validate MTE feature qemu: Generate command line for MTE feature
I wish I'd managed to see this before it got reviewed and merged :/
We can still revert the patches, if needed.
For context, I have been following the development of the MTE feature in QEMU for a while, and was planning to work on the libvirt part later down the line. The main reason why I have not done so yet is that there are still some open questions about the interface.
Specifically, MTE is not just a single thing: there are at least two versions that I'm aware of, MTE and MTE3.
Right now, mte=on gives you MTE3 with TCG and whatever the host supports on KVM. Of course the latter is problematic when it comes to guaranteeing a stable guest ABI... I think a reasonable interface would be similar to what we have for GIC, with a 'version' attribute used to explicitly choose between MTE and MTE3, and some logic to fill in a reasonable value for the host by default.
But there's also the question of whether MTE should be a machine property in the first place, rather than a CPU feature?
I admit that my QEMU code base understanding is limited, but the patch you've linked doesn't really expose any CPU feature that libvirt could set. Instead, it enables MTE for KVM under the same API, i.e. -machine virt,mte=on.
This has been through some iterations... we (as in people working on this in QEMU) need to decide on where to go with cpu features, cpu models, etc. on Arm, but for now, it's a virt machine property. I have considered doing a GIC-like configuration, but for starters, the kernel doesn't support configuring the MTE version yet... and I'm not sure if configuring the MTE version (vs enabling/disabling MTE) should not be modeled as a cpu property instead. Note that my patch adds a migration blocker when enabling MTE, so (1) nothing bad regarding migration compatibility should happen yet, and (2) libvirt should probably not turn it on by default (I haven't checked what the patches actually do ;) [Also, I don't think there is any readily available hardware supporting MTE yet; I have been testing my code on the simulator...]
On Wed, May 17, 2023 at 11:19:17AM +0200, Cornelia Huck wrote:
This has been through some iterations... we (as in people working on this in QEMU) need to decide on where to go with cpu features, cpu models, etc. on Arm, but for now, it's a virt machine property.
I have considered doing a GIC-like configuration, but for starters, the kernel doesn't support configuring the MTE version yet... and I'm not sure if configuring the MTE version (vs enabling/disabling MTE) should not be modeled as a cpu property instead.
Note that my patch adds a migration blocker when enabling MTE, so (1) nothing bad regarding migration compatibility should happen yet
Migration is of course the most obvious failure scenario, but one of the critical features offered by libvirt is guest ABI compatibility. If the user needs MTE3 specifically, rather than any MTE version, to be available to the guest OS, they'll configure the domain with something like <mte version='3'/> and we need to be able to prevent such a VM from running on a host that doesn't have MTE3 support. So the fundamental question is, does the current libvirt interface paint us into a corner when it comes to implementing a more granular one later? Remember that, unlike QEMU, we don't have the luxury of erasing mistakes from our public interfaces, ever :)
libvirt should probably not turn it on by default (I haven't checked what the patches actually do ;)
[Also, I don't think there is any readily available hardware supporting MTE yet; I have been testing my code on the simulator...]
Agreed on not enabling it by default, especially considering the current hardware support situation. The feature remains disabled by default after the patches that have been merged. -- Andrea Bolognani / Red Hat / Virtualization
On Wed, May 17 2023, Andrea Bolognani <abologna@redhat.com> wrote:
On Wed, May 17, 2023 at 11:19:17AM +0200, Cornelia Huck wrote:
This has been through some iterations... we (as in people working on this in QEMU) need to decide on where to go with cpu features, cpu models, etc. on Arm, but for now, it's a virt machine property.
I have considered doing a GIC-like configuration, but for starters, the kernel doesn't support configuring the MTE version yet... and I'm not sure if configuring the MTE version (vs enabling/disabling MTE) should not be modeled as a cpu property instead.
Note that my patch adds a migration blocker when enabling MTE, so (1) nothing bad regarding migration compatibility should happen yet
Migration is of course the most obvious failure scenario, but one of the critical features offered by libvirt is guest ABI compatibility.
If the user needs MTE3 specifically, rather than any MTE version, to be available to the guest OS, they'll configure the domain with something like
<mte version='3'/>
and we need to be able to prevent such a VM from running on a host that doesn't have MTE3 support.
So the fundamental question is, does the current libvirt interface paint us into a corner when it comes to implementing a more granular one later?
Given that an interface allowing to actually control the exposed version is not likely to pop up in the next days, does it make sense to even try to wire it up in libvirt right now?
Remember that, unlike QEMU, we don't have the luxury of erasing mistakes from our public interfaces, ever :)
On Mon, May 22, 2023 at 11:55:15AM +0200, Cornelia Huck wrote:
On Wed, May 17 2023, Andrea Bolognani <abologna@redhat.com> wrote:
Migration is of course the most obvious failure scenario, but one of the critical features offered by libvirt is guest ABI compatibility.
If the user needs MTE3 specifically, rather than any MTE version, to be available to the guest OS, they'll configure the domain with something like
<mte version='3'/>
and we need to be able to prevent such a VM from running on a host that doesn't have MTE3 support.
So the fundamental question is, does the current libvirt interface paint us into a corner when it comes to implementing a more granular one later?
Given that an interface allowing to actually control the exposed version is not likely to pop up in the next days, does it make sense to even try to wire it up in libvirt right now?
I believe it does not. Until the situation has cleared up <qemu:commandline> <qemu:arg value='-machine'/> <qemu:arg value='virt,mte=on'/> </qemu:commandline> while far from an optimal solution, will work in a pinch. -- Andrea Bolognani / Red Hat / Virtualization
On Wed, May 17, 2023 at 09:14:02AM +0200, Michal Prívozník wrote:
On 5/16/23 18:32, Andrea Bolognani wrote:
Last but not least, the way detection has been implemented is not accurate: as of today, QEMU does *not* support enabling MTE with KVM. Patches adding this feature have been posted[1] and are going to be merged soon, but even then just looking at the machine type property is not going to be enough to determine whether MTE can actually be used.
Then it's a matter of:
+ if (kvm_arm_mte_supported()) { + object_class_property_add_bool(oc, "mte", virt_get_mte, virt_set_mte); + object_class_property_set_description(oc, "mte", + "Set on/off to enable/disable emulating a " + "guest CPU which implements the ARM " + "Memory Tagging Extension"); + }
I don't think this would work: even if KVM doesn't support MTE, TCG can still emulate it, so the property still needs to show up.
Or querying KVM extensions in libvirt (we already do that for some features).
That would tell us whether KVM itself is MTE-capable, but not whether the QEMU binary can make use of that feature.
Committing to any specific interface in libvirt at this point in time feels premature, as it's pretty much guaranteed that it will no longer fit once the questions above have been answered.
Fair enough, feel free to revert my patches.
Let's keep the discussion going for now, but if we get very close to the freeze without a clear consensus on the one you have implemented being the interface that we want I'll probably post a revert. -- Andrea Bolognani / Red Hat / Virtualization
participants (5)
-
Andrea Bolognani -
Cornelia Huck -
Martin Kletzander -
Michal Privoznik -
Michal Prívozník