Messages in this thread | | | Date | Fri, 22 Jan 2021 13:26:57 +0000 | From | Will Deacon <> | Subject | Re: [PATCH 1/1] iommu/arm-smmu-v3: add support for BBML |
| |
On Fri, Jan 22, 2021 at 09:21:20PM +0800, Leizhen (ThunderTown) wrote: > On 2021/1/22 21:00, Robin Murphy wrote: > > On 2021-01-22 12:51, Will Deacon wrote: > >> On Thu, Nov 26, 2020 at 11:42:30AM +0800, Zhen Lei wrote: > >>> - tblp = ptep - ARM_LPAE_LVL_IDX(iova, lvl, data); > >>> - if (__arm_lpae_unmap(data, NULL, iova, sz, lvl, tblp) != sz) { > >>> - WARN_ON(1); > >>> - return -EINVAL; > >>> + switch (cfg->bbml) { > >>> + case 0: > >>> + /* > >>> + * We need to unmap and free the old table before > >>> + * overwriting it with a block entry. > >>> + */ > >>> + tblp = ptep - ARM_LPAE_LVL_IDX(iova, lvl, data); > >>> + if (__arm_lpae_unmap(data, NULL, iova, sz, lvl, tblp) != sz) { > >>> + WARN_ON(1); > >>> + return -EINVAL; > >>> + } > >>> + break; > >>> + case 1: > >>> + __arm_lpae_init_pte(data, paddr, prot, lvl, ptep, ARM_LPAE_PTE_nT); > >>> + > >>> + io_pgtable_tlb_flush_walk(iop, iova, sz, ARM_LPAE_GRANULE(data)); > >>> + tblp = iopte_deref(pte, data); > >>> + __arm_lpae_free_pgtable(data, lvl + 1, tblp); > >>> + break; > >>> + case 2: > >>> + __arm_lpae_init_pte(data, paddr, prot, lvl, ptep, 0); > >>> + > >>> + io_pgtable_tlb_flush_walk(iop, iova, sz, ARM_LPAE_GRANULE(data)); > >>> + tblp = iopte_deref(pte, data); > >>> + __arm_lpae_free_pgtable(data, lvl + 1, tblp); > >>> + return 0; > >> > >> Sorry, but I really don't understand what you're trying to do here. The old > >> code uses BBM for the table -> block path so we don't need anything extra > >> here. The dodgy case is when we unmap part of a block, and end up installing > >> a table via arm_lpae_split_blk_unmap(). We can't use BBM there because there > >> could be ongoing DMA to parts of the block mapping that we want to remain in > >> place. > >> > >> Are you seeing a problem in practice? > > > > Right, I was under the assumption that we could ignore BBML because we > > should never have a legitimate reason to split blocks. I'm certainly not > > keen on piling any more complexity into split_blk_unmap, because the > > IOMMU API clearly doesn't have a well-defined behaviour for that case > > anyway - some other drivers will just unmap the entire block, and IIRC > > there was a hint somewhere in VFIO that it might actually expect that > > behaviour. > > I'm going home. I'll answer you two tomorrow.
It can wait until Monday! Have a good weekend :)
Will
| |