On Mon, May 15, 2023 at 01:49:32PM +0100, Robin Murphy wrote: > It was never entirely clear whether the force_aperture flag was supposed > to be a hardware capability or a software policy. So far things seem to > have leant towards the latter, given that the only drivers not setting > the flag are ones where the aperture is seemingly a whole virtual > address space such that accesses outside it wouldn't be possible, and > the one driver which definitely can't enforce it in hardware *does* > still set the flag. > > On reflection, though, it makes little sense for drivers to dictate > usage policy to callers, and the interpretation that a driver setting > the flag might also translate addresses outside the given aperture but > has for some reason chosen not to is not actually a useful one. It seems > a lot more logical to treat the aperture as the absolute limit of what > can be translated, and the flag to indicate what would happen if a > device did try to access an address outside the aperture, i.e. whether > the access would fault or pass through untranslated. As such, reframe > the flag consistent with a hardware capability in the hope of clearing > up the misconception. > > Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx> > --- > drivers/iommu/dma-iommu.c | 19 +++++++------------ > drivers/iommu/mtk_iommu_v1.c | 4 ++++ > drivers/iommu/sprd-iommu.c | 1 + > drivers/iommu/tegra-gart.c | 2 +- > 4 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 7a9f0b0bddbd..4693b021d54f 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -548,24 +548,19 @@ static int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, > if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE) > return -EINVAL; > > + /* > + * If the IOMMU only offers a non-isolated GART-style translation > + * aperture, just let the device use dma-direct. > + */ So we add code to dma-iommu.c for a driver that never enables dma-iommu.c and uses def_default_domain to prevent it from ever being used anyhow? :( > + if (!domain->geometry.force_aperture) > + return -EINVAL; We need more checks than just this, the 'blocking domain is an empty unmanaged' thing needs it, there is a check in virtio-iommu that looks wonky and the name doesn't really convay what it does anymore.. "partial_translation" or something would be better. I still prefer deleting tegra-gart and deleting force_aperture completely. Jason
