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 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 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