[libvirt PATCH 0/2] qemu: support kvm-poll-control performance hint
See the commit message of the first patch and https://bugzilla.redhat.com/show_bug.cgi?id=1895204 Tim Wiederhake (2): qemu: support kvm-poll-control performance hint tests: Add tests for kvm-poll-control feature docs/formatdomain.rst | 14 ++++++++------ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 5 +++++ tests/qemuxml2argvdata/kvm-features-off.xml | 1 + tests/qemuxml2argvdata/kvm-features.args | 2 +- tests/qemuxml2argvdata/kvm-features.xml | 1 + tests/qemuxml2xmloutdata/kvm-features-off.xml | 1 + tests/qemuxml2xmloutdata/kvm-features.xml | 1 + 10 files changed, 28 insertions(+), 7 deletions(-) -- 2.26.2
QEMU version 4.2 introduced a performance feature under commit d645e13287 ("kvm: i386: halt poll control MSR support"). This patch adds a new KVM feature 'poll-control' to set this performance hint for KVM guests. The feature is off by default. To enable this hint and have libvirt add "-cpu host,kvm-poll-control=on" to the QEMU command line, the following XML code needs to be added to the guest's domain description: <features> <kvm> <poll-control state='on'/> </kvm> </features> Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- docs/formatdomain.rst | 14 ++++++++------ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 5 +++++ 5 files changed, 23 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index a6ab845f92..cd5d7aae56 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1766,6 +1766,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. <kvm> <hidden state='on'/> <hint-dedicated state='on'/> + <poll-control='on'/> </kvm> <xen> <e820_host state='on'/> @@ -1848,12 +1849,13 @@ are: ``kvm`` Various features to change the behavior of the KVM hypervisor. - ============== ====================================================================== ======= ============================ - Feature Description Value Since - ============== ====================================================================== ======= ============================ - hidden Hide the KVM hypervisor from standard MSR based discovery on, off :since:`1.2.8 (QEMU 2.1.0)` - hint-dedicated Allows a guest to enable optimizations when running on dedicated vCPUs on, off :since:`5.7.0 (QEMU 2.12.0)` - ============== ====================================================================== ======= ============================ + ============== ============================================================================ ======= ============================ + Feature Description Value Since + ============== ============================================================================ ======= ============================ + hidden Hide the KVM hypervisor from standard MSR based discovery on, off :since:`1.2.8 (QEMU 2.1.0)` + hint-dedicated Allows a guest to enable optimizations when running on dedicated vCPUs on, off :since:`5.7.0 (QEMU 2.12.0)` + poll-control Decrease IO completion latency by introducing a grace period of busy waiting on, off :since:`6.11.0 (QEMU 4.2)` + ============== ============================================================================ ======= ============================ ``xen`` Various features to change the behavior of the Xen hypervisor. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index e0d09f9c03..f86a854863 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6512,6 +6512,11 @@ <ref name="featurestate"/> </element> </optional> + <optional> + <element name="poll-control"> + <ref name="featurestate"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9199771dc0..2b56993845 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -208,6 +208,7 @@ VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST, "hidden", "hint-dedicated", + "poll-control", ); VIR_ENUM_IMPL(virDomainXen, @@ -19823,6 +19824,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def, switch ((virDomainKVM) feature) { case VIR_DOMAIN_KVM_HIDDEN: case VIR_DOMAIN_KVM_DEDICATED: + case VIR_DOMAIN_KVM_POLLCONTROL: if (!(tmp = virXMLPropString(nodes[i], "state"))) { virReportError(VIR_ERR_XML_ERROR, _("missing 'state' attribute for " @@ -23982,6 +23984,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, switch ((virDomainKVM) i) { case VIR_DOMAIN_KVM_HIDDEN: case VIR_DOMAIN_KVM_DEDICATED: + case VIR_DOMAIN_KVM_POLLCONTROL: if (src->kvm_features[i] != dst->kvm_features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%s' differs: " @@ -29695,6 +29698,7 @@ virDomainDefFormatFeatures(virBufferPtr buf, switch ((virDomainKVM) j) { case VIR_DOMAIN_KVM_HIDDEN: case VIR_DOMAIN_KVM_DEDICATED: + case VIR_DOMAIN_KVM_POLLCONTROL: if (def->kvm_features[j]) virBufferAsprintf(&childBuf, "<%s state='%s'/>\n", virDomainKVMTypeToString(j), diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 16c050a3ea..3e3d4bd002 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1912,6 +1912,7 @@ typedef enum { typedef enum { VIR_DOMAIN_KVM_HIDDEN = 0, VIR_DOMAIN_KVM_DEDICATED, + VIR_DOMAIN_KVM_POLLCONTROL, VIR_DOMAIN_KVM_LAST } virDomainKVM; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0eec35da16..34b5746c1a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6464,6 +6464,11 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAddLit(&buf, ",kvm-hint-dedicated=on"); break; + case VIR_DOMAIN_KVM_POLLCONTROL: + if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON) + virBufferAddLit(&buf, ",kvm-poll-control=on"); + break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_KVM_LAST: break; -- 2.26.2
On 11/13/20 9:49 AM, Tim Wiederhake wrote:
QEMU version 4.2 introduced a performance feature under commit d645e13287 ("kvm: i386: halt poll control MSR support").
This patch adds a new KVM feature 'poll-control' to set this performance hint for KVM guests. The feature is off by default.
To enable this hint and have libvirt add "-cpu host,kvm-poll-control=on" to the QEMU command line, the following XML code needs to be added to the guest's domain description:
<features> <kvm> <poll-control state='on'/> </kvm> </features>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- docs/formatdomain.rst | 14 ++++++++------ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 5 +++++ 5 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index a6ab845f92..cd5d7aae56 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1766,6 +1766,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. <kvm> <hidden state='on'/> <hint-dedicated state='on'/> + <poll-control='on'/> </kvm> <xen> <e820_host state='on'/> @@ -1848,12 +1849,13 @@ are: ``kvm`` Various features to change the behavior of the KVM hypervisor.
- ============== ====================================================================== ======= ============================ - Feature Description Value Since - ============== ====================================================================== ======= ============================ - hidden Hide the KVM hypervisor from standard MSR based discovery on, off :since:`1.2.8 (QEMU 2.1.0)` - hint-dedicated Allows a guest to enable optimizations when running on dedicated vCPUs on, off :since:`5.7.0 (QEMU 2.12.0)` - ============== ====================================================================== ======= ============================ + ============== ============================================================================ ======= ============================ + Feature Description Value Since + ============== ============================================================================ ======= ============================ + hidden Hide the KVM hypervisor from standard MSR based discovery on, off :since:`1.2.8 (QEMU 2.1.0)` + hint-dedicated Allows a guest to enable optimizations when running on dedicated vCPUs on, off :since:`5.7.0 (QEMU 2.12.0)` + poll-control Decrease IO completion latency by introducing a grace period of busy waiting on, off :since:`6.11.0 (QEMU 4.2)` + ============== ============================================================================ ======= ============================
``xen`` Various features to change the behavior of the Xen hypervisor. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index e0d09f9c03..f86a854863 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6512,6 +6512,11 @@ <ref name="featurestate"/> </element> </optional> + <optional> + <element name="poll-control"> + <ref name="featurestate"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9199771dc0..2b56993845 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -208,6 +208,7 @@ VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST, "hidden", "hint-dedicated", + "poll-control", );
VIR_ENUM_IMPL(virDomainXen, @@ -19823,6 +19824,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def, switch ((virDomainKVM) feature) { case VIR_DOMAIN_KVM_HIDDEN: case VIR_DOMAIN_KVM_DEDICATED: + case VIR_DOMAIN_KVM_POLLCONTROL: if (!(tmp = virXMLPropString(nodes[i], "state"))) { virReportError(VIR_ERR_XML_ERROR, _("missing 'state' attribute for " @@ -23982,6 +23984,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, switch ((virDomainKVM) i) { case VIR_DOMAIN_KVM_HIDDEN: case VIR_DOMAIN_KVM_DEDICATED: + case VIR_DOMAIN_KVM_POLLCONTROL: if (src->kvm_features[i] != dst->kvm_features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%s' differs: " @@ -29695,6 +29698,7 @@ virDomainDefFormatFeatures(virBufferPtr buf, switch ((virDomainKVM) j) { case VIR_DOMAIN_KVM_HIDDEN: case VIR_DOMAIN_KVM_DEDICATED: + case VIR_DOMAIN_KVM_POLLCONTROL: if (def->kvm_features[j]) virBufferAsprintf(&childBuf, "<%s state='%s'/>\n", virDomainKVMTypeToString(j), diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 16c050a3ea..3e3d4bd002 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1912,6 +1912,7 @@ typedef enum { typedef enum { VIR_DOMAIN_KVM_HIDDEN = 0, VIR_DOMAIN_KVM_DEDICATED, + VIR_DOMAIN_KVM_POLLCONTROL,
VIR_DOMAIN_KVM_LAST } virDomainKVM;
Until here, the change is okay.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0eec35da16..34b5746c1a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6464,6 +6464,11 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAddLit(&buf, ",kvm-hint-dedicated=on"); break;
+ case VIR_DOMAIN_KVM_POLLCONTROL: + if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON) + virBufferAddLit(&buf, ",kvm-poll-control=on"); + break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_KVM_LAST: break;
But this should go into the next patch IMO. The reason is that if somebody wants to backport only XML part they should not be forced to backport QEMU too. In this specific case it probably doesn't doesn't matter that much, because no other driver uses KVM features. But I think it's still wort it (e.g. to gain the habit). In theory, src/conf/ (plus docs/rng change) can be viewed as "frontend" and driver implementation can be viewed as "backend". What is also acceptable is having src/conf + xml2xml tests in one patch, because xml2xml tests do test the change made to src/conf. So in this specific case, it is also acceptable to have src/conf/ and docs/rng change and tests/.../*.xml changes in one patch, qemu + *.args in the other. I'm sorry for being picky. The change itself is okay. Michal
On Mon, 2020-11-16 at 14:27 +0100, Michal Privoznik wrote:
On 11/13/20 9:49 AM, Tim Wiederhake wrote:
QEMU version 4.2 introduced a performance feature under commit d645e13287 ("kvm: i386: halt poll control MSR support").
This patch adds a new KVM feature 'poll-control' to set this performance hint for KVM guests. The feature is off by default.
To enable this hint and have libvirt add "-cpu host,kvm-poll- control=on" to the QEMU command line, the following XML code needs to be added to the guest's domain description:
<features> <kvm> <poll-control state='on'/> </kvm> </features>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- docs/formatdomain.rst | 14 ++++++++------ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 5 +++++ 5 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index a6ab845f92..cd5d7aae56 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1766,6 +1766,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. <kvm> <hidden state='on'/> <hint-dedicated state='on'/> + <poll-control='on'/> </kvm> <xen> <e820_host state='on'/> @@ -1848,12 +1849,13 @@ are: ``kvm`` Various features to change the behavior of the KVM hypervisor.
- ============== =================================================================== === ======= ============================ - Feature Description Value Since - ============== =================================================================== === ======= ============================ - hidden Hide the KVM hypervisor from standard MSR based discovery on, off :since:`1.2.8 (QEMU 2.1.0)` - hint-dedicated Allows a guest to enable optimizations when running on dedicated vCPUs on, off :since:`5.7.0 (QEMU 2.12.0)` - ============== =================================================================== === ======= ============================ + ============== =================================================================== ========= ======= ============================ + Feature Description Value Since + ============== =================================================================== ========= ======= ============================ + hidden Hide the KVM hypervisor from standard MSR based discovery on, off :since:`1.2.8 (QEMU 2.1.0)` + hint-dedicated Allows a guest to enable optimizations when running on dedicated vCPUs on, off :since:`5.7.0 (QEMU 2.12.0)` + poll-control Decrease IO completion latency by introducing a grace period of busy waiting on, off :since:`6.11.0 (QEMU 4.2)` + ============== =================================================================== ========= ======= ============================
``xen`` Various features to change the behavior of the Xen hypervisor. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index e0d09f9c03..f86a854863 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6512,6 +6512,11 @@ <ref name="featurestate"/> </element> </optional> + <optional> + <element name="poll-control"> + <ref name="featurestate"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9199771dc0..2b56993845 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -208,6 +208,7 @@ VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST, "hidden", "hint-dedicated", + "poll-control", );
VIR_ENUM_IMPL(virDomainXen, @@ -19823,6 +19824,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def, switch ((virDomainKVM) feature) { case VIR_DOMAIN_KVM_HIDDEN: case VIR_DOMAIN_KVM_DEDICATED: + case VIR_DOMAIN_KVM_POLLCONTROL: if (!(tmp = virXMLPropString(nodes[i], "state"))) { virReportError(VIR_ERR_XML_ERROR, _("missing 'state' attribute for " @@ -23982,6 +23984,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, switch ((virDomainKVM) i) { case VIR_DOMAIN_KVM_HIDDEN: case VIR_DOMAIN_KVM_DEDICATED: + case VIR_DOMAIN_KVM_POLLCONTROL: if (src->kvm_features[i] != dst->kvm_features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%s' differs: " @@ -29695,6 +29698,7 @@ virDomainDefFormatFeatures(virBufferPtr buf, switch ((virDomainKVM) j) { case VIR_DOMAIN_KVM_HIDDEN: case VIR_DOMAIN_KVM_DEDICATED: + case VIR_DOMAIN_KVM_POLLCONTROL: if (def->kvm_features[j]) virBufferAsprintf(&childBuf, "<%s state='%s'/>\n", virDomainKVMTypeToStrin g(j), diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 16c050a3ea..3e3d4bd002 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1912,6 +1912,7 @@ typedef enum { typedef enum { VIR_DOMAIN_KVM_HIDDEN = 0, VIR_DOMAIN_KVM_DEDICATED, + VIR_DOMAIN_KVM_POLLCONTROL,
VIR_DOMAIN_KVM_LAST } virDomainKVM;
Until here, the change is okay.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0eec35da16..34b5746c1a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6464,6 +6464,11 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAddLit(&buf, ",kvm-hint- dedicated=on"); break;
+ case VIR_DOMAIN_KVM_POLLCONTROL: + if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON) + virBufferAddLit(&buf, ",kvm-poll-control=on"); + break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_KVM_LAST: break;
But this should go into the next patch IMO. The reason is that if somebody wants to backport only XML part they should not be forced to backport QEMU too. In this specific case it probably doesn't doesn't matter that much, because no other driver uses KVM features. But I think it's still wort it (e.g. to gain the habit).
In theory, src/conf/ (plus docs/rng change) can be viewed as "frontend" and driver implementation can be viewed as "backend". What is also acceptable is having src/conf + xml2xml tests in one patch, because xml2xml tests do test the change made to src/conf. So in this specific case, it is also acceptable to have src/conf/ and docs/rng change and tests/.../*.xml changes in one patch, qemu + *.args in the other.
I'm sorry for being picky. The change itself is okay.
Michal
Thanks for the review! I don't quite understand though, if you could please clarify what you mean: If the patch changing src/conf/domain_conf.{c,h} goes in first, we have one commit where "<poll-control state='on'/>" would be accepted but silently discarded, potentially messing with people git-bisect'ing in the future. If the patch changing src/qemu/qemu_command.c goes in first, we need VIR_DOMAIN_KVM_POLLCONTROL defined in src/conf/domain_conf.h, thus breaking the split between "frontend" and "backend" changes as described above. The patches in this series were modeled after commits cb12c59dac04c34dc8250d93e63516b1a1af5260 and 63b2e57cb3b34568f36ad6f0c8c4244e292a679e by the way, which made the split in the same fashion as I did. Thanks in advance for you advice! Tim
On 11/17/20 2:26 PM, Tim Wiederhake wrote:
On Mon, 2020-11-16 at 14:27 +0100, Michal Privoznik wrote:
On 11/13/20 9:49 AM, Tim Wiederhake wrote:
QEMU version 4.2 introduced a performance feature under commit d645e13287 ("kvm: i386: halt poll control MSR support").
This patch adds a new KVM feature 'poll-control' to set this performance hint for KVM guests. The feature is off by default.
To enable this hint and have libvirt add "-cpu host,kvm-poll- control=on" to the QEMU command line, the following XML code needs to be added to the guest's domain description:
<features> <kvm> <poll-control state='on'/> </kvm> </features>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- docs/formatdomain.rst | 14 ++++++++------ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 5 +++++ 5 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index a6ab845f92..cd5d7aae56 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1766,6 +1766,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. <kvm> <hidden state='on'/> <hint-dedicated state='on'/> + <poll-control='on'/> </kvm> <xen> <e820_host state='on'/> @@ -1848,12 +1849,13 @@ are: ``kvm`` Various features to change the behavior of the KVM hypervisor.
- ============== =================================================================== === ======= ============================ - Feature Description Value Since - ============== =================================================================== === ======= ============================ - hidden Hide the KVM hypervisor from standard MSR based discovery on, off :since:`1.2.8 (QEMU 2.1.0)` - hint-dedicated Allows a guest to enable optimizations when running on dedicated vCPUs on, off :since:`5.7.0 (QEMU 2.12.0)` - ============== =================================================================== === ======= ============================ + ============== =================================================================== ========= ======= ============================ + Feature Description Value Since + ============== =================================================================== ========= ======= ============================ + hidden Hide the KVM hypervisor from standard MSR based discovery on, off :since:`1.2.8 (QEMU 2.1.0)` + hint-dedicated Allows a guest to enable optimizations when running on dedicated vCPUs on, off :since:`5.7.0 (QEMU 2.12.0)` + poll-control Decrease IO completion latency by introducing a grace period of busy waiting on, off :since:`6.11.0 (QEMU 4.2)` + ============== =================================================================== ========= ======= ============================
``xen`` Various features to change the behavior of the Xen hypervisor. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index e0d09f9c03..f86a854863 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -6512,6 +6512,11 @@ <ref name="featurestate"/> </element> </optional> + <optional> + <element name="poll-control"> + <ref name="featurestate"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9199771dc0..2b56993845 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -208,6 +208,7 @@ VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST, "hidden", "hint-dedicated", + "poll-control", );
VIR_ENUM_IMPL(virDomainXen, @@ -19823,6 +19824,7 @@ virDomainFeaturesDefParse(virDomainDefPtr def, switch ((virDomainKVM) feature) { case VIR_DOMAIN_KVM_HIDDEN: case VIR_DOMAIN_KVM_DEDICATED: + case VIR_DOMAIN_KVM_POLLCONTROL: if (!(tmp = virXMLPropString(nodes[i], "state"))) { virReportError(VIR_ERR_XML_ERROR, _("missing 'state' attribute for " @@ -23982,6 +23984,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, switch ((virDomainKVM) i) { case VIR_DOMAIN_KVM_HIDDEN: case VIR_DOMAIN_KVM_DEDICATED: + case VIR_DOMAIN_KVM_POLLCONTROL: if (src->kvm_features[i] != dst->kvm_features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%s' differs: " @@ -29695,6 +29698,7 @@ virDomainDefFormatFeatures(virBufferPtr buf, switch ((virDomainKVM) j) { case VIR_DOMAIN_KVM_HIDDEN: case VIR_DOMAIN_KVM_DEDICATED: + case VIR_DOMAIN_KVM_POLLCONTROL: if (def->kvm_features[j]) virBufferAsprintf(&childBuf, "<%s state='%s'/>\n", virDomainKVMTypeToStrin g(j), diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 16c050a3ea..3e3d4bd002 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1912,6 +1912,7 @@ typedef enum { typedef enum { VIR_DOMAIN_KVM_HIDDEN = 0, VIR_DOMAIN_KVM_DEDICATED, + VIR_DOMAIN_KVM_POLLCONTROL,
VIR_DOMAIN_KVM_LAST } virDomainKVM;
Until here, the change is okay.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0eec35da16..34b5746c1a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6464,6 +6464,11 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAddLit(&buf, ",kvm-hint- dedicated=on"); break;
+ case VIR_DOMAIN_KVM_POLLCONTROL: + if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON) + virBufferAddLit(&buf, ",kvm-poll-control=on"); + break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_KVM_LAST: break;
But this should go into the next patch IMO. The reason is that if somebody wants to backport only XML part they should not be forced to backport QEMU too. In this specific case it probably doesn't doesn't matter that much, because no other driver uses KVM features. But I think it's still wort it (e.g. to gain the habit).
In theory, src/conf/ (plus docs/rng change) can be viewed as "frontend" and driver implementation can be viewed as "backend". What is also acceptable is having src/conf + xml2xml tests in one patch, because xml2xml tests do test the change made to src/conf. So in this specific case, it is also acceptable to have src/conf/ and docs/rng change and tests/.../*.xml changes in one patch, qemu + *.args in the other.
I'm sorry for being picky. The change itself is okay.
Michal
Thanks for the review! I don't quite understand though, if you could please clarify what you mean:
If the patch changing src/conf/domain_conf.{c,h} goes in first, we have one commit where "<poll-control state='on'/>" would be accepted but silently discarded, potentially messing with people git-bisect'ing in the future.
Ah, we don't error out? Alright then, let's go with your patches as they are. Sorry for the noise. Michal
On 11/13/20 9:49 AM, Tim Wiederhake wrote:
QEMU version 4.2 introduced a performance feature under commit d645e13287 ("kvm: i386: halt poll control MSR support").
This patch adds a new KVM feature 'poll-control' to set this performance hint for KVM guests. The feature is off by default.
To enable this hint and have libvirt add "-cpu host,kvm-poll-control=on" to the QEMU command line, the following XML code needs to be added to the guest's domain description:
<features> <kvm> <poll-control state='on'/> </kvm> </features>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- docs/formatdomain.rst | 14 ++++++++------ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 5 +++++ 5 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index a6ab845f92..cd5d7aae56 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -1766,6 +1766,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. <kvm> <hidden state='on'/> <hint-dedicated state='on'/> + <poll-control='on'/> </kvm> <xen> <e820_host state='on'/> @@ -1848,12 +1849,13 @@ are: ``kvm`` Various features to change the behavior of the KVM hypervisor.
- ============== ====================================================================== ======= ============================ - Feature Description Value Since - ============== ====================================================================== ======= ============================ - hidden Hide the KVM hypervisor from standard MSR based discovery on, off :since:`1.2.8 (QEMU 2.1.0)` - hint-dedicated Allows a guest to enable optimizations when running on dedicated vCPUs on, off :since:`5.7.0 (QEMU 2.12.0)` - ============== ====================================================================== ======= ============================ + ============== ============================================================================ ======= ============================ + Feature Description Value Since + ============== ============================================================================ ======= ============================ + hidden Hide the KVM hypervisor from standard MSR based discovery on, off :since:`1.2.8 (QEMU 2.1.0)` + hint-dedicated Allows a guest to enable optimizations when running on dedicated vCPUs on, off :since:`5.7.0 (QEMU 2.12.0)` + poll-control Decrease IO completion latency by introducing a grace period of busy waiting on, off :since:`6.11.0 (QEMU 4.2)` + ============== ============================================================================ ======= ============================
Small nit, the next version is 6.10.0 ;-) Michal
Update the KVM feature tests for QEMU's kvm-poll-control performance hint. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/qemuxml2argvdata/kvm-features-off.xml | 1 + tests/qemuxml2argvdata/kvm-features.args | 2 +- tests/qemuxml2argvdata/kvm-features.xml | 1 + tests/qemuxml2xmloutdata/kvm-features-off.xml | 1 + tests/qemuxml2xmloutdata/kvm-features.xml | 1 + 5 files changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/qemuxml2argvdata/kvm-features-off.xml b/tests/qemuxml2argvdata/kvm-features-off.xml index 21d492c8b9..d63a573239 100644 --- a/tests/qemuxml2argvdata/kvm-features-off.xml +++ b/tests/qemuxml2argvdata/kvm-features-off.xml @@ -13,6 +13,7 @@ <kvm> <hidden state='off'/> <hint-dedicated state='off'/> + <poll-control state='off'/> </kvm> </features> <clock offset='utc'/> diff --git a/tests/qemuxml2argvdata/kvm-features.args b/tests/qemuxml2argvdata/kvm-features.args index 5753a3eb3f..629f868fe1 100644 --- a/tests/qemuxml2argvdata/kvm-features.args +++ b/tests/qemuxml2argvdata/kvm-features.args @@ -11,7 +11,7 @@ QEMU_AUDIO_DRV=none \ -name QEMUGuest1 \ -S \ -machine pc,accel=kvm,usb=off,dump-guest-core=off \ --cpu host,kvm=off,kvm-hint-dedicated=on \ +-cpu host,kvm=off,kvm-hint-dedicated=on,kvm-poll-control=on \ -m 214 \ -realtime mlock=off \ -smp 6,sockets=6,cores=1,threads=1 \ diff --git a/tests/qemuxml2argvdata/kvm-features.xml b/tests/qemuxml2argvdata/kvm-features.xml index 85f74786c9..a5159254c6 100644 --- a/tests/qemuxml2argvdata/kvm-features.xml +++ b/tests/qemuxml2argvdata/kvm-features.xml @@ -13,6 +13,7 @@ <kvm> <hidden state='on'/> <hint-dedicated state='on'/> + <poll-control state='on'/> </kvm> </features> <cpu mode='host-passthrough' check='none'/> diff --git a/tests/qemuxml2xmloutdata/kvm-features-off.xml b/tests/qemuxml2xmloutdata/kvm-features-off.xml index 9068be3e7b..2fc9468910 100644 --- a/tests/qemuxml2xmloutdata/kvm-features-off.xml +++ b/tests/qemuxml2xmloutdata/kvm-features-off.xml @@ -13,6 +13,7 @@ <kvm> <hidden state='off'/> <hint-dedicated state='off'/> + <poll-control state='off'/> </kvm> </features> <clock offset='utc'/> diff --git a/tests/qemuxml2xmloutdata/kvm-features.xml b/tests/qemuxml2xmloutdata/kvm-features.xml index 9c53eb4630..458070ac46 100644 --- a/tests/qemuxml2xmloutdata/kvm-features.xml +++ b/tests/qemuxml2xmloutdata/kvm-features.xml @@ -13,6 +13,7 @@ <kvm> <hidden state='on'/> <hint-dedicated state='on'/> + <poll-control state='on'/> </kvm> </features> <cpu mode='host-passthrough' check='none' migratable='off'/> -- 2.26.2
On 11/13/20 9:49 AM, Tim Wiederhake wrote:
See the commit message of the first patch and https://bugzilla.redhat.com/show_bug.cgi?id=1895204
Tim Wiederhake (2): qemu: support kvm-poll-control performance hint tests: Add tests for kvm-poll-control feature
docs/formatdomain.rst | 14 ++++++++------ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 5 +++++ tests/qemuxml2argvdata/kvm-features-off.xml | 1 + tests/qemuxml2argvdata/kvm-features.args | 2 +- tests/qemuxml2argvdata/kvm-features.xml | 1 + tests/qemuxml2xmloutdata/kvm-features-off.xml | 1 + tests/qemuxml2xmloutdata/kvm-features.xml | 1 + 10 files changed, 28 insertions(+), 7 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and pushed. Michal
participants (2)
-
Michal Privoznik -
Tim Wiederhake