diff mbox series

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

Message ID 20200813102657.2588720-8-liangpeng10@huawei.com (mailing list archive)
State New, archived
Headers show
Series Support disable/enable CPU features for AArch64 | expand

Commit Message

Peng Liang Aug. 13, 2020, 10:26 a.m. UTC 
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(+)

Comments

Andrew Jones Aug. 13, 2020, 12:56 p.m. UTC | #1 
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.

> +
> +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

> +
>      if (!qdict_size(qdict_out)) {
>          qobject_unref(qdict_out);
>      } else {
> -- 
> 2.18.4
>
Peng Liang Aug. 15, 2020, 2:19 a.m. UTC | #2 
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
>>
> 
> .
>
Andrew Jones Aug. 15, 2020, 7:02 a.m. UTC | #3 
On Sat, Aug 15, 2020 at 10:19:05AM +0800, Peng Liang wrote:
> 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...

But then the command line requires capital letters for property names,
which isn't very convenient to the user. The field names could be
converted to lowercase when generating the property names.

Thanks,
drew
diff mbox series

Patch

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;
+}
+
+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);
+
     if (!qdict_size(qdict_out)) {
         qobject_unref(qdict_out);
     } else {