Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command

On 12/03/2021 08.42, Marc-André Lureau wrote:
On Fri, Mar 12, 2021 at 3:14 AM Philippe Mathieu-Daudé <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
[...]
+## +# @AcceleratorInfo: +# +# Accelerator information. +# +# @name: The accelerator name. +# +# Since: 6.0 +## +{ 'union': 'AcceleratorInfo', + 'base': {'name': 'Accelerator'}, + 'discriminator': 'name', + 'data': { } }
+
Making room for future details, why not.
I think we definitely need the additional data section here: For KVM on POWER, it would be good to know whether it's KVM-HV or KVM-PR, for KVM on MIPS it would be good to know whether it's KVM_VM_MIPS_VZ or KVM_VM_MIPS_TE, for KVM on x86 whether it's the AMD flavor or Intel, ...
+## +# @query-accels: +# +# Get a list of AcceleratorInfo for all built-in accelerators. +# +# Returns: a list of @AcceleratorInfo describing each accelerator. +# +# Since: 6.0 +# +# Example: +# +# -> { "execute": "query-accels" } +# <- { "return": [ +# { +# "type": "qtest" +# }, +# { +# "type": "kvm" +# } +# ] } +# +## +{ 'command': 'query-accels', + 'returns': ['AcceleratorInfo'] }
That's nice, but how do you know which accels are actually enabled?
I guess we need two commands in the end, one for querying which accelerators are available, and one for querying the data from the currently active accelerator...? Thomas

On Fri, Mar 12, 2021 at 09:11:45AM +0100, Thomas Huth wrote:
On 12/03/2021 08.42, Marc-André Lureau wrote:
On Fri, Mar 12, 2021 at 3:14 AM Philippe Mathieu-Daudé <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
[...]
+## +# @AcceleratorInfo: +# +# Accelerator information. +# +# @name: The accelerator name. +# +# Since: 6.0 +## +{ 'union': 'AcceleratorInfo', + 'base': {'name': 'Accelerator'}, + 'discriminator': 'name', + 'data': { } }
+
Making room for future details, why not.
I think we definitely need the additional data section here: For KVM on POWER, it would be good to know whether it's KVM-HV or KVM-PR, for KVM on MIPS it would be good to know whether it's KVM_VM_MIPS_VZ or KVM_VM_MIPS_TE, for KVM on x86 whether it's the AMD flavor or Intel, ...
Could also pre-expand all of these and list them individually.
+## +# @query-accels: +# +# Get a list of AcceleratorInfo for all built-in accelerators. +# +# Returns: a list of @AcceleratorInfo describing each accelerator. +# +# Since: 6.0 +# +# Example: +# +# -> { "execute": "query-accels" } +# <- { "return": [ +# { +# "type": "qtest" +# }, +# { +# "type": "kvm" +# } +# ] } +# +## +{ 'command': 'query-accels', + 'returns': ['AcceleratorInfo'] }
That's nice, but how do you know which accels are actually enabled?
I guess we need two commands in the end, one for querying which accelerators are available, and one for querying the data from the currently active accelerator...?
If we listed each accelerator individually, then we could use booleans for them, where only the active one would be true. If we can't come up with another need for the accelerator-specific info now, then I'd leave it to be added at a later time when it is needed. Thanks, drew

On 3/12/21 9:48 AM, Andrew Jones wrote:
On Fri, Mar 12, 2021 at 09:11:45AM +0100, Thomas Huth wrote:
On 12/03/2021 08.42, Marc-André Lureau wrote:
On Fri, Mar 12, 2021 at 3:14 AM Philippe Mathieu-Daudé <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
[...]
+## +# @AcceleratorInfo: +# +# Accelerator information. +# +# @name: The accelerator name. +# +# Since: 6.0 +## +{ 'union': 'AcceleratorInfo', + 'base': {'name': 'Accelerator'}, + 'discriminator': 'name', + 'data': { } }
+
Making room for future details, why not.
I think we definitely need the additional data section here: For KVM on POWER, it would be good to know whether it's KVM-HV or KVM-PR, for KVM on MIPS it would be good to know whether it's KVM_VM_MIPS_VZ or KVM_VM_MIPS_TE, for KVM on x86 whether it's the AMD flavor or Intel, ...
Could also pre-expand all of these and list them individually.
Let us consider simplicity for the reader, and which use cases we expect for this.
+## +# @query-accels: +# +# Get a list of AcceleratorInfo for all built-in accelerators. +# +# Returns: a list of @AcceleratorInfo describing each accelerator. +# +# Since: 6.0 +# +# Example: +# +# -> { "execute": "query-accels" } +# <- { "return": [ +# { +# "type": "qtest" +# }, +# { +# "type": "kvm" +# } +# ] } +# +## +{ 'command': 'query-accels', + 'returns': ['AcceleratorInfo'] }
That's nice, but how do you know which accels are actually enabled?
I guess we need two commands in the end, one for querying which accelerators are available, and one for querying the data from the currently active accelerator...?
If we listed each accelerator individually, then we could use booleans for them, where only the active one would be true. If we can't come up with another need for the accelerator-specific info now, then I'd leave it to be added at a later time when it is needed.
Thanks, drew

On Fri, Mar 12, 2021 at 09:52:33AM +0100, Claudio Fontana wrote:
On 3/12/21 9:48 AM, Andrew Jones wrote:
On Fri, Mar 12, 2021 at 09:11:45AM +0100, Thomas Huth wrote:
On 12/03/2021 08.42, Marc-André Lureau wrote:
On Fri, Mar 12, 2021 at 3:14 AM Philippe Mathieu-Daudé <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
[...]
+## +# @AcceleratorInfo: +# +# Accelerator information. +# +# @name: The accelerator name. +# +# Since: 6.0 +## +{ 'union': 'AcceleratorInfo', + 'base': {'name': 'Accelerator'}, + 'discriminator': 'name', + 'data': { } }
+
Making room for future details, why not.
I think we definitely need the additional data section here: For KVM on POWER, it would be good to know whether it's KVM-HV or KVM-PR, for KVM on MIPS it would be good to know whether it's KVM_VM_MIPS_VZ or KVM_VM_MIPS_TE, for KVM on x86 whether it's the AMD flavor or Intel, ...
Could also pre-expand all of these and list them individually.
Let us consider simplicity for the reader, and which use cases we expect for this.
What do you mean by "reader"? If you mean the QMP client that needs this information, then I can't think of anything more simple than a single list of booleans with descriptive names to process. If you mean that the expected use cases don't care about all those variants, then you don't need to worry about that. Only the ones compiled in will be in the list. The expected use cases will never see the other ones. Thanks, drew

On 12/03/21 09:48, Andrew Jones wrote:
I think we definitely need the additional data section here: For KVM on POWER, it would be good to know whether it's KVM-HV or KVM-PR, for KVM on MIPS it would be good to know whether it's KVM_VM_MIPS_VZ or KVM_VM_MIPS_TE, for KVM on x86 whether it's the AMD flavor or Intel, ...
Could also pre-expand all of these and list them individually.
That seems worse (in general) because in a lot of cases you don't care; the basic query-accels output should match the argument to "-accel". Paolo

On Fri, Mar 12, 2021 at 10:01:43AM +0100, Paolo Bonzini wrote:
On 12/03/21 09:48, Andrew Jones wrote:
I think we definitely need the additional data section here: For KVM on POWER, it would be good to know whether it's KVM-HV or KVM-PR, for KVM on MIPS it would be good to know whether it's KVM_VM_MIPS_VZ or KVM_VM_MIPS_TE, for KVM on x86 whether it's the AMD flavor or Intel, ...
Could also pre-expand all of these and list them individually.
That seems worse (in general) because in a lot of cases you don't care; the basic query-accels output should match the argument to "-accel".
For these special subtypes, what's the property/state that indicates it when just using '-accel kvm' on the command line? Because if this qmp list matches the '-accel' parameter list, then qtest and other qmp clients may need to query that other information too, in order for this accel query to be useful. And, do we need an accel-specific qmp query for it? Or, is that information already available? Thanks, drew

On 12/03/21 10:17, Andrew Jones wrote:
On Fri, Mar 12, 2021 at 10:01:43AM +0100, Paolo Bonzini wrote:
On 12/03/21 09:48, Andrew Jones wrote:
I think we definitely need the additional data section here: For KVM on POWER, it would be good to know whether it's KVM-HV or KVM-PR, for KVM on MIPS it would be good to know whether it's KVM_VM_MIPS_VZ or KVM_VM_MIPS_TE, for KVM on x86 whether it's the AMD flavor or Intel, ...
Could also pre-expand all of these and list them individually.
That seems worse (in general) because in a lot of cases you don't care; the basic query-accels output should match the argument to "-accel".
For these special subtypes, what's the property/state that indicates it when just using '-accel kvm' on the command line? Because if this qmp list matches the '-accel' parameter list, then qtest and other qmp clients may need to query that other information too, in order for this accel query to be useful. And, do we need an accel-specific qmp query for it? Or, is that information already available?
It depends. On PPC (if I remember/understand correctly) only pseries supports both HV and PR, while all other machines only support KVM-PR. So in that case it's a kvm-type machine property that is defined only for the pseries machine. On MIPS instead there's no option and VZ always wins over TE. I think it could be made an option on -accel, but I'm not familiar with MIPS machine types. Something like "name: 'kvm', types: ['book3s-hv', 'pr']" would work nicely for KVM-PPC, and likewise for MIPS. Paolo
participants (4)
-
Andrew Jones
-
Claudio Fontana
-
Paolo Bonzini
-
Thomas Huth