Skip to content

Commit f5ccf55

Browse files
Jacob Panjoergroedel
authored andcommitted
dmaengine/idxd: Re-enable kernel workqueue under DMA API
Kernel workqueues were disabled due to flawed use of kernel VA and SVA API. Now that we have the support for attaching PASID to the device's default domain and the ability to reserve global PASIDs from SVA APIs, we can re-enable the kernel work queues and use them under DMA API. We also use non-privileged access for in-kernel DMA to be consistent with the IOMMU settings. Consequently, interrupt for user privilege is enabled for work completion IRQs. Link: https://lore.kernel.org/linux-iommu/[email protected]/ Tested-by: Tony Zhu <[email protected]> Reviewed-by: Dave Jiang <[email protected]> Reviewed-by: Fenghua Yu <[email protected]> Reviewed-by: Lu Baolu <[email protected]> Reviewed-by: Jason Gunthorpe <[email protected]> Acked-by: Vinod Koul <[email protected]> Signed-off-by: Jacob Pan <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Lu Baolu <[email protected]> Signed-off-by: Joerg Roedel <[email protected]>
1 parent 7d0c9da commit f5ccf55

File tree

5 files changed

+76
-38
lines changed

5 files changed

+76
-38
lines changed

drivers/dma/idxd/device.c

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -299,21 +299,6 @@ void idxd_wqs_unmap_portal(struct idxd_device *idxd)
299299
}
300300
}
301301

302-
static void __idxd_wq_set_priv_locked(struct idxd_wq *wq, int priv)
303-
{
304-
struct idxd_device *idxd = wq->idxd;
305-
union wqcfg wqcfg;
306-
unsigned int offset;
307-
308-
offset = WQCFG_OFFSET(idxd, wq->id, WQCFG_PRIVL_IDX);
309-
spin_lock(&idxd->dev_lock);
310-
wqcfg.bits[WQCFG_PRIVL_IDX] = ioread32(idxd->reg_base + offset);
311-
wqcfg.priv = priv;
312-
wq->wqcfg->bits[WQCFG_PRIVL_IDX] = wqcfg.bits[WQCFG_PRIVL_IDX];
313-
iowrite32(wqcfg.bits[WQCFG_PRIVL_IDX], idxd->reg_base + offset);
314-
spin_unlock(&idxd->dev_lock);
315-
}
316-
317302
static void __idxd_wq_set_pasid_locked(struct idxd_wq *wq, int pasid)
318303
{
319304
struct idxd_device *idxd = wq->idxd;
@@ -1423,26 +1408,21 @@ int drv_enable_wq(struct idxd_wq *wq)
14231408
}
14241409

14251410
/*
1426-
* In the event that the WQ is configurable for pasid and priv bits.
1427-
* For kernel wq, the driver should setup the pasid, pasid_en, and priv bit.
1428-
* However, for non-kernel wq, the driver should only set the pasid_en bit for
1429-
* shared wq. A dedicated wq that is not 'kernel' type will configure pasid and
1411+
* In the event that the WQ is configurable for pasid, the driver
1412+
* should setup the pasid, pasid_en bit. This is true for both kernel
1413+
* and user shared workqueues. There is no need to setup priv bit in
1414+
* that in-kernel DMA will also do user privileged requests.
1415+
* A dedicated wq that is not 'kernel' type will configure pasid and
14301416
* pasid_en later on so there is no need to setup.
14311417
*/
14321418
if (test_bit(IDXD_FLAG_CONFIGURABLE, &idxd->flags)) {
1433-
int priv = 0;
1434-
14351419
if (wq_pasid_enabled(wq)) {
14361420
if (is_idxd_wq_kernel(wq) || wq_shared(wq)) {
14371421
u32 pasid = wq_dedicated(wq) ? idxd->pasid : 0;
14381422

14391423
__idxd_wq_set_pasid_locked(wq, pasid);
14401424
}
14411425
}
1442-
1443-
if (is_idxd_wq_kernel(wq))
1444-
priv = 1;
1445-
__idxd_wq_set_priv_locked(wq, priv);
14461426
}
14471427

14481428
rc = 0;
@@ -1550,6 +1530,15 @@ int idxd_device_drv_probe(struct idxd_dev *idxd_dev)
15501530
if (rc < 0)
15511531
return -ENXIO;
15521532

