On Wed, Jul 1, 2026 at 7:23 PM Daniel Henrique Barboza
<[email protected]> wrote:
>
> We're hardcoding faulting type as READ, where it could very well be a
> write access, and we're not recording the faulting addr/iova.
>
> A note was added in the fault_type logic because I wasn't able to
> trivially handle a probable code repeitition it in this same patch.
> Something to do in a later date.
>
> Fixes: 0c54acb8243d ("hw/riscv: add RISC-V IOMMU base emulation")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3564
> Signed-off-by: Daniel Henrique Barboza <[email protected]>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> hw/riscv/riscv-iommu.c | 29 +++++++++++++++++++++++------
> 1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index 70eede3f14..690c21289f 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -1330,6 +1330,7 @@ static void riscv_iommu_ctx_inval(RISCVIOMMUState *s,
> GHFunc func,
> /* Find or allocate translation context for a given {device_id, process_id}
> */
> static RISCVIOMMUContext *riscv_iommu_ctx(RISCVIOMMUState *s,
> unsigned devid, unsigned
> process_id,
> + IOMMUAccessFlags perm, uint64_t
> iova,
> void **ref)
> {
> GHashTable *ctx_cache;
> @@ -1339,6 +1340,7 @@ static RISCVIOMMUContext
> *riscv_iommu_ctx(RISCVIOMMUState *s,
> .process_id = process_id,
> };
> unsigned mode = get_field(s->ddtp, RISCV_IOMMU_DDTP_MODE);
> + uint32_t fault_type;
>
> ctx_cache = g_hash_table_ref(s->ctx_cache);
>
> @@ -1381,8 +1383,21 @@ static RISCVIOMMUContext
> *riscv_iommu_ctx(RISCVIOMMUState *s,
> g_hash_table_unref(ctx_cache);
> *ref = NULL;
>
> - riscv_iommu_report_fault(s, ctx, RISCV_IOMMU_FQ_TTYPE_UADDR_RD,
> - fault, !!process_id, 0, 0);
> + /*
> + * TODO: (1) do we need to distinguish other fault types
> + * for ctx fetching and (2) evaluate putting the 'fault_type'
> + * logic inside riscv_iommu_report_fault() - there's at
> + * least one other place (end of riscv_iommu_translate())
> + * that does something similar.
> + */
> + if (perm & IOMMU_RO) {
> + fault_type = RISCV_IOMMU_FQ_TTYPE_UADDR_RD;
> + } else {
> + fault_type = RISCV_IOMMU_FQ_TTYPE_UADDR_WR;
> + }
> +
> + riscv_iommu_report_fault(s, ctx, fault_type, fault,
> + !!process_id, iova, 0);
>
> g_free(ctx);
> return NULL;
> @@ -2128,6 +2143,8 @@ static void riscv_iommu_process_dbg(RISCVIOMMUState *s)
> uint64_t ctrl = riscv_iommu_reg_get64(s, RISCV_IOMMU_REG_TR_REQ_CTL);
> unsigned devid = get_field(ctrl, RISCV_IOMMU_TR_REQ_CTL_DID);
> unsigned pid = get_field(ctrl, RISCV_IOMMU_TR_REQ_CTL_PID);
> + IOMMUAccessFlags perm = ctrl & RISCV_IOMMU_TR_REQ_CTL_NW
> + ? IOMMU_RO : IOMMU_RW;
> RISCVIOMMUContext *ctx;
> void *ref;
>
> @@ -2135,7 +2152,7 @@ static void riscv_iommu_process_dbg(RISCVIOMMUState *s)
> return;
> }
>
> - ctx = riscv_iommu_ctx(s, devid, pid, &ref);
> + ctx = riscv_iommu_ctx(s, devid, pid, perm, iova, &ref);
> if (ctx == NULL) {
> riscv_iommu_reg_set64(s, RISCV_IOMMU_REG_TR_RESPONSE,
> RISCV_IOMMU_TR_RESPONSE_FAULT |
> @@ -2143,7 +2160,7 @@ static void riscv_iommu_process_dbg(RISCVIOMMUState *s)
> } else {
> IOMMUTLBEntry iotlb = {
> .iova = iova,
> - .perm = ctrl & RISCV_IOMMU_TR_REQ_CTL_NW ? IOMMU_RO : IOMMU_RW,
> + .perm = perm,
> .addr_mask = ~0,
> .target_as = NULL,
> };
> @@ -2482,7 +2499,7 @@ static MemTxResult riscv_iommu_trap_write(void *opaque,
> hwaddr addr,
> /* FIXME: PCIe bus remapping for attached endpoints. */
> devid |= s->bus << 8;
>
> - ctx = riscv_iommu_ctx(s, devid, 0, &ref);
> + ctx = riscv_iommu_ctx(s, devid, 0, IOMMU_RW, addr, &ref);
> if (ctx == NULL) {
> res = MEMTX_ACCESS_ERROR;
> } else {
> @@ -2784,7 +2801,7 @@ static IOMMUTLBEntry
> riscv_iommu_memory_region_translate(
> };
> uint32_t devid = riscv_iommu_space_devid(as);
>
> - ctx = riscv_iommu_ctx(as->iommu, devid, iommu_idx, &ref);
> + ctx = riscv_iommu_ctx(as->iommu, devid, iommu_idx, flag, addr, &ref);
> if (ctx == NULL) {
> /* Translation disabled or invalid. */
> iotlb.addr_mask = 0;
> --
> 2.43.0
>
>
