[PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Lu Baolu]

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 <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.h |   8 +++
 drivers/iommu/intel/iommu.c | 118 +++++++++++++++++++++++++++++++++++-
 2 files changed, 124 insertions(+), 2 deletions(-)

---
Note: This is a follow-up of this patch:
https://lore.kernel.org/linux-iommu/20220607014942.3954894-4-baolu.lu@linux.intel.com/
which, removed the SVM_FLAG_SUPERVISOR_MODE support from the IOMMU SVA
interface and recommended the device driver to handle kernel DMA with
PASID through the kernel DMA APIs. It is nothing new anyway. It's
a simplified version of the previous callbacks which have existed in
the tree for more than one year. Those callbacks have been removed by
commit 241469685d8d ("iommu/vt-d: Remove aux-domain related callbacks")
in order for the new iommufd framework.

diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index 2dd4c5193cc1..a703e0768f47 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -541,6 +541,7 @@ struct dmar_domain {
 
 	spinlock_t lock;		/* Protect device tracking lists */
 	struct list_head devices;	/* all devices' list */
+	struct list_head subdevices;	/* all subdevices' list */
 
 	struct dma_pte	*pgd;		/* virtual address */
 	int		gaw;		/* max guest address width */
@@ -626,6 +627,13 @@ struct device_domain_info {
 	struct pasid_table *pasid_table; /* pasid table */
 };
 
+/* PCI domain-subdevice relationship */
+struct subdev_domain_info {
+	struct list_head link_domain;	/* link to domain siblings */
+	struct device *dev;		/* physical device derived from */
+	ioasid_t pasid;			/* PASID on physical device */
+};
+
 static inline void __iommu_flush_cache(
 	struct intel_iommu *iommu, void *addr, int size)
 {
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index aeeb1185d397..6eced9e87cda 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1393,6 +1393,7 @@ iommu_support_dev_iotlb(struct dmar_domain *domain, struct intel_iommu *iommu,
 
 static void domain_update_iotlb(struct dmar_domain *domain)
 {
+	struct subdev_domain_info *sinfo;
 	struct device_domain_info *info;
 	bool has_iotlb_device = false;
 
@@ -1403,6 +1404,14 @@ static void domain_update_iotlb(struct dmar_domain *domain)
 			break;
 		}
 	}
+
+	list_for_each_entry(sinfo, &domain->subdevices, link_domain) {
+		info = dev_iommu_priv_get(sinfo->dev);
+		if (info->ats_enabled) {
+			has_iotlb_device = true;
+			break;
+		}
+	}
 	domain->has_iotlb_device = has_iotlb_device;
 	spin_unlock(&domain->lock);
 }
@@ -1495,6 +1504,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 subdev_domain_info *sinfo;
 	struct device_domain_info *info;
 
 	if (!domain->has_iotlb_device)
@@ -1503,6 +1513,35 @@ static void iommu_flush_dev_iotlb(struct dmar_domain *domain,
 	spin_lock(&domain->lock);
 	list_for_each_entry(info, &domain->devices, link)
 		__iommu_flush_dev_iotlb(info, addr, mask);
+
+	list_for_each_entry(sinfo, &domain->subdevices, link_domain) {
+		info = dev_iommu_priv_get(sinfo->dev);
+		qi_flush_dev_iotlb_pasid(info->iommu,
+					 PCI_DEVID(info->bus, info->devfn),
+					 info->pfsid, sinfo->pasid,
+					 info->ats_qdep, addr,
+					 mask);
+	}
+	spin_unlock(&domain->lock);
+}
+
+/*
+ * The VT-d spec requires to use PASID-based-IOTLB Invalidation to invalidate
+ * IOTLB and the paging-structure-caches for a first-level page table.
+ */
+static void domain_flush_pasid_iotlb(struct intel_iommu *iommu,
+				     struct dmar_domain *domain, u64 addr,
+				     unsigned long npages, bool ih)
+{
+	u16 did = domain->iommu_did[iommu->seq_id];
+	struct subdev_domain_info *sinfo;
+
+	spin_lock(&domain->lock);
+	list_for_each_entry(sinfo, &domain->subdevices, link_domain)
+		qi_flush_piotlb(iommu, did, sinfo->pasid, addr, npages, ih);
+
+	if (!list_empty(&domain->devices))
+		qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr, npages, ih);
 	spin_unlock(&domain->lock);
 }
 
