qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 02/16] hw/riscv/virt.c: skip 'mmu-type' FDT if satp mode not


From: Daniel Henrique Barboza
Subject: Re: [PATCH 02/16] hw/riscv/virt.c: skip 'mmu-type' FDT if satp mode not set
Date: Tue, 6 Jun 2023 17:07:03 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0



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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]