Re: [PATCH v8] qapi: introduce 'query-kvm-cpuid' action

From: Eric Blake <eblake@redhat.com>
To: Valeriy Vdovin <valeriy.vdovin@virtuozzo.com>
Cc: qemu-devel@nongnu.org, Eduardo Habkost <ehabkost@redhat.com>,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
	Markus Armbruster <armbru@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	Richard Henderson <richard.henderson@linaro.org>,
	Thomas Huth <thuth@redhat.com>,
	Laurent Vivier <lvivier@redhat.com>,
	kvm@vger.kernel.org, Denis Lunev <den@openvz.org>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Subject: Re: [PATCH v8] qapi: introduce 'query-kvm-cpuid' action
Date: Wed, 2 Jun 2021 15:51:02 -0500	[thread overview]
Message-ID: <20210602205102.icdqspki66rwvc3n@redhat.com> (raw)
In-Reply-To: <20210531123806.23030-1-valeriy.vdovin@virtuozzo.com>

On Mon, May 31, 2021 at 03:38:06PM +0300, Valeriy Vdovin wrote:
> Introducing new qapi method 'query-kvm-cpuid'. This method can be used to
> get virtualized cpu model info generated by QEMU during VM initialization in
> the form of cpuid representation.
> 
> 
> Use example:
> qmp_request: {
>   "execute": "query-kvm-cpuid"
> }
> 
> qmp_response: [
>   {
>     "eax": 1073741825,
>     "edx": 77,
>     "in_eax": 1073741824,
>     "ecx": 1447775574,
>     "ebx": 1263359563,
>   },

JSON does not permit a trailing ',' before '}'; which means you did
not actually paste an actual QMP response here.

> ---
>  qapi/machine-target.json   | 43 ++++++++++++++++++++++++++++++++++++++
>  target/i386/kvm/kvm.c      | 37 ++++++++++++++++++++++++++++++++
>  tests/qtest/qmp-cmd-test.c |  1 +
>  3 files changed, 81 insertions(+)
> 
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index e7811654b7..a83180dd24 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -329,3 +329,46 @@
>  ##
>  { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
>    'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
> +
> +##
> +# @CpuidEntry:
> +#
> +# A single entry of a CPUID response.
> +#
> +# One entry holds full set of information (leaf) returned to the guest in response
> +# to it calling a CPUID instruction with eax, ecx used as the agruments to that

arguments

> +# instruction. ecx is an optional argument as not all of the leaves support it.

Is there a default value of ecx for when it is not provided by the
user but needed by the leaf?  Or is it an error if ecx is omitted in
that case?  Similarly, is it an error if ecx is provided but not
needed?

> +#
> +# @in_eax: CPUID argument in eax
> +# @in_ecx: CPUID argument in ecx

Should be in-eax, in-ecx.

> +# @eax: eax
> +# @ebx: ebx
> +# @ecx: ecx
> +# @edx: edx
> +#
> +# Since: 6.1
> +##
> +{ 'struct': 'CpuidEntry',
> +  'data': { 'in_eax' : 'uint32',
> +            '*in_ecx' : 'uint32',
> +            'eax' : 'uint32',
> +            'ebx' : 'uint32',
> +            'ecx' : 'uint32',
> +            'edx' : 'uint32'
> +          },
> +  'if': 'defined(TARGET_I386) && defined(CONFIG_KVM)' }
> +
> +##
> +# @query-kvm-cpuid:
> +#
> +# Returns raw data from the KVM CPUID table for the first VCPU.
> +# The KVM CPUID table defines the response to the CPUID
> +# instruction when executed by the guest operating system.
> +#
> +# Returns: a list of CpuidEntry
> +#
> +# Since: 6.1
> +##
> +{ 'command': 'query-kvm-cpuid',
> +  'returns': ['CpuidEntry'],
> +  'if': 'defined(TARGET_I386) && defined(CONFIG_KVM)' }

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org