Re: [PATCH v2 1/2] build: include Hyper-V enlightenments in CPUID flags list

[Fiona Ebner <f.ebner@proxmox.com>]

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?

> +    check => 1,
> +    'cpuid-0xb' => 1,
> +    enforce => 1,
> +    'fill-mtrr-mask' => 1,
> +    'host-cache-info' => 1,
> +    'host-phys-bits' => 1,
> +    hotpluggable => 1,
> +    hotplugged => 1,
> +    kvm => 1,
> +    'kvm-pv-enforce-cpuid' => 1,
> +    'l3-cache' => 1,
> +    'legacy-cache' => 1,
> +    'legacy-multi-node' => 1,
> +    lmce => 1,
> +    migratable => 1,
> +    pmu => 1,
> +    realized => 1,
> +    'start-powered-off' => 1,
> +    'tcg-cpuid' => 1,
> +    'vmware-cpuid-freq' => 1,
> +    'x-amd-topoext-features-only' => 1,
> +    'x-arch-cap-always-on' => 1,
> +    'x-consistent-cache' => 1,
> +    'x-force-cpuid-0x1f' => 1,
> +    'x-force-features' => 1,
> +    'x-l1-cache-per-thread' => 1,
> +    'x-migrate-error-code' => 1,
> +    'x-migrate-smi-count' => 1,
> +    'x-pdcm-on-even-without-pmu' => 1,
> +    'x-vendor-cpuid-only' => 1,
> +    'x-vendor-cpuid-only-v2' => 1,
> +    'xen-vapic' => 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.

> +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'.
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.

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.

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

>  }
> +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.

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