Hello,
On 9/27/2026 8:18 PM, Ralf Ramsauer wrote:
From: Ralf Ramsauer <[email protected]>
The PLIC device is created from the topology string built by
riscv_plic_hart_config_string(), which gives a hart a supervisor context
only if that hart actually has supervisor mode. The device tree, in
contrast, lists a machine and a supervisor context for every hart,
unconditionally.
The two have disagreed since commit 7d10ff8a4d ("hw/riscv: virt: Use the
PLIC config helper function") taught the device about harts without
S-mode and left the device tree behind. Boot with
-cpu rv64,s=false,u=false -smp 4
and the PLIC implements four contexts while the device tree advertises
eight. Contexts are numbered by their position in 'interrupts-extended',
so a guest that takes its context number from the property gets it wrong
for every hart but the first: hart 1 drives the context of hart 2, and
the upper harts address contexts the PLIC never created. This is what an
M-mode kernel on the virt machine runs into; sifive_u already omits the
supervisor entry for its management hart.
Emit the supervisor entry only for the harts that have supervisor mode.
Fixes: 7d10ff8a4d ("hw/riscv: virt: Use the PLIC config helper function")
Signed-off-by: Ralf Ramsauer <[email protected]>
---
hw/riscv/virt.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index d4b501802f..d0ce9dc862 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -165,14 +165,23 @@ static void create_fdt_socket_plic(RISCVVirtState *s,
plic_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_EXT);
}
} else {
- cells_length = s->soc[socket].num_harts * 4;
- plic_cells = g_new0(uint32_t, cells_length);
+ plic_cells = g_new0(uint32_t, s->soc[socket].num_harts * 4);
+ cells_length = 0;
for (cpu = 0; cpu < s->soc[socket].num_harts; cpu++) {
- plic_cells[cpu * 4 + 0] = cpu_to_be32(intc_phandles[cpu]);
- plic_cells[cpu * 4 + 1] = cpu_to_be32(IRQ_M_EXT);
- plic_cells[cpu * 4 + 2] = cpu_to_be32(intc_phandles[cpu]);
- plic_cells[cpu * 4 + 3] = cpu_to_be32(IRQ_S_EXT);
+ CPURISCVState *env = &s->soc[socket].harts[cpu].env;
There are a few things to consider here. First is that we have a very similar
PLIC
code in the sifive_u.c board. I believe we want this fix also applied there.
Second, we should avoid accessing CPU internals in board code. In an ideal
world
the board would be completely oblivious to what the CPU is about, e.g. its
extensions
and accelerator mode. In practice we have to account for accelerator
differences since
KVM and TCG requires different stuff w.r.t irq controllers and timers and so
on, and
at least for now this code is being open coded in the boards.
For the problem you're addressing here I would like to avoid making the board
accessing
the CPU env and checking for RVS. My suggestion here is:
- we need a pre-patch that adds a helper in cpu.c that tells whether a CPU has
supervisor
mode. Something like:
(cpu.c, could be right after "riscv_cpu_is_vendor")
bool riscv_cpu_has_supervisor_mode(RISCVCPU *cpu)
{
#ifndef CONFIG_USER_ONLY
if (kvm_enabled()) {
return true;
}
return riscv_has_ext(&cpu->env, RVS);
#else
return false;
#endif
}
same thing in cpu.h. The justification for this helper in a separated patch is
that
"next patch needs to verify supervisor mode for CPUs in hw/riscv board code", or
something around those lines.
- in this patch you can then do:
> + if (riscv_cpu_has_supervisor_mode(s->soc[socket].harts[cpu])) {
> + plic_cells[cells_length++] = cpu_to_be32(intc_phandles[cpu]);
> + plic_cells[cells_length++] = cpu_to_be32(IRQ_S_EXT);
> + }
And we're at least trying to obfuscating the CPU internals from the board a bit.
And please do the same in sifive_u.c. Don't need to be in this same patch, so
feel
free to add another patch just for the sifive_u board.
Thanks,
Daniel
+
+ plic_cells[cells_length++] = cpu_to_be32(intc_phandles[cpu]);
+ plic_cells[cells_length++] = cpu_to_be32(IRQ_M_EXT);
+
+ /*
+ * The PLIC implements a supervisor context only for harts that
+ * have supervisor mode, see riscv_plic_hart_config_string().
+ */
+ if (riscv_has_ext(env, RVS)) {
+ plic_cells[cells_length++] = cpu_to_be32(intc_phandles[cpu]);
+ plic_cells[cells_length++] = cpu_to_be32(IRQ_S_EXT);
+ }
}
}
base-commit: 81ce3a87737aa50716c42db8886082d12783e0a1
