Messages in this thread | | | Date | Sat, 22 Jul 2023 21:58:03 +0800 | Subject | Re: [PATCH v2 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains | From | Baolu Lu <> |
| |
On 2023/7/21 11:07, Tian, Kevin wrote: >> From: Lu Baolu <baolu.lu@linux.intel.com> >> Sent: Thursday, July 13, 2023 12:33 PM >> >> @@ -409,6 +409,7 @@ struct iommu_fault_param { >> * @priv: IOMMU Driver private data >> * @max_pasids: number of PASIDs this device can consume >> * @attach_deferred: the dma domain attachment is deferred >> + * @requires_direct: The driver requested IOMMU_RESV_DIRECT > > it's not accurate to say "driver requested" as it's a device attribute. > > s/requires_direct/require_direct/ > > what about "has_resv_direct"?
How about
* @require_direct: device requires IOMMU_RESV_DIRECT reserved regions
?
> >> @@ -959,14 +959,12 @@ static int >> iommu_create_device_direct_mappings(struct iommu_domain *domain, >> 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); >> + pg_size = domain->pgsize_bitmap ? 1UL << __ffs(domain- >>> pgsize_bitmap) : 0; >> INIT_LIST_HEAD(&mappings); >> >> + if (WARN_ON_ONCE(iommu_is_dma_domain(domain) && !pg_size)) >> + return -EINVAL; >> + >> iommu_get_resv_regions(dev, &mappings); >> >> /* We need to consider overlapping regions for different devices */ >> @@ -974,13 +972,17 @@ static int >> iommu_create_device_direct_mappings(struct iommu_domain *domain, >> dma_addr_t start, end, addr; >> size_t map_size = 0; >> >> + if (entry->type == IOMMU_RESV_DIRECT) >> + dev->iommu->requires_direct = 1; >> + >> + if ((entry->type != IOMMU_RESV_DIRECT && >> + entry->type != IOMMU_RESV_DIRECT_RELAXABLE) || >> + !iommu_is_dma_domain(domain)) >> + continue; >> + >> 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; >> - >> for (addr = start; addr <= end; addr += pg_size) { >> phys_addr_t phys_addr; >> > > piggybacking a device attribute detection in a function which tries to > populate domain mappings is a bit confusing. > > Does it work better to introduce a new function to detect this attribute > and has it directly called in the probe path?
Jason answered this.
> >> @@ -2121,6 +2123,21 @@ static int __iommu_device_set_domain(struct >> iommu_group *group, >> { >> int ret; >> >> + /* >> + * If the driver has requested IOMMU_RESV_DIRECT then we cannot > > ditto. It's not requested by the driver. > >> allow >> + * the blocking domain to be attached as it does not contain the >> + * required 1:1 mapping. This test effectively exclusive the device > > s/exclusive/excludes/ >
Updated. Thanks!
Best regards, baolu
| |