Re: RMRR device on non-Intel platform

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Apr 21, 2023 at 01:29:46PM +0100, Robin Murphy wrote:

> Can you clarify why something other than IOMMU_RESV_SW_MSI would be
> needed?

We need iommufd to setup a 1:1 map for the reserved space.

So, of the reserved spaces we have these:

	/* Memory regions which must be mapped 1:1 at all times */
	IOMMU_RESV_DIRECT,

           Block iommufd

	/*
	 * Memory regions which are advertised to be 1:1 but are
	 * commonly considered relaxable in some conditions,
	 * for instance in device assignment use case (USB, Graphics)
	 */
	IOMMU_RESV_DIRECT_RELAXABLE,

           iommufd ignores this one

	/* Arbitrary "never map this or give it to a device" address ranges */
	IOMMU_RESV_RESERVED,

	   iommufd prevents using this IOVA range

	/* Hardware MSI region (untranslated) */
	IOMMU_RESV_MSI,

	   iommufd treats this the same as IOMMU_RESV_RESERVED

	/* Software-managed MSI translation window */
	IOMMU_RESV_SW_MSI,

	   iommufd treats this the same as IOMMU_RESV_RESERVED, also
	   it passes the start to iommu_get_msi_cookie() which
	   eventually maps something, but not 1:1.

I don't think it is a compatible change for IOMMU_RESV_SW_MSI to also
mean 1:1 map?

On baremetal we have no idea what the platform put under that
hardcoded address?

On VM we don't use the iommu_get_msi_cookie() flow because the GIC in
the VM pretends it doesn't have an ITS page?  (did I get that right?)

> MSI regions already represent "safe" direct mappings, either as an inherent
> property of the hardware, or with an actual mapping maintained by software.
> Also RELAXABLE is meant to imply that it is only needed until a driver takes
> over the device, which at face value doesn't make much sense for interrupts.

I used "relxable" to suggest it is safe for userspace.

> We'll still need to set this when the default domain type is identity too -
> see the diff I posted (the other parts below I merely implied).

Right, I missed that!

I suggest like this to avoid the double loop:

@@ -1037,9 +1037,6 @@ static int iommu_create_device_direct_mappings(struct iom>
        unsigned long pg_size;
        int ret = 0;
 
-       if (!iommu_is_dma_domain(domain))
-               return 0;
-
        BUG_ON(!domain->pgsize_bitmap);
 
        pg_size = 1UL << __ffs(domain->pgsize_bitmap);
@@ -1052,13 +1049,18 @@ static int iommu_create_device_direct_mappings(struct i>
                dma_addr_t start, end, addr;
                size_t map_size = 0;
 
-               start = ALIGN(entry->start, pg_size);
-               end   = ALIGN(entry->start + entry->length, pg_size);
-
                if (entry->type != IOMMU_RESV_DIRECT &&
                    entry->type != IOMMU_RESV_DIRECT_RELAXABLE)
                        continue;
 
+               if (entry->type == IOMMU_RESV_DIRECT)
+                       dev->iommu->requires_direct = 1;
+
+               if (!iommu_is_dma_domain(domain))
+                       continue;
+
+               start = ALIGN(entry->start, pg_size);
+               end   = ALIGN(entry->start + entry->length, pg_size);
                for (addr = start; addr <= end; addr += pg_size) {
                        phys_addr_t phys_addr;

Jason