| Message ID | 20230530194623.272652-3-dbarboza@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv, KVM: fixes and enhancements | expand |
On Tue, May 30, 2023 at 04:46:09PM -0300, Daniel Henrique Barboza wrote: > The absence of a satp mode in riscv_host_cpu_init() is causing the > following error: > > $ sudo ./qemu/build/qemu-system-riscv64 -machine virt,accel=kvm \ > -m 2G -smp 1 -nographic -snapshot \ > -kernel ./guest_imgs/Image \ > -initrd ./guest_imgs/rootfs_kvm_riscv64.img \ > -append "earlycon=sbi root=/dev/ram rw" \ > -cpu host > ** > ERROR:../target/riscv/cpu.c:320:satp_mode_str: code should not be > reached > Bail out! ERROR:../target/riscv/cpu.c:320:satp_mode_str: code should > not be reached > Aborted > > The error is triggered from create_fdt_socket_cpus() in hw/riscv/virt.c. > It's trying to get satp_mode_str for a NULL cpu->cfg.satp_mode.map. > > For this KVM 'cpu' we would need to inherit the satp supported modes > from the RISC-V host. At this moment this is not possible because the > KVM driver does not support it. And even when it does we can't just let > this broken for every other older kernel. > > Skip the 'mmu-type' FDT node if there's no satp_mode set. This seems reasonable, since mmu-type is not a required node, according to [1], and there's nothing we can put in it, which would be correct, until we can get the information from KVM. [1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/cpu.yaml > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > hw/riscv/virt.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index 4e3efbee16..8aa907e81f 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -243,13 +243,13 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket, > s->soc[socket].hartid_base + cpu); > qemu_fdt_add_subnode(ms->fdt, cpu_name); > > - satp_mode_max = satp_mode_max_from_map( > - s->soc[socket].harts[cpu].cfg.satp_mode.map); > - sv_name = g_strdup_printf("riscv,%s", > - satp_mode_str(satp_mode_max, is_32_bit)); > - qemu_fdt_setprop_string(ms->fdt, cpu_name, "mmu-type", sv_name); > - g_free(sv_name); > - > + if (cpu_ptr->cfg.satp_mode.supported != 0) { > + satp_mode_max = satp_mode_max_from_map(cpu_ptr->cfg.satp_mode.map); > + sv_name = g_strdup_printf("riscv,%s", > + satp_mode_str(satp_mode_max, is_32_bit)); > + qemu_fdt_setprop_string(ms->fdt, cpu_name, "mmu-type", sv_name); > + g_free(sv_name); > + } > > name = riscv_isa_string(cpu_ptr); > qemu_fdt_setprop_string(ms->fdt, cpu_name, "riscv,isa", name); > -- > 2.40.1 > > Adding a sentence, like what I wrote above, to the commit message in order to provide better justification might be nice, but either way Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Thanks, drew
On 6/6/23 10:13, Andrew Jones wrote: > On Tue, May 30, 2023 at 04:46:09PM -0300, Daniel Henrique Barboza wrote: >> The absence of a satp mode in riscv_host_cpu_init() is causing the >> following error: >> >> $ sudo ./qemu/build/qemu-system-riscv64 -machine virt,accel=kvm \ >> -m 2G -smp 1 -nographic -snapshot \ >> -kernel ./guest_imgs/Image \ >> -initrd ./guest_imgs/rootfs_kvm_riscv64.img \ >> -append "earlycon=sbi root=/dev/ram rw" \ >> -cpu host >> ** >> ERROR:../target/riscv/cpu.c:320:satp_mode_str: code should not be >> reached >> Bail out! ERROR:../target/riscv/cpu.c:320:satp_mode_str: code should >> not be reached >> Aborted >> >> The error is triggered from create_fdt_socket_cpus() in hw/riscv/virt.c. >> It's trying to get satp_mode_str for a NULL cpu->cfg.satp_mode.map. >> >> For this KVM 'cpu' we would need to inherit the satp supported modes >> from the RISC-V host. At this moment this is not possible because the >> KVM driver does not support it. And even when it does we can't just let >> this broken for every other older kernel. >> >> Skip the 'mmu-type' FDT node if there's no satp_mode set. > > This seems reasonable, since mmu-type is not a required node, according to > [1], and there's nothing we can put in it, which would be correct, until > we can get the information from KVM. > > [1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/cpu.yaml > >> >> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >> --- >> hw/riscv/virt.c | 14 +++++++------- >> 1 file changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c >> index 4e3efbee16..8aa907e81f 100644 >> --- a/hw/riscv/virt.c >> +++ b/hw/riscv/virt.c >> @@ -243,13 +243,13 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket, >> s->soc[socket].hartid_base + cpu); >> qemu_fdt_add_subnode(ms->fdt, cpu_name); >> >> - satp_mode_max = satp_mode_max_from_map( >> - s->soc[socket].harts[cpu].cfg.satp_mode.map); >> - sv_name = g_strdup_printf("riscv,%s", >> - satp_mode_str(satp_mode_max, is_32_bit)); >> - qemu_fdt_setprop_string(ms->fdt, cpu_name, "mmu-type", sv_name); >> - g_free(sv_name); >> - >> + if (cpu_ptr->cfg.satp_mode.supported != 0) { >> + satp_mode_max = satp_mode_max_from_map(cpu_ptr->cfg.satp_mode.map); >> + sv_name = g_strdup_printf("riscv,%s", >> + satp_mode_str(satp_mode_max, is_32_bit)); >> + qemu_fdt_setprop_string(ms->fdt, cpu_name, "mmu-type", sv_name); >> + g_free(sv_name); >> + } >> >> name = riscv_isa_string(cpu_ptr); >> qemu_fdt_setprop_string(ms->fdt, cpu_name, "riscv,isa", name); >> -- >> 2.40.1 >> >> > > Adding a sentence, like what I wrote above, to the commit message in order > to provide better justification might be nice, but either way > > Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Thanks! Commit message amended for v2 as follows: (....) Since mmu-type is not a required node, according to [1], skip the 'mmu-type' FDT node if there's no satp_mode set. We'll revisit this logic when we can get satp information from KVM. [1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/cpu.yaml Daniel > > Thanks, > drew
On Wed, May 31, 2023 at 5:48 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > The absence of a satp mode in riscv_host_cpu_init() is causing the > following error: > > $ sudo ./qemu/build/qemu-system-riscv64 -machine virt,accel=kvm \ > -m 2G -smp 1 -nographic -snapshot \ > -kernel ./guest_imgs/Image \ > -initrd ./guest_imgs/rootfs_kvm_riscv64.img \ > -append "earlycon=sbi root=/dev/ram rw" \ > -cpu host > ** > ERROR:../target/riscv/cpu.c:320:satp_mode_str: code should not be > reached > Bail out! ERROR:../target/riscv/cpu.c:320:satp_mode_str: code should > not be reached > Aborted > > The error is triggered from create_fdt_socket_cpus() in hw/riscv/virt.c. > It's trying to get satp_mode_str for a NULL cpu->cfg.satp_mode.map. > > For this KVM 'cpu' we would need to inherit the satp supported modes > from the RISC-V host. At this moment this is not possible because the > KVM driver does not support it. And even when it does we can't just let > this broken for every other older kernel. > > Skip the 'mmu-type' FDT node if there's no satp_mode set. > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > hw/riscv/virt.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index 4e3efbee16..8aa907e81f 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -243,13 +243,13 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket, > s->soc[socket].hartid_base + cpu); > qemu_fdt_add_subnode(ms->fdt, cpu_name); > > - satp_mode_max = satp_mode_max_from_map( > - s->soc[socket].harts[cpu].cfg.satp_mode.map); > - sv_name = g_strdup_printf("riscv,%s", > - satp_mode_str(satp_mode_max, is_32_bit)); > - qemu_fdt_setprop_string(ms->fdt, cpu_name, "mmu-type", sv_name); > - g_free(sv_name); > - > + if (cpu_ptr->cfg.satp_mode.supported != 0) { > + satp_mode_max = satp_mode_max_from_map(cpu_ptr->cfg.satp_mode.map); > + sv_name = g_strdup_printf("riscv,%s", > + satp_mode_str(satp_mode_max, is_32_bit)); > + qemu_fdt_setprop_string(ms->fdt, cpu_name, "mmu-type", sv_name); > + g_free(sv_name); > + } > > name = riscv_isa_string(cpu_ptr); > qemu_fdt_setprop_string(ms->fdt, cpu_name, "riscv,isa", name); > -- > 2.40.1 > >
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index 4e3efbee16..8aa907e81f 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -243,13 +243,13 @@ static void create_fdt_socket_cpus(RISCVVirtState *s, int socket, s->soc[socket].hartid_base + cpu); qemu_fdt_add_subnode(ms->fdt, cpu_name); - satp_mode_max = satp_mode_max_from_map( - s->soc[socket].harts[cpu].cfg.satp_mode.map); - sv_name = g_strdup_printf("riscv,%s", - satp_mode_str(satp_mode_max, is_32_bit)); - qemu_fdt_setprop_string(ms->fdt, cpu_name, "mmu-type", sv_name); - g_free(sv_name); - + if (cpu_ptr->cfg.satp_mode.supported != 0) { + satp_mode_max = satp_mode_max_from_map(cpu_ptr->cfg.satp_mode.map); + sv_name = g_strdup_printf("riscv,%s", + satp_mode_str(satp_mode_max, is_32_bit)); + qemu_fdt_setprop_string(ms->fdt, cpu_name, "mmu-type", sv_name); + g_free(sv_name); + } name = riscv_isa_string(cpu_ptr); qemu_fdt_setprop_string(ms->fdt, cpu_name, "riscv,isa", name);
The absence of a satp mode in riscv_host_cpu_init() is causing the following error: $ sudo ./qemu/build/qemu-system-riscv64 -machine virt,accel=kvm \ -m 2G -smp 1 -nographic -snapshot \ -kernel ./guest_imgs/Image \ -initrd ./guest_imgs/rootfs_kvm_riscv64.img \ -append "earlycon=sbi root=/dev/ram rw" \ -cpu host ** ERROR:../target/riscv/cpu.c:320:satp_mode_str: code should not be reached Bail out! ERROR:../target/riscv/cpu.c:320:satp_mode_str: code should not be reached Aborted The error is triggered from create_fdt_socket_cpus() in hw/riscv/virt.c. It's trying to get satp_mode_str for a NULL cpu->cfg.satp_mode.map. For this KVM 'cpu' we would need to inherit the satp supported modes from the RISC-V host. At this moment this is not possible because the KVM driver does not support it. And even when it does we can't just let this broken for every other older kernel. Skip the 'mmu-type' FDT node if there's no satp_mode set. Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- hw/riscv/virt.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)