On Wed, 2023-01-04 at 13:05 +0100, Niklas Schnelle wrote: > In some virtualized environments, including s390 paged memory guests, > IOTLB flushes are used to update IOMMU shadow tables. Due to this, they > are much more expensive than in typical bare metal environments or > non-paged s390 guests. In addition they may parallelize more poorly in > virtualized environments. This changes the trade off for flushing IOVAs > such that minimizing the number of IOTLB flushes trumps any benefit of > cheaper queuing operations or increased paralellism. > > In this scenario per-CPU flush queues pose several problems. Firstly > per-CPU memory is often quite limited prohibiting larger queues. > Secondly collecting IOVAs per-CPU but flushing via a global timeout > reduces the number of IOVAs flushed for each timeout especially on s390 > where PCI interrupts may not be bound to a specific CPU. > > Thus let's introduce a single flush queue mode IOMMU_DOMAIN_DMA_SQ that > reuses the same queue logic but only allocates a single global queue > allowing larger batches of IOVAs to be freed at once and with larger > timeouts. This is to allow the common IOVA flushing code to more closely > resemble the global flush behavior used on s390's previous internal DMA > API implementation. > > As we now support two different variants of flush queues rename the > existing __IOMMU_DOMAIN_DMA_FQ to __IOMMU_DOMAIN_DMA_LAZY to indicate > the general case of having a flush queue and introduce separate > __IOMMU_DOMAIN_DMA_PERCPU_Q and __IOMMU_DOMAIN_DMA_SINGLE_Q bits to > indicate the two queue variants. > > Link: https://lore.kernel.org/linux-iommu/3e402947-61f9-b7e8-1414-fde006257b6f@xxxxxxx/ > Signed-off-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx> > --- > v2 -> v3: > - Rename __IOMMU_DOMAIN_DMA_FQ to __IOMMU_DOMAIN_DMA_LAZY to make it more clear > that this bit indicates flush queue use independent of the exact queuing > strategy > ---8<--- > > /* sysfs updates are serialised by the mutex of the group owning @domain */ > int iommu_dma_init_fq(struct iommu_domain *domain) > { > struct iommu_dma_cookie *cookie = domain->iova_cookie; > - struct iova_fq __percpu *queue; > - int i, cpu; > + int rc; > > if (cookie->fq_domain) > return 0; > @@ -250,26 +336,16 @@ int iommu_dma_init_fq(struct iommu_domain *domain) > atomic64_set(&cookie->fq_flush_start_cnt, 0); > atomic64_set(&cookie->fq_flush_finish_cnt, 0); > > ---8<--- > + if (domain->type == IOMMU_DOMAIN_DMA_FQ) > + rc = iommu_dma_init_fq_percpu(cookie); > + else > + rc = iommu_dma_init_fq_single(cookie); Found out in testing that the above doesn't work for the "echo XYZ > /sys/../iommu_group/type" interface as domain->type is not set before calling iommu_dma_init_fq() so it would always init for DMA-SQ which is of course the case that I used during earlier testing. I think the easiest fix is to add a type parameter to iommu_dma_init_fq(). > > - for (i = 0; i < IOVA_FQ_SIZE; i++) > - INIT_LIST_HEAD(&fq->entries[i].freelist); > + if (rc) { > + pr_warn("iova flush queue initialization failed\n"); > + return rc; > } > ---8<--- > > mutex_unlock(&group->mutex); > @@ -2896,10 +2900,10 @@ static int iommu_change_dev_def_domain(struct iommu_group *group, > } > > /* We can bring up a flush queue without tearing down the domain */ > - if (type == IOMMU_DOMAIN_DMA_FQ && prev_dom->type == IOMMU_DOMAIN_DMA) { > + if (!!(type & __IOMMU_DOMAIN_DMA_LAZY) && prev_dom->type == IOMMU_DOMAIN_DMA) { > ret = iommu_dma_init_fq(prev_dom); > if (!ret) > - prev_dom->type = IOMMU_DOMAIN_DMA_FQ; > + prev_dom->type = type; Here domain->type is set only after calling iommu_dma_init_fq(). > goto out; > } >
