Re: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature

Hi, Anthony Harivel, Sep 03, 2024 at 14:41: > Daniel P. Berrangé, Sep 03, 2024 at 14:24: > > On Tue, Sep 03, 2024 at 02:16:58PM +0200, Peter Krempa wrote: > > > On Tue, Sep 03, 2024 at 13:29:28 +0200, Anthony Harivel wrote: > > > > Daniel P. Berrangé, Sep 03, 2024 at 12:08: > > > > > On Mon, Sep 02, 2024 at 03:09:42PM +0200, Peter Krempa wrote: > > > > > > On Thu, Aug 22, 2024 at 17:59:47 +0200, Anthony Harivel wrote: > > > > > > > Add the support in libvirt to activate the RAPL feature in QEMU. > > > > > > I suppose that the 'rapl-helper-socket' is a shared (multiple qemu's use > > > > > > it) resource set up beforehand by the admin. Right? > > > > > > > > > > The qemu-pr-helper could be run as a single instnce, or it could be > > > > > run per-QEMU instance. The latter would give us better security > > > > > isolation, for what is a privileged daemon. On the other hand, I > > > > > wonder about the CPU overhead of having 100's of copies of the > > > > > process running on a host. > > > > > > So when I was originally skimming trhough the docs I didn't properly > > > understand that the reason for the helper daemon is that there was a > > > security issue with accessing the measurement counters and thought it > > > was strictly for performance reasons. > > > > Yep, this is only for security reasons. > > > > TL;DR: > "A malicious user application may use RAPL to infer confidential > information of another unprivileged application running on the same or > a different CPU core by observing the power-related behavior of the > victim application." > > The answer from Intel was to put the interface being privileged > permission. > > Link: > https://www.intel.com/content/www/us/en/developer/articles/technical/soft... > > > > > If it runs on a single instance, then the socket needs to be chmod/chown > > > > to something like qemu / libvirt group with access only to root and > > > > group. > > > > > > Another alternative for a shared instance to be used by multiple qemu > > > instances is that libvirt can open the socket and pass it to qemu, which > > > avoids the potential issue at-least with DAC security labels as the > > > socket can be owned by root:root with mode 600. > > > > > > I'm not sure how the selinux policy for that daemon looks though. > > > > I don't believe any policy exists yet, since this is a brand new > > service. > > > > > > Running one helper instance per-QEMU instance would mean that every > > > > instance read 1 MSR / Package every second. The socket is left open > > > > (thanks to Daniel suggestion in QEMU review). The impact would be quite > > > > low I guess on the housekeeping CPU. > > > > > > > > When I designed the daemon with Paolo, the first solution was the main > > > > idea but I'm open to any solution that leads to a better adoption of the > > > > feature. > > > > > > Libvirt can obviously manage also a per VM instance, which should be > > > straightforward, but not as simple as this patch. This can theoretically > > > also be added later, e.g. by adding a 'managed' property enabling the > > > libvirt-managed daemon. > > > > A parallel would be the qemu-pr-helper which this new service was initially > > derived from in terms of archicture. It also runs privileged for security > > reasons and can be single shared instance, or per-VM instance. > > > > Which actually leads to the question to the following: > Why do I "simply" not do like the qemu-pr-helper in libvirt ? > > see qemuProcessStartManagedPRDaemon() in src/qemu/qemu_process.c +2808 > > > > With regards, > > Daniel > > -- > > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > > |: https://libvirt.org -o- https://fstop138.berrange.com :| > > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| If I may resume the conversation: 1) The helper daemon is primarily needed for security reasons to prevent potential leaks of confidential information through RAPL. 2) There are two main ways to handle the helper daemon: > Single instance for all QEMU processes: This requires adjusting socket permissions (chmod/chown) to control access securely. > One instance per QEMU process: This offers better isolation but might increase CPU overhead slightly if the number of instances running get high. 3) Libvirt can manage both approaches, but no existing SELinux policy covers this yet, so we’ll need to consider that. 4) qemu-pr-helper is very similar to the qemu-vmsr-helper in terms of architecture and in terms of privileged needs. It can be single shared instance or per-vm instance. I cannot find the original patch that has added qemu-pr-helper into libvirt. The latest commit in src/qemu/qemu_process is 7eead248c65f and it doesn't look right IMHO. If you have a reference to the original patch, I can use it as a guide to implement this helper in the same way, if you believe that is the best solution. Thanks, Anthony