| Message ID | 8ab7614e5df93ab5267788b73dcd75f9f5615e82.1608142916.git.alistair.francis@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: Start to remove xlen preprocess | expand |
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 <alistair.francis@wdc.com> > --- > 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 <richard.henderson@linaro.org> r~
On Thu, Dec 17, 2020 at 2:23 AM Alistair Francis <alistair.francis@wdc.com> 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 <alistair.francis@wdc.com> > --- > 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 <libfdt.h> > > -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(&hart.env); > } > [snip] Regards, Bin
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~
On Thu, 17 Dec 2020 05:58:11 PST (-0800), richard.henderson@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).
On Thu, Dec 17, 2020 at 9:25 AM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Thu, 17 Dec 2020 05:58:11 PST (-0800), richard.henderson@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).
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 <libfdt.h> -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]; + + return riscv_cpu_is_32bit(&hart.env); } -target_ulong riscv_calc_kernel_start_addr(MachineState *machine, +target_ulong riscv_calc_kernel_start_addr(RISCVHartArrayState harts, target_ulong firmware_end_addr) { - if (riscv_is_32_bit(machine)) { + if (riscv_is_32bit(harts)) { return QEMU_ALIGN_UP(firmware_end_addr, 4 * MiB); } else { return QEMU_ALIGN_UP(firmware_end_addr, 2 * MiB); @@ -259,7 +247,8 @@ void riscv_rom_copy_firmware_info(MachineState *machine, hwaddr rom_base, &address_space_memory); } -void riscv_setup_rom_reset_vec(MachineState *machine, hwaddr start_addr, +void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState harts, + hwaddr start_addr, hwaddr rom_base, hwaddr rom_size, uint64_t kernel_entry, uint32_t fdt_load_addr, void *fdt) @@ -267,7 +256,7 @@ void riscv_setup_rom_reset_vec(MachineState *machine, hwaddr start_addr, int i; uint32_t start_addr_hi32 = 0x00000000; - if (!riscv_is_32_bit(machine)) { + if (!riscv_is_32bit(harts)) { start_addr_hi32 = start_addr >> 32; } /* reset vector */ @@ -284,7 +273,7 @@ void riscv_setup_rom_reset_vec(MachineState *machine, hwaddr start_addr, 0x00000000, /* fw_dyn: */ }; - if (riscv_is_32_bit(machine)) { + if (riscv_is_32bit(harts)) { reset_vec[3] = 0x0202a583; /* lw a1, 32(t0) */ reset_vec[4] = 0x0182a283; /* lw t0, 24(t0) */ } else { diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c index 7216329237..62750007a5 100644 --- a/hw/riscv/sifive_u.c +++ b/hw/riscv/sifive_u.c @@ -453,7 +453,7 @@ static void sifive_u_machine_init(MachineState *machine) /* create device tree */ create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline, - riscv_is_32_bit(machine)); + riscv_is_32bit(s->soc.u_cpus)); if (s->start_in_flash) { /* @@ -482,7 +482,7 @@ static void sifive_u_machine_init(MachineState *machine) break; } - if (riscv_is_32_bit(machine)) { + if (riscv_is_32bit(s->soc.u_cpus)) { firmware_end_addr = riscv_find_and_load_firmware(machine, "opensbi-riscv32-generic-fw_dynamic.bin", start_addr, NULL); @@ -493,7 +493,7 @@ static void sifive_u_machine_init(MachineState *machine) } if (machine->kernel_filename) { - kernel_start_addr = riscv_calc_kernel_start_addr(machine, + kernel_start_addr = riscv_calc_kernel_start_addr(s->soc.u_cpus, firmware_end_addr); kernel_entry = riscv_load_kernel(machine->kernel_filename, @@ -520,7 +520,7 @@ static void sifive_u_machine_init(MachineState *machine) /* Compute the fdt load address in dram */ fdt_load_addr = riscv_load_fdt(memmap[SIFIVE_U_DEV_DRAM].base, machine->ram_size, s->fdt); - if (!riscv_is_32_bit(machine)) { + if (!riscv_is_32bit(s->soc.u_cpus)) { start_addr_hi32 = (uint64_t)start_addr >> 32; } @@ -539,7 +539,7 @@ static void sifive_u_machine_init(MachineState *machine) 0x00000000, /* fw_dyn: */ }; - if (riscv_is_32_bit(machine)) { + if (riscv_is_32bit(s->soc.u_cpus)) { reset_vec[4] = 0x0202a583; /* lw a1, 32(t0) */ reset_vec[5] = 0x0182a283; /* lw t0, 24(t0) */ } else { diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c index 3e47e4579d..e723ca0ac9 100644 --- a/hw/riscv/spike.c +++ b/hw/riscv/spike.c @@ -244,7 +244,7 @@ static void spike_board_init(MachineState *machine) /* create device tree */ create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline, - riscv_is_32_bit(machine)); + riscv_is_32bit(s->soc[0])); /* boot rom */ memory_region_init_rom(mask_rom, NULL, "riscv.spike.mrom", @@ -257,7 +257,7 @@ static void spike_board_init(MachineState *machine) * keeping ELF files here was intentional because BIN files don't work * for the Spike machine as HTIF emulation depends on ELF parsing. */ - if (riscv_is_32_bit(machine)) { + if (riscv_is_32bit(s->soc[0])) { firmware_end_addr = riscv_find_and_load_firmware(machine, "opensbi-riscv32-generic-fw_dynamic.elf", memmap[SPIKE_DRAM].base, @@ -270,7 +270,7 @@ static void spike_board_init(MachineState *machine) } if (machine->kernel_filename) { - kernel_start_addr = riscv_calc_kernel_start_addr(machine, + kernel_start_addr = riscv_calc_kernel_start_addr(s->soc[0], firmware_end_addr); kernel_entry = riscv_load_kernel(machine->kernel_filename, @@ -299,7 +299,7 @@ static void spike_board_init(MachineState *machine) fdt_load_addr = riscv_load_fdt(memmap[SPIKE_DRAM].base, machine->ram_size, s->fdt); /* load the reset vector */ - riscv_setup_rom_reset_vec(machine, memmap[SPIKE_DRAM].base, + riscv_setup_rom_reset_vec(machine, s->soc[0], memmap[SPIKE_DRAM].base, memmap[SPIKE_MROM].base, memmap[SPIKE_MROM].size, kernel_entry, fdt_load_addr, s->fdt); diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index 915e9ae216..485a410da7 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -603,7 +603,7 @@ static void virt_machine_init(MachineState *machine) /* create device tree */ create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline, - riscv_is_32_bit(machine)); + riscv_is_32bit(s->soc[0])); /* boot rom */ memory_region_init_rom(mask_rom, NULL, "riscv_virt_board.mrom", @@ -611,7 +611,7 @@ static void virt_machine_init(MachineState *machine) memory_region_add_subregion(system_memory, memmap[VIRT_MROM].base, mask_rom); - if (riscv_is_32_bit(machine)) { + if (riscv_is_32bit(s->soc[0])) { firmware_end_addr = riscv_find_and_load_firmware(machine, "opensbi-riscv32-generic-fw_dynamic.bin", start_addr, NULL); @@ -622,7 +622,7 @@ static void virt_machine_init(MachineState *machine) } if (machine->kernel_filename) { - kernel_start_addr = riscv_calc_kernel_start_addr(machine, + kernel_start_addr = riscv_calc_kernel_start_addr(s->soc[0], firmware_end_addr); kernel_entry = riscv_load_kernel(machine->kernel_filename, @@ -658,7 +658,8 @@ static void virt_machine_init(MachineState *machine) fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base, machine->ram_size, s->fdt); /* load the reset vector */ - riscv_setup_rom_reset_vec(machine, start_addr, virt_memmap[VIRT_MROM].base, + riscv_setup_rom_reset_vec(machine, s->soc[0], start_addr, + virt_memmap[VIRT_MROM].base, virt_memmap[VIRT_MROM].size, kernel_entry, fdt_load_addr, s->fdt);
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 <alistair.francis@wdc.com> --- 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(-)