* [PATCH 0/3] target/riscv: Add support for 'B' extension
@ 2024-01-09 17:07 Rob Bradford
2024-01-09 17:07 ` [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension Rob Bradford
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Rob Bradford @ 2024-01-09 17:07 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, Rob Bradford
Add support for the new (fast track) 'B' extension [1] this extension
uses the misa.B bit to indicate that the Zba, Zbb and Zbs extensions are
present.
Since this extension is not yet frozen it is exposed via the 'x-b' cpu
option. The validation logic is based on the new approach taken for the
'G' extension. [2]
The specification handles backward compatability: The misa.B bit may be
set if Zba, Zbb and Zbs are present but in order to not break existing
systems the bit is not required to be set if they are present. As such
even though Zba, Zbb and Zbs default to on in QEMU this extension is not
enabled by default in any cpu other than the 'max' variant.
Cheers,
Rob
[1] - https://github.com/riscv/riscv-b
[2] - https://patchew.org/QEMU/20231218125334.37184-1-dbarboza@ventanamicro.com/20231218125334.37184-16-dbarboza@ventanamicro.com/
Rob Bradford (3):
target/riscv: Add infrastructure for 'B' MISA extension
target/riscv: Add step to validate 'B' extension
target/riscv: Enable 'B' extension on max CPU type
target/riscv/cpu.c | 5 +++--
target/riscv/cpu.h | 1 +
target/riscv/tcg/tcg-cpu.c | 37 ++++++++++++++++++++++++++++++++++++-
3 files changed, 40 insertions(+), 3 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
2024-01-09 17:07 [PATCH 0/3] target/riscv: Add support for 'B' extension Rob Bradford
@ 2024-01-09 17:07 ` Rob Bradford
2024-01-10 18:18 ` Daniel Henrique Barboza
` (2 more replies)
2024-01-09 17:07 ` [PATCH 2/3] target/riscv: Add step to validate 'B' extension Rob Bradford
2024-01-09 17:07 ` [PATCH 3/3] target/riscv: Enable 'B' extension on max CPU type Rob Bradford
2 siblings, 3 replies; 19+ messages in thread
From: Rob Bradford @ 2024-01-09 17:07 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, Rob Bradford
Add the infrastructure for the 'B' extension which is the union of the
Zba, Zbb and Zbs instructions.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
---
target/riscv/cpu.c | 5 +++--
target/riscv/cpu.h | 1 +
target/riscv/tcg/tcg-cpu.c | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index b07a76ef6b..22f8e527ff 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -38,9 +38,9 @@
#include "tcg/tcg.h"
/* RISC-V CPU definitions */
-static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
+static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, RVV,
- RVC, RVS, RVU, RVH, RVJ, RVG, 0};
+ RVC, RVS, RVU, RVH, RVJ, RVG, RVB, 0};
/*
* From vector_helper.c
@@ -1251,6 +1251,7 @@ static const MISAExtInfo misa_ext_info_arr[] = {
MISA_EXT_INFO(RVJ, "x-j", "Dynamic translated languages"),
MISA_EXT_INFO(RVV, "v", "Vector operations"),
MISA_EXT_INFO(RVG, "g", "General purpose (IMAFD_Zicsr_Zifencei)"),
+ MISA_EXT_INFO(RVB, "x-b", "Bit manipulation (Zba_Zbb_Zbs)")
};
static int riscv_validate_misa_info_idx(uint32_t bit)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 2725528bb5..756a345513 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -69,6 +69,7 @@ typedef struct CPUArchState CPURISCVState;
#define RVH RV('H')
#define RVJ RV('J')
#define RVG RV('G')
+#define RVB RV('B')
extern const uint32_t misa_bits[];
const char *riscv_get_misa_ext_name(uint32_t bit);
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 8a35683a34..fda54671d5 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -791,6 +791,7 @@ static const RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
MISA_CFG(RVJ, false),
MISA_CFG(RVV, false),
MISA_CFG(RVG, false),
+ MISA_CFG(RVB, false)
};
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/3] target/riscv: Add step to validate 'B' extension
2024-01-09 17:07 [PATCH 0/3] target/riscv: Add support for 'B' extension Rob Bradford
2024-01-09 17:07 ` [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension Rob Bradford
@ 2024-01-09 17:07 ` Rob Bradford
2024-01-10 18:26 ` Daniel Henrique Barboza
2024-01-11 13:09 ` Andrew Jones
2024-01-09 17:07 ` [PATCH 3/3] target/riscv: Enable 'B' extension on max CPU type Rob Bradford
2 siblings, 2 replies; 19+ messages in thread
From: Rob Bradford @ 2024-01-09 17:07 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, Rob Bradford
If the B extension is enabled warn if the user has disabled any of the
required extensions that are part of the 'B' extension. Conversely
enable the extensions that make up the 'B' extension if it is enabled.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
---
target/riscv/tcg/tcg-cpu.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index fda54671d5..f10871d352 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -273,6 +273,35 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
}
}
+static void riscv_cpu_validate_b(RISCVCPU *cpu)
+{
+ const char *warn_msg = "RVB mandates disabled extension %s";
+
+ if (!cpu->cfg.ext_zba) {
+ if (!cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zba))) {
+ cpu->cfg.ext_zba = true;
+ } else {
+ warn_report(warn_msg, "zba");
+ }
+ }
+
+ if (!cpu->cfg.ext_zbb) {
+ if (!cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zbb))) {
+ cpu->cfg.ext_zbb = true;
+ } else {
+ warn_report(warn_msg, "zbb");
+ }
+ }
+
+ if (!cpu->cfg.ext_zbs) {
+ if (!cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zbs))) {
+ cpu->cfg.ext_zbs = true;
+ } else {
+ warn_report(warn_msg, "zbs");
+ }
+ }
+}
+
/*
* Check consistency between chosen extensions while setting
* cpu->cfg accordingly.
@@ -309,6 +338,10 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
env->misa_ext_mask |= RVI | RVM | RVA | RVF | RVD;
}
+ if (riscv_has_ext(env, RVB)) {
+ riscv_cpu_validate_b(cpu);
+ }
+
if (riscv_has_ext(env, RVI) && riscv_has_ext(env, RVE)) {
error_setg(errp,
"I and E extensions are incompatible");
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/3] target/riscv: Enable 'B' extension on max CPU type
2024-01-09 17:07 [PATCH 0/3] target/riscv: Add support for 'B' extension Rob Bradford
2024-01-09 17:07 ` [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension Rob Bradford
2024-01-09 17:07 ` [PATCH 2/3] target/riscv: Add step to validate 'B' extension Rob Bradford
@ 2024-01-09 17:07 ` Rob Bradford
2024-01-10 18:32 ` Daniel Henrique Barboza
2 siblings, 1 reply; 19+ messages in thread
From: Rob Bradford @ 2024-01-09 17:07 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
dbarboza, zhiwei_liu, Rob Bradford
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
---
target/riscv/tcg/tcg-cpu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index f10871d352..9705daec93 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -999,7 +999,8 @@ static void riscv_init_max_cpu_extensions(Object *obj)
const RISCVCPUMultiExtConfig *prop;
/* Enable RVG, RVJ and RVV that are disabled by default */
- riscv_cpu_set_misa(env, env->misa_mxl, env->misa_ext | RVG | RVJ | RVV);
+ riscv_cpu_set_misa(env, env->misa_mxl,
+ env->misa_ext | RVG | RVJ | RVV | RVB);
for (prop = riscv_cpu_extensions; prop && prop->name; prop++) {
isa_ext_update_enabled(cpu, prop->offset, true);
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
2024-01-09 17:07 ` [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension Rob Bradford
@ 2024-01-10 18:18 ` Daniel Henrique Barboza
2024-01-11 13:07 ` Andrew Jones
2024-01-11 13:15 ` Andrew Jones
2 siblings, 0 replies; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-01-10 18:18 UTC (permalink / raw)
To: Rob Bradford, qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
zhiwei_liu
On 1/9/24 14:07, Rob Bradford wrote:
> Add the infrastructure for the 'B' extension which is the union of the
> Zba, Zbb and Zbs instructions.
>
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> ---
> target/riscv/cpu.c | 5 +++--
> target/riscv/cpu.h | 1 +
> target/riscv/tcg/tcg-cpu.c | 1 +
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index b07a76ef6b..22f8e527ff 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -38,9 +38,9 @@
> #include "tcg/tcg.h"
>
> /* RISC-V CPU definitions */
> -static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
> +static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
> const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, RVV,
> - RVC, RVS, RVU, RVH, RVJ, RVG, 0};
> + RVC, RVS, RVU, RVH, RVJ, RVG, RVB, 0};
>
> /*
> * From vector_helper.c
> @@ -1251,6 +1251,7 @@ static const MISAExtInfo misa_ext_info_arr[] = {
> MISA_EXT_INFO(RVJ, "x-j", "Dynamic translated languages"),
> MISA_EXT_INFO(RVV, "v", "Vector operations"),
> MISA_EXT_INFO(RVG, "g", "General purpose (IMAFD_Zicsr_Zifencei)"),
> + MISA_EXT_INFO(RVB, "x-b", "Bit manipulation (Zba_Zbb_Zbs)")
> };
>
> static int riscv_validate_misa_info_idx(uint32_t bit)
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 2725528bb5..756a345513 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -69,6 +69,7 @@ typedef struct CPUArchState CPURISCVState;
> #define RVH RV('H')
> #define RVJ RV('J')
> #define RVG RV('G')
> +#define RVB RV('B')
>
> extern const uint32_t misa_bits[];
> const char *riscv_get_misa_ext_name(uint32_t bit);
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 8a35683a34..fda54671d5 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -791,6 +791,7 @@ static const RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
> MISA_CFG(RVJ, false),
> MISA_CFG(RVV, false),
> MISA_CFG(RVG, false),
> + MISA_CFG(RVB, false)
Please add a comma at the end:
> + MISA_CFG(RVB, false),
This way, when a new bit is added, the change is limited to the new entry.
With this nit fixed:
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> };
>
> /*
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] target/riscv: Add step to validate 'B' extension
2024-01-09 17:07 ` [PATCH 2/3] target/riscv: Add step to validate 'B' extension Rob Bradford
@ 2024-01-10 18:26 ` Daniel Henrique Barboza
2024-01-11 13:09 ` Andrew Jones
1 sibling, 0 replies; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-01-10 18:26 UTC (permalink / raw)
To: Rob Bradford, qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
zhiwei_liu
On 1/9/24 14:07, Rob Bradford wrote:
> If the B extension is enabled warn if the user has disabled any of the
> required extensions that are part of the 'B' extension. Conversely
> enable the extensions that make up the 'B' extension if it is enabled.
>
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> ---
This patch doesn't apply cleanly on current master. Which is normal, since we just
had a RISC-V PR merged.
I'm afraid you'll need to resend the series rebased on top of master (since it's newer
than Alistair's riscv-to-apply.next now).
> target/riscv/tcg/tcg-cpu.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
I fixed the conflicts and applied the patch and it works for me. So:
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index fda54671d5..f10871d352 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -273,6 +273,35 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
> }
> }
>
> +static void riscv_cpu_validate_b(RISCVCPU *cpu)
> +{
> + const char *warn_msg = "RVB mandates disabled extension %s";
> +
> + if (!cpu->cfg.ext_zba) {
> + if (!cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zba))) {
> + cpu->cfg.ext_zba = true;
> + } else {
> + warn_report(warn_msg, "zba");
> + }
> + }
> +
> + if (!cpu->cfg.ext_zbb) {
> + if (!cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zbb))) {
> + cpu->cfg.ext_zbb = true;
> + } else {
> + warn_report(warn_msg, "zbb");
> + }
> + }
> +
> + if (!cpu->cfg.ext_zbs) {
> + if (!cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zbs))) {
> + cpu->cfg.ext_zbs = true;
> + } else {
> + warn_report(warn_msg, "zbs");
> + }
> + }
> +}
> +
> /*
> * Check consistency between chosen extensions while setting
> * cpu->cfg accordingly.
> @@ -309,6 +338,10 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> env->misa_ext_mask |= RVI | RVM | RVA | RVF | RVD;
> }
>
> + if (riscv_has_ext(env, RVB)) {
> + riscv_cpu_validate_b(cpu);
> + }
> +
> if (riscv_has_ext(env, RVI) && riscv_has_ext(env, RVE)) {
> error_setg(errp,
> "I and E extensions are incompatible");
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] target/riscv: Enable 'B' extension on max CPU type
2024-01-09 17:07 ` [PATCH 3/3] target/riscv: Enable 'B' extension on max CPU type Rob Bradford
@ 2024-01-10 18:32 ` Daniel Henrique Barboza
2024-01-10 18:41 ` Daniel Henrique Barboza
2024-01-11 13:02 ` Andrew Jones
0 siblings, 2 replies; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-01-10 18:32 UTC (permalink / raw)
To: Rob Bradford, qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
zhiwei_liu
On 1/9/24 14:07, Rob Bradford wrote:
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> ---
> target/riscv/tcg/tcg-cpu.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index f10871d352..9705daec93 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -999,7 +999,8 @@ static void riscv_init_max_cpu_extensions(Object *obj)
> const RISCVCPUMultiExtConfig *prop;
>
> /* Enable RVG, RVJ and RVV that are disabled by default */
> - riscv_cpu_set_misa(env, env->misa_mxl, env->misa_ext | RVG | RVJ | RVV);
> + riscv_cpu_set_misa(env, env->misa_mxl,
> + env->misa_ext | RVG | RVJ | RVV | RVB);
I'm aware that we decided a while ago the 'max' CPU could only have non-vendor and
non-experimental extensions enabled. RVB is experimental, so in theory we shouldn't
enable it.
But RVB is an alias for zba, zbb and zbs, extensions that the 'max' CPU is already
enabling. In this case I think it's sensible to enable RVB here since it would just
reflect stuff that it's already happening.
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>
> for (prop = riscv_cpu_extensions; prop && prop->name; prop++) {
> isa_ext_update_enabled(cpu, prop->offset, true);
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] target/riscv: Enable 'B' extension on max CPU type
2024-01-10 18:32 ` Daniel Henrique Barboza
@ 2024-01-10 18:41 ` Daniel Henrique Barboza
2024-01-11 13:02 ` Andrew Jones
1 sibling, 0 replies; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-01-10 18:41 UTC (permalink / raw)
To: Rob Bradford, qemu-devel
Cc: qemu-riscv, atishp, palmer, alistair.francis, bin.meng, liwei1518,
zhiwei_liu
Rob,
Given that you'll need to resend the patches due to the conflict in patch 2,
I think it would be nice to mention in this commit message that we're ok with
enabling RVB in the 'max' CPU, even though RVB per se is experimental, because
it's just an alias for extensions that the CPU already uses.
I'm asking this because future Daniel will forget why we're adding an experimental
extension in the 'max' CPU, and this info in the commit msg will spare him from
searching the ML archives to discover why :)
Thanks,
Daniel
On 1/10/24 15:32, Daniel Henrique Barboza wrote:
>
>
> On 1/9/24 14:07, Rob Bradford wrote:
>> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
>> ---
>> target/riscv/tcg/tcg-cpu.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
>> index f10871d352..9705daec93 100644
>> --- a/target/riscv/tcg/tcg-cpu.c
>> +++ b/target/riscv/tcg/tcg-cpu.c
>> @@ -999,7 +999,8 @@ static void riscv_init_max_cpu_extensions(Object *obj)
>> const RISCVCPUMultiExtConfig *prop;
>> /* Enable RVG, RVJ and RVV that are disabled by default */
>> - riscv_cpu_set_misa(env, env->misa_mxl, env->misa_ext | RVG | RVJ | RVV);
>> + riscv_cpu_set_misa(env, env->misa_mxl,
>> + env->misa_ext | RVG | RVJ | RVV | RVB);
>
> I'm aware that we decided a while ago the 'max' CPU could only have non-vendor and
> non-experimental extensions enabled. RVB is experimental, so in theory we shouldn't
> enable it.
>
> But RVB is an alias for zba, zbb and zbs, extensions that the 'max' CPU is already
> enabling. In this case I think it's sensible to enable RVB here since it would just
> reflect stuff that it's already happening.
>
>
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>
>
>
>
>> for (prop = riscv_cpu_extensions; prop && prop->name; prop++) {
>> isa_ext_update_enabled(cpu, prop->offset, true);
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Re: [PATCH 3/3] target/riscv: Enable 'B' extension on max CPU type
2024-01-10 18:32 ` Daniel Henrique Barboza
2024-01-10 18:41 ` Daniel Henrique Barboza
@ 2024-01-11 13:02 ` Andrew Jones
2024-01-11 14:53 ` Daniel Henrique Barboza
1 sibling, 1 reply; 19+ messages in thread
From: Andrew Jones @ 2024-01-11 13:02 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: Rob Bradford, qemu-devel, qemu-riscv, atishp, palmer,
alistair.francis, bin.meng, liwei1518, zhiwei_liu
On Wed, Jan 10, 2024 at 03:32:21PM -0300, Daniel Henrique Barboza wrote:
>
>
> On 1/9/24 14:07, Rob Bradford wrote:
> > Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> > ---
> > target/riscv/tcg/tcg-cpu.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> > index f10871d352..9705daec93 100644
> > --- a/target/riscv/tcg/tcg-cpu.c
> > +++ b/target/riscv/tcg/tcg-cpu.c
> > @@ -999,7 +999,8 @@ static void riscv_init_max_cpu_extensions(Object *obj)
> > const RISCVCPUMultiExtConfig *prop;
> > /* Enable RVG, RVJ and RVV that are disabled by default */
> > - riscv_cpu_set_misa(env, env->misa_mxl, env->misa_ext | RVG | RVJ | RVV);
> > + riscv_cpu_set_misa(env, env->misa_mxl,
> > + env->misa_ext | RVG | RVJ | RVV | RVB);
>
> I'm aware that we decided a while ago the 'max' CPU could only have non-vendor and
> non-experimental extensions enabled. RVB is experimental, so in theory we shouldn't
> enable it.
>
> But RVB is an alias for zba, zbb and zbs, extensions that the 'max' CPU is already
> enabling. In this case I think it's sensible to enable RVB here since it would just
> reflect stuff that it's already happening.
It's also setting the B bit in misa, which, until this spec is at least
frozen, is a reserved bit and reserved bits "must return zero when read".
I don't want to stand in the way of progress and it seems 99.9% likely
that the spec will be frozen and ratified, but, if we want to stick to
our policies (which we should document), then even the 'max' cpu type
should require x-b be added to the command line if it wants the B bit
set in misa.
Thanks,
drew
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
2024-01-09 17:07 ` [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension Rob Bradford
2024-01-10 18:18 ` Daniel Henrique Barboza
@ 2024-01-11 13:07 ` Andrew Jones
2024-01-11 13:14 ` Andrew Jones
2024-01-11 13:15 ` Andrew Jones
2 siblings, 1 reply; 19+ messages in thread
From: Andrew Jones @ 2024-01-11 13:07 UTC (permalink / raw)
To: Rob Bradford
Cc: qemu-devel, qemu-riscv, atishp, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu
On Tue, Jan 09, 2024 at 05:07:35PM +0000, Rob Bradford wrote:
> Add the infrastructure for the 'B' extension which is the union of the
> Zba, Zbb and Zbs instructions.
>
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> ---
> target/riscv/cpu.c | 5 +++--
> target/riscv/cpu.h | 1 +
> target/riscv/tcg/tcg-cpu.c | 1 +
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index b07a76ef6b..22f8e527ff 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -38,9 +38,9 @@
> #include "tcg/tcg.h"
>
> /* RISC-V CPU definitions */
> -static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
> +static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
Is there a corresponding proposed change to table 29.1 of the nonpriv spec
which states B comes after C and before P? If so, can you provide a link
to it? Otherwise, how do we know that?
Thanks,
drew
> const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, RVV,
> - RVC, RVS, RVU, RVH, RVJ, RVG, 0};
> + RVC, RVS, RVU, RVH, RVJ, RVG, RVB, 0};
>
> /*
> * From vector_helper.c
> @@ -1251,6 +1251,7 @@ static const MISAExtInfo misa_ext_info_arr[] = {
> MISA_EXT_INFO(RVJ, "x-j", "Dynamic translated languages"),
> MISA_EXT_INFO(RVV, "v", "Vector operations"),
> MISA_EXT_INFO(RVG, "g", "General purpose (IMAFD_Zicsr_Zifencei)"),
> + MISA_EXT_INFO(RVB, "x-b", "Bit manipulation (Zba_Zbb_Zbs)")
> };
>
> static int riscv_validate_misa_info_idx(uint32_t bit)
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 2725528bb5..756a345513 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -69,6 +69,7 @@ typedef struct CPUArchState CPURISCVState;
> #define RVH RV('H')
> #define RVJ RV('J')
> #define RVG RV('G')
> +#define RVB RV('B')
>
> extern const uint32_t misa_bits[];
> const char *riscv_get_misa_ext_name(uint32_t bit);
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 8a35683a34..fda54671d5 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -791,6 +791,7 @@ static const RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
> MISA_CFG(RVJ, false),
> MISA_CFG(RVV, false),
> MISA_CFG(RVG, false),
> + MISA_CFG(RVB, false)
> };
>
> /*
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] target/riscv: Add step to validate 'B' extension
2024-01-09 17:07 ` [PATCH 2/3] target/riscv: Add step to validate 'B' extension Rob Bradford
2024-01-10 18:26 ` Daniel Henrique Barboza
@ 2024-01-11 13:09 ` Andrew Jones
1 sibling, 0 replies; 19+ messages in thread
From: Andrew Jones @ 2024-01-11 13:09 UTC (permalink / raw)
To: Rob Bradford
Cc: qemu-devel, qemu-riscv, atishp, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu
On Tue, Jan 09, 2024 at 05:07:36PM +0000, Rob Bradford wrote:
> If the B extension is enabled warn if the user has disabled any of the
> required extensions that are part of the 'B' extension. Conversely
> enable the extensions that make up the 'B' extension if it is enabled.
>
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> ---
> target/riscv/tcg/tcg-cpu.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index fda54671d5..f10871d352 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -273,6 +273,35 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
> }
> }
>
> +static void riscv_cpu_validate_b(RISCVCPU *cpu)
> +{
> + const char *warn_msg = "RVB mandates disabled extension %s";
> +
> + if (!cpu->cfg.ext_zba) {
> + if (!cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zba))) {
> + cpu->cfg.ext_zba = true;
> + } else {
> + warn_report(warn_msg, "zba");
> + }
> + }
> +
> + if (!cpu->cfg.ext_zbb) {
> + if (!cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zbb))) {
> + cpu->cfg.ext_zbb = true;
> + } else {
> + warn_report(warn_msg, "zbb");
> + }
> + }
> +
> + if (!cpu->cfg.ext_zbs) {
> + if (!cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zbs))) {
> + cpu->cfg.ext_zbs = true;
> + } else {
> + warn_report(warn_msg, "zbs");
> + }
> + }
> +}
> +
> /*
> * Check consistency between chosen extensions while setting
> * cpu->cfg accordingly.
> @@ -309,6 +338,10 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> env->misa_ext_mask |= RVI | RVM | RVA | RVF | RVD;
> }
>
> + if (riscv_has_ext(env, RVB)) {
> + riscv_cpu_validate_b(cpu);
> + }
> +
> if (riscv_has_ext(env, RVI) && riscv_has_ext(env, RVE)) {
> error_setg(errp,
> "I and E extensions are incompatible");
> --
> 2.43.0
>
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
2024-01-11 13:07 ` Andrew Jones
@ 2024-01-11 13:14 ` Andrew Jones
2024-01-11 15:17 ` Rob Bradford
0 siblings, 1 reply; 19+ messages in thread
From: Andrew Jones @ 2024-01-11 13:14 UTC (permalink / raw)
To: Rob Bradford
Cc: qemu-devel, qemu-riscv, atishp, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu
On Thu, Jan 11, 2024 at 02:07:34PM +0100, Andrew Jones wrote:
> On Tue, Jan 09, 2024 at 05:07:35PM +0000, Rob Bradford wrote:
> > Add the infrastructure for the 'B' extension which is the union of the
> > Zba, Zbb and Zbs instructions.
> >
> > Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> > ---
> > target/riscv/cpu.c | 5 +++--
> > target/riscv/cpu.h | 1 +
> > target/riscv/tcg/tcg-cpu.c | 1 +
> > 3 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index b07a76ef6b..22f8e527ff 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -38,9 +38,9 @@
> > #include "tcg/tcg.h"
> >
> > /* RISC-V CPU definitions */
> > -static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
> > +static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
>
> Is there a corresponding proposed change to table 29.1 of the nonpriv spec
> which states B comes after C and before P? If so, can you provide a link
> to it? Otherwise, how do we know that?
Oh, I see. The unpriv spec B chapter comes after the C chapter (and before
J, P, ...). I still wonder if we'll have a 29.1 table update with the
ratification of this extension though.
Thanks,
drew
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
2024-01-09 17:07 ` [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension Rob Bradford
2024-01-10 18:18 ` Daniel Henrique Barboza
2024-01-11 13:07 ` Andrew Jones
@ 2024-01-11 13:15 ` Andrew Jones
2 siblings, 0 replies; 19+ messages in thread
From: Andrew Jones @ 2024-01-11 13:15 UTC (permalink / raw)
To: Rob Bradford
Cc: qemu-devel, qemu-riscv, atishp, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu
On Tue, Jan 09, 2024 at 05:07:35PM +0000, Rob Bradford wrote:
> Add the infrastructure for the 'B' extension which is the union of the
> Zba, Zbb and Zbs instructions.
>
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> ---
> target/riscv/cpu.c | 5 +++--
> target/riscv/cpu.h | 1 +
> target/riscv/tcg/tcg-cpu.c | 1 +
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index b07a76ef6b..22f8e527ff 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -38,9 +38,9 @@
> #include "tcg/tcg.h"
>
> /* RISC-V CPU definitions */
> -static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
> +static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
> const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, RVV,
> - RVC, RVS, RVU, RVH, RVJ, RVG, 0};
> + RVC, RVS, RVU, RVH, RVJ, RVG, RVB, 0};
>
> /*
> * From vector_helper.c
> @@ -1251,6 +1251,7 @@ static const MISAExtInfo misa_ext_info_arr[] = {
> MISA_EXT_INFO(RVJ, "x-j", "Dynamic translated languages"),
> MISA_EXT_INFO(RVV, "v", "Vector operations"),
> MISA_EXT_INFO(RVG, "g", "General purpose (IMAFD_Zicsr_Zifencei)"),
> + MISA_EXT_INFO(RVB, "x-b", "Bit manipulation (Zba_Zbb_Zbs)")
> };
>
> static int riscv_validate_misa_info_idx(uint32_t bit)
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 2725528bb5..756a345513 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -69,6 +69,7 @@ typedef struct CPUArchState CPURISCVState;
> #define RVH RV('H')
> #define RVJ RV('J')
> #define RVG RV('G')
> +#define RVB RV('B')
>
> extern const uint32_t misa_bits[];
> const char *riscv_get_misa_ext_name(uint32_t bit);
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 8a35683a34..fda54671d5 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -791,6 +791,7 @@ static const RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
> MISA_CFG(RVJ, false),
> MISA_CFG(RVV, false),
> MISA_CFG(RVG, false),
> + MISA_CFG(RVB, false)
> };
>
> /*
> --
> 2.43.0
>
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] target/riscv: Enable 'B' extension on max CPU type
2024-01-11 13:02 ` Andrew Jones
@ 2024-01-11 14:53 ` Daniel Henrique Barboza
2024-01-11 15:49 ` Rob Bradford
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Henrique Barboza @ 2024-01-11 14:53 UTC (permalink / raw)
To: Andrew Jones
Cc: Rob Bradford, qemu-devel, qemu-riscv, atishp, palmer,
alistair.francis, bin.meng, liwei1518, zhiwei_liu
On 1/11/24 10:02, Andrew Jones wrote:
> On Wed, Jan 10, 2024 at 03:32:21PM -0300, Daniel Henrique Barboza wrote:
>>
>>
>> On 1/9/24 14:07, Rob Bradford wrote:
>>> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
>>> ---
>>> target/riscv/tcg/tcg-cpu.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
>>> index f10871d352..9705daec93 100644
>>> --- a/target/riscv/tcg/tcg-cpu.c
>>> +++ b/target/riscv/tcg/tcg-cpu.c
>>> @@ -999,7 +999,8 @@ static void riscv_init_max_cpu_extensions(Object *obj)
>>> const RISCVCPUMultiExtConfig *prop;
>>> /* Enable RVG, RVJ and RVV that are disabled by default */
>>> - riscv_cpu_set_misa(env, env->misa_mxl, env->misa_ext | RVG | RVJ | RVV);
>>> + riscv_cpu_set_misa(env, env->misa_mxl,
>>> + env->misa_ext | RVG | RVJ | RVV | RVB);
>>
>> I'm aware that we decided a while ago the 'max' CPU could only have non-vendor and
>> non-experimental extensions enabled. RVB is experimental, so in theory we shouldn't
>> enable it.
>>
>> But RVB is an alias for zba, zbb and zbs, extensions that the 'max' CPU is already
>> enabling. In this case I think it's sensible to enable RVB here since it would just
>> reflect stuff that it's already happening.
>
> It's also setting the B bit in misa, which, until this spec is at least
> frozen, is a reserved bit and reserved bits "must return zero when read".
This is a side effect that I wasn't aware of.
Rob, given that the 'max' CPU already has the zb* extensions enabled, is there any
other gain in enabling RVB in this CPU? If there isn't I'd rather leave this one
out for now.
Thanks,
Daniel
>
> I don't want to stand in the way of progress and it seems 99.9% likely
> that the spec will be frozen and ratified, but, if we want to stick to
> our policies (which we should document), then even the 'max' cpu type
> should require x-b be added to the command line if it wants the B bit
> set in misa.
>
> Thanks,
> drew
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
2024-01-11 13:14 ` Andrew Jones
@ 2024-01-11 15:17 ` Rob Bradford
2024-01-12 16:08 ` Andrew Jones
0 siblings, 1 reply; 19+ messages in thread
From: Rob Bradford @ 2024-01-11 15:17 UTC (permalink / raw)
To: Andrew Jones
Cc: qemu-devel, qemu-riscv, atishp, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu, ved
+ Ved
On Thu, 2024-01-11 at 14:14 +0100, Andrew Jones wrote:
> On Thu, Jan 11, 2024 at 02:07:34PM +0100, Andrew Jones wrote:
> > On Tue, Jan 09, 2024 at 05:07:35PM +0000, Rob Bradford wrote:
> > > Add the infrastructure for the 'B' extension which is the union
> > > of the
> > > Zba, Zbb and Zbs instructions.
> > >
> > > Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> > > ---
> > > target/riscv/cpu.c | 5 +++--
> > > target/riscv/cpu.h | 1 +
> > > target/riscv/tcg/tcg-cpu.c | 1 +
> > > 3 files changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > index b07a76ef6b..22f8e527ff 100644
> > > --- a/target/riscv/cpu.c
> > > +++ b/target/riscv/cpu.c
> > > @@ -38,9 +38,9 @@
> > > #include "tcg/tcg.h"
> > >
> > > /* RISC-V CPU definitions */
> > > -static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
> > > +static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
> >
> > Is there a corresponding proposed change to table 29.1 of the
> > nonpriv spec
> > which states B comes after C and before P? If so, can you provide a
> > link
> > to it? Otherwise, how do we know that?
>
> Oh, I see. The unpriv spec B chapter comes after the C chapter (and
> before
> J, P, ...). I still wonder if we'll have a 29.1 table update with the
> ratification of this extension though.
>
>
I agree it's a bit confusing - but the order is established by the
table in the unprivileged spec and the table explanation also makes
this clear.
"""
Table 27.1: Standard ISA extension names. The table also defines the
canonical order in which
extension names must appear in the name string, with top-to-bottom in
table indicating first-to-last
in the name string, e.g., RV32IMACV is legal, whereas RV32IMAVC is not.
"""
The proposed B specification does not make any remarks about the
ordering in the ISA definition string. [1] I would worry there would be
a lot of software churn if this ordering were to be changed.
Cheers,
Rob
> Thanks,
> drew
[1] - https://github.com/riscv/riscv-b
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] target/riscv: Enable 'B' extension on max CPU type
2024-01-11 14:53 ` Daniel Henrique Barboza
@ 2024-01-11 15:49 ` Rob Bradford
0 siblings, 0 replies; 19+ messages in thread
From: Rob Bradford @ 2024-01-11 15:49 UTC (permalink / raw)
To: Daniel Henrique Barboza, Andrew Jones
Cc: qemu-devel, qemu-riscv, atishp, palmer, alistair.francis,
bin.meng, liwei1518, zhiwei_liu
On Thu, 2024-01-11 at 11:53 -0300, Daniel Henrique Barboza wrote:
>
>
> On 1/11/24 10:02, Andrew Jones wrote:
> > On Wed, Jan 10, 2024 at 03:32:21PM -0300, Daniel Henrique Barboza
> > wrote:
> > >
> > >
> > > On 1/9/24 14:07, Rob Bradford wrote:
> > > > Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> > > > ---
> > > > target/riscv/tcg/tcg-cpu.c | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-
> > > > cpu.c
> > > > index f10871d352..9705daec93 100644
> > > > --- a/target/riscv/tcg/tcg-cpu.c
> > > > +++ b/target/riscv/tcg/tcg-cpu.c
> > > > @@ -999,7 +999,8 @@ static void
> > > > riscv_init_max_cpu_extensions(Object *obj)
> > > > const RISCVCPUMultiExtConfig *prop;
> > > > /* Enable RVG, RVJ and RVV that are disabled by default
> > > > */
> > > > - riscv_cpu_set_misa(env, env->misa_mxl, env->misa_ext | RVG
> > > > | RVJ | RVV);
> > > > + riscv_cpu_set_misa(env, env->misa_mxl,
> > > > + env->misa_ext | RVG | RVJ | RVV | RVB);
> > >
> > > I'm aware that we decided a while ago the 'max' CPU could only
> > > have non-vendor and
> > > non-experimental extensions enabled. RVB is experimental, so in
> > > theory we shouldn't
> > > enable it.
> > >
> > > But RVB is an alias for zba, zbb and zbs, extensions that the
> > > 'max' CPU is already
> > > enabling. In this case I think it's sensible to enable RVB here
> > > since it would
> > > just
> > >
> > > reflect stuff that it's already happening.
> >
> > It's also setting the B bit in misa, which, until this spec is at
> > least
> > frozen, is a reserved bit and reserved bits "must return zero when
> > read".
>
> This is a side effect that I wasn't aware of.
>
> Rob, given that the 'max' CPU already has the zb* extensions enabled,
> is there any
> other gain in enabling RVB in this CPU? If there isn't I'd rather
> leave this one
> out for now.
>
It seems completely reasonable to me to drop it for now.
Thanks for all the feedback,
Rob
>
> Thanks,
>
> Daniel
>
>
> >
> > I don't want to stand in the way of progress and it seems 99.9%
> > likely
> > that the spec will be frozen and ratified, but, if we want to stick
> > to
> > our policies (which we should document), then even the 'max' cpu
> > type
> > should require x-b be added to the command line if it wants the B
> > bit
> > set in misa.
> >
> > Thanks,
> > drew
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Re: Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
2024-01-11 15:17 ` Rob Bradford
@ 2024-01-12 16:08 ` Andrew Jones
2024-01-12 16:54 ` Rob Bradford
0 siblings, 1 reply; 19+ messages in thread
From: Andrew Jones @ 2024-01-12 16:08 UTC (permalink / raw)
To: Rob Bradford
Cc: qemu-devel, qemu-riscv, atishp, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu, ved
On Thu, Jan 11, 2024 at 03:17:25PM +0000, Rob Bradford wrote:
> + Ved
>
> On Thu, 2024-01-11 at 14:14 +0100, Andrew Jones wrote:
> > On Thu, Jan 11, 2024 at 02:07:34PM +0100, Andrew Jones wrote:
> > > On Tue, Jan 09, 2024 at 05:07:35PM +0000, Rob Bradford wrote:
> > > > Add the infrastructure for the 'B' extension which is the union
> > > > of the
> > > > Zba, Zbb and Zbs instructions.
> > > >
> > > > Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> > > > ---
> > > > target/riscv/cpu.c | 5 +++--
> > > > target/riscv/cpu.h | 1 +
> > > > target/riscv/tcg/tcg-cpu.c | 1 +
> > > > 3 files changed, 5 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > > index b07a76ef6b..22f8e527ff 100644
> > > > --- a/target/riscv/cpu.c
> > > > +++ b/target/riscv/cpu.c
> > > > @@ -38,9 +38,9 @@
> > > > #include "tcg/tcg.h"
> > > >
> > > > /* RISC-V CPU definitions */
> > > > -static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
> > > > +static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
> > >
> > > Is there a corresponding proposed change to table 29.1 of the
> > > nonpriv spec
> > > which states B comes after C and before P? If so, can you provide a
> > > link
> > > to it? Otherwise, how do we know that?
> >
> > Oh, I see. The unpriv spec B chapter comes after the C chapter (and
> > before
> > J, P, ...). I still wonder if we'll have a 29.1 table update with the
> > ratification of this extension though.
> >
> >
>
> I agree it's a bit confusing - but the order is established by the
> table in the unprivileged spec and the table explanation also makes
> this clear.
>
> """
> Table 27.1: Standard ISA extension names. The table also defines the
> canonical order in which
> extension names must appear in the name string, with top-to-bottom in
> table indicating first-to-last
> in the name string, e.g., RV32IMACV is legal, whereas RV32IMAVC is not.
> """
Yes, this is the table I was referring to when I referenced "table 29.1 of
the nonpriv spec". Since there's a chance I was looking at too old a spec
I've now gone straight to the source,
https://github.com/riscv/riscv-isa-manual/blob/main/src/naming.adoc
but I still don't see B there. Do you see B in the table you're looking
at?
>
> The proposed B specification does not make any remarks about the
> ordering in the ISA definition string. [1] I would worry there would be
> a lot of software churn if this ordering were to be changed.
The ordering shouldn't change, but I can't see where it's documented
(beyond the B chapter coming after the C chapter).
Thanks,
drew
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Re: Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
2024-01-12 16:08 ` Andrew Jones
@ 2024-01-12 16:54 ` Rob Bradford
2024-01-13 0:28 ` Ved Shanbhogue
0 siblings, 1 reply; 19+ messages in thread
From: Rob Bradford @ 2024-01-12 16:54 UTC (permalink / raw)
To: Andrew Jones
Cc: qemu-devel, qemu-riscv, atishp, palmer, alistair.francis,
bin.meng, liwei1518, dbarboza, zhiwei_liu, ved
On Fri, 2024-01-12 at 17:08 +0100, Andrew Jones wrote:
> On Thu, Jan 11, 2024 at 03:17:25PM +0000, Rob Bradford wrote:
> > + Ved
> >
> > On Thu, 2024-01-11 at 14:14 +0100, Andrew Jones wrote:
> > > On Thu, Jan 11, 2024 at 02:07:34PM +0100, Andrew Jones wrote:
> > > > On Tue, Jan 09, 2024 at 05:07:35PM +0000, Rob Bradford wrote:
> > > > > Add the infrastructure for the 'B' extension which is the
> > > > > union
> > > > > of the
> > > > > Zba, Zbb and Zbs instructions.
> > > > >
> > > > > Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> > > > > ---
> > > > > target/riscv/cpu.c | 5 +++--
> > > > > target/riscv/cpu.h | 1 +
> > > > > target/riscv/tcg/tcg-cpu.c | 1 +
> > > > > 3 files changed, 5 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > > > index b07a76ef6b..22f8e527ff 100644
> > > > > --- a/target/riscv/cpu.c
> > > > > +++ b/target/riscv/cpu.c
> > > > > @@ -38,9 +38,9 @@
> > > > > #include "tcg/tcg.h"
> > > > >
> > > > > /* RISC-V CPU definitions */
> > > > > -static const char riscv_single_letter_exts[] =
> > > > > "IEMAFDQCPVH";
> > > > > +static const char riscv_single_letter_exts[] =
> > > > > "IEMAFDQCBPVH";
> > > >
> > > > Is there a corresponding proposed change to table 29.1 of the
> > > > nonpriv spec
> > > > which states B comes after C and before P? If so, can you
> > > > provide a
> > > > link
> > > > to it? Otherwise, how do we know that?
> > >
> > > Oh, I see. The unpriv spec B chapter comes after the C chapter
> > > (and
> > > before
> > > J, P, ...). I still wonder if we'll have a 29.1 table update with
> > > the
> > > ratification of this extension though.
> > >
> > >
> >
> > I agree it's a bit confusing - but the order is established by the
> > table in the unprivileged spec and the table explanation also makes
> > this clear.
> >
> > """
> > Table 27.1: Standard ISA extension names. The table also defines
> > the
> > canonical order in which
> > extension names must appear in the name string, with top-to-bottom
> > in
> > table indicating first-to-last
> > in the name string, e.g., RV32IMACV is legal, whereas RV32IMAVC is
> > not.
> > """
>
> Yes, this is the table I was referring to when I referenced "table
> 29.1 of
> the nonpriv spec". Since there's a chance I was looking at too old a
> spec
> I've now gone straight to the source,
>
> https://github.com/riscv/riscv-isa-manual/blob/main/src/naming.adoc
>
> but I still don't see B there. Do you see B in the table you're
> looking
> at?
>
> >
> > The proposed B specification does not make any remarks about the
> > ordering in the ISA definition string. [1] I would worry there
> > would be
> > a lot of software churn if this ordering were to be changed.
>
> The ordering shouldn't change, but I can't see where it's documented
> (beyond the B chapter coming after the C chapter).
I'm using table 27.1 in the published PDF which is the PDF in this
release:
https://github.com/riscv/riscv-isa-manual/releases/tag/Ratified-IMAFDQC
It looks like it was removed in this commit (which is a set of
backports):
https://github.com/riscv/riscv-isa-manual/commit/6ecd735338241583d53396b7065eab7c9ace68aa
Cheers,
Rob
>
> Thanks,
> drew
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Re: Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
2024-01-12 16:54 ` Rob Bradford
@ 2024-01-13 0:28 ` Ved Shanbhogue
0 siblings, 0 replies; 19+ messages in thread
From: Ved Shanbhogue @ 2024-01-13 0:28 UTC (permalink / raw)
To: Rob Bradford
Cc: Andrew Jones, qemu-devel, qemu-riscv, atishp, palmer,
alistair.francis, bin.meng, liwei1518, dbarboza, zhiwei_liu
Rob Bradford wrote:
>I'm using table 27.1 in the published PDF which is the PDF in this
>release:
>https://github.com/riscv/riscv-isa-manual/releases/tag/Ratified-IMAFDQC
>It looks like it was removed in this commit (which is a set of
>backports):
>
We would retain the previously documented canonical order with B
between C and P and that table updated on ratification.
regards
ved
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2024-01-13 1:51 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-09 17:07 [PATCH 0/3] target/riscv: Add support for 'B' extension Rob Bradford
2024-01-09 17:07 ` [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension Rob Bradford
2024-01-10 18:18 ` Daniel Henrique Barboza
2024-01-11 13:07 ` Andrew Jones
2024-01-11 13:14 ` Andrew Jones
2024-01-11 15:17 ` Rob Bradford
2024-01-12 16:08 ` Andrew Jones
2024-01-12 16:54 ` Rob Bradford
2024-01-13 0:28 ` Ved Shanbhogue
2024-01-11 13:15 ` Andrew Jones
2024-01-09 17:07 ` [PATCH 2/3] target/riscv: Add step to validate 'B' extension Rob Bradford
2024-01-10 18:26 ` Daniel Henrique Barboza
2024-01-11 13:09 ` Andrew Jones
2024-01-09 17:07 ` [PATCH 3/3] target/riscv: Enable 'B' extension on max CPU type Rob Bradford
2024-01-10 18:32 ` Daniel Henrique Barboza
2024-01-10 18:41 ` Daniel Henrique Barboza
2024-01-11 13:02 ` Andrew Jones
2024-01-11 14:53 ` Daniel Henrique Barboza
2024-01-11 15:49 ` Rob Bradford
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).