@@ -1522,7 +1561,7 @@ static void iommu_flush_iotlb_psi(struct intel_iommu *iommu,
 		ih = 1 << 6;
 
 	if (domain_use_first_level(domain)) {
-		qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr, pages, ih);
+		domain_flush_pasid_iotlb(iommu, domain, addr, pages, ih);
 	} else {
 		unsigned long bitmask = aligned_pages - 1;
 
@@ -1591,7 +1630,7 @@ static void intel_flush_iotlb_all(struct iommu_domain *domain)
 		u16 did = dmar_domain->iommu_did[iommu->seq_id];
 
 		if (domain_use_first_level(dmar_domain))
-			qi_flush_piotlb(iommu, did, PASID_RID2PASID, 0, -1, 0);
+			domain_flush_pasid_iotlb(iommu, dmar_domain, 0, -1, 0);
 		else
 			iommu->flush.flush_iotlb(iommu, did, 0, 0,
 						 DMA_TLB_DSI_FLUSH);
@@ -1763,6 +1802,7 @@ static struct dmar_domain *alloc_domain(unsigned int type)
 		domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL;
 	domain->has_iotlb_device = false;
 	INIT_LIST_HEAD(&domain->devices);
+	INIT_LIST_HEAD(&domain->subdevices);
 	spin_lock_init(&domain->lock);
 
 	return domain;
@@ -4789,6 +4829,78 @@ static void intel_iommu_iotlb_sync_map(struct iommu_domain *domain,
 	}
 }
 
