2023-07-25 00:31:04

by Jacob Pan

[permalink] [raw]
Subject: [PATCH v11 0/8] Re-enable IDXD kernel workqueue under DMA API

Hi Joerg and all,

IDXD kernel work queues were disabled due to the flawed use of kernel VA
and SVA API.
Link: https://lore.kernel.org/linux-iommu/[email protected]/

The solution is to enable it under DMA API where IDXD shared workqueue users
can use ENQCMDS to submit work on buffers mapped by DMA API.

This patchset adds support for attaching PASID to the device's default
domain and the ability to allocate global PASIDs from IOMMU APIs. IDXD driver
can then re-enable the kernel work queues and use them under DMA API.

This depends on the IOASID removal series. (merged)
https://lore.kernel.org/all/[email protected]/


Thanks,

Jacob

---
Changelog:
v11:
- Rebased onto Joerg's next tree (v6.5-rc1)
- Split RIDPASID check in invalidation code into patch (6/8)
- Renamed iommu_alloc_global_pasid_dev to iommu_alloc_global_pasid (2/8)
- Added WARN_ON if no dev_pasid is found during remove (7/8)
v10:
- Fix global PASID alloc function with device's max_pasid=0
v9:
- Fix an IDXD driver issue where user interrupt enable bit got cleared
during device enable/disable cycle. Reported and tested by
Tony Zhu <[email protected]>
- Rebased to v6.4-rc7
v8:
- further vt-d driver refactoring (3-6) around set/remove device PASID
(Baolu)
- make consistent use of NO_PASID in SMMU code (Jean)
- fix off-by-one error in max PASID check (Kevin)
v7:
- renamed IOMMU_DEF_RID_PASID to be IOMMU_NO_PASID to be more generic
(Jean)
- simplify range checking for sva PASID (Baolu)
v6:
- use a simplified version of vt-d driver change for set_device_pasid
from Baolu.
- check and rename global PASID allocation base
v5:
- exclude two patches related to supervisor mode, taken by VT-d
maintainer Baolu.
- move PASID range check into allocation API so that device drivers
only need to pass in struct device*. (Kevin)
- factor out helper functions in device-domain attach (Baolu)
- make explicit use of RID_PASID across architectures
v4:
- move dummy functions outside ifdef CONFIG_IOMMU_SVA (Baolu)
- dropped domain type check while disabling idxd system PASID (Baolu)

v3:
- moved global PASID allocation API from SVA to IOMMU (Kevin)
- remove #ifdef around global PASID reservation during boot (Baolu)
- remove restriction on PASID 0 allocation (Baolu)
- fix a bug in sysfs domain change when attaching devices
- clear idxd user interrupt enable bit after disabling device( Fenghua)
v2:
- refactored device PASID attach domain ops based on Baolu's early patch
- addressed TLB flush gap
- explicitly reserve RID_PASID from SVA PASID number space
- get dma domain directly, avoid checking domain types



Jacob Pan (3):
iommu: Generalize PASID 0 for normal DMA w/o PASID
iommu: Move global PASID allocation from SVA to core
dmaengine/idxd: Re-enable kernel workqueue under DMA API

Lu Baolu (5):
iommu/vt-d: Add domain_flush_pasid_iotlb()
iommu/vt-d: Remove pasid_mutex
iommu/vt-d: Make prq draining code generic
iommu/vt-d: Prepare for set_dev_pasid callback
iommu/vt-d: Add set_dev_pasid callback for dma domain

drivers/dma/idxd/device.c | 39 ++---
drivers/dma/idxd/dma.c | 5 +-
drivers/dma/idxd/idxd.h | 9 +
drivers/dma/idxd/init.c | 54 +++++-
drivers/dma/idxd/sysfs.c | 7 -
.../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 2 +-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 +-
drivers/iommu/intel/iommu.c | 157 +++++++++++++++---
drivers/iommu/intel/iommu.h | 9 +
drivers/iommu/intel/pasid.c | 2 +-
drivers/iommu/intel/pasid.h | 2 -
drivers/iommu/intel/svm.c | 62 +------
drivers/iommu/iommu-sva.c | 29 ++--
drivers/iommu/iommu.c | 28 ++++
include/linux/iommu.h | 11 ++
15 files changed, 287 insertions(+), 145 deletions(-)

