1. Patchew
  2. QEMU
  3. [RFC PATCH v2 0/6] Add API for list cpu extensions
  4. View patch

[RFC PATCH v2 5/6] target/riscv: Add defalut value for string property

LIU Zhiwei posted 6 patches 1 year, 3 months ago
Maintainers: Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Laurent Vivier <laurent@vivier.eu>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liweiwei@iscas.ac.cn>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
[RFC PATCH v2 5/6] target/riscv: Add defalut value for string property
Posted by LIU Zhiwei 1 year, 3 months ago 
Before this patch,
"""
qemu-system-riscv64 -device rv64-riscv-cpu,v=true,help

...
vext_spec=<str>
...

"""

After this patch,
"""
vext_spec=<str>        -  (default: "v1.0")
"""

Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
---
 hw/core/qdev-prop-internal.h | 2 ++
 hw/core/qdev-properties.c    | 7 +++++++
 include/hw/qdev-properties.h | 8 ++++++++
 target/riscv/cpu.c           | 2 +-
 4 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev-prop-internal.h b/hw/core/qdev-prop-internal.h
index d7b77844fe..f0613b9757 100644
--- a/hw/core/qdev-prop-internal.h
+++ b/hw/core/qdev-prop-internal.h
@@ -13,6 +13,8 @@ void qdev_propinfo_get_enum(Object *obj, Visitor *v, const char *name,
 void qdev_propinfo_set_enum(Object *obj, Visitor *v, const char *name,
                             void *opaque, Error **errp);
 
+void qdev_propinfo_set_default_value_string(ObjectProperty *op,
+                                            const Property *prop);
 void qdev_propinfo_set_default_value_enum(ObjectProperty *op,
                                           const Property *prop);
 void qdev_propinfo_set_default_value_int(ObjectProperty *op,
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 357b8761b5..64f70a7292 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -96,6 +96,12 @@ static ObjectPropertyAccessor *field_prop_setter(const PropertyInfo *info)
     return info->set ? field_prop_set : NULL;
 }
 
+void qdev_propinfo_set_default_value_string(ObjectProperty *op,
+                                            const Property *prop)
+{
+    object_property_set_default_str(op, prop->defval.p);
+}
+
 void qdev_propinfo_get_enum(Object *obj, Visitor *v, const char *name,
                             void *opaque, Error **errp)
 {
@@ -488,6 +494,7 @@ const PropertyInfo qdev_prop_string = {
     .release = release_string,
     .get   = get_string,
     .set   = set_string,
+    .set_default_value = qdev_propinfo_set_default_value_string,
 };
 
 /* --- on/off/auto --- */
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index e1df08876c..8e5651724a 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -22,6 +22,7 @@ struct Property {
     union {
         int64_t i;
         uint64_t u;
+        void *p;
     } defval;
     int          arrayoffset;
     const PropertyInfo *arrayinfo;
@@ -91,6 +92,11 @@ extern const PropertyInfo qdev_prop_link;
                 .set_default = true,                                       \
                 .defval.u  = (_type)_defval)
 
+#define DEFINE_PROP_STR(_name, _state, _field, _defval, _prop, _type)      \
+    DEFINE_PROP(_name, _state, _field, _prop, _type,                       \
+                .set_default = true,                                       \
+                .defval.p  = (_type)_defval)
+
 #define DEFINE_PROP_UNSIGNED_NODEFAULT(_name, _state, _field, _prop, _type) \
     DEFINE_PROP(_name, _state, _field, _prop, _type)
 
@@ -171,6 +177,8 @@ extern const PropertyInfo qdev_prop_link;
     DEFINE_PROP_UNSIGNED(_n, _s, _f, _d, qdev_prop_size, uint64_t)
 #define DEFINE_PROP_STRING(_n, _s, _f)             \
     DEFINE_PROP(_n, _s, _f, qdev_prop_string, char*)
+#define DEFINE_PROP_STRING_DEF(_n, _s, _f, _d)             \
+    DEFINE_PROP_STR(_n, _s, _f, _d, qdev_prop_string, char*)
 #define DEFINE_PROP_ON_OFF_AUTO(_n, _s, _f, _d) \
     DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_on_off_auto, OnOffAuto)
 #define DEFINE_PROP_SIZE32(_n, _s, _f, _d)                       \
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 38838cd2c0..edcd34e62b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1769,7 +1769,7 @@ static Property riscv_cpu_extensions[] = {
     DEFINE_PROP_BOOL("sstc", RISCVCPU, cfg.ext_sstc, true),
 
     DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
-    DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
+    DEFINE_PROP_STRING_DEF("vext_spec", RISCVCPU, cfg.vext_spec, "v1.0"),
     DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
     DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
 
-- 
2.17.1
Re: [RFC PATCH v2 5/6] target/riscv: Add defalut value for string property
Posted by Daniel Henrique Barboza 1 year, 3 months ago 

On 8/28/23 05:45, LIU Zhiwei wrote:
> Before this patch,
> """
> qemu-system-riscv64 -device rv64-riscv-cpu,v=true,help
> 
> ...
> vext_spec=<str>
> ...
> 
> """
> 
> After this patch,
> """
> vext_spec=<str>        -  (default: "v1.0")
> """
> 
> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---

Code LGTM.

Assuming that we'll need a new API to set default strings in hw/core/qdev-properties.c
(seems likely but we'll need more opinions on that) we'll need to split this patch in
two:

- one patch to add the new qdev_propinfo_set_default_value_string() into
hw/core/qdev-properties.c

- one to change target/riscv to use the newly created API


Thanks,


Daniel



>   hw/core/qdev-prop-internal.h | 2 ++
>   hw/core/qdev-properties.c    | 7 +++++++
>   include/hw/qdev-properties.h | 8 ++++++++
>   target/riscv/cpu.c           | 2 +-
>   4 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/core/qdev-prop-internal.h b/hw/core/qdev-prop-internal.h
> index d7b77844fe..f0613b9757 100644
> --- a/hw/core/qdev-prop-internal.h
> +++ b/hw/core/qdev-prop-internal.h
> @@ -13,6 +13,8 @@ void qdev_propinfo_get_enum(Object *obj, Visitor *v, const char *name,
>   void qdev_propinfo_set_enum(Object *obj, Visitor *v, const char *name,
>                               void *opaque, Error **errp);
>   
> +void qdev_propinfo_set_default_value_string(ObjectProperty *op,
> +                                            const Property *prop);
>   void qdev_propinfo_set_default_value_enum(ObjectProperty *op,
>                                             const Property *prop);
>   void qdev_propinfo_set_default_value_int(ObjectProperty *op,
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 357b8761b5..64f70a7292 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -96,6 +96,12 @@ static ObjectPropertyAccessor *field_prop_setter(const PropertyInfo *info)
>       return info->set ? field_prop_set : NULL;
>   }
>   
> +void qdev_propinfo_set_default_value_string(ObjectProperty *op,
> +                                            const Property *prop)
> +{
> +    object_property_set_default_str(op, prop->defval.p);
> +}
> +
>   void qdev_propinfo_get_enum(Object *obj, Visitor *v, const char *name,
>                               void *opaque, Error **errp)
>   {
> @@ -488,6 +494,7 @@ const PropertyInfo qdev_prop_string = {
>       .release = release_string,
>       .get   = get_string,
>       .set   = set_string,
> +    .set_default_value = qdev_propinfo_set_default_value_string,
>   };
>   
>   /* --- on/off/auto --- */
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index e1df08876c..8e5651724a 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -22,6 +22,7 @@ struct Property {
>       union {
>           int64_t i;
>           uint64_t u;
> +        void *p;
>       } defval;
>       int          arrayoffset;
>       const PropertyInfo *arrayinfo;
> @@ -91,6 +92,11 @@ extern const PropertyInfo qdev_prop_link;
>                   .set_default = true,                                       \
>                   .defval.u  = (_type)_defval)
>   
> +#define DEFINE_PROP_STR(_name, _state, _field, _defval, _prop, _type)      \
> +    DEFINE_PROP(_name, _state, _field, _prop, _type,                       \
> +                .set_default = true,                                       \
> +                .defval.p  = (_type)_defval)
> +
>   #define DEFINE_PROP_UNSIGNED_NODEFAULT(_name, _state, _field, _prop, _type) \
>       DEFINE_PROP(_name, _state, _field, _prop, _type)
>   
> @@ -171,6 +177,8 @@ extern const PropertyInfo qdev_prop_link;
>       DEFINE_PROP_UNSIGNED(_n, _s, _f, _d, qdev_prop_size, uint64_t)
>   #define DEFINE_PROP_STRING(_n, _s, _f)             \
>       DEFINE_PROP(_n, _s, _f, qdev_prop_string, char*)
> +#define DEFINE_PROP_STRING_DEF(_n, _s, _f, _d)             \
> +    DEFINE_PROP_STR(_n, _s, _f, _d, qdev_prop_string, char*)
>   #define DEFINE_PROP_ON_OFF_AUTO(_n, _s, _f, _d) \
>       DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_on_off_auto, OnOffAuto)
>   #define DEFINE_PROP_SIZE32(_n, _s, _f, _d)                       \
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 38838cd2c0..edcd34e62b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1769,7 +1769,7 @@ static Property riscv_cpu_extensions[] = {
>       DEFINE_PROP_BOOL("sstc", RISCVCPU, cfg.ext_sstc, true),
>   
>       DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
> -    DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
> +    DEFINE_PROP_STRING_DEF("vext_spec", RISCVCPU, cfg.vext_spec, "v1.0"),
>       DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>       DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.