Re: [PATCH 1/1] vfio/type1: Respect IOMMU reserved regions in vfio_test_domain_fgsp()

[Jason Gunthorpe <jgg@ziepe.ca>]

On Mon, Jan 02, 2023 at 10:34:52AM +0100, Niklas Schnelle wrote:
> Since commit cbf7827bc5dc ("iommu/s390: Fix potential s390_domain
> aperture shrinking") the s390 IOMMU driver uses a reserved region
> instead of an artificially shrunk aperture to restrict IOMMU use based
> on the system provided DMA ranges of devices. In particular on current
> machines this prevents use of DMA addresses below 2^32 for all devices.
> While usually just IOMMU mapping below these addresses is
> harmless. However our virtual ISM PCI device looks at new mappings on
> IOTLB flush and immediately goes into the error state if such a mapping
> violates its allowed DMA ranges. This then breaks pass-through of the
> ISM device to a KVM guest.
> 
> Analysing this we found that vfio_test_domain_fgsp() maps 2 pages at DMA
> address 0 irrespective of the IOMMUs reserved regions. Even if usually
> harmless this seems wrong in the general case so instead go through the
> freshly updated IOVA list and try to find a range that isn't reserved
> and fits 2 pages and use that for testing for fine grained super pages.

Why does it matter? The s390 driver will not set fgsp=true, so if it
fails because map fails or does a proper detection it shouldn't make a
difference.

IOW how does this actualy manifest into a failure?

> -	if (!ret) {
> -		size_t unmapped = iommu_unmap(domain->domain, 0, PAGE_SIZE);
> +	list_for_each_entry(region, regions, list) {
> +		if (region->end - region->start < PAGE_SIZE * 2)
> +			continue;
>  
> -		if (unmapped == PAGE_SIZE)
> -			iommu_unmap(domain->domain, PAGE_SIZE, PAGE_SIZE);
> -		else
> -			domain->fgsp = true;
> +		ret = iommu_map(domain->domain, region->start, page_to_phys(pages), PAGE_SIZE * 2,
> +				IOMMU_READ | IOMMU_WRITE | IOMMU_CACHE);

The region also needs to have 'region->start % (PAGE_SIZE*2) == 0' for the
test to work

Jason