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 <[email protected]>
> ---
> 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
>
