On 8/28/2026 4:01 PM, Chao Liu wrote:
Hi Daniel,
On Fri, Aug 28, 2026 at 12:40:38PM +0800, Daniel Henrique Barboza wrote:
On 8/28/2026 1:13 AM, Chao Liu wrote:
On Thu, Aug 27, 2026 at 07:26:01PM +0800, Daniel Henrique Barboza wrote:
Yet another FDT that we want to move to a helper to avoid copy/pasting
code to other boards that will use a mtimer. In particular the future
'riscv-server-ref' board.
No FDT changes made.
Signed-off-by: Daniel Henrique Barboza <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
---
hw/riscv/fdt-common.c | 96 ++++++++++++++++++++++++++++++++
hw/riscv/virt.c | 101 ++++------------------------------
include/hw/riscv/fdt-common.h | 11 ++++
3 files changed, 119 insertions(+), 89 deletions(-)
diff --git a/hw/riscv/fdt-common.c b/hw/riscv/fdt-common.c
index d815c5adff..76a783709d 100644
--- a/hw/riscv/fdt-common.c
+++ b/hw/riscv/fdt-common.c
@@ -16,6 +16,7 @@
#include "target/riscv/cpu_bits.h"
#include "hw/riscv/riscv-iommu-bits.h"
#include "hw/riscv/iommu.h"
+#include "hw/intc/riscv_aclint.h"
#include "hw/intc/riscv_imsic.h"
#include "hw/pci/pci.h"
#include "hw/pci/pcie_host.h"
@@ -702,3 +703,98 @@ void riscv_create_fdt_socket_aplic(void *fdt,
APLICFdtProps *props,
aplic_phandles[props->socket] = aplic_s_phandle;
}
+
+void riscv_create_fdt_socket_aclint(void *fdt, ACLINTFdtProps *props,
+ uint32_t *intc_phandles)
+{
+ uint32_t aclint_cells_size = props->num_harts * sizeof(uint32_t) * 2;
+ g_autofree uint32_t *aclint_mswi_cells = NULL;
+ g_autofree uint32_t *aclint_sswi_cells = NULL;
+ g_autofree uint32_t *aclint_mtimer_cells = NULL;
+ hwaddr addr, size;
+ char *name;
+ int cpu;
+
+ aclint_mswi_cells = g_new0(uint32_t, props->num_harts * 2);
+ aclint_mtimer_cells = g_new0(uint32_t, props->num_harts * 2);
+ aclint_sswi_cells = g_new0(uint32_t, props->num_harts * 2);
+
+ for (cpu = 0; cpu < props->num_harts; cpu++) {
+ aclint_mswi_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
+ aclint_mswi_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_SOFT);
+ aclint_mtimer_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
+ aclint_mtimer_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_M_TIMER);
+ aclint_sswi_cells[cpu * 2 + 0] = cpu_to_be32(intc_phandles[cpu]);
+ aclint_sswi_cells[cpu * 2 + 1] = cpu_to_be32(IRQ_S_SOFT);
+ }
+
+ if (props->aia_type != AIA_TYPE_APLIC_IMSIC) {
+ addr = props->clint->base + (props->clint->size * props->socket);
+ name = g_strdup_printf("/soc/mswi@%"HWADDR_PRIx, addr);
+
+ qemu_fdt_add_subnode(fdt, name);
+ qemu_fdt_setprop_string(fdt, name, "compatible", "riscv,aclint-mswi");
+ qemu_fdt_setprop_sized_cells(fdt, name, "reg",
+ 2, addr, 2, RISCV_ACLINT_SWI_SIZE);
+ qemu_fdt_setprop(fdt, name, "interrupts-extended",
+ aclint_mswi_cells, aclint_cells_size);
+ qemu_fdt_setprop(fdt, name, "interrupt-controller", NULL, 0);
+ qemu_fdt_setprop_cell(fdt, name, "#interrupt-cells", 0);
+
+ if (props->numa_enabled) {
+ qemu_fdt_setprop_cell(fdt, name, "numa-node-id", props->socket);
+ }
+
+ g_free(name);
+ }
+
+ if (props->aia_type == AIA_TYPE_APLIC_IMSIC) {
+ addr = props->clint->base +
+ (RISCV_ACLINT_DEFAULT_MTIMER_SIZE * props->socket);
+ size = RISCV_ACLINT_DEFAULT_MTIMER_SIZE;
+ } else {
+ addr = props->clint->base + RISCV_ACLINT_SWI_SIZE +
+ (props->clint->size * props->socket);
+ size = props->clint->size - RISCV_ACLINT_SWI_SIZE;
+ }
+
+ name = g_strdup_printf("/soc/mtimer@%"HWADDR_PRIx, addr);
+ qemu_fdt_add_subnode(fdt, name);
+ qemu_fdt_setprop_string(fdt, name, "compatible",
+ "riscv,aclint-mtimer");
+ qemu_fdt_setprop_sized_cells(fdt, name, "reg",
+ 2, addr,
Should be:
2, addr + RISCV_ACLINT_DEFAULT_MTIME,
I found an MTIMER FDT issue in the virt machine FDT.
Command:
```
timeout 30s ./build/qemu-system-riscv64 \
-machine
virt,aia=aplic-imsic,aia-guests=5,aclint=on,dumpdtb=/tmp/virt.dtb \
-smp 2 -display none -nodefaults
dtc -I dtb -O dts /tmp/virt.dtb | grep -A4 mtimer@
```
The generated FDT contains:
mtimer@2000000 {
interrupts-extended = <0x04 0x07 0x02 0x07>;
reg = <0x00 0x2000000 0x00 0x08 0x00 0x2000000 0x00
0x7ff8>;
compatible = "riscv,aclint-mtimer";
};
without this patches:
mtimer@2000000 {
interrupts-extended = <0x04 0x07 0x02 0x07>;
reg = <0x00 0x2007ff8 0x00 0x08 0x00 0x2000000 0x00
0x7ff8>;
compatible = "riscv,aclint-mtimer";
};
The first address should be 0x20007ff8.
That was intentional and I forgot to document it in the commit msg. The
"No FDT changes made" I said doesn't apply here. My bad!
Thanks for clarifying. I also noticed this waring during my previous
review and testing.
If you use 'dtc' in the current 'virt' DT you'll get warnings:
dtc -I dtb -O dts virt.dtb > virt.dts
<stdout>: Warning (simple_bus_reg): /soc/mtimer@2000000: simple-bus unit address format
error, expected "2007ff8"
<stdout>: Warning (simple_bus_reg): /soc/mtimer@2008000: simple-bus unit address format
error, expected "200fff8"
Yes, the warning is valid, and I can reproduce it as well.
This happens because the unit address in the node name doesn't match the
first address in 'reg'. For mtimer@2000000, "reg" must be reg = <0x00 0x2000000
(...)>.
For mtimer@2008000, reg = <0x00 2008000 (...)> and so on.
I agree that the address in the node name should match the first address
in `reg`, as described by the Devicetree Spec [1].
However, I think the value that needs to change is the node name, not
the first address in `reg`.
See ACLINT spec, an MITER device has two separate base addr: one for MTIME
and another for the MTIMECMP registers [2].
In the current QEMU implement, `addr` is the MTIMECMP base address, while
the MTIME register is located at:
addr + RISCV_ACLINT_DEFAULT_METIME
For this example:
MTIMECMP base = 0x2000000
MITIME base = 0x2007ff8
So the existing `reg` property correctly describes the two register
regions:
reg = <0x00 0x2007ff8 0x00 0x08
0x00 0x2000000 0x00 0x7ff8>
Now, does this change break aclint in 'virt'? Because in that case we'll
just keep the "dtc" warning and live with it. Aside from the existing tests
we have in QEMU I didn't do a deep dive in aclint.
Yes, I think using `addr` here would make the ACLINT description
incorrect. `addr` is the MTIMECMP base address, while MTIME is located
at `addr + RISCV_ACLINT_DEFAULT_MTIME`.
Please keep the first `reg` address as it was:
qemu_fdt_setprop_sized_cells(fdt, name, "reg",
- 2, addr,
+ 2, addr + RISCV_ACLINT_DEFAULT_MTIME,
2, size - RISCV_ACLINT_DEFAULT_MTIME,
2, addr + RISCV_ACLINT_DEFAULT_MTIMECMP,
2, RISCV_ACLINT_DEFAULT_MTIME);
Since this patch is mainly a code-duplication cleanup, I suggest
leaving the generated FDT unchanged here. The unit-address fix can be
sent as a separate follow-up patch:
- name = g_strdup_printf("/soc/mtimer@%" HWADDR_PRIx, addr);
+ name = g_strdup_printf("/soc/mtimer@%" HWADDR_PRIx,
+ addr + RISCV_ACLINT_DEFAULT_MTIME);
Oh, I think I get it. The correct fix is changing the nodename to use
addr + RISCV_ACLINT_DEFAULT_MTIME.
I'll do as you said in v5. Thanks for clarifying,
Daniel
--
LINK:
[1] Devicetree Specification, Section 2.2.1, "Node Names":
https://devicetree-specification.readthedocs.io/en/stable/devicetree-basics.html#node-names
[2] RISC-V ACLINT Specification, "Machine-level Timer Device (MTIMER)",
Section "Register Map":
https://github.com/riscvarchive/riscv-aclint/blob/main/riscv-aclint.adoc
Thanks,
Chao
Thanks,
Daniel
+ 2, size - RISCV_ACLINT_DEFAULT_MTIME,
+ 2, addr + RISCV_ACLINT_DEFAULT_MTIMECMP,
+ 2, RISCV_ACLINT_DEFAULT_MTIME);
[...]
+ qemu_fdt_setprop(fdt, name, "interrupts-extended",
- qemu_fdt_setprop_sized_cells(ms->fdt, name, "reg",
- 2, addr + RISCV_ACLINT_DEFAULT_MTIME,
I think this is the original code that was moved here.
Thanks,
Chao
- 2, size - RISCV_ACLINT_DEFAULT_MTIME,
- 2, addr + RISCV_ACLINT_DEFAULT_MTIMECMP,
- 2, RISCV_ACLINT_DEFAULT_MTIME);
