hw/riscv/virt.c: fix 'iommu-map' FDT entry

[PATCH] hw/riscv/virt.c: fix 'iommu-map' FDT entry

Posted by Daniel Henrique Barboza 1 month ago

Based on the DT documentation of 'iommu-map':

https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/pci-iommu.txt

- iommu-map: Maps a Requester ID to an IOMMU and associated IOMMU specifier
  data.

  The property is an arbitrary number of tuples of
  (rid-base,iommu,iommu-base,length).

----------

We're adding a no-op entry (length = 0) in iommu-map:

         qemu_fdt_setprop_cells(ms->fdt, name, "iommu-map",
                                0, iommu_sys_phandle, 0, 0, 0,
                                iommu_sys_phandle, 0, 0xffff);

This is easily seen in the generated DT:

iommu-map = <0x00 0x8000 0x00 0x00 0x00 0x8000 0x00 0xffff>;

The tuple (0 0 0x8000 0) does nothing since it has length = 0.  The
information we want to advertise is in the second tuple only.  Thus
remove the empty tuple.

While we're at it, seems like we've mistaken the API and we're using
0xffff as 'last address', but in fact it is length.  This means that
we're telling the DT we're mapping 0x0 -> 0xfffe, which wasn't our
intention.  Therefore change size to '0x10000' to reflect the address
mapping we want (0x0 -> 0xffff).

Found while reviewing the RISC-V Server Platform DT generation, which
happens to copy a lot of code from the 'virt' board, and this nit is
also present there.

Fixes: 2c12de1460 ("hw/riscv/virt: Add IOMMU as platform device if the option is set")
Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
---
 hw/riscv/virt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index ce64eaaef7..0c489bb412 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -900,8 +900,7 @@ static void create_fdt_pcie(RISCVVirtState *s,
 
     if (virt_is_iommu_sys_enabled(s)) {
         qemu_fdt_setprop_cells(ms->fdt, name, "iommu-map",
-                               0, iommu_sys_phandle, 0, 0, 0,
-                               iommu_sys_phandle, 0, 0xffff);
+                               0, iommu_sys_phandle, 0, 0x10000);
     }
 
     create_pcie_irq_map(s, ms->fdt, name, irq_pcie_phandle);
-- 
2.43.0

Re: [PATCH] hw/riscv/virt.c: fix 'iommu-map' FDT entry

Posted by Alistair Francis 1 month ago

On Tue, Jun 9, 2026 at 7:08 AM Daniel Henrique Barboza
<daniel.barboza@oss.qualcomm.com> wrote:
>
> Based on the DT documentation of 'iommu-map':
>
> https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/pci-iommu.txt
>
> - iommu-map: Maps a Requester ID to an IOMMU and associated IOMMU specifier
>   data.
>
>   The property is an arbitrary number of tuples of
>   (rid-base,iommu,iommu-base,length).
>
> ----------
>
> We're adding a no-op entry (length = 0) in iommu-map:
>
>          qemu_fdt_setprop_cells(ms->fdt, name, "iommu-map",
>                                 0, iommu_sys_phandle, 0, 0, 0,
>                                 iommu_sys_phandle, 0, 0xffff);
>
> This is easily seen in the generated DT:
>
> iommu-map = <0x00 0x8000 0x00 0x00 0x00 0x8000 0x00 0xffff>;
>
> The tuple (0 0 0x8000 0) does nothing since it has length = 0.  The
> information we want to advertise is in the second tuple only.  Thus
> remove the empty tuple.
>
> While we're at it, seems like we've mistaken the API and we're using
> 0xffff as 'last address', but in fact it is length.  This means that
> we're telling the DT we're mapping 0x0 -> 0xfffe, which wasn't our
> intention.  Therefore change size to '0x10000' to reflect the address
> mapping we want (0x0 -> 0xffff).
>
> Found while reviewing the RISC-V Server Platform DT generation, which
> happens to copy a lot of code from the 'virt' board, and this nit is
> also present there.
>
> Fixes: 2c12de1460 ("hw/riscv/virt: Add IOMMU as platform device if the option is set")
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  hw/riscv/virt.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index ce64eaaef7..0c489bb412 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -900,8 +900,7 @@ static void create_fdt_pcie(RISCVVirtState *s,
>
>      if (virt_is_iommu_sys_enabled(s)) {
>          qemu_fdt_setprop_cells(ms->fdt, name, "iommu-map",
> -                               0, iommu_sys_phandle, 0, 0, 0,
> -                               iommu_sys_phandle, 0, 0xffff);
> +                               0, iommu_sys_phandle, 0, 0x10000);
>      }
>
>      create_pcie_irq_map(s, ms->fdt, name, irq_pcie_phandle);
> --
> 2.43.0
>
>

Re: [PATCH] hw/riscv/virt.c: fix 'iommu-map' FDT entry

Posted by Alistair Francis 1 month ago

On Tue, Jun 9, 2026 at 7:08 AM Daniel Henrique Barboza
<daniel.barboza@oss.qualcomm.com> wrote:
>
> Based on the DT documentation of 'iommu-map':
>
> https://www.kernel.org/doc/Documentation/devicetree/bindings/pci/pci-iommu.txt
>
> - iommu-map: Maps a Requester ID to an IOMMU and associated IOMMU specifier
>   data.
>
>   The property is an arbitrary number of tuples of
>   (rid-base,iommu,iommu-base,length).
>
> ----------
>
> We're adding a no-op entry (length = 0) in iommu-map:
>
>          qemu_fdt_setprop_cells(ms->fdt, name, "iommu-map",
>                                 0, iommu_sys_phandle, 0, 0, 0,
>                                 iommu_sys_phandle, 0, 0xffff);
>
> This is easily seen in the generated DT:
>
> iommu-map = <0x00 0x8000 0x00 0x00 0x00 0x8000 0x00 0xffff>;
>
> The tuple (0 0 0x8000 0) does nothing since it has length = 0.  The
> information we want to advertise is in the second tuple only.  Thus
> remove the empty tuple.
>
> While we're at it, seems like we've mistaken the API and we're using
> 0xffff as 'last address', but in fact it is length.  This means that
> we're telling the DT we're mapping 0x0 -> 0xfffe, which wasn't our
> intention.  Therefore change size to '0x10000' to reflect the address
> mapping we want (0x0 -> 0xffff).
>
> Found while reviewing the RISC-V Server Platform DT generation, which
> happens to copy a lot of code from the 'virt' board, and this nit is
> also present there.
>
> Fixes: 2c12de1460 ("hw/riscv/virt: Add IOMMU as platform device if the option is set")
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/riscv/virt.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index ce64eaaef7..0c489bb412 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -900,8 +900,7 @@ static void create_fdt_pcie(RISCVVirtState *s,
>
>      if (virt_is_iommu_sys_enabled(s)) {
>          qemu_fdt_setprop_cells(ms->fdt, name, "iommu-map",
> -                               0, iommu_sys_phandle, 0, 0, 0,
> -                               iommu_sys_phandle, 0, 0xffff);
> +                               0, iommu_sys_phandle, 0, 0x10000);
>      }
>
>      create_pcie_irq_map(s, ms->fdt, name, irq_pcie_phandle);
> --
> 2.43.0
>
>