qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Tim Wiederhake <twiederh@redhat.com>
Cc: qemu-devel@nongnu.org, "Igor Mammedov" <imammedo@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>
Subject: Re: [PATCH v3 0/5] Generate x86 cpu features
Date: Tue, 5 Mar 2024 14:17:59 +0000	[thread overview]
Message-ID: <ZecplxvAjp07vnQ_@redhat.com> (raw)
In-Reply-To: <20240206134739.15345-1-twiederh@redhat.com>

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



  parent reply	other threads:[~2024-03-05 14:19 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-06 13:47 [PATCH v3 0/5] Generate x86 cpu features Tim Wiederhake
2024-02-06 13:47 ` [PATCH v3 1/5] target/i386: Split out feature_word_info Tim Wiederhake
2024-02-06 13:47 ` [PATCH v3 2/5] target/i386: Translate feature_word_info to yaml Tim Wiederhake
2024-02-06 13:47 ` [PATCH v3 3/5] target/i386: Remove comments from feature_word_info.c.inc Tim Wiederhake
2024-02-06 13:47 ` [PATCH v3 4/5] target/i386: Fix feature_word_info.c.inc formatting Tim Wiederhake
2024-02-06 13:47 ` [PATCH v3 5/5] target/i386: Generate feature_word_info.c.inc Tim Wiederhake
2024-03-04 12:04 ` [PATCH v3 0/5] Generate x86 cpu features Tim Wiederhake
2024-03-05 14:17 ` Daniel P. Berrangé [this message]
2024-03-11 11:32   ` Tim Wiederhake
2024-03-11 11:58     ` Daniel P. Berrangé

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=ZecplxvAjp07vnQ_@redhat.com \
    --to=berrange@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=twiederh@redhat.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;
as well as URLs for NNTP newsgroup(s).