Hi Jean-Philippe,
On 03/10/2016 12:52 PM, Jean-Philippe Brucker wrote:
> Hi Eric,
>
> On Tue, Mar 01, 2016 at 06:27:46PM +0000, Eric Auger wrote:
>> [...]
>> +
>> +int iommu_get_single_reserved(struct iommu_domain *domain,
>> + phys_addr_t addr, int prot,
>> + dma_addr_t *iova)
>> +{
>> + unsigned long order = __ffs(domain->ops->pgsize_bitmap);
>> + size_t page_size = 1 << order;
>> + phys_addr_t mask = page_size - 1;
>> + phys_addr_t aligned_addr = addr & ~mask;
>> + phys_addr_t offset = addr - aligned_addr;
>> + struct iommu_reserved_binding *b;
>> + struct iova *p_iova;
>> + struct iova_domain *iovad =
>> + (struct iova_domain *)domain->reserved_iova_cookie;
>> + int ret;
>> +
>> + if (!iovad)
>> + return -EINVAL;
>> +
>> + mutex_lock(&domain->reserved_mutex);
>
> I believe this function could get called from the chunk of __setup_irq
> that is executed atomically:
>
> * request_threaded_irq
> * __setup_irq
> * irq_startup
> * irq_domain_activate_irq
> * msi_domain_activate
> * msi_compose
> * iommu_get_single_reserved
>
> If this is the case, we should probably use a spinlock to protect the
> iova_domain...
Please apologize for the delay, I was in vacation.
Thank you for spotting this flow. I will rework the locking.
>
>> +
>> + b = find_reserved_binding(domain, aligned_addr, page_size);
>> + if (b) {
>> + *iova = b->iova + offset;
>> + kref_get(&b->kref);
>> + ret = 0;
>> + goto unlock;
>> + }
>> +
>> + /* there is no existing reserved iova for this pa */
>> + p_iova = alloc_iova(iovad, 1, iovad->dma_32bit_pfn, true);
>> + if (!p_iova) {
>> + ret = -ENOMEM;
>> + goto unlock;
>> + }
>> + *iova = p_iova->pfn_lo << order;
>> +
>> + b = kzalloc(sizeof(*b), GFP_KERNEL);
>
> ... and GFP_ATOMIC here.
OK
Thank you for your time!
Best Regards
Eric
>
> Thanks,
> Jean-Philippe
>
>> + if (!b) {
>> + ret = -ENOMEM;
>> + goto free_iova_unlock;
>> + }
>> +
>> + ret = iommu_map(domain, *iova, aligned_addr, page_size, prot);
>> + if (ret)
>> + goto free_binding_iova_unlock;
>> +
>> + kref_init(&b->kref);
>> + kref_get(&b->kref);
>> + b->domain = domain;
>> + b->addr = aligned_addr;
>> + b->iova = *iova;
>> + b->size = page_size;
>> +
>> + link_reserved_binding(domain, b);
>> +
>> + *iova += offset;
>> + goto unlock;
>> +
>> +free_binding_iova_unlock:
>> + kfree(b);
>> +free_iova_unlock:
>> + free_iova(iovad, *iova >> order);
>> +unlock:
>> + mutex_unlock(&domain->reserved_mutex);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_get_single_reserved);
