[PATCH] hw/riscv/riscv-iommu.c: fix MSI MRIF interrupt-pending offset

Daniel Henrique Barboza posted 1 patch 4 weeks, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260626220529.3800372-1-daniel.barboza@oss.qualcomm.com
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Chao Liu <chao.liu.zevorn@gmail.com>
hw/riscv/riscv-iommu.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
[PATCH] hw/riscv/riscv-iommu.c: fix MSI MRIF interrupt-pending offset
Posted by Daniel Henrique Barboza 4 weeks, 1 day ago
We're doing a wrong shift when calculating the offset for the
interrupt-pending bits, off by one right shift order.

This went undercover for awhile because the calculation works for
interrupt entities 1 to 63.  The math goes wrong when using interrupt
entities 64 or greater.

Instead of fixing the issue and running we're also adding some notes
on where this calc comes from.

Fixes: 0c54acb8243d ("hw/riscv: add RISC-V IOMMU base emulation")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3561
Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
---
 hw/riscv/riscv-iommu.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
index 3ea9130860..792729cfd5 100644
--- a/hw/riscv/riscv-iommu.c
+++ b/hw/riscv/riscv-iommu.c
@@ -686,7 +686,26 @@ static MemTxResult riscv_iommu_msi_write(RISCVIOMMUState *s,
 
     /* MRIF pending bit address */
     addr = get_field(pte[0], RISCV_IOMMU_MSI_PTE_MRIF_ADDR) << 9;
-    addr = addr | ((data & 0x7c0) >> 3);
+    /*
+     * AIA spec section "Format of a memory-resident interrupt file":
+     * address offset 0x000 contains interrupt-pending bits for
+     * identities 1-63, offfset 0x010 for identities 64-127, and
+     * so it goes up to 0x1F0 for identities 1984-2047.
+     *
+     * Hence each batch of identities advances offset by 16 (0x010)
+     * for every interrupt-pending bits.  This means that doing
+     * (data & 0x7c0) will filter out the first 6 bits, then
+     * a >> 2 will turn the result in the 0x10 steps we need.
+     *
+     * E.g:
+     *
+     * - (1-63 & 0x7c0) = 0, 0 >> 2 = 0, offset 0x000
+     * - (64-127 & 0x7c0) = 64, 64 >> 2 = 16, offset 0x010
+     * - (128-191 & 0x7c0) = 128, 128 >> 2 = 32, offset 0x020
+     *
+     * and so on.
+     */
+    addr = addr | ((data & 0x7c0) >> 2);
 
     trace_riscv_iommu_msi(s->parent_obj.id, PCI_BUS_NUM(ctx->devid),
                           PCI_SLOT(ctx->devid), PCI_FUNC(ctx->devid),
-- 
2.43.0
Re: [PATCH] hw/riscv/riscv-iommu.c: fix MSI MRIF interrupt-pending offset
Posted by Alistair Francis 3 weeks, 6 days ago
On Sat, Jun 27, 2026 at 8:06 AM Daniel Henrique Barboza
<daniel.barboza@oss.qualcomm.com> wrote:
>
> We're doing a wrong shift when calculating the offset for the
> interrupt-pending bits, off by one right shift order.
>
> This went undercover for awhile because the calculation works for
> interrupt entities 1 to 63.  The math goes wrong when using interrupt
> entities 64 or greater.
>
> Instead of fixing the issue and running we're also adding some notes
> on where this calc comes from.
>
> Fixes: 0c54acb8243d ("hw/riscv: add RISC-V IOMMU base emulation")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3561
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  hw/riscv/riscv-iommu.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index 3ea9130860..792729cfd5 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -686,7 +686,26 @@ static MemTxResult riscv_iommu_msi_write(RISCVIOMMUState *s,
>
>      /* MRIF pending bit address */
>      addr = get_field(pte[0], RISCV_IOMMU_MSI_PTE_MRIF_ADDR) << 9;
> -    addr = addr | ((data & 0x7c0) >> 3);
> +    /*
> +     * AIA spec section "Format of a memory-resident interrupt file":
> +     * address offset 0x000 contains interrupt-pending bits for
> +     * identities 1-63, offfset 0x010 for identities 64-127, and
> +     * so it goes up to 0x1F0 for identities 1984-2047.
> +     *
> +     * Hence each batch of identities advances offset by 16 (0x010)
> +     * for every interrupt-pending bits.  This means that doing
> +     * (data & 0x7c0) will filter out the first 6 bits, then
> +     * a >> 2 will turn the result in the 0x10 steps we need.
> +     *
> +     * E.g:
> +     *
> +     * - (1-63 & 0x7c0) = 0, 0 >> 2 = 0, offset 0x000
> +     * - (64-127 & 0x7c0) = 64, 64 >> 2 = 16, offset 0x010
> +     * - (128-191 & 0x7c0) = 128, 128 >> 2 = 32, offset 0x020
> +     *
> +     * and so on.
> +     */
> +    addr = addr | ((data & 0x7c0) >> 2);
>
>      trace_riscv_iommu_msi(s->parent_obj.id, PCI_BUS_NUM(ctx->devid),
>                            PCI_SLOT(ctx->devid), PCI_FUNC(ctx->devid),
> --
> 2.43.0
>
>
Re: [PATCH] hw/riscv/riscv-iommu.c: fix MSI MRIF interrupt-pending offset
Posted by Alistair Francis 3 weeks, 6 days ago
On Sat, Jun 27, 2026 at 8:06 AM Daniel Henrique Barboza
<daniel.barboza@oss.qualcomm.com> wrote:
>
> We're doing a wrong shift when calculating the offset for the
> interrupt-pending bits, off by one right shift order.
>
> This went undercover for awhile because the calculation works for
> interrupt entities 1 to 63.  The math goes wrong when using interrupt
> entities 64 or greater.
>
> Instead of fixing the issue and running we're also adding some notes
> on where this calc comes from.
>
> Fixes: 0c54acb8243d ("hw/riscv: add RISC-V IOMMU base emulation")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3561
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

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

Alistair

> ---
>  hw/riscv/riscv-iommu.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index 3ea9130860..792729cfd5 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -686,7 +686,26 @@ static MemTxResult riscv_iommu_msi_write(RISCVIOMMUState *s,
>
>      /* MRIF pending bit address */
>      addr = get_field(pte[0], RISCV_IOMMU_MSI_PTE_MRIF_ADDR) << 9;
> -    addr = addr | ((data & 0x7c0) >> 3);
> +    /*
> +     * AIA spec section "Format of a memory-resident interrupt file":
> +     * address offset 0x000 contains interrupt-pending bits for
> +     * identities 1-63, offfset 0x010 for identities 64-127, and
> +     * so it goes up to 0x1F0 for identities 1984-2047.
> +     *
> +     * Hence each batch of identities advances offset by 16 (0x010)
> +     * for every interrupt-pending bits.  This means that doing
> +     * (data & 0x7c0) will filter out the first 6 bits, then
> +     * a >> 2 will turn the result in the 0x10 steps we need.
> +     *
> +     * E.g:
> +     *
> +     * - (1-63 & 0x7c0) = 0, 0 >> 2 = 0, offset 0x000
> +     * - (64-127 & 0x7c0) = 64, 64 >> 2 = 16, offset 0x010
> +     * - (128-191 & 0x7c0) = 128, 128 >> 2 = 32, offset 0x020
> +     *
> +     * and so on.
> +     */
> +    addr = addr | ((data & 0x7c0) >> 2);
>
>      trace_riscv_iommu_msi(s->parent_obj.id, PCI_BUS_NUM(ctx->devid),
>                            PCI_SLOT(ctx->devid), PCI_FUNC(ctx->devid),
> --
> 2.43.0
>
>