Re: [PATCH v2 1/3] cpu-models: add "disable-deprecated-feats" option to cpu model expansion
Collin Walling writes: > On 4/25/24 02:31, Markus Armbruster wrote: >> Collin Walling writes: >> >>> On 4/24/24 02:19, Markus Armbruster wrote: Collin Walling writes: > This optional parameter for query-cpu-model-expansion enables CPU > model features flagged as deprecated to appear in the resulting > list of properties. > > This commit does not add support beyond adding a new argument > to the query. All queries with this option present will result > in an error claiming this option is not supported. > > Signed-off-by: Collin Walling > --- > qapi/machine-target.json | 7 ++- > target/arm/arm-qmp-cmds.c| 7 +++ > target/i386/cpu-sysemu.c | 7 +++ > target/s390x/cpu_models_sysemu.c | 7 +++ > 4 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/qapi/machine-target.json b/qapi/machine-target.json > index 29e695aa06..b9da284d2d 100644 > --- a/qapi/machine-target.json > +++ b/qapi/machine-target.json > @@ -285,6 +285,10 @@ > # > # @type: expansion type, specifying how to expand the CPU model > # > +# @disable-deprecated-feats: include CPU model features that are > +# flagged as deprecated. If supported, these features will appear > +# in the properties list paired with false. What's the default? Which command result(s) does this affect? Suggest to explain using unabridged example QMP input and output before and after this series. >>> >>> Fair enough. Bool defaults to false but that's not apparent in the >>> description. I will add more detail. >> >> I didn't mean to ask for example QMP in the doc comment. I need you to >> explain the new member to me. Once I understand what the thing does, I >> may have suggestions on improving the doc comment. >> >> [...] >> > > Ah, I misunderstood. The idea behind this "disable-deprecated-feats" > option is to add/override any CPU features that are flagged as > deprecated to the props dictionary (found in the CpuModelInfo struct) > paired with the value false. > > For example, the csske feature on s390x is flagged as deprecated. If a > query-cpu-model-expansion command is created with > "disable-deprecated-feats": true, then the csske feature will show up in > the props list as "csske": false. This also overrides any user defined > features and props that would show up in the response normally. E.g. if > the same command was executed and "csske": true was provided in the > model's properties by the user, the response will still show "csske": > false since the "disable-deprecated-feats" option takes priority (there > is a discussion with David H regarding which should take precedence -- > this is a flaw in this design). > > In the below QMP samples I provide a static expansion on a host model. > Pay close attention to the bpb, te, cte, and csske properties. [...] > Hope this helps to provide more context. Please let me know if you'd > like more info. I am leaning towards v3's design more, as it seems a > lot cleaner overall. I would appreciate your feedback there as well if > you have the time. > > Thanks! Okay, I'll look at v3 then. Thank you!
Re: [PATCH v2 1/3] cpu-models: add "disable-deprecated-feats" option to cpu model expansion
On 4/25/24 02:31, Markus Armbruster wrote:
> Collin Walling writes:
>
>> On 4/24/24 02:19, Markus Armbruster wrote:
>>> Collin Walling writes:
>>>
This optional parameter for query-cpu-model-expansion enables CPU
model features flagged as deprecated to appear in the resulting
list of properties.
This commit does not add support beyond adding a new argument
to the query. All queries with this option present will result
in an error claiming this option is not supported.
Signed-off-by: Collin Walling
---
qapi/machine-target.json | 7 ++-
target/arm/arm-qmp-cmds.c| 7 +++
target/i386/cpu-sysemu.c | 7 +++
target/s390x/cpu_models_sysemu.c | 7 +++
4 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 29e695aa06..b9da284d2d 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -285,6 +285,10 @@
#
# @type: expansion type, specifying how to expand the CPU model
#
+# @disable-deprecated-feats: include CPU model features that are
+# flagged as deprecated. If supported, these features will appear
+# in the properties list paired with false.
>>>
>>> What's the default?
>>>
>>> Which command result(s) does this affect? Suggest to explain using
>>> unabridged example QMP input and output before and after this series.
>>
>> Fair enough. Bool defaults to false but that's not apparent in the
>> description. I will add more detail.
>
> I didn't mean to ask for example QMP in the doc comment. I need you to
> explain the new member to me. Once I understand what the thing does, I
> may have suggestions on improving the doc comment.
>
> [...]
>
Ah, I misunderstood. The idea behind this "disable-deprecated-feats"
option is to add/override any CPU features that are flagged as
deprecated to the props dictionary (found in the CpuModelInfo struct)
paired with the value false.
For example, the csske feature on s390x is flagged as deprecated. If a
query-cpu-model-expansion command is created with
"disable-deprecated-feats": true, then the csske feature will show up in
the props list as "csske": false. This also overrides any user defined
features and props that would show up in the response normally. E.g. if
the same command was executed and "csske": true was provided in the
model's properties by the user, the response will still show "csske":
false since the "disable-deprecated-feats" option takes priority (there
is a discussion with David H regarding which should take precedence --
this is a flaw in this design).
In the below QMP samples I provide a static expansion on a host model.
Pay close attention to the bpb, te, cte, and csske properties.
Before
This is how query-cpu-model-expansion may be executed today:
{
"execute": "query-cpu-model-expansion",
"arguments": {
"type": "static",
"model": {
"name": "host"
}
}
}
{
"return": {
"model": {
"name": "z14.2-base",
"props": {
"aen": true,
"cmmnt": true,
"aefsi": true,
"diag318": true,
"mepoch": true,
"msa8": true,
"msa7": true,
"msa6": true,
"msa5": true,
"msa4": true,
"msa3": true,
"msa2": true,
"msa1": true,
"sthyi": true,
"edat": true,
"ri": true,
"edat2": true,
"etoken": true,
"vx": true,
"ipter": true,
"mepochptff": true,
"ap": true,
"vxeh": true,
"vxpd": true,
"esop": true,
"apqi": true,
"apft": true,
"els": true,
"iep": true,
"apqci": true,
"cte": true,<--
"ais": true,
"bpb": true,<--
"ctop": true,
"gs": true,
"ppa15": true,
"zpci": true,
"sea_esop2": true,
"te": true, <--
"cmm": true
}
}
}
}
Note: csske does not even show up here.
===
After
===
This is with the optional "disabled-deprecated-feats":
{
"execute": "query-cpu-model-expansion",
"arguments": {
"type": "static",
"model": {
"name": "host"
},
"disabled-deprecated-feats": true
}
}
{
"return": {
"model": {
"name": "z14.2-base",
"props": {
"aen": true,
"cmmnt": true,
"aefsi": true,
"diag318": true,
"mepoch": true,
"msa8": true,
"msa7": true,
"msa6": true,
"msa5": true,
"msa4": true,
"msa3": true,
"msa2": true,
"msa1": true,
"sthyi": true,
"edat": true,
"ri": true,
"edat2": true,
"etoken": true,
"vx": true,
"ipter": true,
"mepochptff": true,
"ap": true,
"vxeh": true,
Re: [PATCH v2 1/3] cpu-models: add "disable-deprecated-feats" option to cpu model expansion
On 4/25/24 09:35, Daniel P. Berrangé wrote:
> On Wed, Apr 24, 2024 at 03:12:42PM -0400, Collin Walling wrote:
>> On 4/24/24 13:51, Collin Walling wrote:
>>> On 4/24/24 04:20, Daniel P. Berrangé wrote:
On Tue, Apr 23, 2024 at 05:06:53PM -0400, Collin Walling wrote:
> This optional parameter for query-cpu-model-expansion enables CPU
> model features flagged as deprecated to appear in the resulting
> list of properties.
>
> This commit does not add support beyond adding a new argument
> to the query. All queries with this option present will result
> in an error claiming this option is not supported.
>
> Signed-off-by: Collin Walling
> ---
> qapi/machine-target.json | 7 ++-
> target/arm/arm-qmp-cmds.c| 7 +++
> target/i386/cpu-sysemu.c | 7 +++
> target/s390x/cpu_models_sysemu.c | 7 +++
> 4 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index 29e695aa06..b9da284d2d 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -285,6 +285,10 @@
> #
> # @type: expansion type, specifying how to expand the CPU model
> #
> +# @disable-deprecated-feats: include CPU model features that are
> +# flagged as deprecated. If supported, these features will appear
> +# in the properties list paired with false.
> +#
> # Returns: a CpuModelExpansionInfo describing the expanded CPU model
> #
> # Errors:
> @@ -298,7 +302,8 @@
> ##
> { 'command': 'query-cpu-model-expansion',
>'data': { 'type': 'CpuModelExpansionType',
> -'model': 'CpuModelInfo' },
> +'model': 'CpuModelInfo',
> +'*disable-deprecated-feats': 'bool' },
>'returns': 'CpuModelExpansionInfo',
>'if': { 'any': [ 'TARGET_S390X',
> 'TARGET_I386',
I think this is an odd design approach. Lets consider the
current output:
(QEMU) query-cpu-model-expansion type=static model={"name":"z14"}
{
"return": {
"model": {
"name": "z14-base",
"props": {
"aefsi": true,
"aen": true,
...snip...
"vxpd": true,
"zpci": true
}
}
}
}
If we want to inform a mgmt app of some features being deprecated,
why not just unconditionally include that info in the reply thus:
(QEMU) query-cpu-model-expansion type=static model={"name":"z14"}
{
"return": {
"model": {
"name": "z14-base",
"props": {
"aefsi": true,
"aen": true,
...snip...
"vxpd": true,
"zpci": true
}
"deprecated-props": ["ppa15", "ri"]
}
}
}
With regards,
Daniel
>>>
>>> That's a good idea. In this way, we're not mucking up any of the CPU
>>> model information and this makes it much more clear as to which features
>>> are actually deprecated... I like this more.
>>>
>>> I'll work on this.
>>>
>>
>> Follow-up question as I look more closely to the QMP response data
>> structures: should the "deprecated-props" list be added to the
>> CpuModelInfo struct, or to the CpuModelExpansionInfo struct?
>>
>> The former makes more sense to me, as the deprecated features are tied
>> to the actual CPU model... but unsure if other QMP commands would even
>> care about this info? I will begin with this approach, and if feedback
>> in the interim strongly sways in the other direction, then it should be
>> an easy change :)
>
> I hink CpuModelInfo makes more sense than CpuModelExpansionInfo.
> The CpuModelExpansionInfo struct feels pretty pointless to me
> in fact, since the only thing it contains is CpuModelInfo !
>
Agreed! :)
> I think it should also be added to 'CpuDefinitionInfo', which
> is the return type of 'query-cpu-defintions'. This command already
> has a 'unavailable-features' array listing features which the host
> does not support. Conceptually having a 'deprecated-features' array
> alongside that is a nice fit.
>
Okay. Pending review on the v3 I posted yesterday -- if those patches
look like they're on the right track, then I can add this
"deprecated-props" array to CpuDefinitionInfo as well for the next
iteration.
>
>
> With regards,
> Daniel
--
Regards,
Collin
Re: [PATCH v2 1/3] cpu-models: add "disable-deprecated-feats" option to cpu model expansion
On Wed, Apr 24, 2024 at 03:12:42PM -0400, Collin Walling wrote:
> On 4/24/24 13:51, Collin Walling wrote:
> > On 4/24/24 04:20, Daniel P. Berrangé wrote:
> >> On Tue, Apr 23, 2024 at 05:06:53PM -0400, Collin Walling wrote:
> >>> This optional parameter for query-cpu-model-expansion enables CPU
> >>> model features flagged as deprecated to appear in the resulting
> >>> list of properties.
> >>>
> >>> This commit does not add support beyond adding a new argument
> >>> to the query. All queries with this option present will result
> >>> in an error claiming this option is not supported.
> >>>
> >>> Signed-off-by: Collin Walling
> >>> ---
> >>> qapi/machine-target.json | 7 ++-
> >>> target/arm/arm-qmp-cmds.c| 7 +++
> >>> target/i386/cpu-sysemu.c | 7 +++
> >>> target/s390x/cpu_models_sysemu.c | 7 +++
> >>> 4 files changed, 27 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> >>> index 29e695aa06..b9da284d2d 100644
> >>> --- a/qapi/machine-target.json
> >>> +++ b/qapi/machine-target.json
> >>> @@ -285,6 +285,10 @@
> >>> #
> >>> # @type: expansion type, specifying how to expand the CPU model
> >>> #
> >>> +# @disable-deprecated-feats: include CPU model features that are
> >>> +# flagged as deprecated. If supported, these features will appear
> >>> +# in the properties list paired with false.
> >>> +#
> >>> # Returns: a CpuModelExpansionInfo describing the expanded CPU model
> >>> #
> >>> # Errors:
> >>> @@ -298,7 +302,8 @@
> >>> ##
> >>> { 'command': 'query-cpu-model-expansion',
> >>>'data': { 'type': 'CpuModelExpansionType',
> >>> -'model': 'CpuModelInfo' },
> >>> +'model': 'CpuModelInfo',
> >>> +'*disable-deprecated-feats': 'bool' },
> >>>'returns': 'CpuModelExpansionInfo',
> >>>'if': { 'any': [ 'TARGET_S390X',
> >>> 'TARGET_I386',
> >>
> >> I think this is an odd design approach. Lets consider the
> >> current output:
> >>
> >> (QEMU) query-cpu-model-expansion type=static model={"name":"z14"}
> >> {
> >> "return": {
> >> "model": {
> >> "name": "z14-base",
> >> "props": {
> >> "aefsi": true,
> >> "aen": true,
> >> ...snip...
> >> "vxpd": true,
> >> "zpci": true
> >> }
> >> }
> >> }
> >> }
> >>
> >>
> >> If we want to inform a mgmt app of some features being deprecated,
> >> why not just unconditionally include that info in the reply thus:
> >>
> >>
> >> (QEMU) query-cpu-model-expansion type=static model={"name":"z14"}
> >> {
> >> "return": {
> >> "model": {
> >> "name": "z14-base",
> >> "props": {
> >> "aefsi": true,
> >> "aen": true,
> >> ...snip...
> >> "vxpd": true,
> >> "zpci": true
> >> }
> >> "deprecated-props": ["ppa15", "ri"]
> >> }
> >> }
> >> }
> >>
> >>
> >>
> >> With regards,
> >> Daniel
> >
> > That's a good idea. In this way, we're not mucking up any of the CPU
> > model information and this makes it much more clear as to which features
> > are actually deprecated... I like this more.
> >
> > I'll work on this.
> >
>
> Follow-up question as I look more closely to the QMP response data
> structures: should the "deprecated-props" list be added to the
> CpuModelInfo struct, or to the CpuModelExpansionInfo struct?
>
> The former makes more sense to me, as the deprecated features are tied
> to the actual CPU model... but unsure if other QMP commands would even
> care about this info? I will begin with this approach, and if feedback
> in the interim strongly sways in the other direction, then it should be
> an easy change :)
I hink CpuModelInfo makes more sense than CpuModelExpansionInfo.
The CpuModelExpansionInfo struct feels pretty pointless to me
in fact, since the only thing it contains is CpuModelInfo !
I think it should also be added to 'CpuDefinitionInfo', which
is the return type of 'query-cpu-defintions'. This command already
has a 'unavailable-features' array listing features which the host
does not support. Conceptually having a 'deprecated-features' array
alongside that is a nice fit.
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 1/3] cpu-models: add "disable-deprecated-feats" option to cpu model expansion
Collin Walling writes: > On 4/24/24 02:19, Markus Armbruster wrote: >> Collin Walling writes: >> >>> This optional parameter for query-cpu-model-expansion enables CPU >>> model features flagged as deprecated to appear in the resulting >>> list of properties. >>> >>> This commit does not add support beyond adding a new argument >>> to the query. All queries with this option present will result >>> in an error claiming this option is not supported. >>> >>> Signed-off-by: Collin Walling >>> --- >>> qapi/machine-target.json | 7 ++- >>> target/arm/arm-qmp-cmds.c| 7 +++ >>> target/i386/cpu-sysemu.c | 7 +++ >>> target/s390x/cpu_models_sysemu.c | 7 +++ >>> 4 files changed, 27 insertions(+), 1 deletion(-) >>> >>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json >>> index 29e695aa06..b9da284d2d 100644 >>> --- a/qapi/machine-target.json >>> +++ b/qapi/machine-target.json >>> @@ -285,6 +285,10 @@ >>> # >>> # @type: expansion type, specifying how to expand the CPU model >>> # >>> +# @disable-deprecated-feats: include CPU model features that are >>> +# flagged as deprecated. If supported, these features will appear >>> +# in the properties list paired with false. >> >> What's the default? >> >> Which command result(s) does this affect? Suggest to explain using >> unabridged example QMP input and output before and after this series. > > Fair enough. Bool defaults to false but that's not apparent in the > description. I will add more detail. I didn't mean to ask for example QMP in the doc comment. I need you to explain the new member to me. Once I understand what the thing does, I may have suggestions on improving the doc comment. [...] > Thanks for your feedback! You're welcome!
Re: [PATCH v2 1/3] cpu-models: add "disable-deprecated-feats" option to cpu model expansion
On 4/24/24 13:51, Collin Walling wrote:
> On 4/24/24 04:20, Daniel P. Berrangé wrote:
>> On Tue, Apr 23, 2024 at 05:06:53PM -0400, Collin Walling wrote:
>>> This optional parameter for query-cpu-model-expansion enables CPU
>>> model features flagged as deprecated to appear in the resulting
>>> list of properties.
>>>
>>> This commit does not add support beyond adding a new argument
>>> to the query. All queries with this option present will result
>>> in an error claiming this option is not supported.
>>>
>>> Signed-off-by: Collin Walling
>>> ---
>>> qapi/machine-target.json | 7 ++-
>>> target/arm/arm-qmp-cmds.c| 7 +++
>>> target/i386/cpu-sysemu.c | 7 +++
>>> target/s390x/cpu_models_sysemu.c | 7 +++
>>> 4 files changed, 27 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>>> index 29e695aa06..b9da284d2d 100644
>>> --- a/qapi/machine-target.json
>>> +++ b/qapi/machine-target.json
>>> @@ -285,6 +285,10 @@
>>> #
>>> # @type: expansion type, specifying how to expand the CPU model
>>> #
>>> +# @disable-deprecated-feats: include CPU model features that are
>>> +# flagged as deprecated. If supported, these features will appear
>>> +# in the properties list paired with false.
>>> +#
>>> # Returns: a CpuModelExpansionInfo describing the expanded CPU model
>>> #
>>> # Errors:
>>> @@ -298,7 +302,8 @@
>>> ##
>>> { 'command': 'query-cpu-model-expansion',
>>>'data': { 'type': 'CpuModelExpansionType',
>>> -'model': 'CpuModelInfo' },
>>> +'model': 'CpuModelInfo',
>>> +'*disable-deprecated-feats': 'bool' },
>>>'returns': 'CpuModelExpansionInfo',
>>>'if': { 'any': [ 'TARGET_S390X',
>>> 'TARGET_I386',
>>
>> I think this is an odd design approach. Lets consider the
>> current output:
>>
>> (QEMU) query-cpu-model-expansion type=static model={"name":"z14"}
>> {
>> "return": {
>> "model": {
>> "name": "z14-base",
>> "props": {
>> "aefsi": true,
>> "aen": true,
>> ...snip...
>> "vxpd": true,
>> "zpci": true
>> }
>> }
>> }
>> }
>>
>>
>> If we want to inform a mgmt app of some features being deprecated,
>> why not just unconditionally include that info in the reply thus:
>>
>>
>> (QEMU) query-cpu-model-expansion type=static model={"name":"z14"}
>> {
>> "return": {
>> "model": {
>> "name": "z14-base",
>> "props": {
>> "aefsi": true,
>> "aen": true,
>> ...snip...
>> "vxpd": true,
>> "zpci": true
>> }
>> "deprecated-props": ["ppa15", "ri"]
>> }
>> }
>> }
>>
>>
>>
>> With regards,
>> Daniel
>
> That's a good idea. In this way, we're not mucking up any of the CPU
> model information and this makes it much more clear as to which features
> are actually deprecated... I like this more.
>
> I'll work on this.
>
Follow-up question as I look more closely to the QMP response data
structures: should the "deprecated-props" list be added to the
CpuModelInfo struct, or to the CpuModelExpansionInfo struct?
The former makes more sense to me, as the deprecated features are tied
to the actual CPU model... but unsure if other QMP commands would even
care about this info? I will begin with this approach, and if feedback
in the interim strongly sways in the other direction, then it should be
an easy change :)
--
Regards,
Collin
Re: [PATCH v2 1/3] cpu-models: add "disable-deprecated-feats" option to cpu model expansion
On 4/24/24 04:20, Daniel P. Berrangé wrote:
> On Tue, Apr 23, 2024 at 05:06:53PM -0400, Collin Walling wrote:
>> This optional parameter for query-cpu-model-expansion enables CPU
>> model features flagged as deprecated to appear in the resulting
>> list of properties.
>>
>> This commit does not add support beyond adding a new argument
>> to the query. All queries with this option present will result
>> in an error claiming this option is not supported.
>>
>> Signed-off-by: Collin Walling
>> ---
>> qapi/machine-target.json | 7 ++-
>> target/arm/arm-qmp-cmds.c| 7 +++
>> target/i386/cpu-sysemu.c | 7 +++
>> target/s390x/cpu_models_sysemu.c | 7 +++
>> 4 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>> index 29e695aa06..b9da284d2d 100644
>> --- a/qapi/machine-target.json
>> +++ b/qapi/machine-target.json
>> @@ -285,6 +285,10 @@
>> #
>> # @type: expansion type, specifying how to expand the CPU model
>> #
>> +# @disable-deprecated-feats: include CPU model features that are
>> +# flagged as deprecated. If supported, these features will appear
>> +# in the properties list paired with false.
>> +#
>> # Returns: a CpuModelExpansionInfo describing the expanded CPU model
>> #
>> # Errors:
>> @@ -298,7 +302,8 @@
>> ##
>> { 'command': 'query-cpu-model-expansion',
>>'data': { 'type': 'CpuModelExpansionType',
>> -'model': 'CpuModelInfo' },
>> +'model': 'CpuModelInfo',
>> +'*disable-deprecated-feats': 'bool' },
>>'returns': 'CpuModelExpansionInfo',
>>'if': { 'any': [ 'TARGET_S390X',
>> 'TARGET_I386',
>
> I think this is an odd design approach. Lets consider the
> current output:
>
> (QEMU) query-cpu-model-expansion type=static model={"name":"z14"}
> {
> "return": {
> "model": {
> "name": "z14-base",
> "props": {
> "aefsi": true,
> "aen": true,
> ...snip...
> "vxpd": true,
> "zpci": true
> }
> }
> }
> }
>
>
> If we want to inform a mgmt app of some features being deprecated,
> why not just unconditionally include that info in the reply thus:
>
>
> (QEMU) query-cpu-model-expansion type=static model={"name":"z14"}
> {
> "return": {
> "model": {
> "name": "z14-base",
> "props": {
> "aefsi": true,
> "aen": true,
> ...snip...
> "vxpd": true,
> "zpci": true
> }
> "deprecated-props": ["ppa15", "ri"]
> }
> }
> }
>
>
>
> With regards,
> Daniel
That's a good idea. In this way, we're not mucking up any of the CPU
model information and this makes it much more clear as to which features
are actually deprecated... I like this more.
I'll work on this.
--
Regards,
Collin
Re: [PATCH v2 1/3] cpu-models: add "disable-deprecated-feats" option to cpu model expansion
On 4/24/24 02:19, Markus Armbruster wrote:
> Collin Walling writes:
>
>> This optional parameter for query-cpu-model-expansion enables CPU
>> model features flagged as deprecated to appear in the resulting
>> list of properties.
>>
>> This commit does not add support beyond adding a new argument
>> to the query. All queries with this option present will result
>> in an error claiming this option is not supported.
>>
>> Signed-off-by: Collin Walling
>> ---
>> qapi/machine-target.json | 7 ++-
>> target/arm/arm-qmp-cmds.c| 7 +++
>> target/i386/cpu-sysemu.c | 7 +++
>> target/s390x/cpu_models_sysemu.c | 7 +++
>> 4 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>> index 29e695aa06..b9da284d2d 100644
>> --- a/qapi/machine-target.json
>> +++ b/qapi/machine-target.json
>> @@ -285,6 +285,10 @@
>> #
>> # @type: expansion type, specifying how to expand the CPU model
>> #
>> +# @disable-deprecated-feats: include CPU model features that are
>> +# flagged as deprecated. If supported, these features will appear
>> +# in the properties list paired with false.
>
> What's the default?
>
> Which command result(s) does this affect? Suggest to explain using
> unabridged example QMP input and output before and after this series.
>
Fair enough. Bool defaults to false but that's not apparent in the
description. I will add more detail.
> We generally avoid abbreviations in QMP names. Let's call this
> @disable-deprecated-features.
>
Okay.
> Separate sentences with two spaces for consistency, please.
>
Understood.
>> +#
>> # Returns: a CpuModelExpansionInfo describing the expanded CPU model
>> #
>> # Errors:
>> @@ -298,7 +302,8 @@
>> ##
>> { 'command': 'query-cpu-model-expansion',
>>'data': { 'type': 'CpuModelExpansionType',
>> -'model': 'CpuModelInfo' },
>> +'model': 'CpuModelInfo',
>> +'*disable-deprecated-feats': 'bool' },
>>'returns': 'CpuModelExpansionInfo',
>>'if': { 'any': [ 'TARGET_S390X',
>> 'TARGET_I386',
> 'TARGET_ARM',
> 'TARGET_LOONGARCH64',
> 'TARGET_RISCV' ] } }
>
> Put a pin into this conditional: [*].
>
>> diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
>> index 3cc8cc738b..1010d654e3 100644
>> --- a/target/arm/arm-qmp-cmds.c
>> +++ b/target/arm/arm-qmp-cmds.c
>> @@ -100,6 +100,8 @@ static const char *cpu_model_advertised_features[] = {
>>
>> CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType
>> type,
>> CpuModelInfo *model,
>> + bool
>> has_disable_deprecated_feats,
>> + bool
>> disable_deprecated_feats,
>> Error **errp)
>> {
>> CpuModelExpansionInfo *expansion_info;
>> @@ -110,6 +112,11 @@ CpuModelExpansionInfo
>> *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>> const char *name;
>> int i;
>>
>> +if (has_disable_deprecated_feats) {
>> +error_setg(, "Unsupported option 'disable-deprecated-feats'");
>> +return NULL;
>> +}
>
> Reject the new argument in the ARM version, ...
>
>> +
>> if (type != CPU_MODEL_EXPANSION_TYPE_FULL) {
>> error_setg(errp, "The requested expansion type is not supported");
>> return NULL;
>> diff --git a/target/i386/cpu-sysemu.c b/target/i386/cpu-sysemu.c
>> index 3f9093d285..c15786fb66 100644
>> --- a/target/i386/cpu-sysemu.c
>> +++ b/target/i386/cpu-sysemu.c
>> @@ -196,6 +196,8 @@ out:
>> CpuModelExpansionInfo *
>> qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>>CpuModelInfo *model,
>> + bool
>> has_disable_deprecated_feats,
>> + bool
>> disable_deprecated_feats,
>>Error **errp)
>> {
>> X86CPU *xc = NULL;
>> @@ -204,6 +206,11 @@ qmp_query_cpu_model_expansion(CpuModelExpansionType
>> type,
>> QDict *props = NULL;
>> const char *base_name;
>>
>> +if (has_disable_deprecated_feats) {
>> +error_setg(, "Unsupported option 'disable-deprecated-feats'");
>> +goto out;
>> +}
>
> ... the i386 version, ...
>
>> +
>> xc = x86_cpu_from_model(model->name, model->props, "model.props", );
>> if (err) {
>> goto out;
>> diff --git a/target/s390x/cpu_models_sysemu.c
>> b/target/s390x/cpu_models_sysemu.c
>> index 2d99218069..ef9fa80efd 100644
>> --- a/target/s390x/cpu_models_sysemu.c
>> +++ b/target/s390x/cpu_models_sysemu.c
>> @@ -210,6 +210,8 @@ static void cpu_info_from_model(CpuModelInfo *info,
>>
Re: [PATCH v2 1/3] cpu-models: add "disable-deprecated-feats" option to cpu model expansion
On Tue, Apr 23, 2024 at 05:06:53PM -0400, Collin Walling wrote:
> This optional parameter for query-cpu-model-expansion enables CPU
> model features flagged as deprecated to appear in the resulting
> list of properties.
>
> This commit does not add support beyond adding a new argument
> to the query. All queries with this option present will result
> in an error claiming this option is not supported.
>
> Signed-off-by: Collin Walling
> ---
> qapi/machine-target.json | 7 ++-
> target/arm/arm-qmp-cmds.c| 7 +++
> target/i386/cpu-sysemu.c | 7 +++
> target/s390x/cpu_models_sysemu.c | 7 +++
> 4 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index 29e695aa06..b9da284d2d 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -285,6 +285,10 @@
> #
> # @type: expansion type, specifying how to expand the CPU model
> #
> +# @disable-deprecated-feats: include CPU model features that are
> +# flagged as deprecated. If supported, these features will appear
> +# in the properties list paired with false.
> +#
> # Returns: a CpuModelExpansionInfo describing the expanded CPU model
> #
> # Errors:
> @@ -298,7 +302,8 @@
> ##
> { 'command': 'query-cpu-model-expansion',
>'data': { 'type': 'CpuModelExpansionType',
> -'model': 'CpuModelInfo' },
> +'model': 'CpuModelInfo',
> +'*disable-deprecated-feats': 'bool' },
>'returns': 'CpuModelExpansionInfo',
>'if': { 'any': [ 'TARGET_S390X',
> 'TARGET_I386',
I think this is an odd design approach. Lets consider the
current output:
(QEMU) query-cpu-model-expansion type=static model={"name":"z14"}
{
"return": {
"model": {
"name": "z14-base",
"props": {
"aefsi": true,
"aen": true,
...snip...
"vxpd": true,
"zpci": true
}
}
}
}
If we want to inform a mgmt app of some features being deprecated,
why not just unconditionally include that info in the reply thus:
(QEMU) query-cpu-model-expansion type=static model={"name":"z14"}
{
"return": {
"model": {
"name": "z14-base",
"props": {
"aefsi": true,
"aen": true,
...snip...
"vxpd": true,
"zpci": true
}
"deprecated-props": ["ppa15", "ri"]
}
}
}
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 1/3] cpu-models: add "disable-deprecated-feats" option to cpu model expansion
Collin Walling writes:
> This optional parameter for query-cpu-model-expansion enables CPU
> model features flagged as deprecated to appear in the resulting
> list of properties.
>
> This commit does not add support beyond adding a new argument
> to the query. All queries with this option present will result
> in an error claiming this option is not supported.
>
> Signed-off-by: Collin Walling
> ---
> qapi/machine-target.json | 7 ++-
> target/arm/arm-qmp-cmds.c| 7 +++
> target/i386/cpu-sysemu.c | 7 +++
> target/s390x/cpu_models_sysemu.c | 7 +++
> 4 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index 29e695aa06..b9da284d2d 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -285,6 +285,10 @@
> #
> # @type: expansion type, specifying how to expand the CPU model
> #
> +# @disable-deprecated-feats: include CPU model features that are
> +# flagged as deprecated. If supported, these features will appear
> +# in the properties list paired with false.
What's the default?
Which command result(s) does this affect? Suggest to explain using
unabridged example QMP input and output before and after this series.
We generally avoid abbreviations in QMP names. Let's call this
@disable-deprecated-features.
Separate sentences with two spaces for consistency, please.
> +#
> # Returns: a CpuModelExpansionInfo describing the expanded CPU model
> #
> # Errors:
> @@ -298,7 +302,8 @@
> ##
> { 'command': 'query-cpu-model-expansion',
>'data': { 'type': 'CpuModelExpansionType',
> -'model': 'CpuModelInfo' },
> +'model': 'CpuModelInfo',
> +'*disable-deprecated-feats': 'bool' },
>'returns': 'CpuModelExpansionInfo',
>'if': { 'any': [ 'TARGET_S390X',
> 'TARGET_I386',
'TARGET_ARM',
'TARGET_LOONGARCH64',
'TARGET_RISCV' ] } }
Put a pin into this conditional: [*].
> diff --git a/target/arm/arm-qmp-cmds.c b/target/arm/arm-qmp-cmds.c
> index 3cc8cc738b..1010d654e3 100644
> --- a/target/arm/arm-qmp-cmds.c
> +++ b/target/arm/arm-qmp-cmds.c
> @@ -100,6 +100,8 @@ static const char *cpu_model_advertised_features[] = {
>
> CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType
> type,
> CpuModelInfo *model,
> + bool
> has_disable_deprecated_feats,
> + bool
> disable_deprecated_feats,
> Error **errp)
> {
> CpuModelExpansionInfo *expansion_info;
> @@ -110,6 +112,11 @@ CpuModelExpansionInfo
> *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
> const char *name;
> int i;
>
> +if (has_disable_deprecated_feats) {
> +error_setg(, "Unsupported option 'disable-deprecated-feats'");
> +return NULL;
> +}
Reject the new argument in the ARM version, ...
> +
> if (type != CPU_MODEL_EXPANSION_TYPE_FULL) {
> error_setg(errp, "The requested expansion type is not supported");
> return NULL;
> diff --git a/target/i386/cpu-sysemu.c b/target/i386/cpu-sysemu.c
> index 3f9093d285..c15786fb66 100644
> --- a/target/i386/cpu-sysemu.c
> +++ b/target/i386/cpu-sysemu.c
> @@ -196,6 +196,8 @@ out:
> CpuModelExpansionInfo *
> qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>CpuModelInfo *model,
> + bool
> has_disable_deprecated_feats,
> + bool
> disable_deprecated_feats,
>Error **errp)
> {
> X86CPU *xc = NULL;
> @@ -204,6 +206,11 @@ qmp_query_cpu_model_expansion(CpuModelExpansionType type,
> QDict *props = NULL;
> const char *base_name;
>
> +if (has_disable_deprecated_feats) {
> +error_setg(, "Unsupported option 'disable-deprecated-feats'");
> +goto out;
> +}
... the i386 version, ...
> +
> xc = x86_cpu_from_model(model->name, model->props, "model.props", );
> if (err) {
> goto out;
> diff --git a/target/s390x/cpu_models_sysemu.c
> b/target/s390x/cpu_models_sysemu.c
> index 2d99218069..ef9fa80efd 100644
> --- a/target/s390x/cpu_models_sysemu.c
> +++ b/target/s390x/cpu_models_sysemu.c
> @@ -210,6 +210,8 @@ static void cpu_info_from_model(CpuModelInfo *info, const
> S390CPUModel *model,
>
> CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType
> type,
>CpuModelInfo *model,
> + bool
> has_disable_deprecated_feats,
> +
