| Message ID | dee09d708405075420b29115c1e9e87910b8da55.1648270894.git.research_trasio@irq.a4lg.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/riscv: misa to ISA string conversion fix | expand |
On Sat, Mar 26, 2022 at 3:46 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote: > > Some bits in RISC-V `misa' CSR should not be reflected in the ISA > string. For instance, `S' and `U' (represents existence of supervisor > and user mode, respectively) in `misa' CSR must not be copied since > neither `S' nor `U' are valid single-letter extensions. Thanks for the patch. > > This commit restricts which bits to copy from `misa' CSR to ISA string > with another fix: `C' extension should be preceded by `L' extension. The L extension has been removed, so it probably makes more sense to just remove it at this stage instead of fixing the order. > > It also clarifies that RISC-V extension order string is actually a > single-letter extension order list. > > Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com> > --- > target/riscv/cpu.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index ddda4906ff..84877cf24a 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -34,7 +34,7 @@ > > /* RISC-V CPU definitions */ > > -static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG"; > +static const char riscv_single_letter_exts[] = "IEMAFDQLCBJTPVNH"; What about K? Why not use IEMAFDQCBKJTPVNH instead? Alistair > > const char * const riscv_int_regnames[] = { > "x0/zero", "x1/ra", "x2/sp", "x3/gp", "x4/tp", "x5/t0", "x6/t1", > @@ -901,12 +901,12 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data) > char *riscv_isa_string(RISCVCPU *cpu) > { > int i; > - const size_t maxlen = sizeof("rv128") + sizeof(riscv_exts) + 1; > + const size_t maxlen = sizeof("rv128") + sizeof(riscv_single_letter_exts); > char *isa_str = g_new(char, maxlen); > char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", TARGET_LONG_BITS); > - for (i = 0; i < sizeof(riscv_exts); i++) { > - if (cpu->env.misa_ext & RV(riscv_exts[i])) { > - *p++ = qemu_tolower(riscv_exts[i]); > + for (i = 0; i < sizeof(riscv_single_letter_exts) - 1; i++) { > + if (cpu->env.misa_ext & RV(riscv_single_letter_exts[i])) { > + *p++ = qemu_tolower(riscv_single_letter_exts[i]); > } > } > *p = '\0'; > -- > 2.32.0 > >
On Mon, Mar 28, 2022 at 7:30 AM Alistair Francis <alistair23@gmail.com> wrote: > On Sat, Mar 26, 2022 at 3:46 PM Tsukasa OI <research_trasio@irq.a4lg.com> > wrote: > > > > Some bits in RISC-V `misa' CSR should not be reflected in the ISA > > string. For instance, `S' and `U' (represents existence of supervisor > > and user mode, respectively) in `misa' CSR must not be copied since > > neither `S' nor `U' are valid single-letter extensions. > > Thanks for the patch. > > > > > This commit restricts which bits to copy from `misa' CSR to ISA string > > with another fix: `C' extension should be preceded by `L' extension. > > The L extension has been removed, so it probably makes more sense to > just remove it at this stage instead of fixing the order. > > > > > It also clarifies that RISC-V extension order string is actually a > > single-letter extension order list. > > > > Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com> > > --- > > target/riscv/cpu.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > index ddda4906ff..84877cf24a 100644 > > --- a/target/riscv/cpu.c > > +++ b/target/riscv/cpu.c > > @@ -34,7 +34,7 @@ > > > > /* RISC-V CPU definitions */ > > > > -static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG"; > > +static const char riscv_single_letter_exts[] = "IEMAFDQLCBJTPVNH"; > > What about K? > > Why not use IEMAFDQCBKJTPVNH instead? > > Alistair > The RISC-V ISA Manual (version 20191213) is quite old. Where "L" was not removed and "K" was not introduced. It seems Unprivileged spec is not "ratified" as often as Privileged spec does (most of them are drafts...). But I agree that it's better to remove "L" and add "K" into ISA string. Regards, Frank Chang > > > > > const char * const riscv_int_regnames[] = { > > "x0/zero", "x1/ra", "x2/sp", "x3/gp", "x4/tp", "x5/t0", "x6/t1", > > @@ -901,12 +901,12 @@ static void riscv_cpu_class_init(ObjectClass *c, > void *data) > > char *riscv_isa_string(RISCVCPU *cpu) > > { > > int i; > > - const size_t maxlen = sizeof("rv128") + sizeof(riscv_exts) + 1; > > + const size_t maxlen = sizeof("rv128") + > sizeof(riscv_single_letter_exts); > > char *isa_str = g_new(char, maxlen); > > char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", > TARGET_LONG_BITS); > > - for (i = 0; i < sizeof(riscv_exts); i++) { > > - if (cpu->env.misa_ext & RV(riscv_exts[i])) { > > - *p++ = qemu_tolower(riscv_exts[i]); > > + for (i = 0; i < sizeof(riscv_single_letter_exts) - 1; i++) { > > + if (cpu->env.misa_ext & RV(riscv_single_letter_exts[i])) { > > + *p++ = qemu_tolower(riscv_single_letter_exts[i]); > > } > > } > > *p = '\0'; > > -- > > 2.32.0 > > > > > >
Hi Alistair, On 2022/03/28 8:29, Alistair Francis wrote: > On Sat, Mar 26, 2022 at 3:46 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote: >> >> Some bits in RISC-V `misa' CSR should not be reflected in the ISA >> string. For instance, `S' and `U' (represents existence of supervisor >> and user mode, respectively) in `misa' CSR must not be copied since >> neither `S' nor `U' are valid single-letter extensions. > > Thanks for the patch. > >> >> This commit restricts which bits to copy from `misa' CSR to ISA string >> with another fix: `C' extension should be preceded by `L' extension. > > The L extension has been removed, so it probably makes more sense to > just remove it at this stage instead of fixing the order. Thanks. I'll reflect this in v2. > >> >> It also clarifies that RISC-V extension order string is actually a >> single-letter extension order list. >> >> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com> >> --- >> target/riscv/cpu.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >> index ddda4906ff..84877cf24a 100644 >> --- a/target/riscv/cpu.c >> +++ b/target/riscv/cpu.c >> @@ -34,7 +34,7 @@ >> >> /* RISC-V CPU definitions */ >> >> -static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG"; >> +static const char riscv_single_letter_exts[] = "IEMAFDQLCBJTPVNH"; > > What about K? Not having "K" in the string is completely intentional in PATCH v1. riscv_single_letter_exts is intended to be valid single-letter extensions (not canonical order of multi-letter Z* extensions). Considering this (and the fact that we can remove "L"), correct fix to this might be to "remove B and J", not to "add K". "B", "K" and "J" are not going to be single-letter extensions (for now) but prefixes of subextensions (Zb*, Zk* and non-ratified proposal(s?) with Zj*). For "B" and "J", this commit might help understanding it: https://github.com/riscv/riscv-isa-manual/commit/c1c77c4902d5e7c58613d725d09d66a2a743da1c For "K", https://github.com/riscv/riscv-crypto/releases/tag/v0.9.3-scalar (with misa.K) https://github.com/riscv/riscv-crypto/releases/tag/v0.9.4-scalar (WITHOUT misa.K) https://github.com/riscv/riscv-crypto/releases/tag/v1.0.0-scalar (ratified; WITHOUT misa.K) https://lists.riscv.org/g/tech-crypto-ext/topic/riscv_k_in_misa_risc_v/85354476 Dropped "N" and "T" extensions are to be removed, too. Not-yet-ratified "P" could be removed for now but will be kept in v2. > > Why not use IEMAFDQCBKJTPVNH instead? I noticed the order "K" -> "J" (in the draft RISC-V ISA Manual). That will make a patch to GNU Binutils (that has "J" -> "K" order). Thanks! Tsukasa > > Alistair > >> >> const char * const riscv_int_regnames[] = { >> "x0/zero", "x1/ra", "x2/sp", "x3/gp", "x4/tp", "x5/t0", "x6/t1", >> @@ -901,12 +901,12 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data) >> char *riscv_isa_string(RISCVCPU *cpu) >> { >> int i; >> - const size_t maxlen = sizeof("rv128") + sizeof(riscv_exts) + 1; >> + const size_t maxlen = sizeof("rv128") + sizeof(riscv_single_letter_exts); >> char *isa_str = g_new(char, maxlen); >> char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", TARGET_LONG_BITS); >> - for (i = 0; i < sizeof(riscv_exts); i++) { >> - if (cpu->env.misa_ext & RV(riscv_exts[i])) { >> - *p++ = qemu_tolower(riscv_exts[i]); >> + for (i = 0; i < sizeof(riscv_single_letter_exts) - 1; i++) { >> + if (cpu->env.misa_ext & RV(riscv_single_letter_exts[i])) { >> + *p++ = qemu_tolower(riscv_single_letter_exts[i]); >> } >> } >> *p = '\0'; >> -- >> 2.32.0 >> >> >
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index ddda4906ff..84877cf24a 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -34,7 +34,7 @@ /* RISC-V CPU definitions */ -static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG"; +static const char riscv_single_letter_exts[] = "IEMAFDQLCBJTPVNH"; const char * const riscv_int_regnames[] = { "x0/zero", "x1/ra", "x2/sp", "x3/gp", "x4/tp", "x5/t0", "x6/t1", @@ -901,12 +901,12 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data) char *riscv_isa_string(RISCVCPU *cpu) { int i; - const size_t maxlen = sizeof("rv128") + sizeof(riscv_exts) + 1; + const size_t maxlen = sizeof("rv128") + sizeof(riscv_single_letter_exts); char *isa_str = g_new(char, maxlen); char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", TARGET_LONG_BITS); - for (i = 0; i < sizeof(riscv_exts); i++) { - if (cpu->env.misa_ext & RV(riscv_exts[i])) { - *p++ = qemu_tolower(riscv_exts[i]); + for (i = 0; i < sizeof(riscv_single_letter_exts) - 1; i++) { + if (cpu->env.misa_ext & RV(riscv_single_letter_exts[i])) { + *p++ = qemu_tolower(riscv_single_letter_exts[i]); } } *p = '\0';
Some bits in RISC-V `misa' CSR should not be reflected in the ISA string. For instance, `S' and `U' (represents existence of supervisor and user mode, respectively) in `misa' CSR must not be copied since neither `S' nor `U' are valid single-letter extensions. This commit restricts which bits to copy from `misa' CSR to ISA string with another fix: `C' extension should be preceded by `L' extension. It also clarifies that RISC-V extension order string is actually a single-letter extension order list. Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com> --- target/riscv/cpu.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)