QEMU-Devel Archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Anthony Harivel <aharivel@redhat.com>
Cc: mtosatti@redhat.com, berrange@redhat.com, qemu-devel@nongnu.org,
	 vchundur@redhat.com, rjarry@redhat.com
Subject: Re: [PATCH v5 0/3] Add support for the RAPL MSRs series
Date: Fri, 12 Apr 2024 12:57:11 +0200	[thread overview]
Message-ID: <CABgObfamLi+Nz1sTC7PaDFg6jXT=4521SO11gbTysBo08jyp3g@mail.gmail.com> (raw)
In-Reply-To: <20240411121434.253353-1-aharivel@redhat.com>

On Thu, Apr 11, 2024 at 2:14 PM Anthony Harivel <aharivel@redhat.com> wrote:
>
> Dear maintainers,
>
> First of all, thank you very much for your review of my patch
> [1].
>
> In this version (v5), I have attempted to address all the problems
> addressed by Daniel during the last review. I've been more careful with
> all the remarks made.
>
> However, one question remains unanswered pointing the issue with the
> location of "/var/local/run/qemu-vmsr-helper.sock", created by
> compute_default_paths(). QEMU is not allowed to reach the socket here.

If I understand correctly the question, that is expected. This is a
privileged functionality and therefore it requires manual intervention
to change the owner of the socket and allow QEMU to access it.

Paolo

> Thank you again for your continued guidance.
>
> v4 -> v5
> --------
>
> - correct qio_channel_get_peerpid: return pid = -1 in case of error
> - Vmsr_helper: compile only for x86
> - Vmsr_helper: use qio_channel_read/write_all
> - Vmsr_helper: abandon user/group
> - Vmsr_energy.c: correct all error_report
> - Vmsr thread: compute default socket path only once
> - Vmsr thread: open socket only once
> - Pass relevant QEMU CI
>
> v3 -> v4
> --------
>
> - Correct memory leaks with AddressSanitizer
> - Add sanity check for QEMU and qemu-vmsr-helper for checking if host is
>   INTEL and if RAPL is activated.
> - Rename poor variables naming for easier comprehension
> - Move code that checks Host before creating the VMSR thread
> - Get rid of libnuma: create function that read sysfs for reading the
>   Host topology instead
>
> v2 -> v3
> --------
>
> - Move all memory allocations from Clib to Glib
> - Compile on *BSD (working on Linux only)
> - No more limitation on the virtual package: each vCPU that belongs to
>   the same virtual package is giving the same results like expected on
>   a real CPU.
>   This has been tested topology like:
>      -smp 4,sockets=2
>      -smp 16,sockets=4,cores=2,threads=2
>
> v1 -> v2
> --------
>
> - To overcome the CVE-2020-8694 a socket communication is created
>   to a priviliged helper
> - Add the priviliged helper (qemu-vmsr-helper)
> - Add SO_PEERCRED in qio channel socket
>
> RFC -> v1
> ---------
>
> - Add vmsr_* in front of all vmsr specific function
> - Change malloc()/calloc()... with all glib equivalent
> - Pre-allocate all dynamic memories when possible
> - Add a Documentation of implementation, limitation and usage
>
> Best regards,
> Anthony
>
> [1]: https://lists.gnu.org/archive/html/qemu-devel/2024-03/msg04417.html
>
> Anthony Harivel (3):
>   qio: add support for SO_PEERCRED for socket channel
>   tools: build qemu-vmsr-helper
>   Add support for RAPL MSRs in KVM/Qemu
>
>  accel/kvm/kvm-all.c                      |  27 ++
>  contrib/systemd/qemu-vmsr-helper.service |  15 +
>  contrib/systemd/qemu-vmsr-helper.socket  |   9 +
>  docs/specs/index.rst                     |   1 +
>  docs/specs/rapl-msr.rst                  | 155 +++++++
>  docs/tools/index.rst                     |   1 +
>  docs/tools/qemu-vmsr-helper.rst          |  89 ++++
>  include/io/channel.h                     |  21 +
>  include/sysemu/kvm.h                     |   2 +
>  include/sysemu/kvm_int.h                 |  32 ++
>  io/channel-socket.c                      |  28 ++
>  io/channel.c                             |  13 +
>  meson.build                              |   7 +
>  target/i386/cpu.h                        |   8 +
>  target/i386/kvm/kvm-cpu.c                |   9 +
>  target/i386/kvm/kvm.c                    | 428 ++++++++++++++++++
>  target/i386/kvm/meson.build              |   1 +
>  target/i386/kvm/vmsr_energy.c            | 335 ++++++++++++++
>  target/i386/kvm/vmsr_energy.h            |  99 +++++
>  tools/i386/qemu-vmsr-helper.c            | 529 +++++++++++++++++++++++
>  tools/i386/rapl-msr-index.h              |  28 ++
>  21 files changed, 1837 insertions(+)
>  create mode 100644 contrib/systemd/qemu-vmsr-helper.service
>  create mode 100644 contrib/systemd/qemu-vmsr-helper.socket
>  create mode 100644 docs/specs/rapl-msr.rst
>  create mode 100644 docs/tools/qemu-vmsr-helper.rst
>  create mode 100644 target/i386/kvm/vmsr_energy.c
>  create mode 100644 target/i386/kvm/vmsr_energy.h
>  create mode 100644 tools/i386/qemu-vmsr-helper.c
>  create mode 100644 tools/i386/rapl-msr-index.h
>
> --
> 2.44.0
>



  parent reply	other threads:[~2024-04-12 10:58 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-11 12:14 [PATCH v5 0/3] Add support for the RAPL MSRs series Anthony Harivel
2024-04-11 12:14 ` [PATCH v5 1/3] qio: add support for SO_PEERCRED for socket channel Anthony Harivel
2024-04-12 10:58   ` Paolo Bonzini
2024-04-11 12:14 ` [PATCH v5 2/3] tools: build qemu-vmsr-helper Anthony Harivel
2024-04-11 12:14 ` [PATCH v5 3/3] Add support for RAPL MSRs in KVM/Qemu Anthony Harivel
2024-04-17 10:07   ` Zhao Liu
2024-04-17 12:27     ` Daniel P. Berrangé
2024-04-17 15:13       ` Zhao Liu
2024-04-18  8:33         ` Anthony Harivel
2024-04-18 10:52     ` Anthony Harivel
2024-04-19 15:47       ` Zhao Liu
2024-04-17 17:55   ` Daniel P. Berrangé
2024-04-18 16:42   ` Daniel P. Berrangé
2024-04-18 16:57     ` Anthony Harivel
2024-04-25 15:34     ` Anthony Harivel
2024-04-25 15:42       ` Daniel P. Berrangé
2024-04-26  8:36         ` Anthony Harivel
2024-05-06  9:41           ` Anthony Harivel
2024-04-12 10:57 ` Paolo Bonzini [this message]
2024-04-17 17:58   ` [PATCH v5 0/3] Add support for the RAPL MSRs series Daniel P. Berrangé
2024-04-19 17:30     ` Paolo Bonzini
2024-04-17 17:23 ` Daniel P. Berrangé
2024-04-18  8:08   ` Anthony Harivel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABgObfamLi+Nz1sTC7PaDFg6jXT=4521SO11gbTysBo08jyp3g@mail.gmail.com' \
    --to=pbonzini@redhat.com \
    --cc=aharivel@redhat.com \
    --cc=berrange@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rjarry@redhat.com \
    --cc=vchundur@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).