+static int intel_iommu_attach_device_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 subdev_domain_info *sinfo;
+	int ret;
+
+	if (!pasid_supported(iommu))
+		return -ENODEV;
+
+	ret = prepare_domain_attach_device(domain, dev);
+	if (ret)
+		return ret;
+
+	sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+	if (!sinfo)
+		return -ENOMEM;
+
+	ret = domain_attach_iommu(dmar_domain, iommu);
+	if (ret)
+		goto out_free_sinfo;
+
+	if (domain_use_first_level(dmar_domain))
+		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;
+
+	sinfo->dev = dev;
+	sinfo->pasid = pasid;
+	spin_lock(&dmar_domain->lock);
+	list_add(&sinfo->link_domain, &dmar_domain->subdevices);
+	spin_unlock(&dmar_domain->lock);
+
+	return 0;
+out_detach_iommu:
+	domain_detach_iommu(dmar_domain, iommu);
+out_free_sinfo:
+	kfree(sinfo);
+	return ret;
+}
+
+static void intel_iommu_detach_device_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 subdev_domain_info *i, *n, *sinfo = NULL;
+	struct intel_iommu *iommu = info->iommu;
+
+	spin_lock(&dmar_domain->lock);
+	list_for_each_entry_safe(i, n, &dmar_domain->subdevices, link_domain) {
+		if (i->dev == dev && i->pasid == pasid) {
+			list_del(&i->link_domain);
+			sinfo = i;
+			break;
+		}
+	}
+	spin_unlock(&dmar_domain->lock);
+
+	if (WARN_ON(!sinfo))
+		return;
+
+	intel_pasid_tear_down_entry(iommu, dev, pasid, false);
+	domain_detach_iommu(dmar_domain, iommu);
+	kfree(sinfo);
+}
+
 const struct iommu_ops intel_iommu_ops = {
 	.capable		= intel_iommu_capable,
 	.domain_alloc		= intel_iommu_domain_alloc,
@@ -4809,6 +4921,8 @@ const struct iommu_ops intel_iommu_ops = {
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev		= intel_iommu_attach_device,
 		.detach_dev		= intel_iommu_detach_device,
+		.set_dev_pasid		= intel_iommu_attach_device_pasid,
+		.block_dev_pasid	= intel_iommu_detach_device_pasid,
 		.map_pages		= intel_iommu_map_pages,
 		.unmap_pages		= intel_iommu_unmap_pages,
 		.iotlb_sync_map		= intel_iommu_iotlb_sync_map,
-- 
2.25.1

[PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Lu Baolu]

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 <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.h |   8 +++
 drivers/iommu/intel/iommu.c | 118 +++++++++++++++++++++++++++++++++++-
 2 files changed, 124 insertions(+), 2 deletions(-)

---
Note: This is a follow-up of this patch:
https://lore.kernel.org/linux-iommu/20220607014942.3954894-4-baolu.lu@linux.intel.com/
which, removed the SVM_FLAG_SUPERVISOR_MODE support from the IOMMU SVA
interface and recommended the device driver to handle kernel DMA with
PASID through the kernel DMA APIs. It is nothing new anyway. It's
a simplified version of the previous callbacks which have existed in
the tree for more than one year. Those callbacks have been removed by
commit 241469685d8d ("iommu/vt-d: Remove aux-domain related callbacks")
in order for the new iommufd framework.

diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index 2dd4c5193cc1..a703e0768f47 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -541,6 +541,7 @@ struct dmar_domain {
 
 	spinlock_t lock;		/* Protect device tracking lists */
 	struct list_head devices;	/* all devices' list */
+	struct list_head subdevices;	/* all subdevices' list */
 
 	struct dma_pte	*pgd;		/* virtual address */
 	int		gaw;		/* max guest address width */
@@ -626,6 +627,13 @@ struct device_domain_info {
 	struct pasid_table *pasid_table; /* pasid table */
 };
 
+/* PCI domain-subdevice relationship */
+struct subdev_domain_info {
+	struct list_head link_domain;	/* link to domain siblings */
+	struct device *dev;		/* physical device derived from */
+	ioasid_t pasid;			/* PASID on physical device */
+};
+
 static inline void __iommu_flush_cache(
 	struct intel_iommu *iommu, void *addr, int size)
 {
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index aeeb1185d397..6eced9e87cda 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1393,6 +1393,7 @@ iommu_support_dev_iotlb(struct dmar_domain *domain, struct intel_iommu *iommu,
 
 static void domain_update_iotlb(struct dmar_domain *domain)
 {
+	struct subdev_domain_info *sinfo;
 	struct device_domain_info *info;
 	bool has_iotlb_device = false;
 
@@ -1403,6 +1404,14 @@ static void domain_update_iotlb(struct dmar_domain *domain)
 			break;
 		}
 	}
+
+	list_for_each_entry(sinfo, &domain->subdevices, link_domain) {
+		info = dev_iommu_priv_get(sinfo->dev);
+		if (info->ats_enabled) {
+			has_iotlb_device = true;
+			break;
+		}
+	}
 	domain->has_iotlb_device = has_iotlb_device;
 	spin_unlock(&domain->lock);
 }
@@ -1495,6 +1504,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 subdev_domain_info *sinfo;
 	struct device_domain_info *info;
 
 	if (!domain->has_iotlb_device)
@@ -1503,6 +1513,35 @@ static void iommu_flush_dev_iotlb(struct dmar_domain *domain,
 	spin_lock(&domain->lock);
 	list_for_each_entry(info, &domain->devices, link)
 		__iommu_flush_dev_iotlb(info, addr, mask);
+
+	list_for_each_entry(sinfo, &domain->subdevices, link_domain) {
+		info = dev_iommu_priv_get(sinfo->dev);
+		qi_flush_dev_iotlb_pasid(info->iommu,
+					 PCI_DEVID(info->bus, info->devfn),
+					 info->pfsid, sinfo->pasid,
+					 info->ats_qdep, addr,
+					 mask);
+	}
+	spin_unlock(&domain->lock);
+}
+
+/*
+ * The VT-d spec requires to use PASID-based-IOTLB Invalidation to invalidate
+ * IOTLB and the paging-structure-caches for a first-level page table.
+ */
+static void domain_flush_pasid_iotlb(struct intel_iommu *iommu,
+				     struct dmar_domain *domain, u64 addr,
+				     unsigned long npages, bool ih)
+{
+	u16 did = domain->iommu_did[iommu->seq_id];
+	struct subdev_domain_info *sinfo;
+
+	spin_lock(&domain->lock);
+	list_for_each_entry(sinfo, &domain->subdevices, link_domain)
+		qi_flush_piotlb(iommu, did, sinfo->pasid, addr, npages, ih);
+
+	if (!list_empty(&domain->devices))
+		qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr, npages, ih);
 	spin_unlock(&domain->lock);
 }
 
@@ -1522,7 +1561,7 @@ static void iommu_flush_iotlb_psi(struct intel_iommu *iommu,
 		ih = 1 << 6;
 
 	if (domain_use_first_level(domain)) {
-		qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr, pages, ih);
+		domain_flush_pasid_iotlb(iommu, domain, addr, pages, ih);
 	} else {
 		unsigned long bitmask = aligned_pages - 1;
 
@@ -1591,7 +1630,7 @@ static void intel_flush_iotlb_all(struct iommu_domain *domain)
 		u16 did = dmar_domain->iommu_did[iommu->seq_id];
 
 		if (domain_use_first_level(dmar_domain))
-			qi_flush_piotlb(iommu, did, PASID_RID2PASID, 0, -1, 0);
+			domain_flush_pasid_iotlb(iommu, dmar_domain, 0, -1, 0);
 		else
 			iommu->flush.flush_iotlb(iommu, did, 0, 0,
 						 DMA_TLB_DSI_FLUSH);
@@ -1763,6 +1802,7 @@ static struct dmar_domain *alloc_domain(unsigned int type)
 		domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL;
 	domain->has_iotlb_device = false;
 	INIT_LIST_HEAD(&domain->devices);
+	INIT_LIST_HEAD(&domain->subdevices);
 	spin_lock_init(&domain->lock);
 
 	return domain;
@@ -4789,6 +4829,78 @@ static void intel_iommu_iotlb_sync_map(struct iommu_domain *domain,
 	}
 }
 
+static int intel_iommu_attach_device_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 subdev_domain_info *sinfo;
+	int ret;
+
+	if (!pasid_supported(iommu))
+		return -ENODEV;
+
+	ret = prepare_domain_attach_device(domain, dev);
+	if (ret)
+		return ret;
+
+	sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
+	if (!sinfo)
+		return -ENOMEM;
+
+	ret = domain_attach_iommu(dmar_domain, iommu);
+	if (ret)
+		goto out_free_sinfo;
+
+	if (domain_use_first_level(dmar_domain))
+		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;
+
+	sinfo->dev = dev;
+	sinfo->pasid = pasid;
+	spin_lock(&dmar_domain->lock);
+	list_add(&sinfo->link_domain, &dmar_domain->subdevices);
+	spin_unlock(&dmar_domain->lock);
+
+	return 0;
+out_detach_iommu:
+	domain_detach_iommu(dmar_domain, iommu);
+out_free_sinfo:
+	kfree(sinfo);
+	return ret;
+}
+
+static void intel_iommu_detach_device_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 subdev_domain_info *i, *n, *sinfo = NULL;
+	struct intel_iommu *iommu = info->iommu;
+
+	spin_lock(&dmar_domain->lock);
+	list_for_each_entry_safe(i, n, &dmar_domain->subdevices, link_domain) {
+		if (i->dev == dev && i->pasid == pasid) {
+			list_del(&i->link_domain);
+			sinfo = i;
+			break;
+		}
+	}
+	spin_unlock(&dmar_domain->lock);
+
+	if (WARN_ON(!sinfo))
+		return;
+
+	intel_pasid_tear_down_entry(iommu, dev, pasid, false);
+	domain_detach_iommu(dmar_domain, iommu);
+	kfree(sinfo);
+}
+
 const struct iommu_ops intel_iommu_ops = {
 	.capable		= intel_iommu_capable,
 	.domain_alloc		= intel_iommu_domain_alloc,
@@ -4809,6 +4921,8 @@ const struct iommu_ops intel_iommu_ops = {
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev		= intel_iommu_attach_device,
 		.detach_dev		= intel_iommu_detach_device,
+		.set_dev_pasid		= intel_iommu_attach_device_pasid,
+		.block_dev_pasid	= intel_iommu_detach_device_pasid,
 		.map_pages		= intel_iommu_map_pages,
 		.unmap_pages		= intel_iommu_unmap_pages,
 		.iotlb_sync_map		= intel_iommu_iotlb_sync_map,
-- 
2.25.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Tian, Kevin]

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Tuesday, June 14, 2022 11:44 AM
> 
> 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.
> 

why is it not part of the series for those use cases? There is no consumer
of added callbacks in this patch...

> +/* PCI domain-subdevice relationship */
> +struct subdev_domain_info {
> +	struct list_head link_domain;	/* link to domain siblings */
> +	struct device *dev;		/* physical device derived from */
> +	ioasid_t pasid;			/* PASID on physical device */
> +};
> +

It's not subdev. Just dev+pasid in iommu's context.

RE: [PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Tian, Kevin]

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Tuesday, June 14, 2022 11:44 AM
> 
> 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.
> 

why is it not part of the series for those use cases? There is no consumer
of added callbacks in this patch...

> +/* PCI domain-subdevice relationship */
> +struct subdev_domain_info {
> +	struct list_head link_domain;	/* link to domain siblings */
> +	struct device *dev;		/* physical device derived from */
> +	ioasid_t pasid;			/* PASID on physical device */
> +};
> +

It's not subdev. Just dev+pasid in iommu's context.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Baolu Lu]

