| Message ID | 20240711203254.49018-1-walling@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] target/s390x: filter deprecated features based on model expansion type | expand |
On 11.07.24 22:32, Collin Walling wrote: > It is beneficial to provide an interface to retrieve *all* deprecated > features in one go. Management applications will need this information > to determine which features need to be disabled regardless of the > host-model's capabilities. > > To remedy this, deprecated features are only filtered during a static > expansion. All deperecated features are reported on a full expansion. > > Suggested-by: Jiri Denemark <jdenemar@redhat.com> > Signed-off-by: Collin Walling <walling@linux.ibm.com> > --- > target/s390x/cpu_models_sysemu.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/target/s390x/cpu_models_sysemu.c b/target/s390x/cpu_models_sysemu.c > index 977fbc6522..76d15f2e4d 100644 > --- a/target/s390x/cpu_models_sysemu.c > +++ b/target/s390x/cpu_models_sysemu.c > @@ -211,7 +211,15 @@ static void cpu_info_from_model(CpuModelInfo *info, const S390CPUModel *model, > bitmap_zero(bitmap, S390_FEAT_MAX); > s390_get_deprecated_features(bitmap); > > - bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX); > + /* > + * For static model expansion, filter out deprecated features that are > + * not a subset of the model's feature set. Otherwise, report the entire > + * deprecated features list. > + */ > + if (delta_changes) { > + bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX); > + } > + This would likely be the only interface where we expose "more" features than a CPU model actually understands? I guess it wouldn't be that bad because disabling unsupported features will just work, even if the CPU model is not aware of them. So no strong opinion. Just noting that libvirt cannot really rely on that information because the behavior would change between QEMU versions? Maybe not an issue. Acked-by: David Hildenbrand <david@redhat.com>
Collin Walling <walling@linux.ibm.com> writes: > It is beneficial to provide an interface to retrieve *all* deprecated > features in one go. Management applications will need this information > to determine which features need to be disabled regardless of the > host-model's capabilities. > > To remedy this, deprecated features are only filtered during a static > expansion. All deperecated features are reported on a full expansion. > > Suggested-by: Jiri Denemark <jdenemar@redhat.com> > Signed-off-by: Collin Walling <walling@linux.ibm.com> Which command(s) exactly are affected? Do they need a doc update? > --- > target/s390x/cpu_models_sysemu.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/target/s390x/cpu_models_sysemu.c b/target/s390x/cpu_models_sysemu.c > index 977fbc6522..76d15f2e4d 100644 > --- a/target/s390x/cpu_models_sysemu.c > +++ b/target/s390x/cpu_models_sysemu.c > @@ -211,7 +211,15 @@ static void cpu_info_from_model(CpuModelInfo *info, const S390CPUModel *model, > bitmap_zero(bitmap, S390_FEAT_MAX); > s390_get_deprecated_features(bitmap); > > - bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX); > + /* > + * For static model expansion, filter out deprecated features that are > + * not a subset of the model's feature set. Otherwise, report the entire > + * deprecated features list. > + */ > + if (delta_changes) { > + bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX); > + } > + > s390_feat_bitmap_to_ascii(bitmap, &info->deprecated_props, list_add_feat); > info->has_deprecated_props = !!info->deprecated_props; > }
On 7/11/24 5:10 PM, David Hildenbrand wrote: > On 11.07.24 22:32, Collin Walling wrote: >> It is beneficial to provide an interface to retrieve *all* deprecated >> features in one go. Management applications will need this information >> to determine which features need to be disabled regardless of the >> host-model's capabilities. >> >> To remedy this, deprecated features are only filtered during a static >> expansion. All deperecated features are reported on a full expansion. >> >> Suggested-by: Jiri Denemark <jdenemar@redhat.com> >> Signed-off-by: Collin Walling <walling@linux.ibm.com> >> --- >> target/s390x/cpu_models_sysemu.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/target/s390x/cpu_models_sysemu.c b/target/s390x/cpu_models_sysemu.c >> index 977fbc6522..76d15f2e4d 100644 >> --- a/target/s390x/cpu_models_sysemu.c >> +++ b/target/s390x/cpu_models_sysemu.c >> @@ -211,7 +211,15 @@ static void cpu_info_from_model(CpuModelInfo *info, const S390CPUModel *model, >> bitmap_zero(bitmap, S390_FEAT_MAX); >> s390_get_deprecated_features(bitmap); >> >> - bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX); >> + /* >> + * For static model expansion, filter out deprecated features that are >> + * not a subset of the model's feature set. Otherwise, report the entire >> + * deprecated features list. >> + */ >> + if (delta_changes) { >> + bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX); >> + } >> + > > This would likely be the only interface where we expose "more" features > than a CPU model actually understands? I guess it wouldn't be that bad > because disabling unsupported features will just work, even if the CPU > model is not aware of them. > Yes, and for a full expansion an exhaustive list of features are reported, even the ones that the model does not support (they're paired with "false"). So I think it makes sense to report *all* features that are deprecated as well. > So no strong opinion. > > Just noting that libvirt cannot really rely on that information because > the behavior would change between QEMU versions? Maybe not an issue. > Right. If it's appropriate, I could handle back porting of this patch if requested. But that's not for me to decide :) > Acked-by: David Hildenbrand <david@redhat.com> > Thanks!
On 7/12/24 1:23 AM, Markus Armbruster wrote: > Collin Walling <walling@linux.ibm.com> writes: > >> It is beneficial to provide an interface to retrieve *all* deprecated >> features in one go. Management applications will need this information >> to determine which features need to be disabled regardless of the >> host-model's capabilities. >> >> To remedy this, deprecated features are only filtered during a static >> expansion. All deperecated features are reported on a full expansion. >> >> Suggested-by: Jiri Denemark <jdenemar@redhat.com> >> Signed-off-by: Collin Walling <walling@linux.ibm.com> > > Which command(s) exactly are affected? > The query-cpu-model-expansion result will now report all deprecated features when a user requests a full expansion. The inputs are not affects, but the output is modified. I will make this more concise on the v2 commit message. > Do they need a doc update? > Yes, I forgot to add this. This is what is currently documented: ## # @CpuModelInfo: # ... # # @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) # I will change to: # # @deprecated-props: a list of properties that are flagged as deprecated # by the CPU vendor. These are a subset of the reported @props. # (since 9.1) # (I will also the correct typo in my commit message). [...] Thanks!
Collin Walling <walling@linux.ibm.com> writes: > On 7/12/24 1:23 AM, Markus Armbruster wrote: >> Collin Walling <walling@linux.ibm.com> writes: >> >>> It is beneficial to provide an interface to retrieve *all* deprecated >>> features in one go. Management applications will need this information >>> to determine which features need to be disabled regardless of the >>> host-model's capabilities. >>> >>> To remedy this, deprecated features are only filtered during a static >>> expansion. All deperecated features are reported on a full expansion. >>> >>> Suggested-by: Jiri Denemark <jdenemar@redhat.com> >>> Signed-off-by: Collin Walling <walling@linux.ibm.com> >> >> Which command(s) exactly are affected? >> > > The query-cpu-model-expansion result will now report all deprecated > features when a user requests a full expansion. The inputs are not > affects, but the output is modified. I will make this more concise on > the v2 commit message. Yes, please. Consider including an example. >> Do they need a doc update? >> > > Yes, I forgot to add this. This is what is currently documented: > > ## > # @CpuModelInfo: > # > ... > # > # @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) > # > > I will change to: > > # > # @deprecated-props: a list of properties that are flagged as deprecated > # by the CPU vendor. These are a subset of the reported @props. > # (since 9.1) > # Hasn't made it into a release, so we don't have to document the old behavior. Fortunate! Separate sentences with two spaces for consistency, please. > (I will also the correct typo in my commit message). > > [...] > > Thanks!
diff --git a/target/s390x/cpu_models_sysemu.c b/target/s390x/cpu_models_sysemu.c index 977fbc6522..76d15f2e4d 100644 --- a/target/s390x/cpu_models_sysemu.c +++ b/target/s390x/cpu_models_sysemu.c @@ -211,7 +211,15 @@ static void cpu_info_from_model(CpuModelInfo *info, const S390CPUModel *model, bitmap_zero(bitmap, S390_FEAT_MAX); s390_get_deprecated_features(bitmap); - bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX); + /* + * For static model expansion, filter out deprecated features that are + * not a subset of the model's feature set. Otherwise, report the entire + * deprecated features list. + */ + if (delta_changes) { + bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX); + } + s390_feat_bitmap_to_ascii(bitmap, &info->deprecated_props, list_add_feat); info->has_deprecated_props = !!info->deprecated_props; }
It is beneficial to provide an interface to retrieve *all* deprecated features in one go. Management applications will need this information to determine which features need to be disabled regardless of the host-model's capabilities. To remedy this, deprecated features are only filtered during a static expansion. All deperecated features are reported on a full expansion. Suggested-by: Jiri Denemark <jdenemar@redhat.com> Signed-off-by: Collin Walling <walling@linux.ibm.com> --- target/s390x/cpu_models_sysemu.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)