Hi All,
This patch series converts s390's PCI support from its platform specific DMA
API implementation in arch/s390/pci/pci_dma.c to the common DMA IOMMU layer.
The conversion itself is done in patch 3 and after applying my the s390 IOMMU
improvements series[0]. It only touches the s390 IOMMU driver and arch code
moving over remaining functions from the s390 DMA API implementation. No
changes to common code are necessary. Though patch 4 does propose an additional
common code change to let the iommu.strict kernel parameter override
ops->def_domain_type.
After patch 3 the basic conversion is done and on our partitioning machine
hypervisor LPAR performance matches or exceeds the existing code. When running
under z/VM or KVM however, performance plummets to about half of the existing
code due to a much higher rate of IOTLB flushes for unmapped pages and we still
need to handle out-of-resource indications (patch 7). Due to the hypervisors
use of IOTLB flushes to synchronize their shadow tables these are very
expensive and minimizing them is key for regaining the performance loss.
To this end patches 5-7 propose a new, single queue, IOTLB flushing scheme as
an alternative to the existing per-CPU flush queues. Introducing an alternative
scheme was also suggested by Robin Murphy[1]. In the previous RFC of this
conversion Robin suggested reusing more of the existing queuing logic which
I incorporated into this version. The single queue mode is introduced in patch
5. It allows batching a much larger number of lazily freed IOVAs and was also
chosen as hypervisors tend to serialize IOTLB flushes removing some of the
gains of multiple queues. Except for going from one per-CPU to a global queue
the queue logic remains untouched.
Then patch 6 enables variable queue sizes using power of 2 queue sizes and
shift/mask to keep performance as close to the existing code as possible.
Finally patch 7 allows triggering a queue flush from the IOMMU driver in order
to handle s390's use of an IOTLB flush out-of-resource indication.
As it is implemented in common code the single queue IOTLB flushing scheme can
of course be used by other platforms with expensive IOTLB flushes. Particularly
virtio-iommu might be a candidate. With this series however only s390 systems
that require IOTLB flushes on map default to it while LPAR uses the per-CPU
queues.
I did verify that the new scheme does work on my x86_64 Ryzen workstation by
locally modifying drivers/iommu/iommu.c:iommu_subsys_init() to default to the
single queue mode and verifying its use via "/sys/.../iommu_group/type". I did
not find problems with an AMD GPU, Intel NIC (with SR-IOV), NVMes or any on
board peripherals though the only performance test was a round of CS:Go :-)
As with previous series this is available via my git.kernel.org tree[3] in the
dma_iommu_v2 branch with s390_dma_iommu_v2 tag.
NOTE: Due to the large drop in performance and the issue of out-of-resource
handling we can't merge the DMA API conversion (patches 1-3) until we have
a more suited IOVA flushing scheme with similar improvements as the proposed
changes of patches 5-7.
Best regards,
Niklas
[0] https://lore.kernel.org/linux-iommu/[email protected]/
[1] https://lore.kernel.org/linux-iommu/[email protected]/
[2] https://lore.kernel.org/linux-iommu/[email protected]/
[3] https://git.kernel.org/pub/scm/linux/kernel/git/niks/linux.git/
Changes since RFC v1:
- Patch 1 uses dma_set_mask_and_coherent() (Christoph)
- Patch 3 now documents and allows the use of iommu.strict=0|1 on s390 and
deprecates s390_iommu=strict while making it an alias.
- Patches 5-7 completely reworked to reuse existing queue logic (Robin)
- Added patch 4 to allow using iommu.strict=0|1 to override
ops->def_domain_type.
Niklas Schnelle (7):
s390/ism: Set DMA coherent mask
s390/pci: prepare is_passed_through() for dma-iommu
s390/pci: Use dma-iommu layer
iommu: Let iommu.strict override ops->def_domain_type
iommu/dma: Allow a single FQ in addition to per-CPU FQs
iommu/dma: Enable variable queue size and use larger single queue
iommu/s390: flush queued IOVAs on RPCIT out of resource indication
.../admin-guide/kernel-parameters.txt | 9 +-
arch/s390/include/asm/pci.h | 7 -
arch/s390/include/asm/pci_dma.h | 120 +--
arch/s390/pci/Makefile | 2 +-
arch/s390/pci/pci.c | 22 +-
arch/s390/pci/pci_bus.c | 5 -
arch/s390/pci/pci_debug.c | 13 +-
arch/s390/pci/pci_dma.c | 732 ------------------
arch/s390/pci/pci_event.c | 17 +-
arch/s390/pci/pci_sysfs.c | 19 +-
drivers/iommu/Kconfig | 3 +-
drivers/iommu/dma-iommu.c | 188 ++++-
drivers/iommu/dma-iommu.h | 1 +
drivers/iommu/iommu.c | 18 +-
drivers/iommu/s390-iommu.c | 409 +++++++++-
drivers/s390/net/ism_drv.c | 2 +-
include/linux/iommu.h | 7 +
17 files changed, 603 insertions(+), 971 deletions(-)
delete mode 100644 arch/s390/pci/pci_dma.c
--
2.34.1
With the IOMMU always controlled through the IOMMU driver testing for
zdev->s390_domain is not a valid indication of the device being
passed-through. Instead test if zdev->kzdev is set.
Reviewed-by: Pierre Morel <[email protected]>
Signed-off-by: Niklas Schnelle <[email protected]>
---
arch/s390/pci/pci_event.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c
index b9324ca2eb94..4ef5a6a1d618 100644
--- a/arch/s390/pci/pci_event.c
+++ b/arch/s390/pci/pci_event.c
@@ -59,9 +59,16 @@ static inline bool ers_result_indicates_abort(pci_ers_result_t ers_res)
}
}
-static bool is_passed_through(struct zpci_dev *zdev)
+static bool is_passed_through(struct pci_dev *pdev)
{
- return zdev->s390_domain;
+ struct zpci_dev *zdev = to_zpci(pdev);
+ bool ret;
+
+ mutex_lock(&zdev->kzdev_lock);
+ ret = !!zdev->kzdev;
+ mutex_unlock(&zdev->kzdev_lock);
+
+ return ret;
}
static bool is_driver_supported(struct pci_driver *driver)
@@ -176,7 +183,7 @@ static pci_ers_result_t zpci_event_attempt_error_recovery(struct pci_dev *pdev)
}
pdev->error_state = pci_channel_io_frozen;
- if (is_passed_through(to_zpci(pdev))) {
+ if (is_passed_through(pdev)) {
pr_info("%s: Cannot be recovered in the host because it is a pass-through device\n",
pci_name(pdev));
goto out_unlock;
@@ -239,7 +246,7 @@ static void zpci_event_io_failure(struct pci_dev *pdev, pci_channel_state_t es)
* we will inject the error event and let the guest recover the device
* itself.
*/
- if (is_passed_through(to_zpci(pdev)))
+ if (is_passed_through(pdev))
goto out;
driver = to_pci_driver(pdev->dev.driver);
if (driver && driver->err_handler && driver->err_handler->error_detected)
--
2.34.1
Flush queues currently use a fixed compile time size of 256 entries.
This being a power of 2 allows the compiler to use shifts and mask
instead of more expensive modulo operations. With per-CPU flush queues
larger queue sizes would hit per-CPU allocation limits, with
a single flush queue these limits do not apply however. As single flush
queue mode is intended for environments with epensive IOTLB flushes it
then makes sense to use a larger queue size and timeout.
To this end re-order struct iova_fq so we can use a dynamic array and
make the flush queue size and timeout variable. So as not to lose the
shift and mask optimization, check that the variable length is a power
of 2 and use explicit shift and mask instead of letting the compiler
optimize this.
For now use a large fixed queue size and timeout for single flush queues
that brings its performance on s390 paged memory guests on par with the
previous s390 specific DMA API implementation. In the future the flush
queue size can then be turned into a config option or kernel parameter.
Signed-off-by: Niklas Schnelle <[email protected]>
---
drivers/iommu/dma-iommu.c | 60 ++++++++++++++++++++++++++-------------
1 file changed, 41 insertions(+), 19 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 1cdbf8579946..3801cdf11aa8 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -61,6 +61,8 @@ struct iommu_dma_cookie {
struct timer_list fq_timer;
/* 1 when timer is active, 0 when not */
atomic_t fq_timer_on;
+ /* timeout in ms */
+ unsigned long fq_timer_timeout;
};
/* Trivial linear page allocator for IOMMU_DMA_MSI_COOKIE */
dma_addr_t msi_iova;
@@ -86,10 +88,16 @@ static int __init iommu_dma_forcedac_setup(char *str)
early_param("iommu.forcedac", iommu_dma_forcedac_setup);
/* Number of entries per flush queue */
-#define IOVA_FQ_SIZE 256
+#define IOVA_DEFAULT_FQ_SIZE 256
+
+/* Number of entries for a single queue */
+#define IOVA_SINGLE_FQ_SIZE 32768
/* Timeout (in ms) after which entries are flushed from the queue */
-#define IOVA_FQ_TIMEOUT 10
+#define IOVA_DEFAULT_FQ_TIMEOUT 10
+
+/* Timeout (in ms) for a single queue */
+#define IOVA_SINGLE_FQ_TIMEOUT 1000
/* Flush queue entry for deferred flushing */
struct iova_fq_entry {
@@ -101,18 +109,19 @@ struct iova_fq_entry {
/* Per-CPU flush queue structure */
struct iova_fq {
- struct iova_fq_entry entries[IOVA_FQ_SIZE];
- unsigned int head, tail;
spinlock_t lock;
+ unsigned int head, tail;
+ unsigned int mod_mask;
+ struct iova_fq_entry entries[];
};
#define fq_ring_for_each(i, fq) \
- for ((i) = (fq)->head; (i) != (fq)->tail; (i) = ((i) + 1) % IOVA_FQ_SIZE)
+ for ((i) = (fq)->head; (i) != (fq)->tail; (i) = ((i) + 1) & (fq)->mod_mask)
static inline bool fq_full(struct iova_fq *fq)
{
assert_spin_locked(&fq->lock);
- return (((fq->tail + 1) % IOVA_FQ_SIZE) == fq->head);
+ return (((fq->tail + 1) & fq->mod_mask) == fq->head);
}
static inline unsigned int fq_ring_add(struct iova_fq *fq)
@@ -121,7 +130,7 @@ static inline unsigned int fq_ring_add(struct iova_fq *fq)
assert_spin_locked(&fq->lock);
- fq->tail = (idx + 1) % IOVA_FQ_SIZE;
+ fq->tail = (idx + 1) & fq->mod_mask;
return idx;
}
@@ -143,7 +152,7 @@ static void fq_ring_free(struct iommu_dma_cookie *cookie, struct iova_fq *fq)
fq->entries[idx].iova_pfn,
fq->entries[idx].pages);
- fq->head = (fq->head + 1) % IOVA_FQ_SIZE;
+ fq->head = (fq->head + 1) & fq->mod_mask;
}
}
@@ -241,7 +250,7 @@ static void queue_iova(struct iommu_dma_cookie *cookie,
if (!atomic_read(&cookie->fq_timer_on) &&
!atomic_xchg(&cookie->fq_timer_on, 1))
mod_timer(&cookie->fq_timer,
- jiffies + msecs_to_jiffies(IOVA_FQ_TIMEOUT));
+ jiffies + msecs_to_jiffies(cookie->fq_timer_timeout));
}
static void iommu_dma_free_fq_single(struct iova_fq *fq)
@@ -283,43 +292,45 @@ static void iommu_dma_free_fq(struct iommu_dma_cookie *cookie)
}
-static void iommu_dma_init_one_fq(struct iova_fq *fq)
+static void iommu_dma_init_one_fq(struct iova_fq *fq, unsigned int fq_size)
{
int i;
fq->head = 0;
fq->tail = 0;
+ fq->mod_mask = fq_size - 1;
spin_lock_init(&fq->lock);
- for (i = 0; i < IOVA_FQ_SIZE; i++)
+ for (i = 0; i < fq_size; i++)
INIT_LIST_HEAD(&fq->entries[i].freelist);
}
-static int iommu_dma_init_fq_single(struct iommu_dma_cookie *cookie)
+static int iommu_dma_init_fq_single(struct iommu_dma_cookie *cookie, unsigned int fq_size)
{
struct iova_fq *queue;
- queue = vzalloc(sizeof(*queue));
+ queue = vzalloc(struct_size(queue, entries, fq_size));
if (!queue)
return -ENOMEM;
- iommu_dma_init_one_fq(queue);
+ iommu_dma_init_one_fq(queue, fq_size);
cookie->single_fq = queue;
return 0;
}
-static int iommu_dma_init_fq_percpu(struct iommu_dma_cookie *cookie)
+static int iommu_dma_init_fq_percpu(struct iommu_dma_cookie *cookie, unsigned int fq_size)
{
struct iova_fq __percpu *queue;
int cpu;
- queue = alloc_percpu(struct iova_fq);
+ queue = __alloc_percpu(struct_size(queue, entries, fq_size),
+ __alignof__(*queue));
if (!queue)
return -ENOMEM;
for_each_possible_cpu(cpu)
- iommu_dma_init_one_fq(per_cpu_ptr(queue, cpu));
+ iommu_dma_init_one_fq(per_cpu_ptr(queue, cpu), fq_size);
cookie->percpu_fq = queue;
return 0;
}
@@ -328,24 +339,35 @@ static int iommu_dma_init_fq_percpu(struct iommu_dma_cookie *cookie)
int iommu_dma_init_fq(struct iommu_domain *domain)
{
struct iommu_dma_cookie *cookie = domain->iova_cookie;
+ unsigned int fq_size = IOVA_DEFAULT_FQ_SIZE;
int rc;
if (cookie->fq_domain)
return 0;
+ if (domain->type == IOMMU_DOMAIN_DMA_SQ)
+ fq_size = IOVA_SINGLE_FQ_SIZE;
+
+ if (!is_power_of_2(fq_size)) {
+ pr_err("FQ size must be a power of 2\n");
+ return -EINVAL;
+ }
+
atomic64_set(&cookie->fq_flush_start_cnt, 0);
atomic64_set(&cookie->fq_flush_finish_cnt, 0);
if (domain->type == IOMMU_DOMAIN_DMA_FQ)
- rc = iommu_dma_init_fq_percpu(cookie);
+ rc = iommu_dma_init_fq_percpu(cookie, fq_size);
else
- rc = iommu_dma_init_fq_single(cookie);
+ rc = iommu_dma_init_fq_single(cookie, fq_size);
if (rc) {
pr_warn("iova flush queue initialization failed\n");
return rc;
}
+ cookie->fq_timer_timeout = (domain->type == IOMMU_DOMAIN_DMA_SQ) ?
+ IOVA_SINGLE_FQ_TIMEOUT : IOVA_DEFAULT_FQ_TIMEOUT;
timer_setup(&cookie->fq_timer, fq_flush_timeout, 0);
atomic_set(&cookie->fq_timer_on, 0);
/*
--
2.34.1