@samitouri / QOSamiQemu / commits / aa51eec004

hw/riscv/riscv-iommu: preserve requested perm in spa_fetch()

b18e3f0e2d0f fixed spa_fetch() faults whose TTYP used the leaf PTE permission instead of the original request permission. However, it kept that request-narrowed value in iotlb->perm after a successful walk, and riscv_iommu_translate() caches iotlb->perm for later accesses to the same IOVA. That means a write to an RW mapping can cache the entry as write-only. A later read then hits the cache and faults even though the mapping allows it, which showed up in NVMe testing as bogus completions and controller timeouts. Keep the requested permission in a separate req_perm and use it for all permission checks and fault-type decisions. Accumulate the leaf permissions separately and copy them to iotlb->perm only after the full walk succeeds, so cached entries describe the mapping rather than the current request. Since faults leave iotlb->perm as the original request, the S-stage and G-stage TTYP fixes remain intact. Fixes: b18e3f0e2d0f ("hw/riscv/riscv-iommu.c: fix fault type for spa_fetch() faults") Signed-off-by: Andrew Jones <andrew.jones@oss.qualcomm.com> Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> Message-ID: <20260717144525.1154204-1-andrew.jones@oss.qualcomm.com> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Andrew Jones committed Jul 17, 2026 at 16:45 UTC aa51eec004461f671f878145d3fc921c6cabee48
1 file changed +29 -21
hw/riscv/riscv-iommu.c
+29 -21
@@ -281,7 +281,7 @@ static hwaddr riscv_iommu_napot_page_mask(hwaddr ppn, hwaddr addr, hwaddr *out)
281 static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
282 IOMMUTLBEntry *iotlb)
283 {
284 - IOMMUAccessFlags pte_perm;
284 + IOMMUAccessFlags trans_perm = IOMMU_NONE;
285 dma_addr_t addr, base;
286 uint64_t satp, gatp, pte;
287 bool en_s, en_g;
@@ -298,6 +298,14 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
298 } pass;
299 MemTxResult ret;
300 bool pv = !!ctx->process_id;
301 + /*
302 + * Keep the request permission separate from iotlb->perm. G-stage
303 + * walks translate S-stage PTE addresses before the real leaf is
304 + * reached, but permission checks and fault types must still use the
305 + * original request. A successful walk leaves iotlb->perm with the
306 + * effective leaf permission for the translation cache.
307 + */
308 + const IOMMUAccessFlags req_perm = iotlb->perm;
309
310 satp = get_field(ctx->satp, RISCV_IOMMU_ATP_MODE_FIELD);
311 gatp = get_field(ctx->gatp, RISCV_IOMMU_ATP_MODE_FIELD);
@@ -316,7 +324,7 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
324 * means we can't do an early MSI check unless we have
325 * strictly !en_s.
326 */
319 - if (!en_s && (iotlb->perm & IOMMU_WO) &&
327 + if (!en_s && (req_perm & IOMMU_WO) &&
328 riscv_iommu_msi_check(s, ctx, iotlb->iova)) {
329 iotlb->target_as = &s->trap_as;
330 iotlb->translated_addr = iotlb->iova;
@@ -434,13 +442,13 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
442 masked_msbs = (addr >> (va_len - 1)) & mask;
443
444 if (masked_msbs != 0 && masked_msbs != mask) {
437 - return (iotlb->perm & IOMMU_WO) ?
445 + return (req_perm & IOMMU_WO) ?
446 RISCV_IOMMU_FQ_CAUSE_WR_FAULT_S :
447 RISCV_IOMMU_FQ_CAUSE_RD_FAULT_S;
448 }
449 } else {
450 if ((addr & va_mask) != addr) {
443 - return (iotlb->perm & IOMMU_WO) ?
451 + return (req_perm & IOMMU_WO) ?
452 RISCV_IOMMU_FQ_CAUSE_WR_FAULT_VS :
453 RISCV_IOMMU_FQ_CAUSE_RD_FAULT_VS;
454 }
@@ -465,8 +473,8 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
473 MEMTXATTRS_UNSPECIFIED);
474 }
475 if (ret != MEMTX_OK) {
468 - return (iotlb->perm & IOMMU_WO) ? RISCV_IOMMU_FQ_CAUSE_WR_FAULT
469 - : RISCV_IOMMU_FQ_CAUSE_RD_FAULT;
476 + return (req_perm & IOMMU_WO) ? RISCV_IOMMU_FQ_CAUSE_WR_FAULT
477 + : RISCV_IOMMU_FQ_CAUSE_RD_FAULT;
478 }
479
480 sc[pass].step++;
@@ -491,13 +499,13 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
499 * non-user mode leaf PTE and !pv we need to fault.
500 */
501 break;
494 - } else if ((iotlb->perm & IOMMU_RO) && !(pte & PTE_R)) {
502 + } else if ((req_perm & IOMMU_RO) && !(pte & PTE_R)) {
503 break; /* Read access check failed */
496 - } else if ((iotlb->perm & IOMMU_WO) && !(pte & PTE_W)) {
504 + } else if ((req_perm & IOMMU_WO) && !(pte & PTE_W)) {
505 break; /* Write access check failed */
506 } else if (!ade && !(pte & PTE_A)) {
507 break; /* Access bit not set */
500 - } else if ((iotlb->perm & IOMMU_WO) && !ade && !(pte & PTE_D)) {
508 + } else if ((req_perm & IOMMU_WO) && !ade && !(pte & PTE_D)) {
509 break; /* Dirty bit not set */
510 } else if (pass == G_STAGE && !(pte & PTE_U)) {
511 /*
@@ -532,21 +540,20 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
540 addr = iotlb->iova;
541 continue;
542 }
543 +
544 + /* Cache the effective permission, not this request's subset. */
545 + IOMMUAccessFlags leaf_perm = (pte & PTE_W) ?
546 + ((pte & PTE_R) ? IOMMU_RW : IOMMU_WO) :
547 + IOMMU_RO;
548 +
549 + trans_perm = trans_perm == IOMMU_NONE ?
550 + leaf_perm : trans_perm & leaf_perm;
551 +
552 /* Translation phase completed (GPA or SPA) */
553 iotlb->translated_addr = base;
554
538 - /*
539 - * Do a bit_and between the PTE bits and the original
540 - * request flags to determine the exact permission we
541 - * need, i.e. if the original request is RO and the
542 - * PTE has RW flags the actual perm is RO.
543 - */
544 - pte_perm = (pte & PTE_W) ? ((pte & PTE_R) ? IOMMU_RW : IOMMU_WO)
545 - : IOMMU_RO;
546 - iotlb->perm &= pte_perm;
547 -
555 /* Check MSI GPA address match */
549 - if (pass == S_STAGE && (iotlb->perm & IOMMU_WO) &&
556 + if (pass == S_STAGE && (req_perm & IOMMU_WO) &&
557 riscv_iommu_msi_check(s, ctx, base)) {
558 /* Trap MSI writes and return GPA address. */
559 iotlb->target_as = &s->trap_as;
@@ -563,6 +570,7 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
570 continue;
571 }
572
573 + iotlb->perm = trans_perm;
574 return 0;
575 }
576
@@ -587,7 +595,7 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
595 */
596 iotlb->translated_addr = addr;
597
590 - return (iotlb->perm & IOMMU_WO) ?
598 + return (req_perm & IOMMU_WO) ?
599 (pass ? RISCV_IOMMU_FQ_CAUSE_WR_FAULT_VS :
600 RISCV_IOMMU_FQ_CAUSE_WR_FAULT_S) :
601 (pass ? RISCV_IOMMU_FQ_CAUSE_RD_FAULT_VS :