docs/formatdomain.rst | 2 ++ src/conf/domain_conf.c | 30 +++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/schemas/domaincommon.rng | 10 +++++++ src/qemu/qemu_command.c | 7 +++++ tests/qemuxmlconfdata/kvm-features-off.xml | 1 + .../kvm-features.x86_64-latest.args | 2 +- tests/qemuxmlconfdata/kvm-features.xml | 1 + 8 files changed, 54 insertions(+), 1 deletion(-)
Add the support in libvirt to activate the RAPL feature in QEMU.
This feature is activated with -accel kvm,rapl=true,path=/path/sock.sock
in QEMU.
The feature is activated in libvirt with the following XML
configuration:
<kvm>
[...]
<rapl state ='on' socket='/run/qemu-vmsr-helper.sock'/>
[...]
</kvm>
See: https://gitlab.com/qemu-project/qemu/-/commit/0418f90809aea5b375c859e744c8e8610e9be446
Signed-off-by: Anthony Harivel <aharivel@redhat.com>
---
docs/formatdomain.rst | 2 ++
src/conf/domain_conf.c | 30 +++++++++++++++++++
src/conf/domain_conf.h | 2 ++
src/conf/schemas/domaincommon.rng | 10 +++++++
src/qemu/qemu_command.c | 7 +++++
tests/qemuxmlconfdata/kvm-features-off.xml | 1 +
.../kvm-features.x86_64-latest.args | 2 +-
tests/qemuxmlconfdata/kvm-features.xml | 1 +
8 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index e1e219bcb5c5..5c66e41e940f 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -1996,6 +1996,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off.
<poll-control state='on'/>
<pv-ipi state='off'/>
<dirty-ring state='on' size='4096'/>
+ <rapl state ='on' socket='/run/qemu-vmsr-helper.sock'/>
</kvm>
<xen>
<e820_host state='on'/>
@@ -2111,6 +2112,7 @@ are:
poll-control Decrease IO completion latency by introducing a grace period of busy waiting on, off :since:`6.10.0 (QEMU 4.2)`
pv-ipi Paravirtualized send IPIs on, off :since:`7.10.0 (QEMU 3.1)`
dirty-ring Enable dirty ring feature on, off; size - must be power of 2, range [1024,65536] :since:`8.0.0 (QEMU 6.1)`
+ rapl Enable rapl feature on, off; socket - rapl helper socket :since:`10.8.0 (QEMU 9.1)`
============== ============================================================================ ====================================================== ============================
``xen``
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d950921667a2..06fbcac4aad1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -220,6 +220,7 @@ VIR_ENUM_IMPL(virDomainKVM,
"poll-control",
"pv-ipi",
"dirty-ring",
+ "rapl"
);
VIR_ENUM_IMPL(virDomainXen,
@@ -16693,6 +16694,14 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
return -1;
}
}
+
+ /* rapl feature should parse socket property */
+ if (feature == VIR_DOMAIN_KVM_RAPL && value == VIR_TRISTATE_SWITCH_ON) {
+ kvm->socket = virXMLPropString(feat, "socket");
+ if (kvm->socket == NULL) {
+ return -1;
+ }
+ }
}
def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON;
@@ -21108,6 +21117,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src,
case VIR_DOMAIN_KVM_POLLCONTROL:
case VIR_DOMAIN_KVM_PVIPI:
case VIR_DOMAIN_KVM_DIRTY_RING:
+ case VIR_DOMAIN_KVM_RAPL:
if (src->kvm_features->features[i] != dst->kvm_features->features[i]) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("State of KVM feature '%1$s' differs: source: '%2$s', destination: '%3$s'"),
@@ -21132,6 +21142,15 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src,
dst->kvm_features->dirty_ring_size);
return false;
}
+
+ if (src->kvm_features->socket != dst->kvm_features->socket) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("rapl helper socket of KVM feature '%1$s' differs: source: '%2$s', destination: '%3$s'"),
+ virDomainKVMTypeToString(i),
+ src->kvm_features->socket,
+ dst->kvm_features->socket);
+ return false;
+ }
}
/* smm */
@@ -27840,6 +27859,17 @@ virDomainDefFormatFeatures(virBuffer *buf,
}
break;
+ case VIR_DOMAIN_KVM_RAPL:
+ if (def->kvm_features->features[j] != VIR_TRISTATE_SWITCH_ABSENT) {
+ virBufferAsprintf(&childBuf, "<%s state='%s'",
+ virDomainKVMTypeToString(j),
+ virTristateSwitchTypeToString(def->kvm_features->features[j]));
+
+ virBufferEscapeString(&childBuf, " socket='%s'", def->kvm_features->socket);
+ virBufferAddLit(&childBuf, "/>\n");
+ }
+ break;
+
case VIR_DOMAIN_KVM_LAST:
break;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index eae621f900b9..aa777850ec72 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2213,6 +2213,7 @@ typedef enum {
VIR_DOMAIN_KVM_POLLCONTROL,
VIR_DOMAIN_KVM_PVIPI,
VIR_DOMAIN_KVM_DIRTY_RING,
+ VIR_DOMAIN_KVM_RAPL,
VIR_DOMAIN_KVM_LAST
} virDomainKVM;
@@ -2400,6 +2401,7 @@ struct _virDomainFeatureKVM {
int features[VIR_DOMAIN_KVM_LAST];
unsigned int dirty_ring_size; /* size of dirty ring for each vCPU, no units */
+ char *socket; /* rapl helper socket */
};
typedef struct _virDomainFeatureTCG virDomainFeatureTCG;
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index 05ba69792470..1883bb115ee5 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -8016,6 +8016,16 @@
</optional>
</element>
</optional>
+ <optional>
+ <element name="rapl">
+ <ref name="featurestate"/>
+ <optional>
+ <attribute name="socket">
+ <data type="string"/>
+ </attribute>
+ </optional>
+ </element>
+ </optional>
</interleave>
</element>
</define>
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 28914c9c346a..d20fb6df5e8e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6562,6 +6562,9 @@ qemuBuildCpuCommandLine(virCommand *cmd,
case VIR_DOMAIN_KVM_DIRTY_RING:
break;
+ case VIR_DOMAIN_KVM_RAPL:
+ break;
+
case VIR_DOMAIN_KVM_LAST:
break;
}
@@ -7176,6 +7179,10 @@ qemuBuildAccelCommandLine(virCommand *cmd,
def->kvm_features->features[VIR_DOMAIN_KVM_DIRTY_RING] == VIR_TRISTATE_SWITCH_ON) {
virBufferAsprintf(&buf, ",dirty-ring-size=%d", def->kvm_features->dirty_ring_size);
}
+ if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON &&
+ def->kvm_features->features[VIR_DOMAIN_KVM_RAPL] == VIR_TRISTATE_SWITCH_ON) {
+ virBufferAsprintf(&buf, ",rapl=true,rapl-helper-socket=%s", def->kvm_features->socket);
+ }
break;
case VIR_DOMAIN_VIRT_HVF:
diff --git a/tests/qemuxmlconfdata/kvm-features-off.xml b/tests/qemuxmlconfdata/kvm-features-off.xml
index 3cd4ff18f283..3e8dbea4b177 100644
--- a/tests/qemuxmlconfdata/kvm-features-off.xml
+++ b/tests/qemuxmlconfdata/kvm-features-off.xml
@@ -16,6 +16,7 @@
<poll-control state='off'/>
<pv-ipi state='off'/>
<dirty-ring state='off'/>
+ <rapl state='off'/>
</kvm>
</features>
<cpu mode='host-passthrough' check='none' migratable='off'/>
diff --git a/tests/qemuxmlconfdata/kvm-features.x86_64-latest.args b/tests/qemuxmlconfdata/kvm-features.x86_64-latest.args
index 955db67eb4b7..2dd613ab338d 100644
--- a/tests/qemuxmlconfdata/kvm-features.x86_64-latest.args
+++ b/tests/qemuxmlconfdata/kvm-features.x86_64-latest.args
@@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
-S \
-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \
-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=on \
--accel kvm,dirty-ring-size=4096 \
+-accel kvm,dirty-ring-size=4096,rapl=true,rapl-helper-socket=/run/qemu-vmsr-helper.sock \
-cpu host,migratable=off,kvm=off,kvm-hint-dedicated=on,kvm-poll-control=on \
-m size=219136k \
-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
diff --git a/tests/qemuxmlconfdata/kvm-features.xml b/tests/qemuxmlconfdata/kvm-features.xml
index 78091064b109..ee601e06de75 100644
--- a/tests/qemuxmlconfdata/kvm-features.xml
+++ b/tests/qemuxmlconfdata/kvm-features.xml
@@ -16,6 +16,7 @@
<poll-control state='on'/>
<pv-ipi state='on'/>
<dirty-ring state='on' size='4096'/>
+ <rapl state='on' socket='/run/qemu-vmsr-helper.sock'/>
</kvm>
</features>
<cpu mode='host-passthrough' check='none' migratable='off'/>
--
2.46.0On Tue, Aug 20, 2024 at 17:10:00 +0200, Anthony Harivel wrote:
> Add the support in libvirt to activate the RAPL feature in QEMU.
>
> This feature is activated with -accel kvm,rapl=true,path=/path/sock.sock
> in QEMU.
>
> The feature is activated in libvirt with the following XML
> configuration:
>
> <kvm>
> [...]
> <rapl state ='on' socket='/run/qemu-vmsr-helper.sock'/>
> [...]
> </kvm>
>
> See: https://gitlab.com/qemu-project/qemu/-/commit/0418f90809aea5b375c859e744c8e8610e9be446
So based on how we've implemented other features configured similarly
this makes sense. Although the documentation states that this works only
on Intel hosts I don't think it makes sense for libvirt do do any
checking.
>
> Signed-off-by: Anthony Harivel <aharivel@redhat.com>
> ---
> docs/formatdomain.rst | 2 ++
> src/conf/domain_conf.c | 30 +++++++++++++++++++
> src/conf/domain_conf.h | 2 ++
> src/conf/schemas/domaincommon.rng | 10 +++++++
> src/qemu/qemu_command.c | 7 +++++
> tests/qemuxmlconfdata/kvm-features-off.xml | 1 +
> .../kvm-features.x86_64-latest.args | 2 +-
> tests/qemuxmlconfdata/kvm-features.xml | 1 +
> 8 files changed, 54 insertions(+), 1 deletion(-)
[...]
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index d950921667a2..06fbcac4aad1 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -220,6 +220,7 @@ VIR_ENUM_IMPL(virDomainKVM,
> "poll-control",
> "pv-ipi",
> "dirty-ring",
> + "rapl"
Missing comma at the end, making the next person add one.
> );
>
> VIR_ENUM_IMPL(virDomainXen,
> @@ -16693,6 +16694,14 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
> return -1;
> }
> }
> +
> + /* rapl feature should parse socket property */
> + if (feature == VIR_DOMAIN_KVM_RAPL && value == VIR_TRISTATE_SWITCH_ON) {
> + kvm->socket = virXMLPropString(feat, "socket");
> + if (kvm->socket == NULL) {
> + return -1;
'virXMLPropString' returns NULL only if the property is missing. With
code like this you'd not report an error, so the user will not know
what's wrong.
If you want to make the field mandatory you'll need to use either
virXMLPropStringRequired or provide your own error.
Or drop the NULL check if 'socket' is optional.
Also since it's a socket path you should check that the user provided an
absolute path (g_path_is_absolute) and report error if not.
> def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON;
> @@ -21108,6 +21117,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src,
> case VIR_DOMAIN_KVM_POLLCONTROL:
> case VIR_DOMAIN_KVM_PVIPI:
> case VIR_DOMAIN_KVM_DIRTY_RING:
> + case VIR_DOMAIN_KVM_RAPL:
> if (src->kvm_features->features[i] != dst->kvm_features->features[i]) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("State of KVM feature '%1$s' differs: source: '%2$s', destination: '%3$s'"),
> @@ -21132,6 +21142,15 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src,
> dst->kvm_features->dirty_ring_size);
> return false;
> }
> +
> + if (src->kvm_features->socket != dst->kvm_features->socket) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("rapl helper socket of KVM feature '%1$s' differs: source: '%2$s', destination: '%3$s'"),
> + virDomainKVMTypeToString(i),
> + src->kvm_features->socket,
> + dst->kvm_features->socket);
> + return false;
> + }
The socket path doesn't look like guest OS ABI to me. (The guest OS
doesn't care where you are pointing the socket to, and on the remote
side of the migration it seems as if this should be possible to change.
Thus this check should be dropped.
(If it's not dropped, you must use NULLSTR around them. Also you most
likely want to use STREQ?)
> }
>
> /* smm */
> @@ -27840,6 +27859,17 @@ virDomainDefFormatFeatures(virBuffer *buf,
> }
> break;
>
> + case VIR_DOMAIN_KVM_RAPL:
> + if (def->kvm_features->features[j] != VIR_TRISTATE_SWITCH_ABSENT) {
> + virBufferAsprintf(&childBuf, "<%s state='%s'",
> + virDomainKVMTypeToString(j),
> + virTristateSwitchTypeToString(def->kvm_features->features[j]));
> +
> + virBufferEscapeString(&childBuf, " socket='%s'", def->kvm_features->socket);
> + virBufferAddLit(&childBuf, "/>\n");
> + }
> + break;
> +
> case VIR_DOMAIN_KVM_LAST:
> break;
> }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index eae621f900b9..aa777850ec72 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2213,6 +2213,7 @@ typedef enum {
> VIR_DOMAIN_KVM_POLLCONTROL,
> VIR_DOMAIN_KVM_PVIPI,
> VIR_DOMAIN_KVM_DIRTY_RING,
> + VIR_DOMAIN_KVM_RAPL,
>
> VIR_DOMAIN_KVM_LAST
> } virDomainKVM;
> @@ -2400,6 +2401,7 @@ struct _virDomainFeatureKVM {
> int features[VIR_DOMAIN_KVM_LAST];
>
> unsigned int dirty_ring_size; /* size of dirty ring for each vCPU, no units */
> + char *socket; /* rapl helper socket */
Name the variable 'rapl_helper_socket', that way it will be obvious
everywhere and not only here at the comment.
> };
>
> typedef struct _virDomainFeatureTCG virDomainFeatureTCG;
> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> index 05ba69792470..1883bb115ee5 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -8016,6 +8016,16 @@
> </optional>
> </element>
> </optional>
> + <optional>
> + <element name="rapl">
> + <ref name="featurestate"/>
> + <optional>
> + <attribute name="socket">
> + <data type="string"/>
We have a type for a local path named 'absFilePath'.
> + </attribute>
> + </optional>
> + </element>
> + </optional>
> </interleave>
> </element>
> </define>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 28914c9c346a..d20fb6df5e8e 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6562,6 +6562,9 @@ qemuBuildCpuCommandLine(virCommand *cmd,
> case VIR_DOMAIN_KVM_DIRTY_RING:
> break;
>
> + case VIR_DOMAIN_KVM_RAPL:
> + break;
> +
> case VIR_DOMAIN_KVM_LAST:
> break;
> }
> @@ -7176,6 +7179,10 @@ qemuBuildAccelCommandLine(virCommand *cmd,
> def->kvm_features->features[VIR_DOMAIN_KVM_DIRTY_RING] == VIR_TRISTATE_SWITCH_ON) {
> virBufferAsprintf(&buf, ",dirty-ring-size=%d", def->kvm_features->dirty_ring_size);
> }
> + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON &&
> + def->kvm_features->features[VIR_DOMAIN_KVM_RAPL] == VIR_TRISTATE_SWITCH_ON) {
> + virBufferAsprintf(&buf, ",rapl=true,rapl-helper-socket=%s", def->kvm_features->socket);
Okay, so socket is mandatory.
Also should we honour also '_OFF' state?
> + }
> break;
>
> case VIR_DOMAIN_VIRT_HVF:
> diff --git a/tests/qemuxmlconfdata/kvm-features-off.xml b/tests/qemuxmlconfdata/kvm-features-off.xml
> index 3cd4ff18f283..3e8dbea4b177 100644
> --- a/tests/qemuxmlconfdata/kvm-features-off.xml
> +++ b/tests/qemuxmlconfdata/kvm-features-off.xml
> @@ -16,6 +16,7 @@
> <poll-control state='off'/>
> <pv-ipi state='off'/>
> <dirty-ring state='off'/>
> + <rapl state='off'/>
> </kvm>
> </features>
> <cpu mode='host-passthrough' check='none' migratable='off'/>
> diff --git a/tests/qemuxmlconfdata/kvm-features.x86_64-latest.args b/tests/qemuxmlconfdata/kvm-features.x86_64-latest.args
> index 955db67eb4b7..2dd613ab338d 100644
> --- a/tests/qemuxmlconfdata/kvm-features.x86_64-latest.args
> +++ b/tests/qemuxmlconfdata/kvm-features.x86_64-latest.args
> @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
> -S \
> -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \
> -machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=on \
> --accel kvm,dirty-ring-size=4096 \
> +-accel kvm,dirty-ring-size=4096,rapl=true,rapl-helper-socket=/run/qemu-vmsr-helper.sock \
> -cpu host,migratable=off,kvm=off,kvm-hint-dedicated=on,kvm-poll-control=on \
> -m size=219136k \
> -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
> diff --git a/tests/qemuxmlconfdata/kvm-features.xml b/tests/qemuxmlconfdata/kvm-features.xml
> index 78091064b109..ee601e06de75 100644
> --- a/tests/qemuxmlconfdata/kvm-features.xml
> +++ b/tests/qemuxmlconfdata/kvm-features.xml
> @@ -16,6 +16,7 @@
> <poll-control state='on'/>
> <pv-ipi state='on'/>
> <dirty-ring state='on' size='4096'/>
> + <rapl state='on' socket='/run/qemu-vmsr-helper.sock'/>
> </kvm>
> </features>
> <cpu mode='host-passthrough' check='none' migratable='off'/>
The rest looks good to me but I'll leave the possibility for others to
chime in even after you post a fixed version.
© 2016 - 2024 Red Hat, Inc.