On 2022/6/14 12:02, Tian, Kevin wrote:
>> From: Lu Baolu <baolu.lu@linux.intel.com>
>> Sent: Tuesday, June 14, 2022 11:44 AM
>>
>> 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.
>>
> 
> why is it not part of the series for those use cases? There is no consumer
> of added callbacks in this patch...

It could be. I just wanted to maintain the integrity of Intel IOMMU
driver implementation.

> 
>> +/* PCI domain-subdevice relationship */
>> +struct subdev_domain_info {
>> +	struct list_head link_domain;	/* link to domain siblings */
>> +	struct device *dev;		/* physical device derived from */
>> +	ioasid_t pasid;			/* PASID on physical device */
>> +};
>> +
> 
> It's not subdev. Just dev+pasid in iommu's context.

How about struct device_pasid_info?

Best regards,
baolu

Re: [PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Baolu Lu]

On 2022/6/14 12:02, Tian, Kevin wrote:
>> From: Lu Baolu <baolu.lu@linux.intel.com>
>> Sent: Tuesday, June 14, 2022 11:44 AM
>>
>> 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.
>>
> 
> why is it not part of the series for those use cases? There is no consumer
> of added callbacks in this patch...

It could be. I just wanted to maintain the integrity of Intel IOMMU
driver implementation.

