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 v2 1/2] build: include Hyper-V enlightenments in CPUID flags list
Date: Tue, 2 Jun 2026 15:13:36 +0200	[thread overview]
Message-ID: <lxqwfcfxnsiwfnv4odnmk3wjv5qxx7tjwvajop3shpixe6437g@jiev4urm3u4u> (raw)
In-Reply-To: <0fbeed5e-3d77-4126-828c-acdb04c109ff@proxmox.com>

On Tue, Jun 02, 2026 at 02:14:05PM +0200, Fiona Ebner wrote:
> Am 01.06.26 um 9:16 AM schrieb Arthur Bied-Charreton:
> > +# Static blacklist to be reviewed on QEMU bumps.
> > +# Currently includes boolean properties from qom-list-properties that are neither CPUID
> > +# flags ('-cpu help') nor Hyper-V enlightenments ('hv-*').
> > +my $blacklist = {
> 
> Maybe {filtered,non-flag}-{,props,bools) is a more telling name?
> 
ack, went with $filtered_props :)
> > +    check => 1,
> > +    'cpuid-0xb' => 1,
> > +    enforce => 1,
> > +    'fill-mtrr-mask' => 1,
> > +    'host-cache-info' => 1,
> > +    'host-phys-bits' => 1,
> > +    hotpluggable => 1,
[...]
> > +};
> >  
> > -    s/^\s+//;
> > +my $flags = {};
> >  
> > -    push @flags, split(/\s+/);
> > +<$out>;
> 
> Nit: Could match the beginning of the line to see that it's the single
> QMP message we expect. And die if there is anything else for catching
> any unexpected warnings/messages.
> 
just to clarify, by beginning of the line you just mean the QMP key in
the return object right?

```
{"QMP": {"version": {"qemu": {"micro": 0, "minor": 0, "major": 11}, "package": "pve-qemu-kvm_11.0.0-4"}, "capabilities": []}}
```

or do you want to fail on new pve-qemu-kvm versions?

> > +qmp('qmp_capabilities');
> > +my $props = qmp('qom-list-properties', typename => 'host-x86_64-cpu');
> 
> This won't work on aarch64. There, typename should be 'max-x86_64-cpu'.
thanks for catching that, I was not aware.
> All the same flags are still present with that model, with the exception
> of 'hv-syndbg'. The reason is that this depends on CONFIG_SYNDBG which
> is only enabled if kvm is present (which is not when the architecture
> mismatches). So either we must consider a second list just for that, or
> my preferred approach, just filter that one out. It's just a special
> debug flag, which devs can still turn on via args if they really need
> to. For some context:
> 
> >     target/i386: Exclude 'hv-syndbg' from 'hv-passthrough'
> >     
> >     Windows with Hyper-V role enabled doesn't boot with 'hv-passthrough' when
> >     no debugger is configured, this significantly limits the usefulness of the
> >     feature as there's no support for subtracting Hyper-V features from CPU
> >     flags at this moment (e.g. "-cpu host,hv-passthrough,-hv-syndbg" does not
> >     work). While this is also theoretically fixable, 'hv-syndbg' is likely
> >     very special and unneeded in the default set. Genuine Hyper-V doesn't seem
> >     to enable it either.
> >     
> >     Introduce 'skip_passthrough' flag to 'kvm_hyperv_properties' and use it as
> >     one-off to skip 'hv-syndbg' when enabling features in 'hv-passthrough'
> >     mode. Note, "-cpu host,hv-passthrough,hv-syndbg" can still be used if
> >     needed.
> >     
> >     As both 'hv-passthrough' and 'hv-syndbg' are debug features, the change
> >     should not have any effect on production environments.
thanks for the context! I agree with just leaving it out. 
> 
> Because, all flag properties are still present, I think we can just use
> 'max-x86_64-cpu' as the typename on x86_64 hosts too. Should there
> really be an important flag missing from max-x86_64-cpu but present in
> host-x86_64-cpu, we can still adapt then.
> 
> What we might want to do already is pass in the typename via ARGV and
> group the filter list by typename. Then we'll have an easier time if we
> need to generate flags for other archs too.
> 
yes, just tried that model, that makes sense. will implement this in v3.

> > +for my $qo ($props->@*) {
> > +    next if $qo->{type} ne 'bool' || defined($blacklist->{$qo->{name}});
> > +    $flags->{$qemu_cpu_flag_alias_map->{$qo->{name}} // $qo->{name}} = 1;
> 
> Style nit: avoid putting the big expression into the hash access, but
> factor it out as a variable.
> 
ack
> >  }
> > +qmp('quit');
> > +waitpid($pid, 0);
> >  
> > -die "no QEMU/KVM CPU flags detected from STDIN input" if scalar (@flags) <= 0;
> > +my @flags = sort keys $flags->%*;
> >  
> > -print join("\n", @flags) or die "$!\n";
> > +print join("\n", @flags) . "\n" or die "$!\n";
> 
> The latter half of the expression is dead code.
> 
thanks for the heads-up :)
> > diff --git a/debian/rules b/debian/rules
> > index c90db29..b2eed71 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)
> > +	./debian/parse-cpu-flags.pl $(destdir)/usr/bin/qemu-system-x86_64 > $(flagfile)
> >  
> >  	# 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-06-02 13:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-01  7:08 [PATCH pve-qemu v2 0/2] include Hyper-V enlightenments in Arthur Bied-Charreton
2026-06-01  7:08 ` [PATCH v2 1/2] build: include Hyper-V enlightenments in CPUID flags list Arthur Bied-Charreton
2026-06-02 12:14   ` Fiona Ebner
2026-06-02 13:13     ` Arthur Bied-Charreton [this message]
2026-06-02 13:34       ` Fiona Ebner
2026-06-01  7:08 ` [PATCH v2 2/2] build: fail when recognized CPUID flags list changes Arthur Bied-Charreton
2026-06-03  7:12 ` superseded: [PATCH pve-qemu v2 0/2] include Hyper-V enlightenments in 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=lxqwfcfxnsiwfnv4odnmk3wjv5qxx7tjwvajop3shpixe6437g@jiev4urm3u4u \
    --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