Re: [PATCH v4 16/16] hw/riscv: Use the CPU to determine if 32-bit
On Thu, Dec 17, 2020 at 9:25 AM Palmer Dabbelt wrote: > > On Thu, 17 Dec 2020 05:58:11 PST (-0800), richard.hender...@linaro.org wrote: > > On 12/17/20 12:44 AM, Bin Meng wrote: > >> What happens if something like ARM big.LITTLE needs to be supported on > >> RISC-V? > > > > I'd say it's the board's job to pass the boot heart. > > (Though even big.LITTLE doesn't mix 64 and 32-bit cores.) > > I guess we can't stop people from building crazy things, but it does seem like > building a system that mixes up base ISAs is unlikely. IDK if 32-bit > compatibility on 64-bit systems is ever going to be important enough to show > up > on RISC-V, as 32-bit might be defunct in portable systems by the point real > RISC-V systems are availiable, but one could imagine systems where only a > subset of cores have 32-bit compatibility. My guess is that they'd all boot > into 64-bit mode, though, so that sort of stuff won't be relevant until one > tries to get to 32-bit code. > > Regardless of where that sort of thing goes, it seems reasonable to just ban > people from spinning up mixed machine XLEN systems in QEMU for the time being. > IIUC that's always been impossible (as it was a #define before), so it's not > like we're regressing on any functionality with that constraint. I agree. Currently we don't get anywhere close to allowing mixed XLEN systems. Allowing 32-bit and 64-bit CPUs on a system is a useful goal, as for example that is what the HiFive 1 has. As richard said though the main interest at this point is the boot CPU, which the board can specify different harts depending on what it is doing. This is just the first step to making RISC-V more flexible with XLEN, there will be more patches in the future to continue improving this. I'm going to apply this series today. Alistair > > Other hetergenous ISA/microarctucture stuff seems reasonable to support in > QEMU, but also not a high priority. It doesn't seem that hard to write the > early boot stuff in a way such that it only depends on the base ISA -- that's > essentially how we're handling stuff like F/D in Linux, you just either probe > or handle the traps. There's already some simple hetergenous stuff floating > around (like the non-MMU cores in the SiFive designs) and that all seems to > work fine so my guess is we have enough stuff there, but I'm sure there'll be > more work do to as more complicated designs start to show up (doubly so as > there's no specs for any of this).
Re: [PATCH v4 16/16] hw/riscv: Use the CPU to determine if 32-bit
On Thu, 17 Dec 2020 05:58:11 PST (-0800), richard.hender...@linaro.org wrote: On 12/17/20 12:44 AM, Bin Meng wrote: What happens if something like ARM big.LITTLE needs to be supported on RISC-V? I'd say it's the board's job to pass the boot heart. (Though even big.LITTLE doesn't mix 64 and 32-bit cores.) I guess we can't stop people from building crazy things, but it does seem like building a system that mixes up base ISAs is unlikely. IDK if 32-bit compatibility on 64-bit systems is ever going to be important enough to show up on RISC-V, as 32-bit might be defunct in portable systems by the point real RISC-V systems are availiable, but one could imagine systems where only a subset of cores have 32-bit compatibility. My guess is that they'd all boot into 64-bit mode, though, so that sort of stuff won't be relevant until one tries to get to 32-bit code. Regardless of where that sort of thing goes, it seems reasonable to just ban people from spinning up mixed machine XLEN systems in QEMU for the time being. IIUC that's always been impossible (as it was a #define before), so it's not like we're regressing on any functionality with that constraint. Other hetergenous ISA/microarctucture stuff seems reasonable to support in QEMU, but also not a high priority. It doesn't seem that hard to write the early boot stuff in a way such that it only depends on the base ISA -- that's essentially how we're handling stuff like F/D in Linux, you just either probe or handle the traps. There's already some simple hetergenous stuff floating around (like the non-MMU cores in the SiFive designs) and that all seems to work fine so my guess is we have enough stuff there, but I'm sure there'll be more work do to as more complicated designs start to show up (doubly so as there's no specs for any of this).
Re: [PATCH v4 16/16] hw/riscv: Use the CPU to determine if 32-bit
On 12/17/20 12:44 AM, Bin Meng wrote: > What happens if something like ARM big.LITTLE needs to be supported on RISC-V? I'd say it's the board's job to pass the boot heart. (Though even big.LITTLE doesn't mix 64 and 32-bit cores.) r~
Re: [PATCH v4 16/16] hw/riscv: Use the CPU to determine if 32-bit
On Thu, Dec 17, 2020 at 2:23 AM Alistair Francis
wrote:
>
> Instead of using string compares to determine if a RISC-V machine is
> using 32-bit or 64-bit CPUs we can use the initalised CPUs. This avoids
> us having to maintain a list of CPU names to compare against.
>
> This commit also fixes the name of the function to match the
> riscv_cpu_is_32bit() function.
>
> Signed-off-by: Alistair Francis
> ---
> include/hw/riscv/boot.h | 8 +---
> hw/riscv/boot.c | 31 ++-
> hw/riscv/sifive_u.c | 10 +-
> hw/riscv/spike.c| 8
> hw/riscv/virt.c | 9 +
> 5 files changed, 29 insertions(+), 37 deletions(-)
>
> diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> index b6d37a91d6..20ff5fe5e5 100644
> --- a/include/hw/riscv/boot.h
> +++ b/include/hw/riscv/boot.h
> @@ -22,10 +22,11 @@
>
> #include "exec/cpu-defs.h"
> #include "hw/loader.h"
> +#include "hw/riscv/riscv_hart.h"
>
> -bool riscv_is_32_bit(MachineState *machine);
> +bool riscv_is_32bit(RISCVHartArrayState harts);
>
> -target_ulong riscv_calc_kernel_start_addr(MachineState *machine,
> +target_ulong riscv_calc_kernel_start_addr(RISCVHartArrayState harts,
>target_ulong firmware_end_addr);
> target_ulong riscv_find_and_load_firmware(MachineState *machine,
>const char
> *default_machine_firmware,
> @@ -41,7 +42,8 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
> hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
> uint64_t kernel_entry, hwaddr *start);
> uint32_t riscv_load_fdt(hwaddr dram_start, uint64_t dram_size, void *fdt);
> -void riscv_setup_rom_reset_vec(MachineState *machine, hwaddr saddr,
> +void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState
> harts,
> + hwaddr saddr,
> hwaddr rom_base, hwaddr rom_size,
> uint64_t kernel_entry,
> uint32_t fdt_load_addr, void *fdt);
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 6bce6fb485..83586aef41 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -33,28 +33,16 @@
>
> #include
>
> -bool riscv_is_32_bit(MachineState *machine)
> +bool riscv_is_32bit(RISCVHartArrayState harts)
> {
> -/*
> - * 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;
> -}
> +RISCVCPU hart = harts.harts[0];
What happens if something like ARM big.LITTLE needs to be supported on RISC-V?
> +
> +return riscv_cpu_is_32bit();
> }
>
[snip]
Regards,
Bin
Re: [PATCH v4 16/16] hw/riscv: Use the CPU to determine if 32-bit
On 12/16/20 12:23 PM, Alistair Francis wrote: > Instead of using string compares to determine if a RISC-V machine is > using 32-bit or 64-bit CPUs we can use the initalised CPUs. This avoids > us having to maintain a list of CPU names to compare against. > > This commit also fixes the name of the function to match the > riscv_cpu_is_32bit() function. > > Signed-off-by: Alistair Francis > --- > include/hw/riscv/boot.h | 8 +--- > hw/riscv/boot.c | 31 ++- > hw/riscv/sifive_u.c | 10 +- > hw/riscv/spike.c| 8 > hw/riscv/virt.c | 9 + > 5 files changed, 29 insertions(+), 37 deletions(-) Reviewed-by: Richard Henderson r~
