| Message ID | 3b6338af937d0d3aa0858ba1a4ad0fd9e759be66.1607967113.git.alistair.francis@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RISC-V: Start to remove xlen preprocess | expand |
On Tue, Dec 15, 2020 at 4:34 AM Alistair Francis <alistair.francis@wdc.com> wrote: > > Currently the riscv_is_32_bit() function only supports the generic rv32 > CPUs. Extend the function to support the SiFive and LowRISC CPUs as > well. > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > hw/riscv/boot.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > index d62f3dc758..3c70ac75d7 100644 > --- a/hw/riscv/boot.c > +++ b/hw/riscv/boot.c > @@ -41,7 +41,17 @@ > > bool riscv_is_32_bit(MachineState *machine) > { > - if (!strncmp(machine->cpu_type, "rv32", 4)) { > + /* > + * To determine if the CPU is 32-bit we need to check a few different CPUs. > + * > + * If the CPU starts with rv32 > + * If the CPU is a sifive 3 seriries CPU (E31, U34) > + * If it's the Ibex CPU > + */ > + if (!strncmp(machine->cpu_type, "rv32", 4) || > + (!strncmp(machine->cpu_type, "sifive", 6) && > + machine->cpu_type[8] == '3') || > + !strncmp(machine->cpu_type, "lowrisc-ibex", 12)) { This does not look like a scalable way. With more and more CPU added in the future, this may end up with a huge list of strncmps.. > return true; > } else { > return false; Regards, Bin
On Tue, Dec 15, 2020 at 1:26 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > On Tue, Dec 15, 2020 at 4:34 AM Alistair Francis > <alistair.francis@wdc.com> wrote: > > > > Currently the riscv_is_32_bit() function only supports the generic rv32 > > CPUs. Extend the function to support the SiFive and LowRISC CPUs as > > well. > > > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > hw/riscv/boot.c | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > > index d62f3dc758..3c70ac75d7 100644 > > --- a/hw/riscv/boot.c > > +++ b/hw/riscv/boot.c > > @@ -41,7 +41,17 @@ > > > > bool riscv_is_32_bit(MachineState *machine) > > { > > - if (!strncmp(machine->cpu_type, "rv32", 4)) { > > + /* > > + * To determine if the CPU is 32-bit we need to check a few different CPUs. > > + * > > + * If the CPU starts with rv32 > > + * If the CPU is a sifive 3 seriries CPU (E31, U34) > > + * If it's the Ibex CPU > > + */ > > + if (!strncmp(machine->cpu_type, "rv32", 4) || > > + (!strncmp(machine->cpu_type, "sifive", 6) && > > + machine->cpu_type[8] == '3') || > > + !strncmp(machine->cpu_type, "lowrisc-ibex", 12)) { > > This does not look like a scalable way. With more and more CPU added > in the future, this may end up with a huge list of strncmps.. Any better ideas? It should handle all SiFive CPUs, besides that we don't have that many CPUs. Alistair > > > return true; > > } else { > > return false; > > Regards, > Bin
On 12/15/20 10:44 AM, Alistair Francis wrote: > On Tue, Dec 15, 2020 at 1:26 AM Bin Meng <bmeng.cn@gmail.com> wrote: >> >> On Tue, Dec 15, 2020 at 4:34 AM Alistair Francis >> <alistair.francis@wdc.com> wrote: >>> >>> Currently the riscv_is_32_bit() function only supports the generic rv32 >>> CPUs. Extend the function to support the SiFive and LowRISC CPUs as >>> well. >>> >>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com> >>> --- >>> hw/riscv/boot.c | 12 +++++++++++- >>> 1 file changed, 11 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c >>> index d62f3dc758..3c70ac75d7 100644 >>> --- a/hw/riscv/boot.c >>> +++ b/hw/riscv/boot.c >>> @@ -41,7 +41,17 @@ >>> >>> bool riscv_is_32_bit(MachineState *machine) >>> { >>> - if (!strncmp(machine->cpu_type, "rv32", 4)) { >>> + /* >>> + * To determine if the CPU is 32-bit we need to check a few different CPUs. >>> + * >>> + * If the CPU starts with rv32 >>> + * If the CPU is a sifive 3 seriries CPU (E31, U34) >>> + * If it's the Ibex CPU >>> + */ >>> + if (!strncmp(machine->cpu_type, "rv32", 4) || >>> + (!strncmp(machine->cpu_type, "sifive", 6) && >>> + machine->cpu_type[8] == '3') || >>> + !strncmp(machine->cpu_type, "lowrisc-ibex", 12)) { >> >> This does not look like a scalable way. With more and more CPU added >> in the future, this may end up with a huge list of strncmps.. > > Any better ideas? It doesn't appear as if you need to query this before you've instantiated the cpu. The first dynamic use that I see is create_fdt, which happens after that point. Verified like so: $ gdb --args ./qemu-system-riscv64 -M virt (gdb) b rv64_base_cpu_init Breakpoint 1 at 0x548390: file ../qemu/target/riscv/cpu.c, line 156. (gdb) b riscv_is_32_bit Breakpoint 2 at 0x490dd0: file ../qemu/hw/riscv/boot.c, line 37. (gdb) run ... Thread 1 "qemu-system-ris" hit Breakpoint 1, rv64_base_cpu_init >From the machine, you can find the boot cpu, and use the riscv_cpu_is_32bit helper on that. Also, just noticed that the two functions spell "32bit" differently. :-) r~
On Tue, Dec 15, 2020 at 1:25 PM Richard Henderson <richard.henderson@linaro.org> wrote: > > On 12/15/20 10:44 AM, Alistair Francis wrote: > > On Tue, Dec 15, 2020 at 1:26 AM Bin Meng <bmeng.cn@gmail.com> wrote: > >> > >> On Tue, Dec 15, 2020 at 4:34 AM Alistair Francis > >> <alistair.francis@wdc.com> wrote: > >>> > >>> Currently the riscv_is_32_bit() function only supports the generic rv32 > >>> CPUs. Extend the function to support the SiFive and LowRISC CPUs as > >>> well. > >>> > >>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > >>> --- > >>> hw/riscv/boot.c | 12 +++++++++++- > >>> 1 file changed, 11 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > >>> index d62f3dc758..3c70ac75d7 100644 > >>> --- a/hw/riscv/boot.c > >>> +++ b/hw/riscv/boot.c > >>> @@ -41,7 +41,17 @@ > >>> > >>> bool riscv_is_32_bit(MachineState *machine) > >>> { > >>> - if (!strncmp(machine->cpu_type, "rv32", 4)) { > >>> + /* > >>> + * To determine if the CPU is 32-bit we need to check a few different CPUs. > >>> + * > >>> + * If the CPU starts with rv32 > >>> + * If the CPU is a sifive 3 seriries CPU (E31, U34) > >>> + * If it's the Ibex CPU > >>> + */ > >>> + if (!strncmp(machine->cpu_type, "rv32", 4) || > >>> + (!strncmp(machine->cpu_type, "sifive", 6) && > >>> + machine->cpu_type[8] == '3') || > >>> + !strncmp(machine->cpu_type, "lowrisc-ibex", 12)) { > >> > >> This does not look like a scalable way. With more and more CPU added > >> in the future, this may end up with a huge list of strncmps.. > > > > Any better ideas? > > It doesn't appear as if you need to query this before you've instantiated the > cpu. The first dynamic use that I see is create_fdt, which happens after that > point. > > Verified like so: > > $ gdb --args ./qemu-system-riscv64 -M virt > (gdb) b rv64_base_cpu_init > Breakpoint 1 at 0x548390: file ../qemu/target/riscv/cpu.c, line 156. > (gdb) b riscv_is_32_bit > Breakpoint 2 at 0x490dd0: file ../qemu/hw/riscv/boot.c, line 37. > (gdb) run > ... > Thread 1 "qemu-system-ris" hit Breakpoint 1, rv64_base_cpu_init > > >From the machine, you can find the boot cpu, and use the riscv_cpu_is_32bit > helper on that. > > Also, just noticed that the two functions spell "32bit" differently. :-) Thanks for the help Richard. I have added a commit in v4 that changes the function like you mention. The rebased ended up being a little tricky so I added the commit to the end instead. Alistair > > > r~
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c index d62f3dc758..3c70ac75d7 100644 --- a/hw/riscv/boot.c +++ b/hw/riscv/boot.c @@ -41,7 +41,17 @@ bool riscv_is_32_bit(MachineState *machine) { - if (!strncmp(machine->cpu_type, "rv32", 4)) { + /* + * To determine if the CPU is 32-bit we need to check a few different CPUs. + * + * If the CPU starts with rv32 + * If the CPU is a sifive 3 seriries CPU (E31, U34) + * If it's the Ibex CPU + */ + if (!strncmp(machine->cpu_type, "rv32", 4) || + (!strncmp(machine->cpu_type, "sifive", 6) && + machine->cpu_type[8] == '3') || + !strncmp(machine->cpu_type, "lowrisc-ibex", 12)) { return true; } else { return false;
Currently the riscv_is_32_bit() function only supports the generic rv32 CPUs. Extend the function to support the SiFive and LowRISC CPUs as well. Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- hw/riscv/boot.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)