Re: [PATCH] hw/riscv/riscv-iommu.c: check for misaligned IOHGATP_PPN

[Chao Liu <chao.liu.zevorn@gmail.com>]

On Tue, Jun 30, 2026 at 05:31:55PM +0800, Daniel Henrique Barboza wrote:
> We must check if IOHGATP_PPN is 16kb aligned for non-bare GATP modes.
> 
> qos-riscv-iommu.h needs change too since G_IOHGATP must now be 16kb
> aligned too.
> 
> Fixes: 69a9ae4836 ("hw/riscv/riscv-iommu: add ATS support")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3550
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
> ---
>  hw/riscv/riscv-iommu.c               | 14 ++++++++++++++
>  tests/qtest/libqos/qos-riscv-iommu.h |  2 +-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index 891a56e731..09990c640f 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -795,6 +795,20 @@ static bool riscv_iommu_validate_device_ctx(RISCVIOMMUState *s,
>          return false;
>      }
>  
> +    if (gatp != RISCV_IOMMU_DC_IOHGATP_MODE_BARE) {
> +        uint64_t iohgatp_ppn = get_field(ctx->gatp,
> +                                         RISCV_IOMMU_DC_IOHGATP_PPN);
> +        /*
> +         * One of the conditions for a misconfigured DDT entry
> +         * according to the riscv-spec: "DC.iohgatp.MODE is not
> +         * Bare and the root page table determined by DC.iohgatp.PPN
> +         * is not aligned to a 16-KiB boundary."
> +         */
> +        if (iohgatp_ppn & ((1ULL << 14) - 1)) {
I believe that if we are checking the PPN of this hgatp register, we should be
checking its lower 2 bits. According to the RISC-V specification, this is meant
to check the 16 KB alignment of the base address for the second-stage page
table. 

It's equal:

 ```text
   addr = PPN << 12
   16 KiB = 2^14
   so, PPN just need lower 14 - 12 = 2 bit is 0
 ```

Reference:

 1. iohgatp section：

 The iohgatp field holds the PPN of the root second-stage page table ...
 The root page table as determined by iohgatp.PPN is 16 KiB and must be aligned
 to a 16-KiB boundary.

 2. Device-context configuration checks：

 A DC with DC.tc.V=1 is considered as misconfigured if any of the following
 conditions are true. If misconfigured then, stop and report "DDT entry
 misconfigured" (cause = 259).
  ...
 DC.iohgatp.MODEis not Bare and the root page table determined by DC.iohgatp.PPN
 is not aligned to a 16-KiB boundary.

So, We can modify it this way.

 ```c
   if (iohgatp_ppn & ((1ULL << 2) - 1)) {
       return false;
   }
 ```

To better align with what you expressed in your comments, we are
obtaining the page table base address directly from the PPN.

 ```c
   if (PPN_PHYS(iohgatp_ppn) & ((1ULL << 14) - 1)) {
       return false;
   }
 ```
> +            return false;
> +        }
> +    }
> +
>      fsc_mode = get_field(ctx->satp, RISCV_IOMMU_DC_FSC_MODE);
>  
>      if (ctx->tc & RISCV_IOMMU_DC_TC_PDTV) {
> diff --git a/tests/qtest/libqos/qos-riscv-iommu.h b/tests/qtest/libqos/qos-riscv-iommu.h
> index 90e69a5d73..4a972401f5 100644
> --- a/tests/qtest/libqos/qos-riscv-iommu.h
> +++ b/tests/qtest/libqos/qos-riscv-iommu.h
> @@ -46,7 +46,7 @@
>  #define QRIOMMU_L1_PTE_VAL          0x0000000000012000ull
>  #define QRIOMMU_L2_PTE_VAL          0x0000000000013000ull
>  
> -#define QRIOMMU_G_IOHGATP           0x0000000000020000ull
> +#define QRIOMMU_G_IOHGATP           0x0000000008000000ull
I believe this isn't an adjustment. as the original value was specifically
set to perform the 16KB alignment check.

The iommu helper function will handle:

 ```c
   iohgatp = qriommu_apply_space_offs(QRIOMMU_G_IOHGATP);
   dc.iohgatp = (iohgatp >> 12) | (8ull << 60);
 ```

So, we can get:

 ```
   iohgatp addr = 0x80000000 + 0x20000 = 0x80020000
   PPN         = 0x80020000 >> 12 = 0x80020
 ```

This is a valid 16 KB byte alignment check:

```
   0x80020000 & 0x3fff == 0
   equal
   0x80020000 & ((1ULL << 14) - 1) == 0
```

But if we use old ppn check code, It just so happens that his check
will fail:

```
   0x80020 & 0x3fff != 0
```

Thanks,
Chao
>  #define QRIOMMU_G_L0_PTE_VAL        0x0000000000021000ull
>  #define QRIOMMU_G_L1_PTE_VAL        0x0000000000022000ull
>  
> -- 
> 2.43.0
>