Re: [PATCH 4/4] iommu: Clean up force_aperture confusion

[Jason Gunthorpe <jgg@nvidia.com>]

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@arm.com>
> ---
>  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