* [PATCH] target/riscv: deprecate capital 'Z' CPU properties
@ 2023-10-07 17:14 Daniel Henrique Barboza
2023-10-08 5:10 ` Tsukasa OI
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Daniel Henrique Barboza @ 2023-10-07 17:14 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liweiwei, zhiwei_liu,
palmer, Daniel Henrique Barboza
At this moment there are eleven CPU extension properties that starts
with capital 'Z': Zifencei, Zicsr, Zihintntl, Zihintpause, Zawrs, Zfa,
Zfh, Zfhmin, Zve32f, Zve64f and Zve64d. All other extensions are named
with lower-case letters.
We want all properties to be named with lower-case letters since it's
consistent with the riscv-isa string that we create in the FDT. Having
these 11 properties to be exceptions can be confusing.
Deprecate all of them. Create their lower-case counterpart to be used as
maintained CPU properties. When trying to use any deprecated property a
warning message will be displayed, recommending users to switch to the
lower-case variant:
./build/qemu-system-riscv64 -M virt -cpu rv64,Zifencei=true --nographic
qemu-system-riscv64: warning: CPU property 'Zifencei' is deprecated. Please use 'zifencei' instead
This will give users some time to change their scripts before we remove
the capital 'Z' properties entirely.
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
docs/about/deprecated.rst | 23 ++++++++++++++++++++++
target/riscv/cpu.c | 39 +++++++++++++++++++++++++++-----------
target/riscv/cpu.h | 1 +
target/riscv/tcg/tcg-cpu.c | 31 +++++++++++++++++++++++++++++-
4 files changed, 82 insertions(+), 12 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 694b878f36..331f10f930 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -378,6 +378,29 @@ of generic CPUs: rv32 and rv64 as default CPUs and 'max' as a feature complete
CPU for both 32 and 64 bit builds. Users are then discouraged to use the 'any'
CPU type starting in 8.2.
+RISC-V CPU properties which start with with capital 'Z' (since 8.2)
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+All RISC-V CPU properties which start with capital 'Z' are being deprecated
+starting in 8.2. The reason is that they were wrongly added with capital 'Z'
+in the past. CPU properties were later added with lower-case names, which
+is the format we want to use from now on.
+
+Users which try to use these deprecated properties will receive a warning
+recommending to switch to their stable counterparts:
+
+- "Zifencei" should be replaced with "zifencei"
+- "Zicsr" should be replaced with "zicsr"
+- "Zihintntl" should be replaced with "zihintntl"
+- "Zihintpause" should be replaced with "zihintpause"
+- "Zawrs" should be replaced with "zawrs"
+- "Zfa" should be replaced with "zfa"
+- "Zfh" should be replaced with "zfh"
+- "Zfhmin" should be replaced with "zfhmin"
+- "Zve32f" should be replaced with "zve32f"
+- "Zve64f" should be replaced with "zve64f"
+- "Zve64d" should be replaced with "zve64d"
+
Block device options
''''''''''''''''''''
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 521bb88538..1cdc3d2609 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1246,17 +1246,17 @@ const char *riscv_get_misa_ext_description(uint32_t bit)
const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
/* Defaults for standard extensions */
MULTI_EXT_CFG_BOOL("sscofpmf", ext_sscofpmf, false),
- MULTI_EXT_CFG_BOOL("Zifencei", ext_ifencei, true),
- MULTI_EXT_CFG_BOOL("Zicsr", ext_icsr, true),
- MULTI_EXT_CFG_BOOL("Zihintntl", ext_zihintntl, true),
- MULTI_EXT_CFG_BOOL("Zihintpause", ext_zihintpause, true),
- MULTI_EXT_CFG_BOOL("Zawrs", ext_zawrs, true),
- MULTI_EXT_CFG_BOOL("Zfa", ext_zfa, true),
- MULTI_EXT_CFG_BOOL("Zfh", ext_zfh, false),
- MULTI_EXT_CFG_BOOL("Zfhmin", ext_zfhmin, false),
- MULTI_EXT_CFG_BOOL("Zve32f", ext_zve32f, false),
- MULTI_EXT_CFG_BOOL("Zve64f", ext_zve64f, false),
- MULTI_EXT_CFG_BOOL("Zve64d", ext_zve64d, false),
+ MULTI_EXT_CFG_BOOL("zifencei", ext_ifencei, true),
+ MULTI_EXT_CFG_BOOL("zicsr", ext_icsr, true),
+ MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
+ MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true),
+ MULTI_EXT_CFG_BOOL("zawrs", ext_zawrs, true),
+ MULTI_EXT_CFG_BOOL("zfa", ext_zfa, true),
+ MULTI_EXT_CFG_BOOL("zfh", ext_zfh, false),
+ MULTI_EXT_CFG_BOOL("zfhmin", ext_zfhmin, false),
+ MULTI_EXT_CFG_BOOL("zve32f", ext_zve32f, false),
+ MULTI_EXT_CFG_BOOL("zve64f", ext_zve64f, false),
+ MULTI_EXT_CFG_BOOL("zve64d", ext_zve64d, false),
MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
@@ -1349,6 +1349,23 @@ const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[] = {
DEFINE_PROP_END_OF_LIST(),
};
+/* Deprecated entries marked for future removal */
+const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[] = {
+ MULTI_EXT_CFG_BOOL("Zifencei", ext_ifencei, true),
+ MULTI_EXT_CFG_BOOL("Zicsr", ext_icsr, true),
+ MULTI_EXT_CFG_BOOL("Zihintntl", ext_zihintntl, true),
+ MULTI_EXT_CFG_BOOL("Zihintpause", ext_zihintpause, true),
+ MULTI_EXT_CFG_BOOL("Zawrs", ext_zawrs, true),
+ MULTI_EXT_CFG_BOOL("Zfa", ext_zfa, true),
+ MULTI_EXT_CFG_BOOL("Zfh", ext_zfh, false),
+ MULTI_EXT_CFG_BOOL("Zfhmin", ext_zfhmin, false),
+ MULTI_EXT_CFG_BOOL("Zve32f", ext_zve32f, false),
+ MULTI_EXT_CFG_BOOL("Zve64f", ext_zve64f, false),
+ MULTI_EXT_CFG_BOOL("Zve64d", ext_zve64d, false),
+
+ DEFINE_PROP_END_OF_LIST(),
+};
+
Property riscv_cpu_options[] = {
DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 3f11e69223..e98f5de32e 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -722,6 +722,7 @@ typedef struct RISCVCPUMultiExtConfig {
extern const RISCVCPUMultiExtConfig riscv_cpu_extensions[];
extern const RISCVCPUMultiExtConfig riscv_cpu_vendor_exts[];
extern const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[];
+extern const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[];
extern Property riscv_cpu_options[];
typedef struct isa_ext_data {
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 08b806dc07..00676593f7 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -732,6 +732,25 @@ static void riscv_cpu_add_misa_properties(Object *cpu_obj)
}
}
+static bool cpu_ext_is_deprecated(const char *ext_name)
+{
+ return isupper(ext_name[0]);
+}
+
+/*
+ * String will be allocated in the heap. Caller is responsible
+ * for freeing it.
+ */
+static char *cpu_ext_to_lower(const char *ext_name)
+{
+ char *ret = g_malloc0(strlen(ext_name) + 1);
+
+ strcpy(ret, ext_name);
+ ret[0] = tolower(ret[0]);
+
+ return ret;
+}
+
static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
@@ -744,6 +763,13 @@ static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
return;
}
+ if (cpu_ext_is_deprecated(multi_ext_cfg->name)) {
+ g_autofree char *lower = cpu_ext_to_lower(multi_ext_cfg->name);
+
+ warn_report("CPU property '%s' is deprecated. Please use '%s' instead",
+ multi_ext_cfg->name, lower);
+ }
+
g_hash_table_insert(multi_ext_user_opts,
GUINT_TO_POINTER(multi_ext_cfg->offset),
(gpointer)value);
@@ -777,13 +803,14 @@ static void cpu_add_multi_ext_prop(Object *cpu_obj,
const RISCVCPUMultiExtConfig *multi_cfg)
{
bool generic_cpu = riscv_cpu_is_generic(cpu_obj);
+ bool deprecated_ext = cpu_ext_is_deprecated(multi_cfg->name);
object_property_add(cpu_obj, multi_cfg->name, "bool",
cpu_get_multi_ext_cfg,
cpu_set_multi_ext_cfg,
NULL, (void *)multi_cfg);
- if (!generic_cpu) {
+ if (!generic_cpu || deprecated_ext) {
return;
}
@@ -826,6 +853,8 @@ static void riscv_cpu_add_user_properties(Object *obj)
riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_vendor_exts);
riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_experimental_exts);
+ riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_deprecated_exts);
+
for (Property *prop = riscv_cpu_options; prop && prop->name; prop++) {
qdev_property_add_static(DEVICE(obj), prop);
}
--
2.41.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: deprecate capital 'Z' CPU properties
2023-10-07 17:14 [PATCH] target/riscv: deprecate capital 'Z' CPU properties Daniel Henrique Barboza
@ 2023-10-08 5:10 ` Tsukasa OI
2023-10-09 1:34 ` Alistair Francis
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Tsukasa OI @ 2023-10-08 5:10 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: open list:RISC-V, qemu-devel@nongnu.org Developers
That's great!
I submitted a patch set to deal with the exact problem:
<https://lists.gnu.org/archive/html/qemu-riscv/2022-05/msg00227.html>
<https://lists.gnu.org/archive/html/qemu-riscv/2022-05/msg00411.html>
But this one is simpler than mine (and also fits to the latest QEMU).
I support this patch set and want it to be merged.
Thanks,
Tsukasa
On 2023/10/08 2:14, Daniel Henrique Barboza wrote:
> At this moment there are eleven CPU extension properties that starts
> with capital 'Z': Zifencei, Zicsr, Zihintntl, Zihintpause, Zawrs, Zfa,
> Zfh, Zfhmin, Zve32f, Zve64f and Zve64d. All other extensions are named
> with lower-case letters.
>
> We want all properties to be named with lower-case letters since it's
> consistent with the riscv-isa string that we create in the FDT. Having
> these 11 properties to be exceptions can be confusing.
>
> Deprecate all of them. Create their lower-case counterpart to be used as
> maintained CPU properties. When trying to use any deprecated property a
> warning message will be displayed, recommending users to switch to the
> lower-case variant:
>
> ./build/qemu-system-riscv64 -M virt -cpu rv64,Zifencei=true --nographic
> qemu-system-riscv64: warning: CPU property 'Zifencei' is deprecated. Please use 'zifencei' instead
>
> This will give users some time to change their scripts before we remove
> the capital 'Z' properties entirely.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> docs/about/deprecated.rst | 23 ++++++++++++++++++++++
> target/riscv/cpu.c | 39 +++++++++++++++++++++++++++-----------
> target/riscv/cpu.h | 1 +
> target/riscv/tcg/tcg-cpu.c | 31 +++++++++++++++++++++++++++++-
> 4 files changed, 82 insertions(+), 12 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 694b878f36..331f10f930 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -378,6 +378,29 @@ of generic CPUs: rv32 and rv64 as default CPUs and 'max' as a feature complete
> CPU for both 32 and 64 bit builds. Users are then discouraged to use the 'any'
> CPU type starting in 8.2.
>
> +RISC-V CPU properties which start with with capital 'Z' (since 8.2)
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +All RISC-V CPU properties which start with capital 'Z' are being deprecated
> +starting in 8.2. The reason is that they were wrongly added with capital 'Z'
> +in the past. CPU properties were later added with lower-case names, which
> +is the format we want to use from now on.
> +
> +Users which try to use these deprecated properties will receive a warning
> +recommending to switch to their stable counterparts:
> +
> +- "Zifencei" should be replaced with "zifencei"
> +- "Zicsr" should be replaced with "zicsr"
> +- "Zihintntl" should be replaced with "zihintntl"
> +- "Zihintpause" should be replaced with "zihintpause"
> +- "Zawrs" should be replaced with "zawrs"
> +- "Zfa" should be replaced with "zfa"
> +- "Zfh" should be replaced with "zfh"
> +- "Zfhmin" should be replaced with "zfhmin"
> +- "Zve32f" should be replaced with "zve32f"
> +- "Zve64f" should be replaced with "zve64f"
> +- "Zve64d" should be replaced with "zve64d"
> +
> Block device options
> ''''''''''''''''''''
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 521bb88538..1cdc3d2609 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1246,17 +1246,17 @@ const char *riscv_get_misa_ext_description(uint32_t bit)
> const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
> /* Defaults for standard extensions */
> MULTI_EXT_CFG_BOOL("sscofpmf", ext_sscofpmf, false),
> - MULTI_EXT_CFG_BOOL("Zifencei", ext_ifencei, true),
> - MULTI_EXT_CFG_BOOL("Zicsr", ext_icsr, true),
> - MULTI_EXT_CFG_BOOL("Zihintntl", ext_zihintntl, true),
> - MULTI_EXT_CFG_BOOL("Zihintpause", ext_zihintpause, true),
> - MULTI_EXT_CFG_BOOL("Zawrs", ext_zawrs, true),
> - MULTI_EXT_CFG_BOOL("Zfa", ext_zfa, true),
> - MULTI_EXT_CFG_BOOL("Zfh", ext_zfh, false),
> - MULTI_EXT_CFG_BOOL("Zfhmin", ext_zfhmin, false),
> - MULTI_EXT_CFG_BOOL("Zve32f", ext_zve32f, false),
> - MULTI_EXT_CFG_BOOL("Zve64f", ext_zve64f, false),
> - MULTI_EXT_CFG_BOOL("Zve64d", ext_zve64d, false),
> + MULTI_EXT_CFG_BOOL("zifencei", ext_ifencei, true),
> + MULTI_EXT_CFG_BOOL("zicsr", ext_icsr, true),
> + MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
> + MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true),
> + MULTI_EXT_CFG_BOOL("zawrs", ext_zawrs, true),
> + MULTI_EXT_CFG_BOOL("zfa", ext_zfa, true),
> + MULTI_EXT_CFG_BOOL("zfh", ext_zfh, false),
> + MULTI_EXT_CFG_BOOL("zfhmin", ext_zfhmin, false),
> + MULTI_EXT_CFG_BOOL("zve32f", ext_zve32f, false),
> + MULTI_EXT_CFG_BOOL("zve64f", ext_zve64f, false),
> + MULTI_EXT_CFG_BOOL("zve64d", ext_zve64d, false),
> MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
>
> MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
> @@ -1349,6 +1349,23 @@ const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
> +/* Deprecated entries marked for future removal */
> +const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[] = {
> + MULTI_EXT_CFG_BOOL("Zifencei", ext_ifencei, true),
> + MULTI_EXT_CFG_BOOL("Zicsr", ext_icsr, true),
> + MULTI_EXT_CFG_BOOL("Zihintntl", ext_zihintntl, true),
> + MULTI_EXT_CFG_BOOL("Zihintpause", ext_zihintpause, true),
> + MULTI_EXT_CFG_BOOL("Zawrs", ext_zawrs, true),
> + MULTI_EXT_CFG_BOOL("Zfa", ext_zfa, true),
> + MULTI_EXT_CFG_BOOL("Zfh", ext_zfh, false),
> + MULTI_EXT_CFG_BOOL("Zfhmin", ext_zfhmin, false),
> + MULTI_EXT_CFG_BOOL("Zve32f", ext_zve32f, false),
> + MULTI_EXT_CFG_BOOL("Zve64f", ext_zve64f, false),
> + MULTI_EXT_CFG_BOOL("Zve64d", ext_zve64d, false),
> +
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> Property riscv_cpu_options[] = {
> DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 3f11e69223..e98f5de32e 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -722,6 +722,7 @@ typedef struct RISCVCPUMultiExtConfig {
> extern const RISCVCPUMultiExtConfig riscv_cpu_extensions[];
> extern const RISCVCPUMultiExtConfig riscv_cpu_vendor_exts[];
> extern const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[];
> +extern const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[];
> extern Property riscv_cpu_options[];
>
> typedef struct isa_ext_data {
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 08b806dc07..00676593f7 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -732,6 +732,25 @@ static void riscv_cpu_add_misa_properties(Object *cpu_obj)
> }
> }
>
> +static bool cpu_ext_is_deprecated(const char *ext_name)
> +{
> + return isupper(ext_name[0]);
> +}
> +
> +/*
> + * String will be allocated in the heap. Caller is responsible
> + * for freeing it.
> + */
> +static char *cpu_ext_to_lower(const char *ext_name)
> +{
> + char *ret = g_malloc0(strlen(ext_name) + 1);
> +
> + strcpy(ret, ext_name);
> + ret[0] = tolower(ret[0]);
> +
> + return ret;
> +}
> +
> static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> @@ -744,6 +763,13 @@ static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
> return;
> }
>
> + if (cpu_ext_is_deprecated(multi_ext_cfg->name)) {
> + g_autofree char *lower = cpu_ext_to_lower(multi_ext_cfg->name);
> +
> + warn_report("CPU property '%s' is deprecated. Please use '%s' instead",
> + multi_ext_cfg->name, lower);
> + }
> +
> g_hash_table_insert(multi_ext_user_opts,
> GUINT_TO_POINTER(multi_ext_cfg->offset),
> (gpointer)value);
> @@ -777,13 +803,14 @@ static void cpu_add_multi_ext_prop(Object *cpu_obj,
> const RISCVCPUMultiExtConfig *multi_cfg)
> {
> bool generic_cpu = riscv_cpu_is_generic(cpu_obj);
> + bool deprecated_ext = cpu_ext_is_deprecated(multi_cfg->name);
>
> object_property_add(cpu_obj, multi_cfg->name, "bool",
> cpu_get_multi_ext_cfg,
> cpu_set_multi_ext_cfg,
> NULL, (void *)multi_cfg);
>
> - if (!generic_cpu) {
> + if (!generic_cpu || deprecated_ext) {
> return;
> }
>
> @@ -826,6 +853,8 @@ static void riscv_cpu_add_user_properties(Object *obj)
> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_vendor_exts);
> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_experimental_exts);
>
> + riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_deprecated_exts);
> +
> for (Property *prop = riscv_cpu_options; prop && prop->name; prop++) {
> qdev_property_add_static(DEVICE(obj), prop);
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: deprecate capital 'Z' CPU properties
2023-10-07 17:14 [PATCH] target/riscv: deprecate capital 'Z' CPU properties Daniel Henrique Barboza
2023-10-08 5:10 ` Tsukasa OI
@ 2023-10-09 1:34 ` Alistair Francis
2023-10-09 1:39 ` Alistair Francis
2023-10-09 7:53 ` Andrew Jones
3 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2023-10-09 1:34 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liweiwei,
zhiwei_liu, palmer
On Sun, Oct 8, 2023 at 3:15 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> At this moment there are eleven CPU extension properties that starts
> with capital 'Z': Zifencei, Zicsr, Zihintntl, Zihintpause, Zawrs, Zfa,
> Zfh, Zfhmin, Zve32f, Zve64f and Zve64d. All other extensions are named
> with lower-case letters.
>
> We want all properties to be named with lower-case letters since it's
> consistent with the riscv-isa string that we create in the FDT. Having
> these 11 properties to be exceptions can be confusing.
>
> Deprecate all of them. Create their lower-case counterpart to be used as
> maintained CPU properties. When trying to use any deprecated property a
> warning message will be displayed, recommending users to switch to the
> lower-case variant:
>
> ./build/qemu-system-riscv64 -M virt -cpu rv64,Zifencei=true --nographic
> qemu-system-riscv64: warning: CPU property 'Zifencei' is deprecated. Please use 'zifencei' instead
>
> This will give users some time to change their scripts before we remove
> the capital 'Z' properties entirely.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> docs/about/deprecated.rst | 23 ++++++++++++++++++++++
> target/riscv/cpu.c | 39 +++++++++++++++++++++++++++-----------
> target/riscv/cpu.h | 1 +
> target/riscv/tcg/tcg-cpu.c | 31 +++++++++++++++++++++++++++++-
> 4 files changed, 82 insertions(+), 12 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 694b878f36..331f10f930 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -378,6 +378,29 @@ of generic CPUs: rv32 and rv64 as default CPUs and 'max' as a feature complete
> CPU for both 32 and 64 bit builds. Users are then discouraged to use the 'any'
> CPU type starting in 8.2.
>
> +RISC-V CPU properties which start with with capital 'Z' (since 8.2)
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +All RISC-V CPU properties which start with capital 'Z' are being deprecated
> +starting in 8.2. The reason is that they were wrongly added with capital 'Z'
> +in the past. CPU properties were later added with lower-case names, which
> +is the format we want to use from now on.
> +
> +Users which try to use these deprecated properties will receive a warning
> +recommending to switch to their stable counterparts:
> +
> +- "Zifencei" should be replaced with "zifencei"
> +- "Zicsr" should be replaced with "zicsr"
> +- "Zihintntl" should be replaced with "zihintntl"
> +- "Zihintpause" should be replaced with "zihintpause"
> +- "Zawrs" should be replaced with "zawrs"
> +- "Zfa" should be replaced with "zfa"
> +- "Zfh" should be replaced with "zfh"
> +- "Zfhmin" should be replaced with "zfhmin"
> +- "Zve32f" should be replaced with "zve32f"
> +- "Zve64f" should be replaced with "zve64f"
> +- "Zve64d" should be replaced with "zve64d"
> +
> Block device options
> ''''''''''''''''''''
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 521bb88538..1cdc3d2609 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1246,17 +1246,17 @@ const char *riscv_get_misa_ext_description(uint32_t bit)
> const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
> /* Defaults for standard extensions */
> MULTI_EXT_CFG_BOOL("sscofpmf", ext_sscofpmf, false),
> - MULTI_EXT_CFG_BOOL("Zifencei", ext_ifencei, true),
> - MULTI_EXT_CFG_BOOL("Zicsr", ext_icsr, true),
> - MULTI_EXT_CFG_BOOL("Zihintntl", ext_zihintntl, true),
> - MULTI_EXT_CFG_BOOL("Zihintpause", ext_zihintpause, true),
> - MULTI_EXT_CFG_BOOL("Zawrs", ext_zawrs, true),
> - MULTI_EXT_CFG_BOOL("Zfa", ext_zfa, true),
> - MULTI_EXT_CFG_BOOL("Zfh", ext_zfh, false),
> - MULTI_EXT_CFG_BOOL("Zfhmin", ext_zfhmin, false),
> - MULTI_EXT_CFG_BOOL("Zve32f", ext_zve32f, false),
> - MULTI_EXT_CFG_BOOL("Zve64f", ext_zve64f, false),
> - MULTI_EXT_CFG_BOOL("Zve64d", ext_zve64d, false),
> + MULTI_EXT_CFG_BOOL("zifencei", ext_ifencei, true),
> + MULTI_EXT_CFG_BOOL("zicsr", ext_icsr, true),
> + MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
> + MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true),
> + MULTI_EXT_CFG_BOOL("zawrs", ext_zawrs, true),
> + MULTI_EXT_CFG_BOOL("zfa", ext_zfa, true),
> + MULTI_EXT_CFG_BOOL("zfh", ext_zfh, false),
> + MULTI_EXT_CFG_BOOL("zfhmin", ext_zfhmin, false),
> + MULTI_EXT_CFG_BOOL("zve32f", ext_zve32f, false),
> + MULTI_EXT_CFG_BOOL("zve64f", ext_zve64f, false),
> + MULTI_EXT_CFG_BOOL("zve64d", ext_zve64d, false),
> MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
>
> MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
> @@ -1349,6 +1349,23 @@ const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
> +/* Deprecated entries marked for future removal */
> +const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[] = {
> + MULTI_EXT_CFG_BOOL("Zifencei", ext_ifencei, true),
> + MULTI_EXT_CFG_BOOL("Zicsr", ext_icsr, true),
> + MULTI_EXT_CFG_BOOL("Zihintntl", ext_zihintntl, true),
> + MULTI_EXT_CFG_BOOL("Zihintpause", ext_zihintpause, true),
> + MULTI_EXT_CFG_BOOL("Zawrs", ext_zawrs, true),
> + MULTI_EXT_CFG_BOOL("Zfa", ext_zfa, true),
> + MULTI_EXT_CFG_BOOL("Zfh", ext_zfh, false),
> + MULTI_EXT_CFG_BOOL("Zfhmin", ext_zfhmin, false),
> + MULTI_EXT_CFG_BOOL("Zve32f", ext_zve32f, false),
> + MULTI_EXT_CFG_BOOL("Zve64f", ext_zve64f, false),
> + MULTI_EXT_CFG_BOOL("Zve64d", ext_zve64d, false),
> +
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> Property riscv_cpu_options[] = {
> DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 3f11e69223..e98f5de32e 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -722,6 +722,7 @@ typedef struct RISCVCPUMultiExtConfig {
> extern const RISCVCPUMultiExtConfig riscv_cpu_extensions[];
> extern const RISCVCPUMultiExtConfig riscv_cpu_vendor_exts[];
> extern const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[];
> +extern const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[];
> extern Property riscv_cpu_options[];
>
> typedef struct isa_ext_data {
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 08b806dc07..00676593f7 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -732,6 +732,25 @@ static void riscv_cpu_add_misa_properties(Object *cpu_obj)
> }
> }
>
> +static bool cpu_ext_is_deprecated(const char *ext_name)
> +{
> + return isupper(ext_name[0]);
> +}
> +
> +/*
> + * String will be allocated in the heap. Caller is responsible
> + * for freeing it.
> + */
> +static char *cpu_ext_to_lower(const char *ext_name)
> +{
> + char *ret = g_malloc0(strlen(ext_name) + 1);
> +
> + strcpy(ret, ext_name);
> + ret[0] = tolower(ret[0]);
> +
> + return ret;
> +}
> +
> static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> @@ -744,6 +763,13 @@ static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
> return;
> }
>
> + if (cpu_ext_is_deprecated(multi_ext_cfg->name)) {
> + g_autofree char *lower = cpu_ext_to_lower(multi_ext_cfg->name);
> +
> + warn_report("CPU property '%s' is deprecated. Please use '%s' instead",
> + multi_ext_cfg->name, lower);
> + }
> +
> g_hash_table_insert(multi_ext_user_opts,
> GUINT_TO_POINTER(multi_ext_cfg->offset),
> (gpointer)value);
> @@ -777,13 +803,14 @@ static void cpu_add_multi_ext_prop(Object *cpu_obj,
> const RISCVCPUMultiExtConfig *multi_cfg)
> {
> bool generic_cpu = riscv_cpu_is_generic(cpu_obj);
> + bool deprecated_ext = cpu_ext_is_deprecated(multi_cfg->name);
>
> object_property_add(cpu_obj, multi_cfg->name, "bool",
> cpu_get_multi_ext_cfg,
> cpu_set_multi_ext_cfg,
> NULL, (void *)multi_cfg);
>
> - if (!generic_cpu) {
> + if (!generic_cpu || deprecated_ext) {
> return;
> }
>
> @@ -826,6 +853,8 @@ static void riscv_cpu_add_user_properties(Object *obj)
> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_vendor_exts);
> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_experimental_exts);
>
> + riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_deprecated_exts);
> +
> for (Property *prop = riscv_cpu_options; prop && prop->name; prop++) {
> qdev_property_add_static(DEVICE(obj), prop);
> }
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: deprecate capital 'Z' CPU properties
2023-10-07 17:14 [PATCH] target/riscv: deprecate capital 'Z' CPU properties Daniel Henrique Barboza
2023-10-08 5:10 ` Tsukasa OI
2023-10-09 1:34 ` Alistair Francis
@ 2023-10-09 1:39 ` Alistair Francis
2023-10-09 11:30 ` Daniel Henrique Barboza
2023-10-09 7:53 ` Andrew Jones
3 siblings, 1 reply; 6+ messages in thread
From: Alistair Francis @ 2023-10-09 1:39 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liweiwei,
zhiwei_liu, palmer
On Sun, Oct 8, 2023 at 3:15 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> At this moment there are eleven CPU extension properties that starts
> with capital 'Z': Zifencei, Zicsr, Zihintntl, Zihintpause, Zawrs, Zfa,
> Zfh, Zfhmin, Zve32f, Zve64f and Zve64d. All other extensions are named
> with lower-case letters.
>
> We want all properties to be named with lower-case letters since it's
> consistent with the riscv-isa string that we create in the FDT. Having
> these 11 properties to be exceptions can be confusing.
>
> Deprecate all of them. Create their lower-case counterpart to be used as
> maintained CPU properties. When trying to use any deprecated property a
> warning message will be displayed, recommending users to switch to the
> lower-case variant:
>
> ./build/qemu-system-riscv64 -M virt -cpu rv64,Zifencei=true --nographic
> qemu-system-riscv64: warning: CPU property 'Zifencei' is deprecated. Please use 'zifencei' instead
>
> This will give users some time to change their scripts before we remove
> the capital 'Z' properties entirely.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> docs/about/deprecated.rst | 23 ++++++++++++++++++++++
> target/riscv/cpu.c | 39 +++++++++++++++++++++++++++-----------
> target/riscv/cpu.h | 1 +
> target/riscv/tcg/tcg-cpu.c | 31 +++++++++++++++++++++++++++++-
> 4 files changed, 82 insertions(+), 12 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 694b878f36..331f10f930 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -378,6 +378,29 @@ of generic CPUs: rv32 and rv64 as default CPUs and 'max' as a feature complete
> CPU for both 32 and 64 bit builds. Users are then discouraged to use the 'any'
> CPU type starting in 8.2.
>
> +RISC-V CPU properties which start with with capital 'Z' (since 8.2)
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +All RISC-V CPU properties which start with capital 'Z' are being deprecated
> +starting in 8.2. The reason is that they were wrongly added with capital 'Z'
> +in the past. CPU properties were later added with lower-case names, which
> +is the format we want to use from now on.
> +
> +Users which try to use these deprecated properties will receive a warning
> +recommending to switch to their stable counterparts:
> +
> +- "Zifencei" should be replaced with "zifencei"
> +- "Zicsr" should be replaced with "zicsr"
> +- "Zihintntl" should be replaced with "zihintntl"
> +- "Zihintpause" should be replaced with "zihintpause"
> +- "Zawrs" should be replaced with "zawrs"
> +- "Zfa" should be replaced with "zfa"
> +- "Zfh" should be replaced with "zfh"
> +- "Zfhmin" should be replaced with "zfhmin"
> +- "Zve32f" should be replaced with "zve32f"
> +- "Zve64f" should be replaced with "zve64f"
> +- "Zve64d" should be replaced with "zve64d"
> +
> Block device options
> ''''''''''''''''''''
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 521bb88538..1cdc3d2609 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1246,17 +1246,17 @@ const char *riscv_get_misa_ext_description(uint32_t bit)
> const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
> /* Defaults for standard extensions */
> MULTI_EXT_CFG_BOOL("sscofpmf", ext_sscofpmf, false),
> - MULTI_EXT_CFG_BOOL("Zifencei", ext_ifencei, true),
> - MULTI_EXT_CFG_BOOL("Zicsr", ext_icsr, true),
> - MULTI_EXT_CFG_BOOL("Zihintntl", ext_zihintntl, true),
> - MULTI_EXT_CFG_BOOL("Zihintpause", ext_zihintpause, true),
> - MULTI_EXT_CFG_BOOL("Zawrs", ext_zawrs, true),
> - MULTI_EXT_CFG_BOOL("Zfa", ext_zfa, true),
> - MULTI_EXT_CFG_BOOL("Zfh", ext_zfh, false),
> - MULTI_EXT_CFG_BOOL("Zfhmin", ext_zfhmin, false),
> - MULTI_EXT_CFG_BOOL("Zve32f", ext_zve32f, false),
> - MULTI_EXT_CFG_BOOL("Zve64f", ext_zve64f, false),
> - MULTI_EXT_CFG_BOOL("Zve64d", ext_zve64d, false),
> + MULTI_EXT_CFG_BOOL("zifencei", ext_ifencei, true),
> + MULTI_EXT_CFG_BOOL("zicsr", ext_icsr, true),
> + MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
> + MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true),
> + MULTI_EXT_CFG_BOOL("zawrs", ext_zawrs, true),
> + MULTI_EXT_CFG_BOOL("zfa", ext_zfa, true),
> + MULTI_EXT_CFG_BOOL("zfh", ext_zfh, false),
> + MULTI_EXT_CFG_BOOL("zfhmin", ext_zfhmin, false),
> + MULTI_EXT_CFG_BOOL("zve32f", ext_zve32f, false),
> + MULTI_EXT_CFG_BOOL("zve64f", ext_zve64f, false),
> + MULTI_EXT_CFG_BOOL("zve64d", ext_zve64d, false),
> MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
>
> MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
> @@ -1349,6 +1349,23 @@ const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
> +/* Deprecated entries marked for future removal */
> +const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[] = {
> + MULTI_EXT_CFG_BOOL("Zifencei", ext_ifencei, true),
> + MULTI_EXT_CFG_BOOL("Zicsr", ext_icsr, true),
> + MULTI_EXT_CFG_BOOL("Zihintntl", ext_zihintntl, true),
> + MULTI_EXT_CFG_BOOL("Zihintpause", ext_zihintpause, true),
> + MULTI_EXT_CFG_BOOL("Zawrs", ext_zawrs, true),
> + MULTI_EXT_CFG_BOOL("Zfa", ext_zfa, true),
> + MULTI_EXT_CFG_BOOL("Zfh", ext_zfh, false),
> + MULTI_EXT_CFG_BOOL("Zfhmin", ext_zfhmin, false),
> + MULTI_EXT_CFG_BOOL("Zve32f", ext_zve32f, false),
> + MULTI_EXT_CFG_BOOL("Zve64f", ext_zve64f, false),
> + MULTI_EXT_CFG_BOOL("Zve64d", ext_zve64d, false),
> +
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> Property riscv_cpu_options[] = {
> DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 3f11e69223..e98f5de32e 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -722,6 +722,7 @@ typedef struct RISCVCPUMultiExtConfig {
> extern const RISCVCPUMultiExtConfig riscv_cpu_extensions[];
> extern const RISCVCPUMultiExtConfig riscv_cpu_vendor_exts[];
> extern const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[];
> +extern const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[];
> extern Property riscv_cpu_options[];
>
> typedef struct isa_ext_data {
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 08b806dc07..00676593f7 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -732,6 +732,25 @@ static void riscv_cpu_add_misa_properties(Object *cpu_obj)
> }
> }
>
> +static bool cpu_ext_is_deprecated(const char *ext_name)
> +{
> + return isupper(ext_name[0]);
> +}
> +
> +/*
> + * String will be allocated in the heap. Caller is responsible
> + * for freeing it.
> + */
> +static char *cpu_ext_to_lower(const char *ext_name)
> +{
> + char *ret = g_malloc0(strlen(ext_name) + 1);
> +
> + strcpy(ret, ext_name);
> + ret[0] = tolower(ret[0]);
> +
> + return ret;
> +}
> +
> static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> @@ -744,6 +763,13 @@ static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
> return;
> }
>
> + if (cpu_ext_is_deprecated(multi_ext_cfg->name)) {
> + g_autofree char *lower = cpu_ext_to_lower(multi_ext_cfg->name);
> +
> + warn_report("CPU property '%s' is deprecated. Please use '%s' instead",
> + multi_ext_cfg->name, lower);
> + }
> +
> g_hash_table_insert(multi_ext_user_opts,
> GUINT_TO_POINTER(multi_ext_cfg->offset),
> (gpointer)value);
> @@ -777,13 +803,14 @@ static void cpu_add_multi_ext_prop(Object *cpu_obj,
> const RISCVCPUMultiExtConfig *multi_cfg)
> {
> bool generic_cpu = riscv_cpu_is_generic(cpu_obj);
> + bool deprecated_ext = cpu_ext_is_deprecated(multi_cfg->name);
>
> object_property_add(cpu_obj, multi_cfg->name, "bool",
> cpu_get_multi_ext_cfg,
> cpu_set_multi_ext_cfg,
> NULL, (void *)multi_cfg);
>
> - if (!generic_cpu) {
> + if (!generic_cpu || deprecated_ext) {
> return;
> }
>
> @@ -826,6 +853,8 @@ static void riscv_cpu_add_user_properties(Object *obj)
> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_vendor_exts);
> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_experimental_exts);
>
> + riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_deprecated_exts);
> +
> for (Property *prop = riscv_cpu_options; prop && prop->name; prop++) {
> qdev_property_add_static(DEVICE(obj), prop);
> }
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: deprecate capital 'Z' CPU properties
2023-10-07 17:14 [PATCH] target/riscv: deprecate capital 'Z' CPU properties Daniel Henrique Barboza
` (2 preceding siblings ...)
2023-10-09 1:39 ` Alistair Francis
@ 2023-10-09 7:53 ` Andrew Jones
3 siblings, 0 replies; 6+ messages in thread
From: Andrew Jones @ 2023-10-09 7:53 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liweiwei,
zhiwei_liu, palmer
On Sat, Oct 07, 2023 at 02:14:27PM -0300, Daniel Henrique Barboza wrote:
> At this moment there are eleven CPU extension properties that starts
> with capital 'Z': Zifencei, Zicsr, Zihintntl, Zihintpause, Zawrs, Zfa,
> Zfh, Zfhmin, Zve32f, Zve64f and Zve64d. All other extensions are named
> with lower-case letters.
>
> We want all properties to be named with lower-case letters since it's
> consistent with the riscv-isa string that we create in the FDT. Having
> these 11 properties to be exceptions can be confusing.
>
> Deprecate all of them. Create their lower-case counterpart to be used as
> maintained CPU properties. When trying to use any deprecated property a
> warning message will be displayed, recommending users to switch to the
> lower-case variant:
>
> ./build/qemu-system-riscv64 -M virt -cpu rv64,Zifencei=true --nographic
> qemu-system-riscv64: warning: CPU property 'Zifencei' is deprecated. Please use 'zifencei' instead
>
> This will give users some time to change their scripts before we remove
> the capital 'Z' properties entirely.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
> docs/about/deprecated.rst | 23 ++++++++++++++++++++++
> target/riscv/cpu.c | 39 +++++++++++++++++++++++++++-----------
> target/riscv/cpu.h | 1 +
> target/riscv/tcg/tcg-cpu.c | 31 +++++++++++++++++++++++++++++-
> 4 files changed, 82 insertions(+), 12 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 694b878f36..331f10f930 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -378,6 +378,29 @@ of generic CPUs: rv32 and rv64 as default CPUs and 'max' as a feature complete
> CPU for both 32 and 64 bit builds. Users are then discouraged to use the 'any'
> CPU type starting in 8.2.
>
> +RISC-V CPU properties which start with with capital 'Z' (since 8.2)
^ double with
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +All RISC-V CPU properties which start with capital 'Z' are being deprecated
> +starting in 8.2. The reason is that they were wrongly added with capital 'Z'
> +in the past. CPU properties were later added with lower-case names, which
> +is the format we want to use from now on.
> +
> +Users which try to use these deprecated properties will receive a warning
> +recommending to switch to their stable counterparts:
> +
> +- "Zifencei" should be replaced with "zifencei"
> +- "Zicsr" should be replaced with "zicsr"
> +- "Zihintntl" should be replaced with "zihintntl"
> +- "Zihintpause" should be replaced with "zihintpause"
> +- "Zawrs" should be replaced with "zawrs"
> +- "Zfa" should be replaced with "zfa"
> +- "Zfh" should be replaced with "zfh"
> +- "Zfhmin" should be replaced with "zfhmin"
> +- "Zve32f" should be replaced with "zve32f"
> +- "Zve64f" should be replaced with "zve64f"
> +- "Zve64d" should be replaced with "zve64d"
> +
> Block device options
> ''''''''''''''''''''
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 521bb88538..1cdc3d2609 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1246,17 +1246,17 @@ const char *riscv_get_misa_ext_description(uint32_t bit)
> const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
> /* Defaults for standard extensions */
> MULTI_EXT_CFG_BOOL("sscofpmf", ext_sscofpmf, false),
> - MULTI_EXT_CFG_BOOL("Zifencei", ext_ifencei, true),
> - MULTI_EXT_CFG_BOOL("Zicsr", ext_icsr, true),
> - MULTI_EXT_CFG_BOOL("Zihintntl", ext_zihintntl, true),
> - MULTI_EXT_CFG_BOOL("Zihintpause", ext_zihintpause, true),
> - MULTI_EXT_CFG_BOOL("Zawrs", ext_zawrs, true),
> - MULTI_EXT_CFG_BOOL("Zfa", ext_zfa, true),
> - MULTI_EXT_CFG_BOOL("Zfh", ext_zfh, false),
> - MULTI_EXT_CFG_BOOL("Zfhmin", ext_zfhmin, false),
> - MULTI_EXT_CFG_BOOL("Zve32f", ext_zve32f, false),
> - MULTI_EXT_CFG_BOOL("Zve64f", ext_zve64f, false),
> - MULTI_EXT_CFG_BOOL("Zve64d", ext_zve64d, false),
> + MULTI_EXT_CFG_BOOL("zifencei", ext_ifencei, true),
> + MULTI_EXT_CFG_BOOL("zicsr", ext_icsr, true),
> + MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
> + MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true),
> + MULTI_EXT_CFG_BOOL("zawrs", ext_zawrs, true),
> + MULTI_EXT_CFG_BOOL("zfa", ext_zfa, true),
> + MULTI_EXT_CFG_BOOL("zfh", ext_zfh, false),
> + MULTI_EXT_CFG_BOOL("zfhmin", ext_zfhmin, false),
> + MULTI_EXT_CFG_BOOL("zve32f", ext_zve32f, false),
> + MULTI_EXT_CFG_BOOL("zve64f", ext_zve64f, false),
> + MULTI_EXT_CFG_BOOL("zve64d", ext_zve64d, false),
> MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
>
> MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
> @@ -1349,6 +1349,23 @@ const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
> +/* Deprecated entries marked for future removal */
> +const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[] = {
> + MULTI_EXT_CFG_BOOL("Zifencei", ext_ifencei, true),
> + MULTI_EXT_CFG_BOOL("Zicsr", ext_icsr, true),
> + MULTI_EXT_CFG_BOOL("Zihintntl", ext_zihintntl, true),
> + MULTI_EXT_CFG_BOOL("Zihintpause", ext_zihintpause, true),
> + MULTI_EXT_CFG_BOOL("Zawrs", ext_zawrs, true),
> + MULTI_EXT_CFG_BOOL("Zfa", ext_zfa, true),
> + MULTI_EXT_CFG_BOOL("Zfh", ext_zfh, false),
> + MULTI_EXT_CFG_BOOL("Zfhmin", ext_zfhmin, false),
> + MULTI_EXT_CFG_BOOL("Zve32f", ext_zve32f, false),
> + MULTI_EXT_CFG_BOOL("Zve64f", ext_zve64f, false),
> + MULTI_EXT_CFG_BOOL("Zve64d", ext_zve64d, false),
> +
> + DEFINE_PROP_END_OF_LIST(),
> +};
> +
> Property riscv_cpu_options[] = {
> DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 3f11e69223..e98f5de32e 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -722,6 +722,7 @@ typedef struct RISCVCPUMultiExtConfig {
> extern const RISCVCPUMultiExtConfig riscv_cpu_extensions[];
> extern const RISCVCPUMultiExtConfig riscv_cpu_vendor_exts[];
> extern const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[];
> +extern const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[];
> extern Property riscv_cpu_options[];
>
> typedef struct isa_ext_data {
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 08b806dc07..00676593f7 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -732,6 +732,25 @@ static void riscv_cpu_add_misa_properties(Object *cpu_obj)
> }
> }
>
> +static bool cpu_ext_is_deprecated(const char *ext_name)
> +{
> + return isupper(ext_name[0]);
> +}
> +
> +/*
> + * String will be allocated in the heap. Caller is responsible
> + * for freeing it.
> + */
> +static char *cpu_ext_to_lower(const char *ext_name)
> +{
> + char *ret = g_malloc0(strlen(ext_name) + 1);
> +
> + strcpy(ret, ext_name);
> + ret[0] = tolower(ret[0]);
> +
> + return ret;
> +}
> +
> static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> @@ -744,6 +763,13 @@ static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
> return;
> }
>
> + if (cpu_ext_is_deprecated(multi_ext_cfg->name)) {
> + g_autofree char *lower = cpu_ext_to_lower(multi_ext_cfg->name);
> +
> + warn_report("CPU property '%s' is deprecated. Please use '%s' instead",
> + multi_ext_cfg->name, lower);
> + }
> +
> g_hash_table_insert(multi_ext_user_opts,
> GUINT_TO_POINTER(multi_ext_cfg->offset),
> (gpointer)value);
> @@ -777,13 +803,14 @@ static void cpu_add_multi_ext_prop(Object *cpu_obj,
> const RISCVCPUMultiExtConfig *multi_cfg)
> {
> bool generic_cpu = riscv_cpu_is_generic(cpu_obj);
> + bool deprecated_ext = cpu_ext_is_deprecated(multi_cfg->name);
>
> object_property_add(cpu_obj, multi_cfg->name, "bool",
> cpu_get_multi_ext_cfg,
> cpu_set_multi_ext_cfg,
> NULL, (void *)multi_cfg);
>
> - if (!generic_cpu) {
> + if (!generic_cpu || deprecated_ext) {
> return;
> }
>
> @@ -826,6 +853,8 @@ static void riscv_cpu_add_user_properties(Object *obj)
> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_vendor_exts);
> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_experimental_exts);
>
> + riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_deprecated_exts);
> +
> for (Property *prop = riscv_cpu_options; prop && prop->name; prop++) {
> qdev_property_add_static(DEVICE(obj), prop);
> }
> --
> 2.41.0
>
>
Other than the text issue,
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: deprecate capital 'Z' CPU properties
2023-10-09 1:39 ` Alistair Francis
@ 2023-10-09 11:30 ` Daniel Henrique Barboza
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Henrique Barboza @ 2023-10-09 11:30 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liweiwei,
zhiwei_liu, palmer
On 10/8/23 22:39, Alistair Francis wrote:
> On Sun, Oct 8, 2023 at 3:15 AM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>>
>> At this moment there are eleven CPU extension properties that starts
>> with capital 'Z': Zifencei, Zicsr, Zihintntl, Zihintpause, Zawrs, Zfa,
>> Zfh, Zfhmin, Zve32f, Zve64f and Zve64d. All other extensions are named
>> with lower-case letters.
>>
>> We want all properties to be named with lower-case letters since it's
>> consistent with the riscv-isa string that we create in the FDT. Having
>> these 11 properties to be exceptions can be confusing.
>>
>> Deprecate all of them. Create their lower-case counterpart to be used as
>> maintained CPU properties. When trying to use any deprecated property a
>> warning message will be displayed, recommending users to switch to the
>> lower-case variant:
>>
>> ./build/qemu-system-riscv64 -M virt -cpu rv64,Zifencei=true --nographic
>> qemu-system-riscv64: warning: CPU property 'Zifencei' is deprecated. Please use 'zifencei' instead
>>
>> This will give users some time to change their scripts before we remove
>> the capital 'Z' properties entirely.
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>
> Thanks!
>
> Applied to riscv-to-apply.next
>
> Alistair
I just sent a v2 with the doc fix Drew mentioned. Feel free to either pick the v2 or
amend this one in-tree. Whatever is more convenient for you.
Thanks,
Daniel
>
>> ---
>> docs/about/deprecated.rst | 23 ++++++++++++++++++++++
>> target/riscv/cpu.c | 39 +++++++++++++++++++++++++++-----------
>> target/riscv/cpu.h | 1 +
>> target/riscv/tcg/tcg-cpu.c | 31 +++++++++++++++++++++++++++++-
>> 4 files changed, 82 insertions(+), 12 deletions(-)
>>
>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> index 694b878f36..331f10f930 100644
>> --- a/docs/about/deprecated.rst
>> +++ b/docs/about/deprecated.rst
>> @@ -378,6 +378,29 @@ of generic CPUs: rv32 and rv64 as default CPUs and 'max' as a feature complete
>> CPU for both 32 and 64 bit builds. Users are then discouraged to use the 'any'
>> CPU type starting in 8.2.
>>
>> +RISC-V CPU properties which start with with capital 'Z' (since 8.2)
>> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> +
>> +All RISC-V CPU properties which start with capital 'Z' are being deprecated
>> +starting in 8.2. The reason is that they were wrongly added with capital 'Z'
>> +in the past. CPU properties were later added with lower-case names, which
>> +is the format we want to use from now on.
>> +
>> +Users which try to use these deprecated properties will receive a warning
>> +recommending to switch to their stable counterparts:
>> +
>> +- "Zifencei" should be replaced with "zifencei"
>> +- "Zicsr" should be replaced with "zicsr"
>> +- "Zihintntl" should be replaced with "zihintntl"
>> +- "Zihintpause" should be replaced with "zihintpause"
>> +- "Zawrs" should be replaced with "zawrs"
>> +- "Zfa" should be replaced with "zfa"
>> +- "Zfh" should be replaced with "zfh"
>> +- "Zfhmin" should be replaced with "zfhmin"
>> +- "Zve32f" should be replaced with "zve32f"
>> +- "Zve64f" should be replaced with "zve64f"
>> +- "Zve64d" should be replaced with "zve64d"
>> +
>> Block device options
>> ''''''''''''''''''''
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 521bb88538..1cdc3d2609 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -1246,17 +1246,17 @@ const char *riscv_get_misa_ext_description(uint32_t bit)
>> const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
>> /* Defaults for standard extensions */
>> MULTI_EXT_CFG_BOOL("sscofpmf", ext_sscofpmf, false),
>> - MULTI_EXT_CFG_BOOL("Zifencei", ext_ifencei, true),
>> - MULTI_EXT_CFG_BOOL("Zicsr", ext_icsr, true),
>> - MULTI_EXT_CFG_BOOL("Zihintntl", ext_zihintntl, true),
>> - MULTI_EXT_CFG_BOOL("Zihintpause", ext_zihintpause, true),
>> - MULTI_EXT_CFG_BOOL("Zawrs", ext_zawrs, true),
>> - MULTI_EXT_CFG_BOOL("Zfa", ext_zfa, true),
>> - MULTI_EXT_CFG_BOOL("Zfh", ext_zfh, false),
>> - MULTI_EXT_CFG_BOOL("Zfhmin", ext_zfhmin, false),
>> - MULTI_EXT_CFG_BOOL("Zve32f", ext_zve32f, false),
>> - MULTI_EXT_CFG_BOOL("Zve64f", ext_zve64f, false),
>> - MULTI_EXT_CFG_BOOL("Zve64d", ext_zve64d, false),
>> + MULTI_EXT_CFG_BOOL("zifencei", ext_ifencei, true),
>> + MULTI_EXT_CFG_BOOL("zicsr", ext_icsr, true),
>> + MULTI_EXT_CFG_BOOL("zihintntl", ext_zihintntl, true),
>> + MULTI_EXT_CFG_BOOL("zihintpause", ext_zihintpause, true),
>> + MULTI_EXT_CFG_BOOL("zawrs", ext_zawrs, true),
>> + MULTI_EXT_CFG_BOOL("zfa", ext_zfa, true),
>> + MULTI_EXT_CFG_BOOL("zfh", ext_zfh, false),
>> + MULTI_EXT_CFG_BOOL("zfhmin", ext_zfhmin, false),
>> + MULTI_EXT_CFG_BOOL("zve32f", ext_zve32f, false),
>> + MULTI_EXT_CFG_BOOL("zve64f", ext_zve64f, false),
>> + MULTI_EXT_CFG_BOOL("zve64d", ext_zve64d, false),
>> MULTI_EXT_CFG_BOOL("sstc", ext_sstc, true),
>>
>> MULTI_EXT_CFG_BOOL("smstateen", ext_smstateen, false),
>> @@ -1349,6 +1349,23 @@ const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[] = {
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>> +/* Deprecated entries marked for future removal */
>> +const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[] = {
>> + MULTI_EXT_CFG_BOOL("Zifencei", ext_ifencei, true),
>> + MULTI_EXT_CFG_BOOL("Zicsr", ext_icsr, true),
>> + MULTI_EXT_CFG_BOOL("Zihintntl", ext_zihintntl, true),
>> + MULTI_EXT_CFG_BOOL("Zihintpause", ext_zihintpause, true),
>> + MULTI_EXT_CFG_BOOL("Zawrs", ext_zawrs, true),
>> + MULTI_EXT_CFG_BOOL("Zfa", ext_zfa, true),
>> + MULTI_EXT_CFG_BOOL("Zfh", ext_zfh, false),
>> + MULTI_EXT_CFG_BOOL("Zfhmin", ext_zfhmin, false),
>> + MULTI_EXT_CFG_BOOL("Zve32f", ext_zve32f, false),
>> + MULTI_EXT_CFG_BOOL("Zve64f", ext_zve64f, false),
>> + MULTI_EXT_CFG_BOOL("Zve64d", ext_zve64d, false),
>> +
>> + DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> Property riscv_cpu_options[] = {
>> DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
>>
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 3f11e69223..e98f5de32e 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -722,6 +722,7 @@ typedef struct RISCVCPUMultiExtConfig {
>> extern const RISCVCPUMultiExtConfig riscv_cpu_extensions[];
>> extern const RISCVCPUMultiExtConfig riscv_cpu_vendor_exts[];
>> extern const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[];
>> +extern const RISCVCPUMultiExtConfig riscv_cpu_deprecated_exts[];
>> extern Property riscv_cpu_options[];
>>
>> typedef struct isa_ext_data {
>> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
>> index 08b806dc07..00676593f7 100644
>> --- a/target/riscv/tcg/tcg-cpu.c
>> +++ b/target/riscv/tcg/tcg-cpu.c
>> @@ -732,6 +732,25 @@ static void riscv_cpu_add_misa_properties(Object *cpu_obj)
>> }
>> }
>>
>> +static bool cpu_ext_is_deprecated(const char *ext_name)
>> +{
>> + return isupper(ext_name[0]);
>> +}
>> +
>> +/*
>> + * String will be allocated in the heap. Caller is responsible
>> + * for freeing it.
>> + */
>> +static char *cpu_ext_to_lower(const char *ext_name)
>> +{
>> + char *ret = g_malloc0(strlen(ext_name) + 1);
>> +
>> + strcpy(ret, ext_name);
>> + ret[0] = tolower(ret[0]);
>> +
>> + return ret;
>> +}
>> +
>> static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
>> void *opaque, Error **errp)
>> {
>> @@ -744,6 +763,13 @@ static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
>> return;
>> }
>>
>> + if (cpu_ext_is_deprecated(multi_ext_cfg->name)) {
>> + g_autofree char *lower = cpu_ext_to_lower(multi_ext_cfg->name);
>> +
>> + warn_report("CPU property '%s' is deprecated. Please use '%s' instead",
>> + multi_ext_cfg->name, lower);
>> + }
>> +
>> g_hash_table_insert(multi_ext_user_opts,
>> GUINT_TO_POINTER(multi_ext_cfg->offset),
>> (gpointer)value);
>> @@ -777,13 +803,14 @@ static void cpu_add_multi_ext_prop(Object *cpu_obj,
>> const RISCVCPUMultiExtConfig *multi_cfg)
>> {
>> bool generic_cpu = riscv_cpu_is_generic(cpu_obj);
>> + bool deprecated_ext = cpu_ext_is_deprecated(multi_cfg->name);
>>
>> object_property_add(cpu_obj, multi_cfg->name, "bool",
>> cpu_get_multi_ext_cfg,
>> cpu_set_multi_ext_cfg,
>> NULL, (void *)multi_cfg);
>>
>> - if (!generic_cpu) {
>> + if (!generic_cpu || deprecated_ext) {
>> return;
>> }
>>
>> @@ -826,6 +853,8 @@ static void riscv_cpu_add_user_properties(Object *obj)
>> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_vendor_exts);
>> riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_experimental_exts);
>>
>> + riscv_cpu_add_multiext_prop_array(obj, riscv_cpu_deprecated_exts);
>> +
>> for (Property *prop = riscv_cpu_options; prop && prop->name; prop++) {
>> qdev_property_add_static(DEVICE(obj), prop);
>> }
>> --
>> 2.41.0
>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-10-09 11:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-07 17:14 [PATCH] target/riscv: deprecate capital 'Z' CPU properties Daniel Henrique Barboza
2023-10-08 5:10 ` Tsukasa OI
2023-10-09 1:34 ` Alistair Francis
2023-10-09 1:39 ` Alistair Francis
2023-10-09 11:30 ` Daniel Henrique Barboza
2023-10-09 7:53 ` Andrew Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).