Re: [PATCH v6] qapi: introduce 'query-cpu-model-cpuid' action

[Markus Armbruster]

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