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

[Alistair Francis]

On Thu, May 29, 2025 at 10:52 PM Nutty Liu
<liujin...@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.
>
> Signed-off-by: Nutty Liu <liujin...@lanxincomputing.com>
> ---
>  hw/riscv/riscv-iommu.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index a877e5da84..f529a6a3d7 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -1935,8 +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;
> +            iova = set_field(0, RISCV_IOMMU_TR_RESPONSE_PPN, PPN_DOWN(iova));

I don't see how this is different.

PPN_DOWN(iova)
is the same as
iova >> TARGET_PAGE_BITS

and

set_field(0, RISCV_IOMMU_TR_RESPONSE_PPN, PPN_DOWN(iova))
should just return
(0 & ~RISCV_IOMMU_TR_RESPONSE_PPN) |
    (RISCV_IOMMU_TR_RESPONSE_PPN & PPN_DOWN(iova))

Can you describe the issue with the original implementation and why
this fixes it in the commit message?

Alistair

>
>              /* We do not support superpages (> 4kbs) for now */
>              iova &= ~RISCV_IOMMU_TR_RESPONSE_S;
> --
> 2.49.0.windows.1
>