Re: [RFC 7/9] target/arm: Add CPU features to query-cpu-model-expansion

[Peng Liang]

On 8/13/2020 8:56 PM, Andrew Jones wrote:
> On Thu, Aug 13, 2020 at 06:26:55PM +0800, Peng Liang wrote:
>> Add CPU features to the result of query-cpu-model-expansion so that
>> other applications (such as libvirt) can know the supported CPU
>> features.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Peng Liang <liangpeng10@huawei.com>
>> ---
>>  target/arm/cpu.c     | 41 +++++++++++++++++++++++++++++++++++++++++
>>  target/arm/cpu.h     |  2 ++
>>  target/arm/monitor.c |  2 ++
>>  3 files changed, 45 insertions(+)
>>
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index 3fc54cb3a4..0f620e8afe 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -25,6 +25,8 @@
>>  #include "qemu/module.h"
>>  #include "qapi/error.h"
>>  #include "qapi/visitor.h"
>> +#include "qapi/qmp/qdict.h"
>> +#include "qom/qom-qobject.h"
>>  #include "cpu.h"
>>  #include "internals.h"
>>  #include "exec/exec-all.h"
>> @@ -1515,6 +1517,45 @@ static const CPUFeatureDep feature_dependencies[] = {
>>      },
>>  };
>>  
>> +static char *strtolower(char *src)
>> +{
>> +    char *start = src;
>> +
>> +    for (; *src; ++src) {
>> +        *src = tolower(*src);
>> +    }
>> +
>> +    return start;
>> +}
> 
> Shouldn't need this. The CPU property names should already be lowercase.
> 

For convenience, we use the field part defined in FIELD macro as the name of
a CPU feature.  So, the names of CPU features are upper...

>> +
>> +void arm_cpu_features_to_dict(ARMCPU *cpu, QDict *features)
>> +{
>> +    Object *obj = OBJECT(cpu);
>> +    const char *name;
>> +    ObjectProperty *prop;
>> +    bool is_32bit = !arm_feature(&cpu->env, ARM_FEATURE_AARCH64);
>> +    int i;
>> +
>> +    for (i = 0; i < ARRAY_SIZE(cpu_features); ++i) {
>> +        if (is_32bit != cpu_features[i].is_32bit) {
>> +            continue;
>> +        }
>> +
>> +        name = cpu_features[i].name;
>> +        prop = object_property_find(obj, name, NULL);
>> +        if (prop) {
>> +            QObject *value;
>> +            g_autofree char *tmp;
>> +
>> +            assert(prop->get);
>> +            value = object_property_get_qobject(obj, name, &error_abort);
>> +            tmp = strtolower(g_strdup(name));
>> +
>> +            qdict_put_obj(features, tmp, value);
>> +        }
>> +    }
>> +}
>> +
>>  static void arm_cpu_get_bit_prop(Object *obj, Visitor *v, const char *name,
>>                                   void *opaque, Error **errp)
>>  {
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index 5d8074d03b..da68b7f8f4 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -3933,4 +3933,6 @@ static inline bool isar_feature_any_tts2uxn(const 
>> ARMISARegisters *id)
>>  #define cpu_isar_feature(name, cpu) \
>>      ({ ARMCPU *cpu_ = (cpu); isar_feature_##name(&cpu_->isar); })
>>  
>> +void arm_cpu_features_to_dict(ARMCPU *cpu, QDict *features);
>> +
>>  #endif
>> diff --git a/target/arm/monitor.c b/target/arm/monitor.c
>> index ba6e01abd0..f8eb29efec 100644
>> --- a/target/arm/monitor.c
>> +++ b/target/arm/monitor.c
>> @@ -225,6 +225,8 @@ CpuModelExpansionInfo 
>> *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>>          }
>>      }
>>  
>> +    arm_cpu_features_to_dict(ARM_CPU(obj), qdict_out);
> 
> Since nobody is looking for these features in qdict_in, then none
> of these features can be changed by QMP user. How does the QMP
> user probe whether or not the feature will work when enabled?
> 
> Thanks,
> drew
> 

My fault, I'll correct it.

Thanks,
Peng

>> +
>>      if (!qdict_size(qdict_out)) {
>>          qobject_unref(qdict_out);
>>      } else {
>> -- 
>> 2.18.4
>>
> 
> .
>