1533+
/*
1534+
* System PASID is preserved across device disable/enable cycle, but
1535+
* genconfig register content gets cleared during device reset. We
1536+
* need to re-enable user interrupts for kernel work queue completion
1537+
* IRQ to function.
1538+
*/
1539+
if (idxd->pasid != IOMMU_PASID_INVALID)
1540+
idxd_set_user_intr(idxd, 1);
1541+
15531542
rc = idxd_device_evl_setup(idxd);
15541543
if (rc < 0) {
15551544
idxd->cmd_status = IDXD_SCMD_DEV_EVL_ERR;

drivers/dma/idxd/dma.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,10 @@ static inline void idxd_prep_desc_common(struct idxd_wq *wq,
7575
hw->xfer_size = len;
7676
/*
7777
* For dedicated WQ, this field is ignored and HW will use the WQCFG.priv
78-
* field instead. This field should be set to 1 for kernel descriptors.
78+
* field instead. This field should be set to 0 for kernel descriptors
79+
* since kernel DMA on VT-d supports "user" privilege only.
7980
*/
80-
hw->priv = 1;
81+
hw->priv = 0;
8182
hw->completion_addr = compl;
8283
}
8384

drivers/dma/idxd/idxd.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,15 @@ static inline struct idxd_device *ie_to_idxd(struct idxd_irq_entry *ie)
473473
return container_of(ie, struct idxd_device, ie);
474474
}
475475

476+
static inline void idxd_set_user_intr(struct idxd_device *idxd, bool enable)
477+
{
478+
union gencfg_reg reg;
479+
480+
reg.bits = ioread32(idxd->reg_base + IDXD_GENCFG_OFFSET);
481+
reg.user_int_en = enable;
482+
iowrite32(reg.bits, idxd->reg_base + IDXD_GENCFG_OFFSET);
483+
}
484+
476485
extern struct bus_type dsa_bus_type;
477486

478487
extern bool support_enqcmd;

drivers/dma/idxd/init.c

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -550,14 +550,59 @@ static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_d
550550

551551
static int idxd_enable_system_pasid(struct idxd_device *idxd)
552552
{
553-
return -EOPNOTSUPP;
553+
struct pci_dev *pdev = idxd->pdev;
554+
struct device *dev = &pdev->dev;
555+
struct iommu_domain *domain;
556+
ioasid_t pasid;
557+
int ret;
558+
559+
/*
560+
* Attach a global PASID to the DMA domain so that we can use ENQCMDS
561+
* to submit work on buffers mapped by DMA API.
562+
*/
563+
domain = iommu_get_domain_for_dev(dev);
564+
if (!domain)
565+
return -EPERM;
566+
567+
pasid = iommu_alloc_global_pasid(dev);
568+
if (pasid == IOMMU_PASID_INVALID)
569+
return -ENOSPC;
570+
571+
/*
572+
* DMA domain is owned by the driver, it should support all valid
573+
* types such as DMA-FQ, identity, etc.
574+
*/
575+
ret = iommu_attach_device_pasid(domain, dev, pasid);
576+
if (ret) {
577+
dev_err(dev, "failed to attach device pasid %d, domain type %d",
578+
pasid, domain->type);
579+
iommu_free_global_pasid(pasid);
580+
return ret;
581+
}
582+
583+
/* Since we set user privilege for kernel DMA, enable completion IRQ */
584+
idxd_set_user_intr(idxd, 1);
585+
idxd->pasid = pasid;
586+
587+
return ret;
554588
}
555589

556590
static void idxd_disable_system_pasid(struct idxd_device *idxd)
557591
{
592+
struct pci_dev *pdev = idxd->pdev;
593+
struct device *dev = &pdev->dev;
594+
struct iommu_domain *domain;
595+
596+
domain = iommu_get_domain_for_dev(dev);
597+
if (!domain)
598+
return;
599+
600+
iommu_detach_device_pasid(domain, dev, idxd->pasid);
601+
iommu_free_global_pasid(idxd->pasid);
558602

559-
iommu_sva_unbind_device(idxd->sva);
603+
idxd_set_user_intr(idxd, 0);
560604
idxd->sva = NULL;
605+
idxd->pasid = IOMMU_PASID_INVALID;
561606
}
562607

563608
static int idxd_enable_sva(struct pci_dev *pdev)
@@ -600,8 +645,9 @@ static int idxd_probe(struct idxd_device *idxd)
600645
} else {
601646
set_bit(IDXD_FLAG_USER_PASID_ENABLED, &idxd->flags);
602647

603-
if (idxd_enable_system_pasid(idxd))
604-
dev_warn(dev, "No in-kernel DMA with PASID.\n");
648+
rc = idxd_enable_system_pasid(idxd);
649+
if (rc)
650+
dev_warn(dev, "No in-kernel DMA with PASID. %d\n", rc);
605651
else
606652
set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
607653
}

drivers/dma/idxd/sysfs.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -948,13 +948,6 @@ static ssize_t wq_name_store(struct device *dev,
948948
if (strlen(buf) > WQ_NAME_SIZE || strlen(buf) == 0)
949949
return -EINVAL;
950950

951-
/*
952-
* This is temporarily placed here until we have SVM support for
953-
* dmaengine.
954-
*/
955-
if (wq->type == IDXD_WQT_KERNEL && device_pasid_enabled(wq->idxd))
956-
return -EOPNOTSUPP;
957-
958951
input = kstrndup(buf, count, GFP_KERNEL);
959952
if (!input)
960953
return -ENOMEM;

0 commit comments

Comments
 (0)