[libvirt PATCH 0/5] Add bus-lock-ratelimit
This was introduced recently in QEMU, see https://bugzilla.redhat.com/show_bug.cgi?id=1982165: Bus locks disrupts overall performance since it blocks all other cores (which must wait for the bus lock to be released before their memory operations). For VMM, it can detect every bus lock acquired by guest VMs and induces a VM exit. So VMM can count the number/frequency of bus lock and take some throttling action or just kill the guest. Tim Wiederhake (5): conf: Add bus-lock-ratelimit qemu: Add bus-lock-ratelimit doc: schema: Add bus-lock-ratelimit to domain schema tests: Add tests for bus-lock-ratelimit doc: Document new bus-lock-ratelimit option docs/formatdomain.rst | 4 +++ docs/schemas/domaincommon.rng | 7 +++++ src/conf/domain_conf.c | 34 +++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/qemu/qemu_command.c | 4 +++ src/qemu/qemu_validate.c | 1 + tests/qemuxml2argvdata/kvm-features.args | 2 +- tests/qemuxml2argvdata/kvm-features.xml | 1 + tests/qemuxml2xmloutdata/kvm-features.xml | 1 + 9 files changed, 55 insertions(+), 1 deletion(-) -- 2.31.1
QEMU recently introduced a way to limit the rate of VM bus locks. Add libvirt support. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 34 ++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/qemu/qemu_validate.c | 1 + 3 files changed, 37 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bdcc3dc2c1..d2202fb6c6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -172,6 +172,7 @@ VIR_ENUM_IMPL(virDomainFeature, "cfpc", "sbbc", "ibs", + "bus-lock-ratelimit", ); VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, @@ -17860,6 +17861,13 @@ virDomainFeaturesDefParse(virDomainDef *def, break; } + case VIR_DOMAIN_FEATURE_BUS_LOCK_RATELIMIT: + if (virXMLPropULongLong(nodes[i], "value", 0, VIR_XML_PROP_REQUIRED, + &def->bus_lock_ratelimit) < 0) + return -1; + def->features[val] = VIR_TRISTATE_SWITCH_ON; + break; + case VIR_DOMAIN_FEATURE_LAST: break; } @@ -21666,6 +21674,23 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_FEATURE_MSRS: break; + case VIR_DOMAIN_FEATURE_BUS_LOCK_RATELIMIT: + if (src->features[i] != dst->features[i] || + src->bus_lock_ratelimit != dst->bus_lock_ratelimit) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("State of feature '%s' differs: " + "source: '%s,%s=%llu', destination: '%s,%s=%llu'"), + featureName, + virTristateSwitchTypeToString(src->features[i]), + "bus_lock_ratelimit", + src->bus_lock_ratelimit, + virTristateSwitchTypeToString(dst->features[i]), + "bus_lock_ratelimit", + dst->bus_lock_ratelimit); + return false; + } + break; + case VIR_DOMAIN_FEATURE_LAST: break; } @@ -27972,6 +27997,15 @@ virDomainDefFormatFeatures(virBuffer *buf, virDomainIBSTypeToString(def->features[i])); break; + case VIR_DOMAIN_FEATURE_BUS_LOCK_RATELIMIT: + if (def->features[i] == VIR_TRISTATE_SWITCH_ON) { + virBufferAsprintf(&childBuf, + "<%s value='%llu'/>\n", + virDomainFeatureTypeToString(i), + def->bus_lock_ratelimit); + } + break; + case VIR_DOMAIN_FEATURE_LAST: break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c0c07ea6ba..80bf7f4395 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2054,6 +2054,7 @@ typedef enum { VIR_DOMAIN_FEATURE_CFPC, VIR_DOMAIN_FEATURE_SBBC, VIR_DOMAIN_FEATURE_IBS, + VIR_DOMAIN_FEATURE_BUS_LOCK_RATELIMIT, VIR_DOMAIN_FEATURE_LAST } virDomainFeature; @@ -2824,6 +2825,7 @@ struct _virDomainDef { unsigned long long hpt_maxpagesize; /* Stored in KiB */ char *hyperv_vendor_id; virTristateSwitch apic_eoi; + unsigned long long bus_lock_ratelimit; bool tseg_specified; unsigned long long tseg_size; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 1de6e05101..e1068ad68a 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -304,6 +304,7 @@ qemuValidateDomainDefFeatures(const virDomainDef *def, case VIR_DOMAIN_FEATURE_PRIVNET: case VIR_DOMAIN_FEATURE_CAPABILITIES: case VIR_DOMAIN_FEATURE_MSRS: + case VIR_DOMAIN_FEATURE_BUS_LOCK_RATELIMIT: case VIR_DOMAIN_FEATURE_LAST: break; } -- 2.31.1
Pass bus-lock-ratelimit to qemu. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_command.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c47998aabd..dee803511b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7197,6 +7197,10 @@ qemuBuildMachineCommandLine(virCommand *cmd, virBufferAsprintf(&buf, ",cap-ibs=%s", str); } + if (def->features[VIR_DOMAIN_FEATURE_BUS_LOCK_RATELIMIT] != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&buf, ",bus-lock-ratelimit=%llu", def->bus_lock_ratelimit); + } + if (cpu && cpu->model && cpu->mode == VIR_CPU_MODE_HOST_MODEL && qemuDomainIsPSeries(def) && -- 2.31.1
Value is VMM exits per second. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- docs/schemas/domaincommon.rng | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f01b7a6470..e3461ab792 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6232,6 +6232,13 @@ <optional> <ref name="ibs"/> </optional> + <optional> + <element name="bus-lock-ratelimit"> + <attribute name="value"> + <ref name="positiveInteger"/> + </attribute> + </element> + </optional> </interleave> </element> </optional> -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/qemuxml2argvdata/kvm-features.args | 2 +- tests/qemuxml2argvdata/kvm-features.xml | 1 + tests/qemuxml2xmloutdata/kvm-features.xml | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/qemuxml2argvdata/kvm-features.args b/tests/qemuxml2argvdata/kvm-features.args index 371c382b47..e4fc1bd76c 100644 --- a/tests/qemuxml2argvdata/kvm-features.args +++ b/tests/qemuxml2argvdata/kvm-features.args @@ -11,7 +11,7 @@ QEMU_AUDIO_DRV=none \ -name guest=QEMUGuest1,debug-threads=on \ -S \ -object secret,id=masterKey0,format=raw,file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ --machine pc,usb=off,dump-guest-core=off \ +-machine pc,usb=off,dump-guest-core=off,bus-lock-ratelimit=1234 \ -accel kvm \ -cpu host,kvm=off,kvm-hint-dedicated=on,kvm-poll-control=on \ -m 214 \ diff --git a/tests/qemuxml2argvdata/kvm-features.xml b/tests/qemuxml2argvdata/kvm-features.xml index 51229a6c37..3cab81771d 100644 --- a/tests/qemuxml2argvdata/kvm-features.xml +++ b/tests/qemuxml2argvdata/kvm-features.xml @@ -16,6 +16,7 @@ <poll-control state='on'/> <pv-ipi state='on'/> </kvm> + <bus-lock-ratelimit value='1234'/> </features> <cpu mode='host-passthrough' check='none'/> <clock offset='utc'/> diff --git a/tests/qemuxml2xmloutdata/kvm-features.xml b/tests/qemuxml2xmloutdata/kvm-features.xml index 72e66fcbf5..39d462c7d3 100644 --- a/tests/qemuxml2xmloutdata/kvm-features.xml +++ b/tests/qemuxml2xmloutdata/kvm-features.xml @@ -16,6 +16,7 @@ <poll-control state='on'/> <pv-ipi state='on'/> </kvm> + <bus-lock-ratelimit value='1234'/> </features> <cpu mode='host-passthrough' check='none' migratable='off'/> <clock offset='utc'/> -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- docs/formatdomain.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index eb8c973cf1..5b38564894 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1864,6 +1864,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. <cfpc value='workaround'/> <sbbc value='workaround'/> <ibs value='fixed-na'/> + <bus-lock-ratelimit value='1234'/> </features> ... @@ -2065,6 +2066,9 @@ 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) +``bus-lock-ratelimit`` + Configure the limit for bus-lock VM-Exits per second. + :since:`Since 8.0.0` (QEMU/KVM only) :anchor:`<a id="elementsTime"/>` -- 2.31.1
On Thu, Dec 02, 2021 at 06:01:03PM +0100, Tim Wiederhake wrote:
This was introduced recently in QEMU, see https://bugzilla.redhat.com/show_bug.cgi?id=1982165:
Bus locks disrupts overall performance since it blocks all other cores (which must wait for the bus lock to be released before their memory operations).
For VMM, it can detect every bus lock acquired by guest VMs and induces a VM exit. So VMM can count the number/frequency of bus lock and take some throttling action or just kill the guest.
NB this feedback is not directed at you, but rather the requester of this feature. This request is awful as it exists today. The bugs and upstream commit provide little to no information for apps or libvirt to make sensible decisions on. We shouldn't expose this in libvirt until someone can explain sensible usage policy for applications and why QEMU isn't just setting a sensible value in the machine type by default, given this is claimed to be an important guest denial of service mitigation.. 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 :|
participants (2)
-
Daniel P. Berrangé -
Tim Wiederhake