lkml.org 
[lkml]   [2023]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains
From
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

\
 
 \ /
  Last update: 2023-07-22 15:58    [W:0.105 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site