> 
>> +/* PCI domain-subdevice relationship */
>> +struct subdev_domain_info {
>> +	struct list_head link_domain;	/* link to domain siblings */
>> +	struct device *dev;		/* physical device derived from */
>> +	ioasid_t pasid;			/* PASID on physical device */
>> +};
>> +
> 
> It's not subdev. Just dev+pasid in iommu's context.

How about struct device_pasid_info?

Best regards,
baolu

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Tian, Kevin]

> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Tuesday, June 14, 2022 12:48 PM
> 
> On 2022/6/14 12:02, Tian, Kevin wrote:
> >> From: Lu Baolu <baolu.lu@linux.intel.com>
> >> Sent: Tuesday, June 14, 2022 11:44 AM
> >>
> >> 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.
> >>
> >
> > why is it not part of the series for those use cases? There is no consumer
> > of added callbacks in this patch...
> 
> It could be. I just wanted to maintain the integrity of Intel IOMMU
> driver implementation.

but let's not add dead code. and this patch is actually a right step
simply from set_dev_pasid() p.o.v hence you should include in any
series which first tries to use that interface.

> 
> >
> >> +/* PCI domain-subdevice relationship */
> >> +struct subdev_domain_info {
> >> +	struct list_head link_domain;	/* link to domain siblings */
> >> +	struct device *dev;		/* physical device derived from */
> >> +	ioasid_t pasid;			/* PASID on physical device */
> >> +};
> >> +
> >
> > It's not subdev. Just dev+pasid in iommu's context.
> 
> How about struct device_pasid_info?
> 

