Hi Peter, Peter Krempa, Feb 12, 2025 at 17:17:
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/0418f90809aea5b375c859e744c8e8...
So what are the plans regarding the managed mode here?
I think I will go first, the way Paolo suggested, deliver the helper thought a RPM, i.e unmanaged by libvirt. I did not find any relevant justification for this simple stateless helper to be managed by libvirt. Technically, the helper could also be distributed as a container. But I guess mixing RPM & container for an install is not very appreciated. At the moment it comes with qemu RPM. The systemd files are included as well.
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@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.
Thanks for all your feedback. I've added all in my todo list for my next sprint. I understand that it needs way more documentation ! Anthony