[PATCH v2] hw/riscv/riscv-iommu: Fix PPN field of Translation-reponse register

[PATCH v2] hw/riscv/riscv-iommu: Fix PPN field of Translation-reponse register

[Nutty Liu]

The original implementation incorrectly performed a bitwise AND
operation between the PPN of iova and PPN Mask, leading to an
incorrect PPN field in Translation-reponse register.

The PPN of iova should be set entirely in the PPN field of
Translation-reponse register.

Also remove the code that was used to clear S field since this
field is already zero.

Signed-off-by: Nutty Liu <liujingqi@lanxincomputing.com>
Reviewed-by: Tomasz Jeznach <tjeznach@rivosinc.com>
---
V1 -> V2:
 - Commented by Alistair
 - Reviewed by Tomasz
 - Remove the extra code
---
 hw/riscv/riscv-iommu.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
index a877e5da84..d8b1cb03a8 100644
--- a/hw/riscv/riscv-iommu.c
+++ b/hw/riscv/riscv-iommu.c
@@ -1935,11 +1935,7 @@ static void riscv_iommu_process_dbg(RISCVIOMMUState *s)
             iova = RISCV_IOMMU_TR_RESPONSE_FAULT | (((uint64_t) fault) << 10);
         } else {
             iova = iotlb.translated_addr & ~iotlb.addr_mask;
-            iova >>= TARGET_PAGE_BITS;
-            iova &= RISCV_IOMMU_TR_RESPONSE_PPN;
-
-            /* We do not support superpages (> 4kbs) for now */
-            iova &= ~RISCV_IOMMU_TR_RESPONSE_S;
+            iova = set_field(0, RISCV_IOMMU_TR_RESPONSE_PPN, PPN_DOWN(iova));
         }
         riscv_iommu_reg_set64(s, RISCV_IOMMU_REG_TR_RESPONSE, iova);
     }
-- 
2.49.0.windows.1

Re: [PATCH v2] hw/riscv/riscv-iommu: Fix PPN field of Translation-reponse register

[Alistair Francis]

On Thu, Jun 5, 2025 at 10:50 PM Nutty Liu <liujingqi@lanxincomputing.com> wrote:
>
> The original implementation incorrectly performed a bitwise AND
> operation between the PPN of iova and PPN Mask, leading to an
> incorrect PPN field in Translation-reponse register.
>
> The PPN of iova should be set entirely in the PPN field of
> Translation-reponse register.
>
> Also remove the code that was used to clear S field since this
> field is already zero.
>
> Signed-off-by: Nutty Liu <liujingqi@lanxincomputing.com>
> Reviewed-by: Tomasz Jeznach <tjeznach@rivosinc.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
> V1 -> V2:
>  - Commented by Alistair
>  - Reviewed by Tomasz
>  - Remove the extra code
> ---
>  hw/riscv/riscv-iommu.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index a877e5da84..d8b1cb03a8 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -1935,11 +1935,7 @@ static void riscv_iommu_process_dbg(RISCVIOMMUState *s)
>              iova = RISCV_IOMMU_TR_RESPONSE_FAULT | (((uint64_t) fault) << 10);
>          } else {
>              iova = iotlb.translated_addr & ~iotlb.addr_mask;
> -            iova >>= TARGET_PAGE_BITS;
> -            iova &= RISCV_IOMMU_TR_RESPONSE_PPN;
> -
> -            /* We do not support superpages (> 4kbs) for now */
> -            iova &= ~RISCV_IOMMU_TR_RESPONSE_S;
> +            iova = set_field(0, RISCV_IOMMU_TR_RESPONSE_PPN, PPN_DOWN(iova));
>          }
>          riscv_iommu_reg_set64(s, RISCV_IOMMU_REG_TR_RESPONSE, iova);
>      }
> --
> 2.49.0.windows.1
>