--
2.25.1



2023-07-25 00:36:53

by Jacob Pan

[permalink] [raw]
Subject: [PATCH v11 7/8] iommu/vt-d: Add set_dev_pasid callback for dma domain

From: Lu Baolu <[email protected]>

This allows the upper layers to set a domain to a PASID of a device
if the PASID feature is supported by the IOMMU hardware. The typical
use cases are, for example, kernel DMA with PASID and hardware
assisted mediated device drivers.

The attaching device and pasid information is tracked in a per-domain
list and is used for IOTLB and devTLB invalidation.

Signed-off-by: Lu Baolu <[email protected]>
Signed-off-by: Jacob Pan <[email protected]>
---
drivers/iommu/intel/iommu.c | 104 ++++++++++++++++++++++++++++++++++--
drivers/iommu/intel/iommu.h | 7 +++
2 files changed, 106 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 4a41aca6a2ba..f939fc2aa66c 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1359,6 +1359,7 @@ domain_lookup_dev_info(struct dmar_domain *domain,

static void domain_update_iotlb(struct dmar_domain *domain)
{
+ struct dev_pasid_info *dev_pasid;
struct device_domain_info *info;
bool has_iotlb_device = false;
unsigned long flags;
@@ -1370,6 +1371,14 @@ static void domain_update_iotlb(struct dmar_domain *domain)
break;
}
}
+
+ list_for_each_entry(dev_pasid, &domain->dev_pasids, link_domain) {
+ info = dev_iommu_priv_get(dev_pasid->dev);
+ if (info->ats_enabled) {
+ has_iotlb_device = true;
+ break;
+ }
+ }
domain->has_iotlb_device = has_iotlb_device;
spin_unlock_irqrestore(&domain->lock, flags);
}
@@ -1455,6 +1464,7 @@ static void __iommu_flush_dev_iotlb(struct device_domain_info *info,
static void iommu_flush_dev_iotlb(struct dmar_domain *domain,
u64 addr, unsigned mask)
{
+ struct dev_pasid_info *dev_pasid;
struct device_domain_info *info;
unsigned long flags;

@@ -1464,6 +1474,19 @@ static void iommu_flush_dev_iotlb(struct dmar_domain *domain,
spin_lock_irqsave(&domain->lock, flags);
list_for_each_entry(info, &domain->devices, link)
__iommu_flush_dev_iotlb(info, addr, mask);
+
+ list_for_each_entry(dev_pasid, &domain->dev_pasids, link_domain) {
+ info = dev_iommu_priv_get(dev_pasid->dev);
+
+ if (!info->ats_enabled)
+ continue;
+
+ qi_flush_dev_iotlb_pasid(info->iommu,
+ PCI_DEVID(info->bus, info->devfn),
+ info->pfsid, dev_pasid->pasid,
+ info->ats_qdep, addr,
+ mask);
+ }
spin_unlock_irqrestore(&domain->lock, flags);
}

@@ -1472,9 +1495,13 @@ static void domain_flush_pasid_iotlb(struct intel_iommu *iommu,
unsigned long npages, bool ih)
{
u16 did = domain_id_iommu(domain, iommu);
+ struct dev_pasid_info *dev_pasid;
unsigned long flags;

spin_lock_irqsave(&domain->lock, flags);
+ list_for_each_entry(dev_pasid, &domain->dev_pasids, link_domain)
+ qi_flush_piotlb(iommu, did, dev_pasid->pasid, addr, npages, ih);
+
if (!list_empty(&domain->devices))
qi_flush_piotlb(iommu, did, IOMMU_NO_PASID, addr, npages, ih);
spin_unlock_irqrestore(&domain->lock, flags);
@@ -1739,6 +1766,7 @@ static struct dmar_domain *alloc_domain(unsigned int type)
domain->use_first_level = true;
domain->has_iotlb_device = false;
INIT_LIST_HEAD(&domain->devices);
+ INIT_LIST_HEAD(&domain->dev_pasids);
spin_lock_init(&domain->lock);
xa_init(&domain->iommu_array);

@@ -4719,7 +4747,10 @@ static void intel_iommu_iotlb_sync_map(struct iommu_domain *domain,
static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
{
struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
+ struct dev_pasid_info *curr, *dev_pasid = NULL;
+ struct dmar_domain *dmar_domain;
struct iommu_domain *domain;
+ unsigned long flags;

domain = iommu_get_domain_for_dev_pasid(dev, pasid, 0);
if (WARN_ON_ONCE(!domain))
@@ -4735,17 +4766,79 @@ static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
goto out_tear_down;
}

- /*
- * Should never reach here until we add support for attaching
- * non-SVA domain to a pasid.
- */
- WARN_ON(1);
+ dmar_domain = to_dmar_domain(domain);
+ spin_lock_irqsave(&dmar_domain->lock, flags);
+ list_for_each_entry(curr, &dmar_domain->dev_pasids, link_domain) {
+ if (curr->dev == dev && curr->pasid == pasid) {
+ list_del(&curr->link_domain);
+ dev_pasid = curr;
+ break;
+ }
+ }
+ WARN_ON_ONCE(!dev_pasid);
+ spin_unlock_irqrestore(&dmar_domain->lock, flags);

+ domain_detach_iommu(dmar_domain, iommu);
+ kfree(dev_pasid);
out_tear_down:
intel_pasid_tear_down_entry(iommu, dev, pasid, false);
intel_drain_pasid_prq(dev, pasid);
}

+static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
+ struct device *dev, ioasid_t pasid)
+{
+ struct device_domain_info *info = dev_iommu_priv_get(dev);
+ struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+ struct intel_iommu *iommu = info->iommu;
+ struct dev_pasid_info *dev_pasid;
+ unsigned long flags;
+ int ret;
+
+ if (!pasid_supported(iommu) || dev_is_real_dma_subdevice(dev))
+ return -EOPNOTSUPP;
+
+ if (context_copied(iommu, info->bus, info->devfn))
+ return -EBUSY;
+
+ ret = prepare_domain_attach_device(domain, dev);
+ if (ret)
+ return ret;
+
+ dev_pasid = kzalloc(sizeof(*dev_pasid), GFP_KERNEL);
+ if (!dev_pasid)
+ return -ENOMEM;
+
+ ret = domain_attach_iommu(dmar_domain, iommu);
+ if (ret)
+ goto out_free;
+
+ if (domain_type_is_si(dmar_domain))
+ ret = intel_pasid_setup_pass_through(iommu, dmar_domain,
+ dev, pasid);
+ else if (dmar_domain->use_first_level)
+ ret = domain_setup_first_level(iommu, dmar_domain,
+ dev, pasid);
+ else
+ ret = intel_pasid_setup_second_level(iommu, dmar_domain,
+ dev, pasid);
+ if (ret)
+ goto out_detach_iommu;
+
+ dev_pasid->dev = dev;
+ dev_pasid->pasid = pasid;
+ spin_lock_irqsave(&dmar_domain->lock, flags);
+ list_add(&dev_pasid->link_domain, &dmar_domain->dev_pasids);
+ spin_unlock_irqrestore(&dmar_domain->lock, flags);
+
+ return 0;
+out_detach_iommu:
+ domain_detach_iommu(dmar_domain, iommu);
+out_free:
+ kfree(dev_pasid);
+ return ret;
+}
+
const struct iommu_ops intel_iommu_ops = {
.capable = intel_iommu_capable,
.domain_alloc = intel_iommu_domain_alloc,
@@ -4765,6 +4858,7 @@ const struct iommu_ops intel_iommu_ops = {
#endif
.default_domain_ops = &(const struct iommu_domain_ops) {
.attach_dev = intel_iommu_attach_device,
+ .set_dev_pasid = intel_iommu_set_dev_pasid,
.map_pages = intel_iommu_map_pages,
.unmap_pages = intel_iommu_unmap_pages,
.iotlb_sync_map = intel_iommu_iotlb_sync_map,
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index 6d94a29f5d52..c18fb699c87a 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -595,6 +595,7 @@ struct dmar_domain {

spinlock_t lock; /* Protect device tracking lists */
struct list_head devices; /* all devices' list */
+ struct list_head dev_pasids; /* all attached pasids */

struct dma_pte *pgd; /* virtual address */
int gaw; /* max guest address width */
@@ -717,6 +718,12 @@ struct device_domain_info {
struct pasid_table *pasid_table; /* pasid table */
};

+struct dev_pasid_info {
+ struct list_head link_domain; /* link to domain siblings */
+ struct device *dev;
+ ioasid_t pasid;
+};
+
static inline void __iommu_flush_cache(
struct intel_iommu *iommu, void *addr, int size)
{
--
2.25.1


2023-07-27 06:35:05

by Tian, Kevin

[permalink] [raw]
Subject: RE: [PATCH v11 7/8] iommu/vt-d: Add set_dev_pasid callback for dma domain

> From: Jacob Pan <[email protected]>
> Sent: Tuesday, July 25, 2023 6:26 AM
>
> From: Lu Baolu <[email protected]>
>
> This allows the upper layers to set a domain to a PASID of a device
> if the PASID feature is supported by the IOMMU hardware. The typical
> use cases are, for example, kernel DMA with PASID and hardware
> assisted mediated device drivers.
>
> The attaching device and pasid information is tracked in a per-domain
> list and is used for IOTLB and devTLB invalidation.
>
> Signed-off-by: Lu Baolu <[email protected]>
> Signed-off-by: Jacob Pan <[email protected]>

Reviewed-by: Kevin Tian <[email protected]>

2023-08-02 15:35:50

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH v11 0/8] Re-enable IDXD kernel workqueue under DMA API

On Mon, Jul 24, 2023 at 03:25:30PM -0700, Jacob Pan wrote:
> Hi Joerg and all,
>
> IDXD kernel work queues were disabled due to the flawed use of kernel VA
> and SVA API.
> Link: https://lore.kernel.org/linux-iommu/[email protected]/
>
> The solution is to enable it under DMA API where IDXD shared workqueue users
> can use ENQCMDS to submit work on buffers mapped by DMA API.
>
> This patchset adds support for attaching PASID to the device's default
> domain and the ability to allocate global PASIDs from IOMMU APIs. IDXD driver
> can then re-enable the kernel work queues and use them under DMA API.
>
> This depends on the IOASID removal series. (merged)
> https://lore.kernel.org/all/[email protected]/

If the vt-d driver bits are fine I think this series is OK

Thanks,
Jason

2023-08-02 16:53:40

by Jacob Pan

[permalink] [raw]
Subject: Re: [PATCH v11 0/8] Re-enable IDXD kernel workqueue under DMA API

Hi Joerg/Baolu,

Any more comments on this series? Thanks

On Mon, 24 Jul 2023 15:25:30 -0700, Jacob Pan
<[email protected]> wrote:

> Hi Joerg and all,
>
> IDXD kernel work queues were disabled due to the flawed use of kernel VA
> and SVA API.
> Link:
> https://lore.kernel.org/linux-iommu/[email protected]/
>
> The solution is to enable it under DMA API where IDXD shared workqueue
> users can use ENQCMDS to submit work on buffers mapped by DMA API.
>
> This patchset adds support for attaching PASID to the device's default
> domain and the ability to allocate global PASIDs from IOMMU APIs. IDXD
> driver can then re-enable the kernel work queues and use them under DMA
> API.
>
> This depends on the IOASID removal series. (merged)
> https://lore.kernel.org/all/[email protected]/
>
>
> Thanks,
>
> Jacob
>
> ---
> Changelog:
> v11:
> - Rebased onto Joerg's next tree (v6.5-rc1)
> - Split RIDPASID check in invalidation code into patch (6/8)
> - Renamed iommu_alloc_global_pasid_dev to
> iommu_alloc_global_pasid (2/8)
> - Added WARN_ON if no dev_pasid is found during remove (7/8)
> v10:
> - Fix global PASID alloc function with device's max_pasid=0
> v9:
> - Fix an IDXD driver issue where user interrupt enable bit got
> cleared during device enable/disable cycle. Reported and tested by
> Tony Zhu <[email protected]>
> - Rebased to v6.4-rc7
> v8:
> - further vt-d driver refactoring (3-6) around set/remove device
> PASID (Baolu)
> - make consistent use of NO_PASID in SMMU code (Jean)
> - fix off-by-one error in max PASID check (Kevin)
> v7:
> - renamed IOMMU_DEF_RID_PASID to be IOMMU_NO_PASID to be more
> generic (Jean)
> - simplify range checking for sva PASID (Baolu)
> v6:
> - use a simplified version of vt-d driver change for
> set_device_pasid from Baolu.
> - check and rename global PASID allocation base
> v5:
> - exclude two patches related to supervisor mode, taken by VT-d
> maintainer Baolu.
> - move PASID range check into allocation API so that device
> drivers only need to pass in struct device*. (Kevin)
> - factor out helper functions in device-domain attach (Baolu)
> - make explicit use of RID_PASID across architectures
> v4:
> - move dummy functions outside ifdef CONFIG_IOMMU_SVA (Baolu)
> - dropped domain type check while disabling idxd system PASID
> (Baolu)
>
> v3:
> - moved global PASID allocation API from SVA to IOMMU (Kevin)
> - remove #ifdef around global PASID reservation during boot
> (Baolu)
> - remove restriction on PASID 0 allocation (Baolu)
> - fix a bug in sysfs domain change when attaching devices
> - clear idxd user interrupt enable bit after disabling device(
> Fenghua) v2:
> - refactored device PASID attach domain ops based on Baolu's
> early patch
> - addressed TLB flush gap
> - explicitly reserve RID_PASID from SVA PASID number space
> - get dma domain directly, avoid checking domain types
>
>
>
> Jacob Pan (3):
> iommu: Generalize PASID 0 for normal DMA w/o PASID
> iommu: Move global PASID allocation from SVA to core
> dmaengine/idxd: Re-enable kernel workqueue under DMA API
>
> Lu Baolu (5):
> iommu/vt-d: Add domain_flush_pasid_iotlb()
> iommu/vt-d: Remove pasid_mutex
> iommu/vt-d: Make prq draining code generic
> iommu/vt-d: Prepare for set_dev_pasid callback
> iommu/vt-d: Add set_dev_pasid callback for dma domain
>
> drivers/dma/idxd/device.c | 39 ++---
> drivers/dma/idxd/dma.c | 5 +-
> drivers/dma/idxd/idxd.h | 9 +
> drivers/dma/idxd/init.c | 54 +++++-
> drivers/dma/idxd/sysfs.c | 7 -
> .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 2 +-
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 16 +-
> drivers/iommu/intel/iommu.c | 157 +++++++++++++++---
> drivers/iommu/intel/iommu.h | 9 +
> drivers/iommu/intel/pasid.c | 2 +-
> drivers/iommu/intel/pasid.h | 2 -
> drivers/iommu/intel/svm.c | 62 +------
> drivers/iommu/iommu-sva.c | 29 ++--
> drivers/iommu/iommu.c | 28 ++++
> include/linux/iommu.h | 11 ++
> 15 files changed, 287 insertions(+), 145 deletions(-)
>


Thanks,

Jacob