10:17 a.m.
On Wed, Feb 12, 2025 at 16:36:48 +0100, 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/0418f90809aea5b375c859e744c...
So what are the plans regarding the managed mode here?
I don't necessarily need you to commit to anything but if libvirt is
ever expected to support the managed mode we should at least have a
strategy how to integrate that and document that only non-managed mode
is supported.
E.g. prepare for
<rapl state ='on' mode='unmanaged'
socket='/run/qemu-vmsr-helper.sock'/>
Or at least imply in the documentation that if socket is provided we
expect an manually managed daemon to listen there.
Signed-off-by: Anthony Harivel <aharivel(a)redhat.com>
---
docs/formatdomain.rst | 2 ++
src/conf/domain_conf.c | 18 ++++++++++++++++++
src/conf/domain_conf.h | 2 ++
src/conf/schemas/domaincommon.rng | 10 ++++++++++
src/qemu/qemu_command.c | 12 ++++++++++++
tests/qemuxmlconfdata/kvm-features-off.xml | 1 +
.../kvm-features.x86_64-latest.args | 2 +-
tests/qemuxmlconfdata/kvm-features.xml | 1 +
8 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 83aeaa32c296..eceee1531085 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2016,6 +2016,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'/>
@@ -2135,6 +2136,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)`
==============
============================================================================
====================================================== ============================
The libvirt version here will need updating.
Since this works only on intel:
$ qemu-system-x86_64 --accel kvm,rapl=true
qemu-system-x86_64: --accel kvm,rapl=true: The RAPL feature can only be enabled on hosts
with Intel CPU models
qemu-system-x86_64: --accel kvm,rapl=true: kvm : error RAPL feature requirement not met
you should document that. I don't think we need a specific CPU check for
this niche feature.
@@ -16837,6 +16838,11 @@ virDomainFeaturesKVMDefParse(virDomainDef
*def,
return -1;
}
}
+
+ /* rapl feature should parse socket property */
+ if (feature == VIR_DOMAIN_KVM_RAPL && value == VIR_TRISTATE_SWITCH_ON)
{
+ def->kvm_features->rapl_helper_socket = virXMLPropString(feat,
"socket");
+ }
}
Qemu will fail without a socket.
$ qemu-system-x86_64 --accel kvm,rapl=true
qemu-system-x86_64: --accel kvm,rapl=true: vmsr socket opening failed
If you make the socket mandatory the presence of the socket could later
be used to mean un-managed mode.
def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON;
@@ -21308,6 +21314,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'"),
@@ -28107,6 +28114,17 @@ virDomainDefFormatFeatures(virBuffer *buf,
[...]
typedef struct _virDomainFeatureTCG virDomainFeatureTCG;
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index 96cedb85e867..c910df8c75a0 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -8143,6 +8143,16 @@
</optional>
</element>
</optional>
+ <optional>
+ <element name="rapl">
+ <ref name="featurestate"/>
+ <optional>
+ <attribute name="socket">
+ <ref name="absFilePath"/>
+ </attribute>
+ </optional>
+ </element>
+ </optional>
</interleave>
</element>
</define>
[...]
@@ -7062,6 +7065,15 @@ 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) {
+ virBufferAddLit(&buf, ",rapl=true");
+
+ if (def->kvm_features->rapl_helper_socket != NULL) {
+ virBufferAsprintf(&buf, ",rapl-helper-socket=%s",
def->kvm_features->rapl_helper_socket);
+ }
As noted above; qemu makes the socket mandatory so the check doesn't
make much sense.
Also we'll most likely need to use virQEMUBuildBufferEscapeComma here to
properly format paths with a comma since they are user-provided.
+ }
break;
case VIR_DOMAIN_VIRT_HVF:
The code doesn't also do any security labelling of the socket. That
means that the socket needs to be properly labelled manually. I think
that should be documetned as well, so that there's no expectation of
libvirt to do this.