| Message ID | 4a4c11213a161a7eedabe46abe58b351bb0e2ef2.1648473008.git.research_trasio@irq.a4lg.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: misa to ISA string conversion fix | expand |
Hello, Le 28/03/2022 à 15:11, Tsukasa OI a écrit : > 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 also removes all reserved/dropped single-letter "extensions" > from the list. > > - "B": Not going to be a single-letter extension (misa.B is reserved). > - "J": Not going to be a single-letter extension (misa.J is reserved). > - "K": Not going to be a single-letter extension (misa.K is reserved). > - "L": Dropped. > - "N": Dropped. > - "T": Dropped. > > It also clarifies that the variable `riscv_single_letter_exts' is 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..1f68c696eb 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[] = "IEMAFDQCPVH"; > > 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); To avoid one dependency on TARGET_LONG_BITS that is not necessary now that we have mxl, I would suggest to replace the previous line with: char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", 1 << (4 + cpu->env.misa_mxl_max)); Frédéric
On 2022/03/30 1:29, Frédéric Pétrot wrote: > Hello, > > Le 28/03/2022 à 15:11, Tsukasa OI a écrit : >> 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 also removes all reserved/dropped single-letter "extensions" >> from the list. >> >> - "B": Not going to be a single-letter extension (misa.B is reserved). >> - "J": Not going to be a single-letter extension (misa.J is reserved). >> - "K": Not going to be a single-letter extension (misa.K is reserved). >> - "L": Dropped. >> - "N": Dropped. >> - "T": Dropped. >> >> It also clarifies that the variable `riscv_single_letter_exts' is 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..1f68c696eb 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[] = "IEMAFDQCPVH"; >> 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); > > To avoid one dependency on TARGET_LONG_BITS that is not necessary now that we > have mxl, I would suggest to replace the previous line with: > char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", > 1 << (4 + cpu->env.misa_mxl_max)); LGTM except... that won't be a part of my patch (you are trying to fix separate issue). You can submit separate patch for this. To comment, I like 16 << cpu->env.misa_mxl_max rather than 1 << (4 + cpu->env.misa_mxl_max) for consistency with target/riscv/gdbstub.c: int bitsize = 16 << env->misa_mxl_max; Tsukasa > > Frédéric >
On Mon, Mar 28, 2022 at 11:11 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. > > This commit also removes all reserved/dropped single-letter "extensions" > from the list. > > - "B": Not going to be a single-letter extension (misa.B is reserved). > - "J": Not going to be a single-letter extension (misa.J is reserved). > - "K": Not going to be a single-letter extension (misa.K is reserved). Interesting, these are still listed in "ISA Extension Naming Conventions". They can just be re-added if they are used in the future. Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > - "L": Dropped. > - "N": Dropped. > - "T": Dropped. > > It also clarifies that the variable `riscv_single_letter_exts' is 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..1f68c696eb 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[] = "IEMAFDQCPVH"; > > 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 >
Le 30/03/2022 à 04:08, Tsukasa OI a écrit : > On 2022/03/30 1:29, Frédéric Pétrot wrote: >> Hello, >> >> Le 28/03/2022 à 15:11, Tsukasa OI a écrit : >>> 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 also removes all reserved/dropped single-letter "extensions" >>> from the list. >>> >>> - "B": Not going to be a single-letter extension (misa.B is reserved). >>> - "J": Not going to be a single-letter extension (misa.J is reserved). >>> - "K": Not going to be a single-letter extension (misa.K is reserved). >>> - "L": Dropped. >>> - "N": Dropped. >>> - "T": Dropped. >>> >>> It also clarifies that the variable `riscv_single_letter_exts' is 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..1f68c696eb 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[] = "IEMAFDQCPVH"; >>> 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); >> >> To avoid one dependency on TARGET_LONG_BITS that is not necessary now that we >> have mxl, I would suggest to replace the previous line with: >> char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", >> 1 << (4 + cpu->env.misa_mxl_max)); > > LGTM except... that won't be a part of my patch (you are trying to fix > separate issue). You can submit separate patch for this. Sure, I'll do that. > To comment, I like > 16 << cpu->env.misa_mxl_max > rather than > 1 << (4 + cpu->env.misa_mxl_max) > for consistency with target/riscv/gdbstub.c: > int bitsize = 16 << env->misa_mxl_max; Good point. However, perhaps I should rather change target/riscv/gdbstub.c, to better fit the note page 16 (no pun intended) of the priviledged spec that expresses bitsize as $2^{MXL+4}$. Frédéric > > Tsukasa > >> >> Frédéric >>
On Mon, Mar 28, 2022 at 11:11 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. > > This commit also removes all reserved/dropped single-letter "extensions" > from the list. > > - "B": Not going to be a single-letter extension (misa.B is reserved). > - "J": Not going to be a single-letter extension (misa.J is reserved). > - "K": Not going to be a single-letter extension (misa.K is reserved). > - "L": Dropped. > - "N": Dropped. > - "T": Dropped. > > It also clarifies that the variable `riscv_single_letter_exts' is a > single-letter extension order list. > > Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com> Thanks! Applied to riscv-to-apply.next Alistair > --- > 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..1f68c696eb 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[] = "IEMAFDQCPVH"; > > 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..1f68c696eb 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[] = "IEMAFDQCPVH"; 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 also removes all reserved/dropped single-letter "extensions" from the list. - "B": Not going to be a single-letter extension (misa.B is reserved). - "J": Not going to be a single-letter extension (misa.J is reserved). - "K": Not going to be a single-letter extension (misa.K is reserved). - "L": Dropped. - "N": Dropped. - "T": Dropped. It also clarifies that the variable `riscv_single_letter_exts' is 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(-)