all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal