From: Markus Armbruster <armbru@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: "Vladimir Sementsov-Ogievskiy" <vsementsov@virtuozzo.com>,
"Daniel P. Berrangé" <berrange@redhat.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
qemu-devel@nongnu.org,
"Valeriy Vdovin" <valeriy.vdovin@virtuozzo.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Denis Lunev" <den@openvz.org>
Subject: Re: [PATCH v6] qapi: introduce 'query-cpu-model-cpuid' action
Date: Thu, 22 Apr 2021 11:41:24 +0200 [thread overview]
Message-ID: <871rb2wta3.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <20210421201759.utsmhuopdmlhghbx@habkost.net> (Eduardo Habkost's message of "Wed, 21 Apr 2021 16:17:59 -0400")
Eduardo Habkost <ehabkost@redhat.com> writes:
> On Wed, Apr 21, 2021 at 08:39:42PM +0300, Valeriy Vdovin wrote:
>> On Tue, Apr 20, 2021 at 01:09:00PM -0400, Eduardo Habkost wrote:
>> > On Tue, Apr 20, 2021 at 07:19:40PM +0300, Valeriy Vdovin wrote:
>> > [...]
>> > > +##
>> > > +# @query-cpu-model-cpuid:
>> > > +#
>> > > +# Returns description of a virtual CPU model, created by QEMU after cpu
>> > > +# initialization routines. The resulting information is a reflection of a parsed
>> > > +# '-cpu' command line option, filtered by available host cpu features.
>> > > +#
>> > > +# Returns: @CpuModelCpuidDescription
>> > > +#
>> > > +# Example:
>> > > +#
>> > > +# -> { "execute": "query-cpu-model-cpuid" }
>> > > +# <- { "return": 'CpuModelCpuidDescription' }
>> > > +#
>> > > +# Since: 6.1
>> > > +##
>> > > +{ 'command': 'query-cpu-model-cpuid',
>> > > + 'returns': 'CpuModelCpuidDescription',
>> > > + 'if': 'defined(TARGET_I386)' }
>> >
>> > I was assuming the command was going to get a CPU model name as
>> > argument.
>> >
>> > If you are only going to return info on the current CPUs, the
>> > interface could be simplified a lot.
>> >
>> > What about a simple `query-cpuid` command that only takes:
>> >
>> > { 'qom-path': 'str', # qom-path is returned by query-cpus-fast
>> > 'eax': 'uint32',
>> > '*ecx': 'uint32' }
>> >
>> > as argument, and returns
>> >
>> > { 'present': 'bool',
>> > 'max_eax': 'uint32', # max value of EAX for this range
>> > '*max_ecx': 'uint32', # max value of ECX if there are subleaves
>> > 'eax': 'uint32',
>> > 'ebx': 'uint32',
>> > 'ecx': 'uint32',
>> > 'edx': 'uint32' }
>> >
>> > ?
>> Hi. The interface that you suggest looks good. But it has one critical
>> point that deems it unusable for our initial needs. The point of this
>> whole new API is to take away the strain of knowing about leaf ranges
>> from the caller of this API. In my current patch this goal works. So
>> the caller does not need to know in advance what ranges there are in
>> original CPUID as well as in it's tweaked QEMU's version.
>>
>
> Raw CPUID data is a pretty low level interface, already. Is it
> really too much of a burden for callers to know that CPUID ranges
> start at 0, 0x40000000, 0x80000000, and 0xC0000000?
>
> (Especially considering that it would save us ~100 extra lines of
> C code and maybe 50-100 extra lines of QAPI schema code.)
>
>
>> But you suggested API is not so kind to the caller, so he would need
>> to add some logic around the call that knows about exact leaf ranges.
>> If you have a solution to that also, I'll be happy to discuss it.
>
> Would be following (Python-like pseudocode) be too much of a
> burden for consumers of the command?
>
> for start in (0, 0x40000000, 0x80000000, 0xC0000000):
> leaf = query_cpuid(qom_path, start)
> for eax in range(start, leaf.max_eax + 1):
> for ecx in range(0, leaf.get('max_ecx', 0) + 1):
> all_leaves.append(query_cpuid(qom_path, eax, ecx))
>
>>
>> The obvious thing that comes to mind is changing the exists/max_ecx pair
>> to something like next_eax, next_ecx. But this idea will probably require
>> the same amount of complexity that I currently have in this patch.
>
> I agree. I'm trying to reduce the complexity of the interface
> and of the command implementation.
This command appears to be primarily motivated by a container use case
that doesn't involve QEMU (other than as a provider of a language to
construct CPU models)[1]. It has secondary applications that do involve
QEMU, but they seem quite limited (automated tests[2], debugging).
This is rather weak justification for QMP command. It may suffice for a
really simple patch along the lines Eduardo proposed. Any additional
complexity would be a hard sell, though.
[1] Message-ID: <20210329112153.GA413337@dhcp-172-16-24-191.sw.ru>
https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg09463.html
[2] Message-ID: <20210419202336.shf3yo7lmr7tmzvp@habkost.net>
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg03697.html
prev parent reply other threads:[~2021-04-22 9:42 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-20 16:19 [PATCH v6] qapi: introduce 'query-cpu-model-cpuid' action Valeriy Vdovin
2021-04-20 16:42 ` no-reply
2021-04-20 17:00 ` Vladimir Sementsov-Ogievskiy
2021-04-20 17:09 ` Eduardo Habkost
2021-04-21 17:39 ` Valeriy Vdovin
2021-04-21 20:17 ` Eduardo Habkost
2021-04-22 9:02 ` Valeriy Vdovin
2021-04-23 20:32 ` Eduardo Habkost
2021-04-22 9:41 ` Markus Armbruster [this message]
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=871rb2wta3.fsf@dusky.pond.sub.org \
--to=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=den@openvz.org \
--cc=ehabkost@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=valeriy.vdovin@virtuozzo.com \
--cc=vsementsov@virtuozzo.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).