From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 19B9F1FF13C for ; Thu, 28 May 2026 17:30:23 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8D18832B57; Thu, 28 May 2026 17:30:08 +0200 (CEST) Date: Thu, 28 May 2026 17:30:00 +0200 From: Arthur Bied-Charreton To: Fiona Ebner Subject: Re: [PATCH pve-qemu 2/3] build: query Hyper-V enlightenment flags for CPU flags list Message-ID: References: <20260522132122.712794-1-a.bied-charreton@proxmox.com> <20260522132122.712794-3-a.bied-charreton@proxmox.com> <9d6b0d73-1a43-4590-84b7-65b7517abac5@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9d6b0d73-1a43-4590-84b7-65b7517abac5@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1779982174341 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.761 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [parse-cpu-flags.pl,parse-machines.pl] Message-ID-Hash: J7P3ZR2PAP56RVOYOEXBEGS5VSCEGR3O X-Message-ID-Hash: J7P3ZR2PAP56RVOYOEXBEGS5VSCEGR3O X-MailFrom: a.bied-charreton@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 > > Signed-off-by: Arthur Bied-Charreton > > --- > > 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 () { > > > > 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) >