[v4] qapi: remove all TARGET_* conditionals from the schema

[PATCH v4 08/15] qapi: Make CpuModelExpansionInfo::deprecated-props optional and generic

Pierrick Bouvier posted 15 patches 7 months ago

Diff against v1 v2 v3
Download mbox

Maintainers: David Woodhouse <dwmw2@infradead.org>, Paul Durrant <paul@xen.org>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Nina Schoetterl-Glausch <nsg@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Gerd Hoffmann <kraxel@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, "Gonglei (Arei)" <arei.gonglei@huawei.com>, Zhenwei Pi <pizhenwei@bytedance.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, Michael Roth <michael.roth@amd.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Peter Maydell <peter.maydell@linaro.org>, Marcelo Tosatti <mtosatti@redhat.com>, Song Gao <gaosong@loongson.cn>, Huacai Chen <chenhuacai@kernel.org>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <arikalo@gmail.com>, Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Laurent Vivier <lvivier@redhat.com>

Expand all Fold all

[PATCH v4 08/15] qapi: Make CpuModelExpansionInfo::deprecated-props optional and generic

Posted by Pierrick Bouvier 7 months ago

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

We'd like to have some unified QAPI schema. Having a structure field
conditional to a target being built in is not very practical.

While @deprecated-props is only used by s390x target, it is generic
enough and could be used by other targets (assuming we expand
CpuModelExpansionType enum values).

Let's always include this field, regardless of the target, but make it
optional. This is not a compatibility break only because the field
remains present always on S390x.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 qapi/machine-target.json | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 426ce4ee82d..d8dbda4b508 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -244,19 +244,18 @@
 #
 # @model: the expanded CpuModelInfo.
 #
-# @deprecated-props: a list of properties that are flagged as
+# @deprecated-props: an optional list of properties that are flagged as
 #     deprecated by the CPU vendor.  The list depends on the
 #     CpuModelExpansionType: "static" properties are a subset of the
 #     enabled-properties for the expanded model; "full" properties are
 #     a set of properties that are deprecated across all models for
-#     the architecture.  (since: 9.1).
+#     the architecture.  (since: 10.1 -- since 9.1 on s390x --).
 #
 # Since: 2.8
 ##
 { 'struct': 'CpuModelExpansionInfo',
   'data': { 'model': 'CpuModelInfo',
-            'deprecated-props' : { 'type': ['str'],
-                                   'if': 'TARGET_S390X' } },
+            '*deprecated-props' : ['str'] },
   'if': { 'any': [ 'TARGET_S390X',
                    'TARGET_I386',
                    'TARGET_ARM',
-- 
2.47.2

Re: [PATCH v4 08/15] qapi: Make CpuModelExpansionInfo::deprecated-props optional and generic

Posted by Markus Armbruster 6 months, 3 weeks ago

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> We'd like to have some unified QAPI schema. Having a structure field
> conditional to a target being built in is not very practical.
>
> While @deprecated-props is only used by s390x target, it is generic
> enough and could be used by other targets (assuming we expand
> CpuModelExpansionType enum values).
>
> Let's always include this field, regardless of the target, but make it
> optional. This is not a compatibility break only because the field
> remains present always on S390x.
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  qapi/machine-target.json | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index 426ce4ee82d..d8dbda4b508 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -244,19 +244,18 @@
>  #
>  # @model: the expanded CpuModelInfo.
>  #
> -# @deprecated-props: a list of properties that are flagged as
> +# @deprecated-props: an optional list of properties that are flagged as
>  #     deprecated by the CPU vendor.  The list depends on the
>  #     CpuModelExpansionType: "static" properties are a subset of the
>  #     enabled-properties for the expanded model; "full" properties are
>  #     a set of properties that are deprecated across all models for
> -#     the architecture.  (since: 9.1).
> +#     the architecture.  (since: 10.1 -- since 9.1 on s390x --).
>  #
>  # Since: 2.8
>  ##
>  { 'struct': 'CpuModelExpansionInfo',
>    'data': { 'model': 'CpuModelInfo',
> -            'deprecated-props' : { 'type': ['str'],
> -                                   'if': 'TARGET_S390X' } },
> +            '*deprecated-props' : ['str'] },
>    'if': { 'any': [ 'TARGET_S390X',
>                     'TARGET_I386',
>                     'TARGET_ARM',

[Copied from review of prior posts]

When I see "optional array", I wonder about the difference between
"absent" and "present and empty".  The doc comment doesn't quite explain
it.  I figure "present and empty" means empty, while "absent" means we
don't know / not implemented.

Is the difference useful?

Daniel doubts it is.

[end of copy]

Let's take with this patch as is.  If we prefer to remove the
difference, we can do so on top.

Re: [PATCH v4 08/15] qapi: Make CpuModelExpansionInfo::deprecated-props optional and generic

Posted by Pierrick Bouvier 6 months, 3 weeks ago

On 5/27/25 4:32 AM, Markus Armbruster wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> We'd like to have some unified QAPI schema. Having a structure field
>> conditional to a target being built in is not very practical.
>>
>> While @deprecated-props is only used by s390x target, it is generic
>> enough and could be used by other targets (assuming we expand
>> CpuModelExpansionType enum values).
>>
>> Let's always include this field, regardless of the target, but make it
>> optional. This is not a compatibility break only because the field
>> remains present always on S390x.
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>   qapi/machine-target.json | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>> index 426ce4ee82d..d8dbda4b508 100644
>> --- a/qapi/machine-target.json
>> +++ b/qapi/machine-target.json
>> @@ -244,19 +244,18 @@
>>   #
>>   # @model: the expanded CpuModelInfo.
>>   #
>> -# @deprecated-props: a list of properties that are flagged as
>> +# @deprecated-props: an optional list of properties that are flagged as
>>   #     deprecated by the CPU vendor.  The list depends on the
>>   #     CpuModelExpansionType: "static" properties are a subset of the
>>   #     enabled-properties for the expanded model; "full" properties are
>>   #     a set of properties that are deprecated across all models for
>> -#     the architecture.  (since: 9.1).
>> +#     the architecture.  (since: 10.1 -- since 9.1 on s390x --).
>>   #
>>   # Since: 2.8
>>   ##
>>   { 'struct': 'CpuModelExpansionInfo',
>>     'data': { 'model': 'CpuModelInfo',
>> -            'deprecated-props' : { 'type': ['str'],
>> -                                   'if': 'TARGET_S390X' } },
>> +            '*deprecated-props' : ['str'] },
>>     'if': { 'any': [ 'TARGET_S390X',
>>                      'TARGET_I386',
>>                      'TARGET_ARM',
> 
> [Copied from review of prior posts]
> 
> When I see "optional array", I wonder about the difference between
> "absent" and "present and empty".  The doc comment doesn't quite explain
> it.  I figure "present and empty" means empty, while "absent" means we
> don't know / not implemented.
> 
> Is the difference useful?
> 
> Daniel doubts it is.
> 
> [end of copy]
> 
> Let's take with this patch as is.  If we prefer to remove the
> difference, we can do so on top.
> 

I didn't answer to this question because I have no opinion on it.
If I have to pick an answer, following Daniel, I doubt the difference is 
useful also, as it's not changing anything for s390, which is the only 
producer for this entry.

Thanks,
Pierrick