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
