[libvirt] [PATCH 0/2] add support for kvm-hint-dedicated performance hint

QEMU version 2.12.1 introduced a performance feature under commit be7773268d98 ("target-i386: add KVM_HINTS_DEDICATED performance hint") This patch adds a new KVM feature 'hint-dedicated' to set this performance hint for KVM guests. Wim ten Have (2): qemu: support for kvm-hint-dedicated performance hint tests: add tests for kvm-hint-dedicated feature docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++++++ tests/qemuxml2argvdata/kvm-features-off.xml | 1 + tests/qemuxml2argvdata/kvm-features.args | 4 ++-- tests/qemuxml2argvdata/kvm-features.xml | 4 +++- tests/qemuxml2xmloutdata/kvm-features-off.xml | 1 + tests/qemuxml2xmloutdata/kvm-features.xml | 4 +++- 10 files changed, 40 insertions(+), 4 deletions(-) -- 2.21.0

From: Wim ten Have <wim.ten.have@oracle.com> QEMU version 2.12.1 introduced a performance feature under commit be7773268d98 ("target-i386: add KVM_HINTS_DEDICATED performance hint") This patch adds a new KVM feature 'hint-dedicated' 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-hint-dedicated=on" to the QEMU command line, the following XML code needs to be added to the guest's domain description in conjunction with CPU mode='host-passthrough'. <features> <kvm> <hint-dedicated state='on'/> </kvm> </features> ... <cpu mode='host-passthrough ... /> Signed-off-by: Wim ten Have <wim.ten.have@oracle.com> Signed-off-by: Menno Lageman <menno.lageman@oracle.com> --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++++++ 5 files changed, 30 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6d084d7c0472..c9b18b57e1fc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2044,6 +2044,7 @@ </hyperv> <kvm> <hidden state='on'/> + <hint-dedicated state='on'/> </kvm> <pvspinlock state='on'/> <gic version='2'/> @@ -2217,6 +2218,12 @@ <td>on, off</td> <td><span class="since">1.2.8 (QEMU 2.1.0)</span></td> </tr> + <tr> + <td>hint-dedicated</td> + <td>Set the "dedicated physical CPU" performance hint ("-cpu kvm-hint-dedicated=on")</td> + <td>on, off</td> + <td><span class="since">5.7.0 (QEMU 2.12.1)</span></td> + </tr> </table> </dd> <dt><code>pmu</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a0771da45b5d..08853f9d9e92 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5965,6 +5965,11 @@ <ref name="featurestate"/> </element> </optional> + <optional> + <element name="hint-dedicated"> + <ref name="featurestate"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 03312afaaff8..3907fcf6e5ca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -202,6 +202,7 @@ VIR_ENUM_IMPL(virDomainHyperv, VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST, "hidden", + "hint-dedicated", ); VIR_ENUM_IMPL(virDomainMsrsUnknown, @@ -20412,6 +20413,7 @@ virDomainDefParseXML(xmlDocPtr xml, switch ((virDomainKVM) feature) { case VIR_DOMAIN_KVM_HIDDEN: + case VIR_DOMAIN_KVM_DEDICATED: if (!(tmp = virXMLPropString(nodes[i], "state"))) { virReportError(VIR_ERR_XML_ERROR, _("missing 'state' attribute for " @@ -22624,6 +22626,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, for (i = 0; i < VIR_DOMAIN_KVM_LAST; i++) { switch ((virDomainKVM) i) { case VIR_DOMAIN_KVM_HIDDEN: + case VIR_DOMAIN_KVM_DEDICATED: if (src->kvm_features[i] != dst->kvm_features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%s' differs: " @@ -28124,6 +28127,7 @@ virDomainDefFormatFeatures(virBufferPtr buf, for (j = 0; j < VIR_DOMAIN_KVM_LAST; j++) { switch ((virDomainKVM) j) { case VIR_DOMAIN_KVM_HIDDEN: + case VIR_DOMAIN_KVM_DEDICATED: 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 bce47443c858..f7423b1b6f89 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1765,6 +1765,7 @@ typedef enum { typedef enum { VIR_DOMAIN_KVM_HIDDEN = 0, + VIR_DOMAIN_KVM_DEDICATED, VIR_DOMAIN_KVM_LAST } virDomainKVM; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 71a36ff63a81..ab535af5efb6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7212,6 +7212,19 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAddLit(&buf, ",kvm=off"); break; + case VIR_DOMAIN_KVM_DEDICATED: + if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON) { + if (def->cpu && def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) { + virBufferAddLit(&buf, ",kvm-hint-dedicated=on"); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("kvm-hint-dedicated=on is only applicable when " + "<cpu mode=\"host-passthrough\" .../> is in effect")); + goto cleanup; + } + } + break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_KVM_LAST: break; -- 2.21.0

On 8/9/19 5:19 PM, Menno Lageman wrote:
From: Wim ten Have <wim.ten.have@oracle.com>
QEMU version 2.12.1 introduced a performance feature under commit be7773268d98 ("target-i386: add KVM_HINTS_DEDICATED performance hint")
This patch adds a new KVM feature 'hint-dedicated' 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-hint-dedicated=on" to the QEMU command line, the following XML code needs to be added to the guest's domain description in conjunction with CPU mode='host-passthrough'.
<features> <kvm> <hint-dedicated state='on'/> </kvm> </features> ... <cpu mode='host-passthrough ... />
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com> Signed-off-by: Menno Lageman <menno.lageman@oracle.com> --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++++++ 5 files changed, 30 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6d084d7c0472..c9b18b57e1fc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2044,6 +2044,7 @@ </hyperv> <kvm> <hidden state='on'/> + <hint-dedicated state='on'/> </kvm> <pvspinlock state='on'/> <gic version='2'/> @@ -2217,6 +2218,12 @@ <td>on, off</td> <td><span class="since">1.2.8 (QEMU 2.1.0)</span></td> </tr> + <tr> + <td>hint-dedicated</td> + <td>Set the "dedicated physical CPU" performance hint ("-cpu kvm-hint-dedicated=on")</td>
I'd rather not document the command line libvirt generates here. Not only it's an internal thing of libvirt, it'd also be a promise we might not keep up (if qemu changes way how this is configured for instance). But what we should document is what this feature actually is. I've tried to dig out KVM patches and now I have a faint idea, but we can't expect our users to go through patches when deciding whether to turn this on or not.
+ <td>on, off</td> + <td><span class="since">5.7.0 (QEMU 2.12.1)</span></td> + </tr> </table> </dd> <dt><code>pmu</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a0771da45b5d..08853f9d9e92 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5965,6 +5965,11 @@ <ref name="featurestate"/> </element> </optional> + <optional> + <element name="hint-dedicated"> + <ref name="featurestate"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 03312afaaff8..3907fcf6e5ca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -202,6 +202,7 @@ VIR_ENUM_IMPL(virDomainHyperv, VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST, "hidden", + "hint-dedicated", );
VIR_ENUM_IMPL(virDomainMsrsUnknown, @@ -20412,6 +20413,7 @@ virDomainDefParseXML(xmlDocPtr xml,
switch ((virDomainKVM) feature) { case VIR_DOMAIN_KVM_HIDDEN: + case VIR_DOMAIN_KVM_DEDICATED: if (!(tmp = virXMLPropString(nodes[i], "state"))) { virReportError(VIR_ERR_XML_ERROR, _("missing 'state' attribute for " @@ -22624,6 +22626,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, for (i = 0; i < VIR_DOMAIN_KVM_LAST; i++) { switch ((virDomainKVM) i) { case VIR_DOMAIN_KVM_HIDDEN: + case VIR_DOMAIN_KVM_DEDICATED: if (src->kvm_features[i] != dst->kvm_features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%s' differs: " @@ -28124,6 +28127,7 @@ virDomainDefFormatFeatures(virBufferPtr buf, for (j = 0; j < VIR_DOMAIN_KVM_LAST; j++) { switch ((virDomainKVM) j) { case VIR_DOMAIN_KVM_HIDDEN: + case VIR_DOMAIN_KVM_DEDICATED: 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 bce47443c858..f7423b1b6f89 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1765,6 +1765,7 @@ typedef enum {
typedef enum { VIR_DOMAIN_KVM_HIDDEN = 0, + VIR_DOMAIN_KVM_DEDICATED,
VIR_DOMAIN_KVM_LAST } virDomainKVM; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 71a36ff63a81..ab535af5efb6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7212,6 +7212,19 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAddLit(&buf, ",kvm=off"); break;
+ case VIR_DOMAIN_KVM_DEDICATED: + if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON) { + if (def->cpu && def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) { + virBufferAddLit(&buf, ",kvm-hint-dedicated=on"); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("kvm-hint-dedicated=on is only applicable when " + "<cpu mode=\"host-passthrough\" .../> is in effect")); + goto cleanup; + } + } + break;
Ou, I don't think this is the correct place to check for valid domain config. How about, you move the check somewhere to qemuDomainDefValidate() and leave here just the cmd line generator part? Also, the error message is kind of hairy. Long story short, this is what I have on mind: diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c index ab535af5ef..f096e8f27e 100644 --- i/src/qemu/qemu_command.c +++ w/src/qemu/qemu_command.c @@ -7213,16 +7213,8 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, break; case VIR_DOMAIN_KVM_DEDICATED: - if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON) { - if (def->cpu && def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) { - virBufferAddLit(&buf, ",kvm-hint-dedicated=on"); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("kvm-hint-dedicated=on is only applicable when " - "<cpu mode=\"host-passthrough\" .../> is in effect")); - goto cleanup; - } - } + if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON) + virBufferAddLit(&buf, ",kvm-hint-dedicated=on"); break; /* coverity[dead_error_begin] */ diff --git i/src/qemu/qemu_domain.c w/src/qemu/qemu_domain.c index 937b461a8b..e1c31d58a4 100644 --- i/src/qemu/qemu_domain.c +++ w/src/qemu/qemu_domain.c @@ -4698,6 +4698,14 @@ qemuDomainDefValidate(const virDomainDef *def, } } + if (def->kvm_features[VIR_DOMAIN_KVM_DEDICATED] == VIR_TRISTATE_SWITCH_ON && + (!def->cpu || def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("kvm-hint-dedicated=on is only applicable " + "for cpu host-passthrough")); + goto cleanup; + } + ret = 0; cleanup: Michal

On 12-08-19 11:23, Michal Privoznik wrote:
On 8/9/19 5:19 PM, Menno Lageman wrote:
From: Wim ten Have <wim.ten.have@oracle.com>
QEMU version 2.12.1 introduced a performance feature under commit be7773268d98 ("target-i386: add KVM_HINTS_DEDICATED performance hint")
This patch adds a new KVM feature 'hint-dedicated' 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-hint-dedicated=on" to the QEMU command line, the following XML code needs to be added to the guest's domain description in conjunction with CPU mode='host-passthrough'.
<features> <kvm> <hint-dedicated state='on'/> </kvm> </features> ... <cpu mode='host-passthrough ... />
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com> Signed-off-by: Menno Lageman <menno.lageman@oracle.com> --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++++++ 5 files changed, 30 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6d084d7c0472..c9b18b57e1fc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2044,6 +2044,7 @@ </hyperv> <kvm> <hidden state='on'/> + <hint-dedicated state='on'/> </kvm> <pvspinlock state='on'/> <gic version='2'/> @@ -2217,6 +2218,12 @@ <td>on, off</td> <td><span class="since">1.2.8 (QEMU 2.1.0)</span></td> </tr> + <tr> + <td>hint-dedicated</td> + <td>Set the "dedicated physical CPU" performance hint ("-cpu kvm-hint-dedicated=on")</td>
I'd rather not document the command line libvirt generates here. Not only it's an internal thing of libvirt, it'd also be a promise we might not keep up (if qemu changes way how this is configured for instance). But what we should document is what this feature actually is. I've tried to dig out KVM patches and now I have a faint idea, but we can't expect our users to go through patches when deciding whether to turn this on or not.
How about replacing it with "Allows a guest to enable optimizations when running on dedicated vCPU" ?
+ <td>on, off</td> + <td><span class="since">5.7.0 (QEMU 2.12.1)</span></td> + </tr> </table> </dd> <dt><code>pmu</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a0771da45b5d..08853f9d9e92 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5965,6 +5965,11 @@ <ref name="featurestate"/> </element> </optional> + <optional> + <element name="hint-dedicated"> + <ref name="featurestate"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 03312afaaff8..3907fcf6e5ca 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -202,6 +202,7 @@ VIR_ENUM_IMPL(virDomainHyperv, VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST, "hidden", + "hint-dedicated", );
VIR_ENUM_IMPL(virDomainMsrsUnknown, @@ -20412,6 +20413,7 @@ virDomainDefParseXML(xmlDocPtr xml,
switch ((virDomainKVM) feature) { case VIR_DOMAIN_KVM_HIDDEN: + case VIR_DOMAIN_KVM_DEDICATED: if (!(tmp = virXMLPropString(nodes[i], "state"))) { virReportError(VIR_ERR_XML_ERROR, _("missing 'state' attribute for " @@ -22624,6 +22626,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, for (i = 0; i < VIR_DOMAIN_KVM_LAST; i++) { switch ((virDomainKVM) i) { case VIR_DOMAIN_KVM_HIDDEN: + case VIR_DOMAIN_KVM_DEDICATED: if (src->kvm_features[i] != dst->kvm_features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%s' differs: " @@ -28124,6 +28127,7 @@ virDomainDefFormatFeatures(virBufferPtr buf, for (j = 0; j < VIR_DOMAIN_KVM_LAST; j++) { switch ((virDomainKVM) j) { case VIR_DOMAIN_KVM_HIDDEN: + case VIR_DOMAIN_KVM_DEDICATED: 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 bce47443c858..f7423b1b6f89 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1765,6 +1765,7 @@ typedef enum {
typedef enum { VIR_DOMAIN_KVM_HIDDEN = 0, + VIR_DOMAIN_KVM_DEDICATED,
VIR_DOMAIN_KVM_LAST } virDomainKVM; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 71a36ff63a81..ab535af5efb6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7212,6 +7212,19 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, virBufferAddLit(&buf, ",kvm=off"); break;
+ case VIR_DOMAIN_KVM_DEDICATED: + if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON) { + if (def->cpu && def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) { + virBufferAddLit(&buf, ",kvm-hint-dedicated=on"); + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("kvm-hint-dedicated=on is only applicable when " + "<cpu mode=\"host-passthrough\" .../> is in effect")); + goto cleanup; + } + } + break;
Ou, I don't think this is the correct place to check for valid domain config. How about, you move the check somewhere to qemuDomainDefValidate() and leave here just the cmd line generator part? Also, the error message is kind of hairy.
Long story short, this is what I have on mind:
diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c index ab535af5ef..f096e8f27e 100644 --- i/src/qemu/qemu_command.c +++ w/src/qemu/qemu_command.c @@ -7213,16 +7213,8 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, break;
case VIR_DOMAIN_KVM_DEDICATED: - if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON) { - if (def->cpu && def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) { - virBufferAddLit(&buf, ",kvm-hint-dedicated=on"); - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("kvm-hint-dedicated=on is only applicable when " - "<cpu mode=\"host-passthrough\" .../> is in effect")); - goto cleanup; - } - } + if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON) + virBufferAddLit(&buf, ",kvm-hint-dedicated=on"); break;
/* coverity[dead_error_begin] */ diff --git i/src/qemu/qemu_domain.c w/src/qemu/qemu_domain.c index 937b461a8b..e1c31d58a4 100644 --- i/src/qemu/qemu_domain.c +++ w/src/qemu/qemu_domain.c @@ -4698,6 +4698,14 @@ qemuDomainDefValidate(const virDomainDef *def, } }
+ if (def->kvm_features[VIR_DOMAIN_KVM_DEDICATED] == VIR_TRISTATE_SWITCH_ON && + (!def->cpu || def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("kvm-hint-dedicated=on is only applicable " + "for cpu host-passthrough")); + goto cleanup; + } + ret = 0;
cleanup:
Yup, makes sense. I'll send a v2. Thanks, Menno
Michal

On 8/12/19 1:18 PM, Menno Lageman wrote:
On 12-08-19 11:23, Michal Privoznik wrote:
On 8/9/19 5:19 PM, Menno Lageman wrote:
From: Wim ten Have <wim.ten.have@oracle.com>
QEMU version 2.12.1 introduced a performance feature under commit be7773268d98 ("target-i386: add KVM_HINTS_DEDICATED performance hint")
This patch adds a new KVM feature 'hint-dedicated' 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-hint-dedicated=on" to the QEMU command line, the following XML code needs to be added to the guest's domain description in conjunction with CPU mode='host-passthrough'.
<features> <kvm> <hint-dedicated state='on'/> </kvm> </features> ... <cpu mode='host-passthrough ... />
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com> Signed-off-by: Menno Lageman <menno.lageman@oracle.com> --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++++++ 5 files changed, 30 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6d084d7c0472..c9b18b57e1fc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2044,6 +2044,7 @@ </hyperv> <kvm> <hidden state='on'/> + <hint-dedicated state='on'/> </kvm> <pvspinlock state='on'/> <gic version='2'/> @@ -2217,6 +2218,12 @@ <td>on, off</td> <td><span class="since">1.2.8 (QEMU 2.1.0)</span></td> </tr> + <tr> + <td>hint-dedicated</td> + <td>Set the "dedicated physical CPU" performance hint ("-cpu kvm-hint-dedicated=on")</td>
I'd rather not document the command line libvirt generates here. Not only it's an internal thing of libvirt, it'd also be a promise we might not keep up (if qemu changes way how this is configured for instance). But what we should document is what this feature actually is. I've tried to dig out KVM patches and now I have a faint idea, but we can't expect our users to go through patches when deciding whether to turn this on or not.
How about replacing it with "Allows a guest to enable optimizations when running on dedicated vCPU" ?
What do you mean by 'dedicated vCPU'? So far I only know dedicated physical CPU (or CPU core) - and that's not easy to set up, you'll need numa pinning, cpu pinning, isolcpus, and whatnot. Also, what opimizations does it enable? I'm not saying we need to put everything in the docs, but also I don't want the docs to be undestandable by developers who developed given feature. Michal

On 12-08-19 13:39, Michal Privoznik wrote:
On 8/12/19 1:18 PM, Menno Lageman wrote:
On 12-08-19 11:23, Michal Privoznik wrote:
On 8/9/19 5:19 PM, Menno Lageman wrote:
From: Wim ten Have <wim.ten.have@oracle.com>
QEMU version 2.12.1 introduced a performance feature under commit be7773268d98 ("target-i386: add KVM_HINTS_DEDICATED performance hint")
This patch adds a new KVM feature 'hint-dedicated' 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-hint-dedicated=on" to the QEMU command line, the following XML code needs to be added to the guest's domain description in conjunction with CPU mode='host-passthrough'.
<features> <kvm> <hint-dedicated state='on'/> </kvm> </features> ... <cpu mode='host-passthrough ... />
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com> Signed-off-by: Menno Lageman <menno.lageman@oracle.com> --- docs/formatdomain.html.in | 7 +++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_conf.c | 4 ++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++++++ 5 files changed, 30 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6d084d7c0472..c9b18b57e1fc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2044,6 +2044,7 @@ </hyperv> <kvm> <hidden state='on'/> + <hint-dedicated state='on'/> </kvm> <pvspinlock state='on'/> <gic version='2'/> @@ -2217,6 +2218,12 @@ <td>on, off</td> <td><span class="since">1.2.8 (QEMU 2.1.0)</span></td> </tr> + <tr> + <td>hint-dedicated</td> + <td>Set the "dedicated physical CPU" performance hint ("-cpu kvm-hint-dedicated=on")</td>
I'd rather not document the command line libvirt generates here. Not only it's an internal thing of libvirt, it'd also be a promise we might not keep up (if qemu changes way how this is configured for instance). But what we should document is what this feature actually is. I've tried to dig out KVM patches and now I have a faint idea, but we can't expect our users to go through patches when deciding whether to turn this on or not.
How about replacing it with "Allows a guest to enable optimizations when running on dedicated vCPU" ?
What do you mean by 'dedicated vCPU'? So far I only know dedicated physical CPU (or CPU core) - and that's not easy to set up, you'll need numa pinning, cpu pinning, isolcpus, and whatnot. Also, what opimizations does it enable?
I tried to extract the essence of the QEMU commit message ("Add KVM_HINTS_DEDICATED performance hint, guest checks this feature bit to determine if they run on dedicated vCPUs, allowing optimizations such as usage of qspinlocks.") without adding too much detail about exactly what optimizations are available since it is the guest that does (or does not) enable these optimizations. Menno
I'm not saying we need to put everything in the docs, but also I don't want the docs to be undestandable by developers who developed given feature.
Michal
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

From: Wim ten Have <wim.ten.have@oracle.com> Update the KVM feature tests for QEMU's kvm-hint-dedicated performance hint. Signed-off-by: Wim ten Have <wim.ten.have@oracle.com> Signed-off-by: Menno Lageman <menno.lageman@oracle.com> --- tests/qemuxml2argvdata/kvm-features-off.xml | 1 + tests/qemuxml2argvdata/kvm-features.args | 4 ++-- tests/qemuxml2argvdata/kvm-features.xml | 4 +++- tests/qemuxml2xmloutdata/kvm-features-off.xml | 1 + tests/qemuxml2xmloutdata/kvm-features.xml | 4 +++- 5 files changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/qemuxml2argvdata/kvm-features-off.xml b/tests/qemuxml2argvdata/kvm-features-off.xml index e893f9343754..ab2c16202c3d 100644 --- a/tests/qemuxml2argvdata/kvm-features-off.xml +++ b/tests/qemuxml2argvdata/kvm-features-off.xml @@ -12,6 +12,7 @@ <acpi/> <kvm> <hidden state='off'/> + <hint-dedicated state='off'/> </kvm> </features> <clock offset='utc'/> diff --git a/tests/qemuxml2argvdata/kvm-features.args b/tests/qemuxml2argvdata/kvm-features.args index 55d2b8be02ea..8372ca897d8d 100644 --- a/tests/qemuxml2argvdata/kvm-features.args +++ b/tests/qemuxml2argvdata/kvm-features.args @@ -10,8 +10,8 @@ QEMU_AUDIO_DRV=none \ /usr/bin/qemu-system-i686 \ -name QEMUGuest1 \ -S \ --machine pc,accel=tcg,usb=off,dump-guest-core=off \ --cpu qemu32,kvm=off \ +-machine pc,accel=kvm,usb=off,dump-guest-core=off \ +-cpu host,kvm=off,kvm-hint-dedicated=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 68bdee42bacf..b942e89a9280 100644 --- a/tests/qemuxml2argvdata/kvm-features.xml +++ b/tests/qemuxml2argvdata/kvm-features.xml @@ -1,4 +1,4 @@ -<domain type='qemu'> +<domain type='kvm'> <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219100</memory> @@ -12,8 +12,10 @@ <acpi/> <kvm> <hidden state='on'/> + <hint-dedicated state='on'/> </kvm> </features> + <cpu mode='host-passthrough' check='none'/> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> diff --git a/tests/qemuxml2xmloutdata/kvm-features-off.xml b/tests/qemuxml2xmloutdata/kvm-features-off.xml index b355061c5b01..a0ef9c7e9e15 100644 --- a/tests/qemuxml2xmloutdata/kvm-features-off.xml +++ b/tests/qemuxml2xmloutdata/kvm-features-off.xml @@ -12,6 +12,7 @@ <acpi/> <kvm> <hidden state='off'/> + <hint-dedicated state='off'/> </kvm> </features> <clock offset='utc'/> diff --git a/tests/qemuxml2xmloutdata/kvm-features.xml b/tests/qemuxml2xmloutdata/kvm-features.xml index 544b9d860755..b6f16ced1dab 100644 --- a/tests/qemuxml2xmloutdata/kvm-features.xml +++ b/tests/qemuxml2xmloutdata/kvm-features.xml @@ -1,4 +1,4 @@ -<domain type='qemu'> +<domain type='kvm'> <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> <memory unit='KiB'>219100</memory> @@ -12,8 +12,10 @@ <acpi/> <kvm> <hidden state='on'/> + <hint-dedicated state='on'/> </kvm> </features> + <cpu mode='host-passthrough' check='none'/> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> -- 2.21.0

On 8/9/19 5:19 PM, Menno Lageman wrote:
From: Wim ten Have <wim.ten.have@oracle.com>
Update the KVM feature tests for QEMU's kvm-hint-dedicated performance hint.
Signed-off-by: Wim ten Have <wim.ten.have@oracle.com> Signed-off-by: Menno Lageman <menno.lageman@oracle.com> --- tests/qemuxml2argvdata/kvm-features-off.xml | 1 + tests/qemuxml2argvdata/kvm-features.args | 4 ++-- tests/qemuxml2argvdata/kvm-features.xml | 4 +++- tests/qemuxml2xmloutdata/kvm-features-off.xml | 1 + tests/qemuxml2xmloutdata/kvm-features.xml | 4 +++- 5 files changed, 10 insertions(+), 4 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Menno Lageman
-
Michal Privoznik