Re: [PATCH v3 01/15] hw/riscv: Expand the is 32-bit check to support more CPUs

[Richard Henderson]

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~