Re: [PATCH v3 0/5] Generate x86 cpu features

[Daniel P . Berrangé]

On Tue, Feb 06, 2024 at 02:47:34PM +0100, Tim Wiederhake wrote:
> Synchronizing the list of cpu features and models with qemu is a recurring
> task in libvirt. For x86, this is done by reading qom-list-properties for
> max-x86_64-cpu and manually filtering out everthing that does not look like
> a feature name, as well as parsing target/i386/cpu.c for cpu models.
> 
> This is a flawed, tedious and error-prone procedure. Ideally, qemu
> and libvirt would query a common source for cpu feature and model
> related information. Meanwhile, converting this information into an easier
> to parse format would help libvirt a lot.
> 
> This patch series converts the cpu feature information present in
> target/i386/cpu.c (`feature_word_info`) into a yaml file and adds a
> script to generate the c code from this data.

Looking at this fresh, I'm left wondering why I didn't suggested
using 'QMP' to expose this information when reviewing the earlier
versions. I see Igor did indeed suggest this:

  https://lists.nongnu.org/archive/html/qemu-devel/2023-09/msg03905.html

Your commentry that "qom-list-properties" doesn't distinguish
between CPU features and other random QOM properties is bang
on the money.

I think what this highlights, is that 'qom-list-properties'
is a very poor design/fit for the problem that management apps
need to solve in this regard.

Libvirt should not need to manually exclude non-feature properties
like 'check' 'enforce' 'migratable' etc.

QEMU already has this knowledge, as IIUC, 'query-cpu-model-expansion'
can distinguish this:

query-cpu-model-expansion type=static model={'name':'Nehalem'}
{
    "return": {
        "model": {
            "name": "base",
            "props": {
                "3dnow": false,
                ...snip...
                "xtpr": false
            }
        }
    }
}

We still have the problem that we're not exposing the CPUID/MSR
leafs/register bits. So query-cpu-model-expansion isn't a fit
for the problem.

Rather than try to design something super general purpose, I'd
suggest we take a short cut and design something entirley x86
specific, and simply mark the QMP command as "unstable"
eg a 'x-query-x86-cpu-model-features', and then basically
report all the information libvirt needs there.

This is functionally equivalent to what you expose in the YAML
file, while still using QEMU's formal 'QMP' API mechanism, so
we avoid inventing a new API concept via YAML.

I think this would avoid need to have a code generator refactor
the CPU definitions too. We just need to expose the values of
the existing CPUID_xxx constants against each register.



With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|