this is better.

RE: [PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Tian, Kevin]

> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Tuesday, June 14, 2022 12:48 PM
> 
> On 2022/6/14 12:02, Tian, Kevin wrote:
> >> From: Lu Baolu <baolu.lu@linux.intel.com>
> >> Sent: Tuesday, June 14, 2022 11:44 AM
> >>
> >> 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.
> >>
> >
> > why is it not part of the series for those use cases? There is no consumer
> > of added callbacks in this patch...
> 
> It could be. I just wanted to maintain the integrity of Intel IOMMU
> driver implementation.

but let's not add dead code. and this patch is actually a right step
simply from set_dev_pasid() p.o.v hence you should include in any
series which first tries to use that interface.

> 
> >
> >> +/* PCI domain-subdevice relationship */
> >> +struct subdev_domain_info {
> >> +	struct list_head link_domain;	/* link to domain siblings */
> >> +	struct device *dev;		/* physical device derived from */
> >> +	ioasid_t pasid;			/* PASID on physical device */
> >> +};
> >> +
> >
> > It's not subdev. Just dev+pasid in iommu's context.
> 
> How about struct device_pasid_info?
> 

this is better.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Baolu Lu]

On 2022/6/14 13:36, Tian, Kevin wrote:
>> From: Baolu Lu<baolu.lu@linux.intel.com>
>> Sent: Tuesday, June 14, 2022 12:48 PM
>>
>> On 2022/6/14 12:02, Tian, Kevin wrote:
>>>> From: Lu Baolu<baolu.lu@linux.intel.com>
>>>> Sent: Tuesday, June 14, 2022 11:44 AM
>>>>
>>>> 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.
>>>>
>>> why is it not part of the series for those use cases? There is no consumer
>>> of added callbacks in this patch...
>> It could be. I just wanted to maintain the integrity of Intel IOMMU
>> driver implementation.
> but let's not add dead code. and this patch is actually a right step
> simply from set_dev_pasid() p.o.v hence you should include in any
> series which first tries to use that interface.
> 

Yes, that's my intention. If it reviews well, we can include it in the
driver's implementation.

Best regards,
baolu

Re: [PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Baolu Lu]

On 2022/6/14 13:36, Tian, Kevin wrote:
>> From: Baolu Lu<baolu.lu@linux.intel.com>
>> Sent: Tuesday, June 14, 2022 12:48 PM
>>
>> On 2022/6/14 12:02, Tian, Kevin wrote:
>>>> From: Lu Baolu<baolu.lu@linux.intel.com>
>>>> Sent: Tuesday, June 14, 2022 11:44 AM
>>>>
>>>> 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.
>>>>
>>> why is it not part of the series for those use cases? There is no consumer
>>> of added callbacks in this patch...
>> It could be. I just wanted to maintain the integrity of Intel IOMMU
>> driver implementation.
> but let's not add dead code. and this patch is actually a right step
> simply from set_dev_pasid() p.o.v hence you should include in any
> series which first tries to use that interface.
> 

Yes, that's my intention. If it reviews well, we can include it in the
driver's implementation.

Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Tian, Kevin]

> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Tuesday, June 14, 2022 2:13 PM
> 
> On 2022/6/14 13:36, Tian, Kevin wrote:
> >> From: Baolu Lu<baolu.lu@linux.intel.com>
> >> Sent: Tuesday, June 14, 2022 12:48 PM
> >>
> >> On 2022/6/14 12:02, Tian, Kevin wrote:
> >>>> From: Lu Baolu<baolu.lu@linux.intel.com>
> >>>> Sent: Tuesday, June 14, 2022 11:44 AM
> >>>>
> >>>> 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.
> >>>>
> >>> why is it not part of the series for those use cases? There is no consumer
> >>> of added callbacks in this patch...
> >> It could be. I just wanted to maintain the integrity of Intel IOMMU
> >> driver implementation.
> > but let's not add dead code. and this patch is actually a right step
> > simply from set_dev_pasid() p.o.v hence you should include in any
> > series which first tries to use that interface.
> >
> 
> Yes, that's my intention. If it reviews well, we can include it in the
> driver's implementation.
> 

