public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
To: Fiona Ebner <f.ebner@proxmox.com>
Cc: pve-devel@lists.proxmox.com
Subject: Re: [PATCH pve-qemu 2/3] build: query Hyper-V enlightenment flags for CPU flags list
Date: Thu, 28 May 2026 17:30:00 +0200	[thread overview]
Message-ID: <yhptfclfgptyi5ukipx7eor3qx6woewq6h3eacdywunyh6hw62@53fea7tz2n3n> (raw)
In-Reply-To: <9d6b0d73-1a43-4590-84b7-65b7517abac5@proxmox.com>

On Thu, May 28, 2026 at 03:38:43PM +0200, Fiona Ebner wrote:
> Am 22.05.26 um 3:21 PM schrieb Arthur Bied-Charreton:
> > The recognized CPU flags list shipped with pve-qemu-kvm must include
> > Hyper-V enlightenment flags so the custom CPU models API can offer them.
> > 
> > Query them from the 'host-x86_64-cpu' type via QMP (qom-list-properties)
> > in parse-cpu-flags.pl, filtering for properties with the 'hv-' prefix,
> > and include them in the recognized flags list.
> > 
> > Filter out non-boolean enlightenments, since the custom CPU model config
> > only supports enable/disable.
> > 
> > Suggested-by: Fiona Ebner <f.ebner@proxmox.com>
> > Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> > ---
> >  debian/parse-cpu-flags.pl | 42 +++++++++++++++++++++++++++++++++++++++
> >  debian/rules              |  2 +-
> >  2 files changed, 43 insertions(+), 1 deletion(-)
> > 
> > diff --git a/debian/parse-cpu-flags.pl b/debian/parse-cpu-flags.pl
> > index 1847b3e..2b5d32a 100755
> > --- a/debian/parse-cpu-flags.pl
> > +++ b/debian/parse-cpu-flags.pl
> > @@ -2,7 +2,10 @@
> >  
> >  use warnings;
> >  use strict;
> 
> Style nit: please add a blank line here.
> 
> You could go ahead and use v5.36 and function signatures while at it.
> 
Ah yes I forgot to do that, will add in v2, thanks!
> > +use IPC::Open2;
> > +use JSON;
> >  
> > +my ($qemu_bin) = @ARGV;
> >  my @flags = ();
> >  my $got_flags_section;
> >  
> > @@ -20,4 +23,43 @@ while (<STDIN>) {
> >  
> >  die "no QEMU/KVM CPU flags detected from STDIN input" if scalar (@flags) <= 0;
> >  
> > +my $pid = open2(
> > +    my $out,
> > +    my $in,
> > +    $qemu_bin,
> > +    '-machine',
> > +    'none',
> > +    '-display',
> > +    'none',
> > +    '-S',
> > +    '-qmp',
> > +    'stdio',
> 
> I'd add a -nodefaults for good measure
> 
ACK
> > +);
> > +
> > +sub qmp {
> > +    my ($cmd, %args) = @_;
> > +    print $in encode_json({ execute => $cmd, %args ? (arguments => \%args) : () }), "\n";
> > +    while (my $line = <$out>) {
> > +        my $msg = decode_json($line);
> > +        next if $msg->{event};
> > +        return $msg->{return} if exists $msg->{return};
> 
> Style nit: use parentheses for exists()
> 
ACk
> > +        die "QMP error: " . encode_json($msg->{error}) if $msg->{error};
> > +    }
> > +}
> > +
> > +<$out>;
> > +qmp('qmp_capabilities');
> > +my $props = qmp('qom-list-properties', typename => 'host-x86_64-cpu');
> 
> Nice!
> 
> > +for my $qo ($props->@*) {
> > +    # Filter out non-boolean flags, our custom CPU model config only supports
> > +    # enable/disable.
> > +    if (index($qo->{name}, 'hv-') == 0 && $qo->{type} eq 'bool') {
> > +        push @flags, $qo->{name};
> > +    }
> > +}
> > +qmp('quit');
> > +waitpid($pid, 0);
> > +
> > +@flags = sort @flags;
> > +
> >  print join("\n", @flags) or die "$!\n";
> > diff --git a/debian/rules b/debian/rules
> > index c90db29..e3eebe8 100755
> > --- a/debian/rules
> > +++ b/debian/rules
> > @@ -123,7 +123,7 @@ install: build
> >  	rm -f $(destdir)/usr/lib/kvm/virtfs-proxy-helper
> >  
> >  	# CPU flags are static for QEMU version, allows avoiding more costly checks
> > -	$(destdir)/usr/bin/qemu-system-x86_64 -cpu help | ./debian/parse-cpu-flags.pl > $(flagfile)
> > +	$(destdir)/usr/bin/qemu-system-x86_64 -cpu help | ./debian/parse-cpu-flags.pl $(destdir)/usr/bin/qemu-system-x86_64 > $(flagfile)
> 
> Should we rather also do the call to -cpu help inside the script then?
> It feels strange to mix things like this (half way via STDIN, half way
> execute command with passed-in binary).
> 
Yes that's true, I was trying to avoid moving around too much stuff but
it would be cleaner to do everything the same way 

> I'm also thinking about whether we should go ahead and use just the QOM
> CPU properties rather than relying on the -cpu help output. We would
> then need a hardcoded blacklist of boolean properties that are not to be
> expose as flags (and also the aliases to avoid duplication). It's not
> that long of a list, but going forward, we'll need to be slightly more
> careful not to add anything as a flag that we don't want. In practice,
> after patch 3/3, every new boolean option needs to be looked at anyways
> and then the person doing the rebase needs to check if it is actually a
> flag. I'm not fully sure, but it would avoid the mixing info from
> different sources altogether. What do you think?
> 
>From a quick diff the only flag present in -cpu help that is not also in
qom-list-properties is kvmclock (on 11.0.0). In the other direction it's
62 (I guess 58 after resolving QEMU aliases) properties, without 
counting the Hyper-V enlightenments, since we want to include those.

Some thoughts:

QEMU accepts all flags that would be added with qom-list-properties 
(just successfully booted a VM with a custom CPU model that had all of 
them set), so on that front we would be okay AFAICT.

The blocker is query_supported_cpu_flags, it reports a lot of the new 
flags I checked as unsupported. There is likely a way to make the 
temporary VM we query for flag support advertise those as well, like 
there was for the Hyper-V enlightenments, but I think that should be 
figured out before we start including those flags. 

I would suggest keeping '-cpu help' as the base set for now and just 
moving the qom-list-properties call for hv-* into the script so we stop 
mixing STDIN and a passed-in binary. At least until we have a good way 
to query support for the new flags. This way we avoid recreating the 
Hyper-V enlightenments issue at a larger scale.

I might still be missing a piece though, will definitely look into the
support-query path with fresh eyes tomorrow. 
> >  
> >  	# Supported machine versions are static for a given QEMU binary.
> >  	$(destdir)/usr/bin/qemu-system-x86_64 -machine help | ./debian/parse-machines.pl > $(machine_file_x86_64)
> 




  reply	other threads:[~2026-05-28 15:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-22 13:21 [PATCH qemu/qemu-server 0/3] include Hyper-V enlightenment flags in CPU flags lists Arthur Bied-Charreton
2026-05-22 13:21 ` [PATCH qemu-server 1/3] cpu flags: include Hyper-V enlightenment flags in supported flags Arthur Bied-Charreton
2026-05-26  8:43   ` applied: " Fiona Ebner
2026-05-22 13:21 ` [PATCH pve-qemu 2/3] build: query Hyper-V enlightenment flags for CPU flags list Arthur Bied-Charreton
2026-05-28 13:38   ` Fiona Ebner
2026-05-28 15:30     ` Arthur Bied-Charreton [this message]
2026-05-28 15:43       ` Fiona Ebner
2026-05-29  5:53         ` Arthur Bied-Charreton
2026-05-22 13:21 ` [PATCH pve-qemu 3/3] build: fail when recognized CPU flags list changes Arthur Bied-Charreton

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=yhptfclfgptyi5ukipx7eor3qx6woewq6h3eacdywunyh6hw62@53fea7tz2n3n \
    --to=a.bied-charreton@proxmox.com \
    --cc=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal