qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH-for-9.1 21/21] qapi: Make @query-cpu-definitions target-a


From: Markus Armbruster
Subject: Re: [RFC PATCH-for-9.1 21/21] qapi: Make @query-cpu-definitions target-agnostic
Date: Tue, 26 Mar 2024 14:32:13 +0100
User-agent: Gnus/5.13 (Gnus v5.13)

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> All targets use the generic_query_cpu_definitions() method,
> which is not target-specific. Make the command target agnostic
> by moving it to machine.json. Rename generic_query_cpu_definitions
> as qmp_query_cpu_definitions.
>
> This is an introspection change for the target that were not
> implementing qmp_query_cpu_definitions(): now query-cpu-definitions
> returns an their CPUs list.

Do you mean "returns their CPUs list"?

>
> Example with SH4 before:
>
>   { "execute": "query-cpu-definitions" }
>
>   { "error": {"class": "CommandNotFound", "desc": "The command 
> query-cpu-definitions has not been found"} }
>
> and after:
>
>   { "execute": "query-cpu-definitions" }
>
>   { "return": [
>           {"name": "sh7751r", "typename": "sh7751r-superh-cpu", "static": 
> false, "deprecated": false},
>           {"name": "sh7750r", "typename": "sh7750r-superh-cpu", "static": 
> false, "deprecated": false},
>           {"name": "sh7785", "typename": "sh7785-superh-cpu", "static": 
> false, "deprecated": false}
>       ]
>   }

Does the result make sense?  What do the callbacks add for the targets
that define them, and why do the other targets don't need them?

Conversion steps:

0. Create a generic version of the command, with optional callbacks
   [PATCH 14]

1. Turn the target-specific versions of the command into thin wrappers
   around the generic version one by one, supplying callbacks to
   preserve behavior [PATCH 15-20]

2. Peel off the wrappers [this patch]

3. Enable the command for all the other targets [also this patch]

Correct?

Split this patch?  Question, not a demand.

I think the commit messages of the step 1 patches should explain the
target-specific behavior, i.e. the difference between their callbacks
and the default behavior.

Obvious for cpu_list_compare(): sorted vs. unsorted.

Not obvious for add_definition().

> However this allows heterogeneous emulation to return a correct list.

Cryptic.  It made me go back to PATCH 14, where I discovered the loop
over the arch bits.

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Well, not all target got converted, I left the s390x one for later :)

Haha!




reply via email to

[Prev in Thread] Current Thread [Next in Thread]