| Message ID | 20240716173253.28533-1-walling@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] target/s390x: filter deprecated properties based on model expansion type | expand |
On 16.07.24 19:32, Collin Walling wrote: > As s390 CPU models progress and deprecated properties are dropped > outright, it will be cumbersome for management apps to query the host > for a comprehensive list of deprecated properties that will need to be > disabled on older models. To remedy this, the query-cpu-model-expansion > output now behaves by filtering deprecated properties based on the > expansion type instead of filtering based off of the model's full set > of features: > > When reporting a static CPU model, only show deprecated properties that > are a subset of the model's enabled features. > > When reporting a full CPU model, show the entire list of deprecated > properties regardless if they are supported on the model. > > Suggested-by: Jiri Denemark <jdenemar@redhat.com> > Signed-off-by: Collin Walling <walling@linux.ibm.com> > --- > > Changelog: > > v2 > - Changed commit message > - Added documentation reflecting this change > - Made code changes that more accurately filter the deprecated > properties based on expansion type. This change makes it > so that the deprecated-properties reported for a static model > expansion are a subset of the model's properties instead of > the model's full-definition properties. > > For example: > > Previously, the z900 static model would report 'bpb' in the > list of deprecated-properties. However, this prop is *not* > a part of the model's feature set, leading to some inaccuracy > (albeit harmless). > > Now, this feature will not show during a static expansion. > It will, however, show up in a full expansion (along with > the rest of the list: 'csske', 'te', 'cte'). > > @David, I've elected to respectully forgo adding your ack-by on this > iteration since I have changed the code (and therefore the behavior) > between this version and the previous in case you do not agree with > these adjustments. > > --- > qapi/machine-target.json | 8 ++++++-- > target/s390x/cpu_models_sysemu.c | 16 +++++++++------- > 2 files changed, 15 insertions(+), 9 deletions(-) > > diff --git a/qapi/machine-target.json b/qapi/machine-target.json > index a8d9ec87f5..d151504f25 100644 > --- a/qapi/machine-target.json > +++ b/qapi/machine-target.json > @@ -21,8 +21,12 @@ > # @props: a dictionary of QOM properties to be applied > # > # @deprecated-props: a list of properties that are flagged as deprecated > -# by the CPU vendor. These props are a subset of the full model's > -# definition list of properties. (since 9.1) > +# by the CPU vendor. (since 9.1). > +# > +# .. note:: Since 9.1, the list of deprecated props were always a subset > +# of the model's full-definition list of properites. Now, this list is s/properites/properties/ > +# populated with the model's enabled property set when delta changes > +# are applied. All deprecated properties are reported otherwise. > # > # Since: 2.8 > ## Acked-by: David Hildenbrand <david@redhat.com>
Collin Walling <walling@linux.ibm.com> writes: > As s390 CPU models progress and deprecated properties are dropped > outright, it will be cumbersome for management apps to query the host > for a comprehensive list of deprecated properties that will need to be > disabled on older models. To remedy this, the query-cpu-model-expansion > output now behaves by filtering deprecated properties based on the > expansion type instead of filtering based off of the model's full set > of features: > > When reporting a static CPU model, only show deprecated properties that > are a subset of the model's enabled features. > > When reporting a full CPU model, show the entire list of deprecated > properties regardless if they are supported on the model. > > Suggested-by: Jiri Denemark <jdenemar@redhat.com> > Signed-off-by: Collin Walling <walling@linux.ibm.com> > --- > > Changelog: > > v2 > - Changed commit message > - Added documentation reflecting this change > - Made code changes that more accurately filter the deprecated > properties based on expansion type. This change makes it > so that the deprecated-properties reported for a static model > expansion are a subset of the model's properties instead of > the model's full-definition properties. > > For example: > > Previously, the z900 static model would report 'bpb' in the > list of deprecated-properties. However, this prop is *not* > a part of the model's feature set, leading to some inaccuracy > (albeit harmless). > > Now, this feature will not show during a static expansion. > It will, however, show up in a full expansion (along with > the rest of the list: 'csske', 'te', 'cte'). > > @David, I've elected to respectully forgo adding your ack-by on this > iteration since I have changed the code (and therefore the behavior) > between this version and the previous in case you do not agree with > these adjustments. > > --- > qapi/machine-target.json | 8 ++++++-- > target/s390x/cpu_models_sysemu.c | 16 +++++++++------- > 2 files changed, 15 insertions(+), 9 deletions(-) > > diff --git a/qapi/machine-target.json b/qapi/machine-target.json > index a8d9ec87f5..d151504f25 100644 > --- a/qapi/machine-target.json > +++ b/qapi/machine-target.json > @@ -21,8 +21,12 @@ > # @props: a dictionary of QOM properties to be applied > # > # @deprecated-props: a list of properties that are flagged as deprecated > -# by the CPU vendor. These props are a subset of the full model's > -# definition list of properties. (since 9.1) > +# by the CPU vendor. (since 9.1). > +# > +# .. note:: Since 9.1, the list of deprecated props were always a subset > +# of the model's full-definition list of properites. Now, this list is > +# populated with the model's enabled property set when delta changes > +# are applied. All deprecated properties are reported otherwise. I'm confused. "Since 9.1, the list of deprecated props were ..." and "Now, this list is" sounds like you're explaining behavior before and after a change. What change? Since only released behavior matters, and @deprecated-props is new, there is no old behavior to document, isn't it? docs/devel/qapi-code-gen.rst section "Documentation markup": For legibility, wrap text paragraphs so every line is at most 70 characters long. Separate sentences with two spaces. > # > # Since: 2.8 > ## [...]
On 7/18/24 9:39 AM, Markus Armbruster wrote: > Collin Walling <walling@linux.ibm.com> writes: > >> As s390 CPU models progress and deprecated properties are dropped >> outright, it will be cumbersome for management apps to query the host >> for a comprehensive list of deprecated properties that will need to be >> disabled on older models. To remedy this, the query-cpu-model-expansion >> output now behaves by filtering deprecated properties based on the >> expansion type instead of filtering based off of the model's full set >> of features: >> >> When reporting a static CPU model, only show deprecated properties that >> are a subset of the model's enabled features. >> >> When reporting a full CPU model, show the entire list of deprecated >> properties regardless if they are supported on the model. >> >> Suggested-by: Jiri Denemark <jdenemar@redhat.com> >> Signed-off-by: Collin Walling <walling@linux.ibm.com> >> --- >> >> Changelog: >> >> v2 >> - Changed commit message >> - Added documentation reflecting this change >> - Made code changes that more accurately filter the deprecated >> properties based on expansion type. This change makes it >> so that the deprecated-properties reported for a static model >> expansion are a subset of the model's properties instead of >> the model's full-definition properties. >> >> For example: >> >> Previously, the z900 static model would report 'bpb' in the >> list of deprecated-properties. However, this prop is *not* >> a part of the model's feature set, leading to some inaccuracy >> (albeit harmless). >> >> Now, this feature will not show during a static expansion. >> It will, however, show up in a full expansion (along with >> the rest of the list: 'csske', 'te', 'cte'). >> >> @David, I've elected to respectully forgo adding your ack-by on this >> iteration since I have changed the code (and therefore the behavior) >> between this version and the previous in case you do not agree with >> these adjustments. >> >> --- >> qapi/machine-target.json | 8 ++++++-- >> target/s390x/cpu_models_sysemu.c | 16 +++++++++------- >> 2 files changed, 15 insertions(+), 9 deletions(-) >> >> diff --git a/qapi/machine-target.json b/qapi/machine-target.json >> index a8d9ec87f5..d151504f25 100644 >> --- a/qapi/machine-target.json >> +++ b/qapi/machine-target.json >> @@ -21,8 +21,12 @@ >> # @props: a dictionary of QOM properties to be applied >> # >> # @deprecated-props: a list of properties that are flagged as deprecated >> -# by the CPU vendor. These props are a subset of the full model's >> -# definition list of properties. (since 9.1) >> +# by the CPU vendor. (since 9.1). >> +# >> +# .. note:: Since 9.1, the list of deprecated props were always a subset >> +# of the model's full-definition list of properites. Now, this list is >> +# populated with the model's enabled property set when delta changes >> +# are applied. All deprecated properties are reported otherwise. > > I'm confused. > > "Since 9.1, the list of deprecated props were ..." and "Now, this list > is" sounds like you're explaining behavior before and after a change. > What change? Since only released behavior matters, and > @deprecated-props is new, there is no old behavior to document, isn't > it? I admittedly had some difficulty articulating the change introduced by this patch. The @deprecated-props array, as well as a way for s390x to populate it, was introduced in release 9.1. Prior to this patch, the deprecated-props list was filtered by the CPU model's full feature set. I attempted to explain this with: "Since 9.1, the list of deprecated props were always a subset of the model's full-definition list of properties." This patch modifies what is populated in the list by filtering it via an intersection of the model's /default/ feature set. The reason for this change was that the deprecated-props list reported for very old s390 models was showing features that were not *actually* a subset of the model's feature set. One of the changes proposed by this patch corrects this for static model expansions. Explained by: "Now, this list is populated with the model's enabled property set when delta changes are applied." The other change introduced by this patch is that the entire list of deprecated-properties is now reported via a full model expansion, explained by: "All deprecated properties are reported otherwise." My thoughts were to acknowledge this change in case a user sees different results between QEMU versions. However, as this @deprecated-props thing is relatively new (a few months), perhaps it does not make sense to include this note? What would you suggest? > > docs/devel/qapi-code-gen.rst section "Documentation markup": > > For legibility, wrap text paragraphs so every line is at most 70 > characters long. > > Separate sentences with two spaces. > Forgot about the 70 rule for these docs. Missed the double space. Thanks for reminding me. Will update. >> # >> # Since: 2.8 >> ## > > [...] > >
On 18/07/2024 20.22, Collin Walling wrote: > On 7/18/24 9:39 AM, Markus Armbruster wrote: >> Collin Walling <walling@linux.ibm.com> writes: >> >>> As s390 CPU models progress and deprecated properties are dropped >>> outright, it will be cumbersome for management apps to query the host >>> for a comprehensive list of deprecated properties that will need to be >>> disabled on older models. To remedy this, the query-cpu-model-expansion >>> output now behaves by filtering deprecated properties based on the >>> expansion type instead of filtering based off of the model's full set >>> of features: >>> >>> When reporting a static CPU model, only show deprecated properties that >>> are a subset of the model's enabled features. >>> >>> When reporting a full CPU model, show the entire list of deprecated >>> properties regardless if they are supported on the model. >>> >>> Suggested-by: Jiri Denemark <jdenemar@redhat.com> >>> Signed-off-by: Collin Walling <walling@linux.ibm.com> >>> --- >>> >>> Changelog: >>> >>> v2 >>> - Changed commit message >>> - Added documentation reflecting this change >>> - Made code changes that more accurately filter the deprecated >>> properties based on expansion type. This change makes it >>> so that the deprecated-properties reported for a static model >>> expansion are a subset of the model's properties instead of >>> the model's full-definition properties. >>> >>> For example: >>> >>> Previously, the z900 static model would report 'bpb' in the >>> list of deprecated-properties. However, this prop is *not* >>> a part of the model's feature set, leading to some inaccuracy >>> (albeit harmless). >>> >>> Now, this feature will not show during a static expansion. >>> It will, however, show up in a full expansion (along with >>> the rest of the list: 'csske', 'te', 'cte'). >>> >>> @David, I've elected to respectully forgo adding your ack-by on this >>> iteration since I have changed the code (and therefore the behavior) >>> between this version and the previous in case you do not agree with >>> these adjustments. >>> >>> --- >>> qapi/machine-target.json | 8 ++++++-- >>> target/s390x/cpu_models_sysemu.c | 16 +++++++++------- >>> 2 files changed, 15 insertions(+), 9 deletions(-) >>> >>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json >>> index a8d9ec87f5..d151504f25 100644 >>> --- a/qapi/machine-target.json >>> +++ b/qapi/machine-target.json >>> @@ -21,8 +21,12 @@ >>> # @props: a dictionary of QOM properties to be applied >>> # >>> # @deprecated-props: a list of properties that are flagged as deprecated >>> -# by the CPU vendor. These props are a subset of the full model's >>> -# definition list of properties. (since 9.1) >>> +# by the CPU vendor. (since 9.1). >>> +# >>> +# .. note:: Since 9.1, the list of deprecated props were always a subset >>> +# of the model's full-definition list of properites. Now, this list is >>> +# populated with the model's enabled property set when delta changes >>> +# are applied. All deprecated properties are reported otherwise. >> >> I'm confused. >> >> "Since 9.1, the list of deprecated props were ..." and "Now, this list >> is" sounds like you're explaining behavior before and after a change. >> What change? Since only released behavior matters, and >> @deprecated-props is new, there is no old behavior to document, isn't >> it? > > I admittedly had some difficulty articulating the change introduced by > this patch. The @deprecated-props array, as well as a way for s390x to > populate it, was introduced in release 9.1. Prior to this patch, the > deprecated-props list was filtered by the CPU model's full feature set. > I attempted to explain this with: > > "Since 9.1, the list of deprecated props were always a subset of the > model's full-definition list of properties." Version 9.1 has not been released yet (see https://wiki.qemu.org/Planning/9.1), so I agree with Markus, this sounds confusing/wrong to me, too. Thomas
Thomas Huth <thuth@redhat.com> writes: > On 18/07/2024 20.22, Collin Walling wrote: >> On 7/18/24 9:39 AM, Markus Armbruster wrote: >>> Collin Walling <walling@linux.ibm.com> writes: >>> >>>> As s390 CPU models progress and deprecated properties are dropped >>>> outright, it will be cumbersome for management apps to query the host >>>> for a comprehensive list of deprecated properties that will need to be >>>> disabled on older models. To remedy this, the query-cpu-model-expansion >>>> output now behaves by filtering deprecated properties based on the >>>> expansion type instead of filtering based off of the model's full set >>>> of features: >>>> >>>> When reporting a static CPU model, only show deprecated properties that >>>> are a subset of the model's enabled features. >>>> >>>> When reporting a full CPU model, show the entire list of deprecated >>>> properties regardless if they are supported on the model. >>>> >>>> Suggested-by: Jiri Denemark <jdenemar@redhat.com> >>>> Signed-off-by: Collin Walling <walling@linux.ibm.com> [...] >>>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json >>>> index a8d9ec87f5..d151504f25 100644 >>>> --- a/qapi/machine-target.json >>>> +++ b/qapi/machine-target.json >>>> @@ -21,8 +21,12 @@ >>>> # @props: a dictionary of QOM properties to be applied >>>> # >>>> # @deprecated-props: a list of properties that are flagged as deprecated >>>> -# by the CPU vendor. These props are a subset of the full model's >>>> -# definition list of properties. (since 9.1) >>>> +# by the CPU vendor. (since 9.1). >>>> +# >>>> +# .. note:: Since 9.1, the list of deprecated props were always a subset >>>> +# of the model's full-definition list of properites. Now, this list is >>>> +# populated with the model's enabled property set when delta changes >>>> +# are applied. All deprecated properties are reported otherwise. >>> >>> I'm confused. >>> >>> "Since 9.1, the list of deprecated props were ..." and "Now, this list >>> is" sounds like you're explaining behavior before and after a change. >>> What change? Since only released behavior matters, and >>> @deprecated-props is new, there is no old behavior to document, isn't >>> it? >> >> I admittedly had some difficulty articulating the change introduced by >> this patch. The @deprecated-props array, as well as a way for s390x to >> populate it, was introduced in release 9.1. Prior to this patch, the >> deprecated-props list was filtered by the CPU model's full feature set. >> I attempted to explain this with: >> "Since 9.1, the list of deprecated props were always a subset of the >> model's full-definition list of properties." > > Version 9.1 has not been released yet (see https://wiki.qemu.org/Planning/9.1), so I agree with Markus, this sounds confusing/wrong to me, too. User-visible changes between releases need to be documented in release notes and the manual. Don't for changes within a release, instead update documentation to reflect the new state of things. Regardless, do explain the change in the commit message. Specifics are more helpful than such generalities, so let me try despite my relative ignorance of the subject matter. 1. Update the description of @deprecated-props to match the new code. Ask yourself what people need to know to use this interface. 2. Explain in the commit message how semantics of @deprecated-props change in this patch.
On 7/19/24 6:16 AM, Thomas Huth wrote: > On 18/07/2024 20.22, Collin Walling wrote: >> On 7/18/24 9:39 AM, Markus Armbruster wrote: >>> Collin Walling <walling@linux.ibm.com> writes: >>> >>>> As s390 CPU models progress and deprecated properties are dropped >>>> outright, it will be cumbersome for management apps to query the host >>>> for a comprehensive list of deprecated properties that will need to be >>>> disabled on older models. To remedy this, the query-cpu-model-expansion >>>> output now behaves by filtering deprecated properties based on the >>>> expansion type instead of filtering based off of the model's full set >>>> of features: >>>> >>>> When reporting a static CPU model, only show deprecated properties that >>>> are a subset of the model's enabled features. >>>> >>>> When reporting a full CPU model, show the entire list of deprecated >>>> properties regardless if they are supported on the model. >>>> >>>> Suggested-by: Jiri Denemark <jdenemar@redhat.com> >>>> Signed-off-by: Collin Walling <walling@linux.ibm.com> >>>> --- >>>> >>>> Changelog: >>>> >>>> v2 >>>> - Changed commit message >>>> - Added documentation reflecting this change >>>> - Made code changes that more accurately filter the deprecated >>>> properties based on expansion type. This change makes it >>>> so that the deprecated-properties reported for a static model >>>> expansion are a subset of the model's properties instead of >>>> the model's full-definition properties. >>>> >>>> For example: >>>> >>>> Previously, the z900 static model would report 'bpb' in the >>>> list of deprecated-properties. However, this prop is *not* >>>> a part of the model's feature set, leading to some inaccuracy >>>> (albeit harmless). >>>> >>>> Now, this feature will not show during a static expansion. >>>> It will, however, show up in a full expansion (along with >>>> the rest of the list: 'csske', 'te', 'cte'). >>>> >>>> @David, I've elected to respectully forgo adding your ack-by on this >>>> iteration since I have changed the code (and therefore the behavior) >>>> between this version and the previous in case you do not agree with >>>> these adjustments. >>>> >>>> --- >>>> qapi/machine-target.json | 8 ++++++-- >>>> target/s390x/cpu_models_sysemu.c | 16 +++++++++------- >>>> 2 files changed, 15 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json >>>> index a8d9ec87f5..d151504f25 100644 >>>> --- a/qapi/machine-target.json >>>> +++ b/qapi/machine-target.json >>>> @@ -21,8 +21,12 @@ >>>> # @props: a dictionary of QOM properties to be applied >>>> # >>>> # @deprecated-props: a list of properties that are flagged as deprecated >>>> -# by the CPU vendor. These props are a subset of the full model's >>>> -# definition list of properties. (since 9.1) >>>> +# by the CPU vendor. (since 9.1). >>>> +# >>>> +# .. note:: Since 9.1, the list of deprecated props were always a subset >>>> +# of the model's full-definition list of properites. Now, this list is >>>> +# populated with the model's enabled property set when delta changes >>>> +# are applied. All deprecated properties are reported otherwise. >>> >>> I'm confused. >>> >>> "Since 9.1, the list of deprecated props were ..." and "Now, this list >>> is" sounds like you're explaining behavior before and after a change. >>> What change? Since only released behavior matters, and >>> @deprecated-props is new, there is no old behavior to document, isn't >>> it? >> >> I admittedly had some difficulty articulating the change introduced by >> this patch. The @deprecated-props array, as well as a way for s390x to >> populate it, was introduced in release 9.1. Prior to this patch, the >> deprecated-props list was filtered by the CPU model's full feature set. >> I attempted to explain this with: >> >> "Since 9.1, the list of deprecated props were always a subset of the >> model's full-definition list of properties." > > Version 9.1 has not been released yet (see > https://wiki.qemu.org/Planning/9.1), so I agree with Markus, this sounds > confusing/wrong to me, too. > > Thomas > > > Ah, well then pardon my ignorance of the release schedule. With that said my `note` makes 0 sense to keep here.
On 7/19/24 7:11 AM, Markus Armbruster wrote: > Thomas Huth <thuth@redhat.com> writes: > >> On 18/07/2024 20.22, Collin Walling wrote: >>> On 7/18/24 9:39 AM, Markus Armbruster wrote: >>>> Collin Walling <walling@linux.ibm.com> writes: >>>> >>>>> As s390 CPU models progress and deprecated properties are dropped >>>>> outright, it will be cumbersome for management apps to query the host >>>>> for a comprehensive list of deprecated properties that will need to be >>>>> disabled on older models. To remedy this, the query-cpu-model-expansion >>>>> output now behaves by filtering deprecated properties based on the >>>>> expansion type instead of filtering based off of the model's full set >>>>> of features: >>>>> >>>>> When reporting a static CPU model, only show deprecated properties that >>>>> are a subset of the model's enabled features. >>>>> >>>>> When reporting a full CPU model, show the entire list of deprecated >>>>> properties regardless if they are supported on the model. >>>>> >>>>> Suggested-by: Jiri Denemark <jdenemar@redhat.com> >>>>> Signed-off-by: Collin Walling <walling@linux.ibm.com> > > [...] > >>>>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json >>>>> index a8d9ec87f5..d151504f25 100644 >>>>> --- a/qapi/machine-target.json >>>>> +++ b/qapi/machine-target.json >>>>> @@ -21,8 +21,12 @@ >>>>> # @props: a dictionary of QOM properties to be applied >>>>> # >>>>> # @deprecated-props: a list of properties that are flagged as deprecated >>>>> -# by the CPU vendor. These props are a subset of the full model's >>>>> -# definition list of properties. (since 9.1) >>>>> +# by the CPU vendor. (since 9.1). >>>>> +# >>>>> +# .. note:: Since 9.1, the list of deprecated props were always a subset >>>>> +# of the model's full-definition list of properites. Now, this list is >>>>> +# populated with the model's enabled property set when delta changes >>>>> +# are applied. All deprecated properties are reported otherwise. >>>> >>>> I'm confused. >>>> >>>> "Since 9.1, the list of deprecated props were ..." and "Now, this list >>>> is" sounds like you're explaining behavior before and after a change. >>>> What change? Since only released behavior matters, and >>>> @deprecated-props is new, there is no old behavior to document, isn't >>>> it? >>> >>> I admittedly had some difficulty articulating the change introduced by >>> this patch. The @deprecated-props array, as well as a way for s390x to >>> populate it, was introduced in release 9.1. Prior to this patch, the >>> deprecated-props list was filtered by the CPU model's full feature set. >>> I attempted to explain this with: >>> "Since 9.1, the list of deprecated props were always a subset of the >>> model's full-definition list of properties." >> >> Version 9.1 has not been released yet (see https://wiki.qemu.org/Planning/9.1), so I agree with Markus, this sounds confusing/wrong to me, too. > > User-visible changes between releases need to be documented in release > notes and the manual. Don't for changes within a release, instead > update documentation to reflect the new state of things. > > Regardless, do explain the change in the commit message. > > Specifics are more helpful than such generalities, so let me try despite > my relative ignorance of the subject matter. > > 1. Update the description of @deprecated-props to match the new code. > Ask yourself what people need to know to use this interface. > > 2. Explain in the commit message how semantics of @deprecated-props > change in this patch. > > Understood, thank you. As per Thomas' comment, I did not do my due diligence to check the QEMU release schedule. My comment does not make sense anyway, since 9.1 is not even released. I will remove it `note` entirely. Apologies for all of this confusion, and I appreciate your patience. I will post v3 with the appropriate corrections.
diff --git a/qapi/machine-target.json b/qapi/machine-target.json index a8d9ec87f5..d151504f25 100644 --- a/qapi/machine-target.json +++ b/qapi/machine-target.json @@ -21,8 +21,12 @@ # @props: a dictionary of QOM properties to be applied # # @deprecated-props: a list of properties that are flagged as deprecated -# by the CPU vendor. These props are a subset of the full model's -# definition list of properties. (since 9.1) +# by the CPU vendor. (since 9.1). +# +# .. note:: Since 9.1, the list of deprecated props were always a subset +# of the model's full-definition list of properites. Now, this list is +# populated with the model's enabled property set when delta changes +# are applied. All deprecated properties are reported otherwise. # # Since: 2.8 ## diff --git a/target/s390x/cpu_models_sysemu.c b/target/s390x/cpu_models_sysemu.c index 977fbc6522..e28ecf7ab9 100644 --- a/target/s390x/cpu_models_sysemu.c +++ b/target/s390x/cpu_models_sysemu.c @@ -174,11 +174,15 @@ static void cpu_info_from_model(CpuModelInfo *info, const S390CPUModel *model, bool delta_changes) { QDict *qdict = qdict_new(); - S390FeatBitmap bitmap; + S390FeatBitmap bitmap, deprecated; /* always fallback to the static base model */ info->name = g_strdup_printf("%s-base", model->def->name); + /* features flagged as deprecated */ + bitmap_zero(deprecated, S390_FEAT_MAX); + s390_get_deprecated_features(deprecated); + if (delta_changes) { /* features deleted from the base feature set */ bitmap_andnot(bitmap, model->def->base_feat, model->features, @@ -193,6 +197,9 @@ static void cpu_info_from_model(CpuModelInfo *info, const S390CPUModel *model, if (!bitmap_empty(bitmap, S390_FEAT_MAX)) { s390_feat_bitmap_to_ascii(bitmap, qdict, qdict_add_enabled_feat); } + + /* deprecated features that are a subset of the model's features */ + bitmap_and(deprecated, deprecated, model->features, S390_FEAT_MAX); } else { /* expand all features */ s390_feat_bitmap_to_ascii(model->features, qdict, @@ -207,12 +214,7 @@ static void cpu_info_from_model(CpuModelInfo *info, const S390CPUModel *model, info->props = QOBJECT(qdict); } - /* features flagged as deprecated */ - bitmap_zero(bitmap, S390_FEAT_MAX); - s390_get_deprecated_features(bitmap); - - bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX); - s390_feat_bitmap_to_ascii(bitmap, &info->deprecated_props, list_add_feat); + s390_feat_bitmap_to_ascii(deprecated, &info->deprecated_props, list_add_feat); info->has_deprecated_props = !!info->deprecated_props; }
As s390 CPU models progress and deprecated properties are dropped outright, it will be cumbersome for management apps to query the host for a comprehensive list of deprecated properties that will need to be disabled on older models. To remedy this, the query-cpu-model-expansion output now behaves by filtering deprecated properties based on the expansion type instead of filtering based off of the model's full set of features: When reporting a static CPU model, only show deprecated properties that are a subset of the model's enabled features. When reporting a full CPU model, show the entire list of deprecated properties regardless if they are supported on the model. Suggested-by: Jiri Denemark <jdenemar@redhat.com> Signed-off-by: Collin Walling <walling@linux.ibm.com> --- Changelog: v2 - Changed commit message - Added documentation reflecting this change - Made code changes that more accurately filter the deprecated properties based on expansion type. This change makes it so that the deprecated-properties reported for a static model expansion are a subset of the model's properties instead of the model's full-definition properties. For example: Previously, the z900 static model would report 'bpb' in the list of deprecated-properties. However, this prop is *not* a part of the model's feature set, leading to some inaccuracy (albeit harmless). Now, this feature will not show during a static expansion. It will, however, show up in a full expansion (along with the rest of the list: 'csske', 'te', 'cte'). @David, I've elected to respectully forgo adding your ack-by on this iteration since I have changed the code (and therefore the behavior) between this version and the previous in case you do not agree with these adjustments. --- qapi/machine-target.json | 8 ++++++-- target/s390x/cpu_models_sysemu.c | 16 +++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-)