Re: [PATCH v1] target/s390x: filter deprecated features based on model expansion type

[Collin Walling]

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!

-- 
Regards,
  Collin