On 8/27/2026 10:46 AM, Daniel Henrique Barboza wrote:
On 8/27/2026 2:55 AM, Joel Stanley wrote:
On Thu, 27 Aug 2026 at 07:16, Daniel Henrique Barboza
<[email protected]> wrote:
void *riscv_create_board_device_tree(const char *model, const char
*compatible,
int *fdt_size)
@@ -348,3 +350,41 @@ void riscv_create_fdt_syscon(void *fdt, uint32_t *phandle,
qemu_fdt_setprop_cell(fdt, name, "value", poweroff);
g_free(name);
}
+
+void riscv_create_fdt_riscv_iommu_sys(void *fdt, hwaddr addr, hwaddr size,
+ uint32_t irq_chip,
+ uint32_t msi_phandle,
+ uint32_t *iommu_sys_phandle,
+ uint32_t iommu_sys_irq)
+{
+ const char comp[] = "riscv,iommu";
+ uint32_t iommu_phandle;
+ g_autofree char *iommu_node = NULL;
+ uint32_t iommu_irq_map[RISCV_IOMMU_INTR_COUNT] = {
+ iommu_sys_irq + RISCV_IOMMU_INTR_CQ,
+ iommu_sys_irq + RISCV_IOMMU_INTR_FQ,
+ iommu_sys_irq + RISCV_IOMMU_INTR_PM,
+ iommu_sys_irq + RISCV_IOMMU_INTR_PQ,
+ };
Any reason to pack them into an array? Seems like they could be part
of the qemu_fdt_setprop_cells call:
qemu_fdt_setprop_cells(fdt, iommu_node, "interrupts",
iommu_sys_irq + RISCV_IOMMU_INTR_CQ, FDT_IRQ_TYPE_EDGE_LOW,
iommu_sys_irq + RISCV_IOMMU_INTR_FQ, FDT_IRQ_TYPE_EDGE_LOW,
iommu_sys_irq + RISCV_IOMMU_INTR_PM, FDT_IRQ_TYPE_EDGE_LOW,
iommu_sys_irq + RISCV_IOMMU_INTR_PQ, FDT_IRQ_TYPE_EDGE_LOW);
No particular reason. I'll make the change.
+
+ iommu_node = g_strdup_printf("/soc/iommu@%"HWADDR_PRIx, addr);
+ iommu_phandle = qemu_fdt_alloc_phandle(fdt);
Ohh, that's neat. Any reason we don't use this for all of phandle
allocation needs?
So back in the day we were using a fdt_phandle int variable around, incrementing
it and using as phandle for nodes. As you might notice in the boards we still
have
a similar pattern going on.
The choice I made with this particular fdt is because I didn't want to pass a
'phandle' argument to the function. Given that the iommu FDT is the last DT
that
declares a new phandle I could get away with it.
Otherwise, if there are other DTs that follows it and are assigning DTs using
integer
increment, there's a risk of phandle collision.
In the end I decided to do this:
if (next_phandle != NULL) {
*iommu_sys_phandle = (*next_phandle)++;
} else {
*iommu_sys_phandle = qemu_fdt_alloc_phandle(fdt);
}
And now the helper is able to accept a phandle allocation from the board or, in
case the board doesn't care about that, we'll use qemu_fdt_alloc_phandle().
I believe this will make it easier to switch for a strategy where we'll always
use
alloc_phandle() in the future.
Cheers,
Daniel
We should be consistent, so switch this one to the pattern used by the
rest of the file, or (preferably, IMO) switch them all over.
I agree that we should allocate phandles in an uniform manner, whatever that
strategy
is. To use alloc_phandle() everywhere we would need to take a look at how
every board
is handling their own phandles first. I believe this is worth doing, but out
of scope
for this mostly "move stuff around" series we have here.
+ qemu_fdt_add_subnode(fdt, iommu_node);
+
+ qemu_fdt_setprop(fdt, iommu_node, "compatible", comp, sizeof(comp));
+ qemu_fdt_setprop_cell(fdt, iommu_node, "#iommu-cells", 1);
+ qemu_fdt_setprop_cell(fdt, iommu_node, "phandle", iommu_phandle);
+
+ qemu_fdt_setprop_sized_cells(fdt, iommu_node, "reg", 2, addr, 2, size);
+ qemu_fdt_setprop_cell(fdt, iommu_node, "interrupt-parent", irq_chip);
+
+ qemu_fdt_setprop_cells(fdt, iommu_node, "interrupts",
+ iommu_irq_map[0], FDT_IRQ_TYPE_EDGE_LOW,
+ iommu_irq_map[1], FDT_IRQ_TYPE_EDGE_LOW,
+ iommu_irq_map[2], FDT_IRQ_TYPE_EDGE_LOW,
+ iommu_irq_map[3], FDT_IRQ_TYPE_EDGE_LOW);
+
+ qemu_fdt_setprop_cell(fdt, iommu_node, "msi-parent", msi_phandle);
+
+ *iommu_sys_phandle = iommu_phandle;
Could we just return it? Or have it passed in, so we don't need to return it?
I guess I could not use alloc_phandle() and receive the phandle from the caller.
It would be consistent with what we already do in other cases, and then when/if
we
decide to change everything to alloc_phandle() we'll do it in a consistent
manner.
I'll remove "iommu_phandle = qemu_fdt_alloc_phandle(fdt);" and make the caller
provide a
valid phandle in iommu_phandle. Thanks,
Daniel