Then you should make it clear in the first place. otherwise a patch
like this implies a review for merge. 😊
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Tian, Kevin]

> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Tuesday, June 14, 2022 2:13 PM
> 
> On 2022/6/14 13:36, Tian, Kevin wrote:
> >> From: Baolu Lu<baolu.lu@linux.intel.com>
> >> Sent: Tuesday, June 14, 2022 12:48 PM
> >>
> >> On 2022/6/14 12:02, Tian, Kevin wrote:
> >>>> From: Lu Baolu<baolu.lu@linux.intel.com>
> >>>> Sent: Tuesday, June 14, 2022 11:44 AM
> >>>>
> >>>> 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.
> >>>>
> >>> why is it not part of the series for those use cases? There is no consumer
> >>> of added callbacks in this patch...
> >> It could be. I just wanted to maintain the integrity of Intel IOMMU
> >> driver implementation.
> > but let's not add dead code. and this patch is actually a right step
> > simply from set_dev_pasid() p.o.v hence you should include in any
> > series which first tries to use that interface.
> >
> 
> Yes, that's my intention. If it reviews well, we can include it in the
> driver's implementation.
> 

Then you should make it clear in the first place. otherwise a patch
like this implies a review for merge. 😊

Re: [PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Baolu Lu]

On 2022/6/14 15:19, Tian, Kevin wrote:
>> From: Baolu Lu <baolu.lu@linux.intel.com>
>> Sent: Tuesday, June 14, 2022 2:13 PM
>>
>> On 2022/6/14 13:36, Tian, Kevin wrote:
>>>> From: Baolu Lu<baolu.lu@linux.intel.com>
>>>> Sent: Tuesday, June 14, 2022 12:48 PM
>>>>
>>>> On 2022/6/14 12:02, Tian, Kevin wrote:
>>>>>> From: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>> Sent: Tuesday, June 14, 2022 11:44 AM
>>>>>>
>>>>>> 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.
>>>>>>
>>>>> why is it not part of the series for those use cases? There is no consumer
>>>>> of added callbacks in this patch...
>>>> It could be. I just wanted to maintain the integrity of Intel IOMMU
>>>> driver implementation.
>>> but let's not add dead code. and this patch is actually a right step
>>> simply from set_dev_pasid() p.o.v hence you should include in any
>>> series which first tries to use that interface.
>>>
>>
>> Yes, that's my intention. If it reviews well, we can include it in the
>> driver's implementation.
>>
> 
> Then you should make it clear in the first place. otherwise a patch
> like this implies a review for merge. 😊

Yeah! Will update this in the next version.

Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain

[Baolu Lu]

On 2022/6/14 15:19, Tian, Kevin wrote:
>> From: Baolu Lu <baolu.lu@linux.intel.com>
>> Sent: Tuesday, June 14, 2022 2:13 PM
>>
>> On 2022/6/14 13:36, Tian, Kevin wrote:
>>>> From: Baolu Lu<baolu.lu@linux.intel.com>
>>>> Sent: Tuesday, June 14, 2022 12:48 PM
>>>>
>>>> On 2022/6/14 12:02, Tian, Kevin wrote:
>>>>>> From: Lu Baolu<baolu.lu@linux.intel.com>
>>>>>> Sent: Tuesday, June 14, 2022 11:44 AM
>>>>>>
>>>>>> 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.
>>>>>>
>>>>> why is it not part of the series for those use cases? There is no consumer
>>>>> of added callbacks in this patch...
>>>> It could be. I just wanted to maintain the integrity of Intel IOMMU
>>>> driver implementation.
>>> but let's not add dead code. and this patch is actually a right step
>>> simply from set_dev_pasid() p.o.v hence you should include in any
>>> series which first tries to use that interface.
>>>
>>
>> Yes, that's my intention. If it reviews well, we can include it in the
>> driver's implementation.
>>
> 
> Then you should make it clear in the first place. otherwise a patch
> like this implies a review for merge. 😊

Yeah! Will update this in the next version.

Best regards,
baolu