Hi Pavel,
On 8/3/19 10:34 PM, Pavel Machek wrote:
> Hi!
>
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -3721,7 +3721,7 @@ static void intel_unmap(struct device *d
>>
>> freelist = domain_unmap(domain, start_pfn, last_pfn);
>>
>> - if (intel_iommu_strict) {
>> + if (intel_iommu_strict || !has_iova_flush_queue(&domain->iovad)) {
>> iommu_flush_iotlb_psi(iommu, domain, start_pfn,
>> nrpages, !freelist, 0);
>> /* free iova */
>> --- a/drivers/iommu/iova.c
>> +++ b/drivers/iommu/iova.c
>> @@ -65,9 +65,14 @@ init_iova_domain(struct iova_domain *iov
>> }
>> EXPORT_SYMBOL_GPL(init_iova_domain);
>>
>> +bool has_iova_flush_queue(struct iova_domain *iovad)
>> +{
>> + return !!iovad->fq;
>
> Should this be READ_ONCE()?
Why? Compiler can't anyhow assume that it's always true/false and there
is a clear data dependency between this and:
: queue_iova(&domain->iovad, iova_pfn, nrpages,
: (unsigned long)freelist);
>
>> @@ -100,13 +106,17 @@ int init_iova_flush_queue(struct iova_do
>> for_each_possible_cpu(cpu) {
>> struct iova_fq *fq;
>>
>> - fq = per_cpu_ptr(iovad->fq, cpu);
>> + fq = per_cpu_ptr(queue, cpu);
>> fq->head = 0;
>> fq->tail = 0;
>>
>> spin_lock_init(&fq->lock);
>> }
>>
>> + smp_wmb();
>> +
>> + iovad->fq = queue;
>> +
>
> Could we have a comment why the barrier is needed,
I'm up for the comment if you feel like it - in my POV it's quite
obvious that we want finish initializing the queue's internals before
assigning the queue. I didn't put the comment exactly because I felt
like it would state something evident [in my POV].
> and perhaps there
> should be oposing smp_rmb() somewhere? Does this need to be
> WRITE_ONCE() as it is racing against reader?
I feel confused. I might have forgotten everything about barriers, but
again if I'm not mistaken, one doesn't need a barrier in:
: if (A->a != NULL)
: use(A); /* dereferences A->a */
: else
: /* don't use `a' */
Thanks,
Dmitry
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
