Hi Chao,
On Mon, Mar 2, 2026 at 1:51 PM Chao Liu <[email protected]> wrote: > > Hi Jim, > > On Mon, Feb 02, 2026 at 03:53:58PM +0800, Jim Shu wrote: > > Instead of IOMMU_NONE, address_space_translate_for_iotlb() now can pass > > the correct iommu_flags to the IOMMU translate function from the > > access_type. > > > > Since RISC-V wgChecker [1] could permit access in RO or WO permission > > only, the IOMMUMemoryRegion could return different section for > > read and write access. To support this kind of IOMMUMemoryRegion > > in the path of CPU access, we should pass correct iommu_flags here. > > > > [1] RISC-V WG: > > https://patchew.org/QEMU/[email protected]/ > > > > Signed-off-by: Jim Shu <[email protected]> > > --- > > accel/tcg/cputlb.c | 3 ++- > > include/accel/tcg/iommu.h | 3 ++- > > system/physmem.c | 16 +++++++++++----- > > 3 files changed, 15 insertions(+), 7 deletions(-) > > > > diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c > > index 2a0f4cfff62..404a8607b9b 100644 > > --- a/accel/tcg/cputlb.c > > +++ b/accel/tcg/cputlb.c > > @@ -1050,7 +1050,8 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx, > > prot = full->prot; > > asidx = cpu_asidx_from_attrs(cpu, full->attrs); > > section = address_space_translate_for_iotlb(cpu, asidx, paddr_page, > > - &xlat, &sz, full->attrs, > > &prot); > > + &xlat, &sz, full->attrs, > > &prot, > > + access_type); > > assert(sz >= TARGET_PAGE_SIZE); > > > > tlb_debug("vaddr=%016" VADDR_PRIx " paddr=0x" HWADDR_FMT_plx > > diff --git a/include/accel/tcg/iommu.h b/include/accel/tcg/iommu.h > > index 547f8ea0ef0..2a79f859834 100644 > > --- a/include/accel/tcg/iommu.h > > +++ b/include/accel/tcg/iommu.h > > @@ -20,7 +20,8 @@ MemoryRegionSection > > *address_space_translate_for_iotlb(CPUState *cpu, > > hwaddr *xlat, > > hwaddr *plen, > > MemTxAttrs attrs, > > - int *prot); > > + int *prot, > > + MMUAccessType > > access_type); > > > > #endif > > > > diff --git a/system/physmem.c b/system/physmem.c > > index 2fb0c25c93b..337137489de 100644 > > --- a/system/physmem.c > > +++ b/system/physmem.c > > @@ -683,12 +683,14 @@ void tcg_iommu_init_notifier_list(CPUState *cpu) > > MemoryRegionSection * > > address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr > > orig_addr, > > hwaddr *xlat, hwaddr *plen, > > - MemTxAttrs attrs, int *prot) > > + MemTxAttrs attrs, int *prot, > > + MMUAccessType access_type) > > { > > MemoryRegionSection *section; > > IOMMUMemoryRegion *iommu_mr; > > IOMMUMemoryRegionClass *imrc; > > IOMMUTLBEntry iotlb; > > + IOMMUAccessFlags iommu_flags; > > int iommu_idx; > > hwaddr addr = orig_addr; > > AddressSpaceDispatch *d = > > address_space_to_dispatch(cpu->cpu_ases[asidx].as); > > @@ -705,10 +707,14 @@ address_space_translate_for_iotlb(CPUState *cpu, int > > asidx, hwaddr orig_addr, > > > > iommu_idx = imrc->attrs_to_index(iommu_mr, attrs); > > tcg_register_iommu_notifier(cpu, iommu_mr, iommu_idx); > > - /* We need all the permissions, so pass IOMMU_NONE so the IOMMU > > - * doesn't short-cut its translation table walk. > > - */ > > - iotlb = imrc->translate(iommu_mr, addr, IOMMU_NONE, iommu_idx); > > + > > + if (access_type == MMU_DATA_STORE) { > > + iommu_flags = IOMMU_WO; > > + } else { > > + iommu_flags = IOMMU_RO; > > + } > This maps both MMU_INST_FETCH and MMU_DATA_LOAD to IOMMU_RO. Is this > intentional? > Some IOMMU implementations might want to distinguish between instruction > fetch and data read. Thanks for pointing out it! I don't know IOMMUAccessFlags enum now has IOMMU_EXEC. I think mapping three access_types to IOMMU_RO/WO/EXEC is more accurate. I will fix it in the v2 series. > > Or is the current mapping sufficient for your use case (RISC-V wgChecker)? Yes, RW is enough for RISC-V wgChecker now. We don't distinguish EXEC permission from READ permission. But I think supporting RWX permissions in the generic code is better. Perhaps future devices will distinguish RWX permissions. > > Thanks, > Chao > > + > > + iotlb = imrc->translate(iommu_mr, addr, iommu_flags, iommu_idx); > > addr = ((iotlb.translated_addr & ~iotlb.addr_mask) > > | (addr & iotlb.addr_mask)); > > /* Update the caller's prot bits to remove permissions the IOMMU > > -- > > 2.43.0 > > > >
