Eduardo Habkost <ehabk...@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.ga413...@dhcp-172-16-24-191.sw.ru>
https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg09463.html
[2] Message-ID: <20210419202336.shf3yo7lmr7tm...@habkost.net>
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg03697.html
