RE: [PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain
> From: Baolu Lu
> Sent: Tuesday, June 14, 2022 12:48 PM
>
> On 2022/6/14 12:02, Tian, Kevin wrote:
> >> From: Lu Baolu
> >> 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
On 2022/6/14 12:02, Tian, Kevin wrote:
From: Lu Baolu
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
> From: Lu Baolu
> 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
[PATCH 1/1] iommu/vt-d: Add set_dev_pasid callbacks for default domain
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
---
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...@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, >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(>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(>lock);
list_for_each_entry(info, >devices, link)
__iommu_flush_dev_iotlb(info, addr, mask);
+
+ list_for_each_entry(sinfo, >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(>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(>lock);
+ list_for_each_entry(sinfo, >subdevices, link_domain)
+ qi_flush_piotlb(iommu, did, sinfo->pasid, addr, npages, ih);
+
+ if (!list_empty(>devices))
+ qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr, npages, ih);
spin_unlock(>lock);
}
@@
[PATCH v2 12/12] iommu/vt-d: Convert global spinlock into per domain ones
Using a global device_domain_lock spinlock to protect per-domain device
tracking lists is an inefficient way, especially considering this lock
is also needed in the hot paths. This optimizes the locking mechanism
by converting the global lock to per domain lock.
On the other hand, as the device tracking lists are never accessed in
any interrupt context, there is no need to disable interrupts while
spinning. Replace irqsave variant with spinlock calls.
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/iommu.h | 1 +
drivers/iommu/intel/iommu.c | 45 +++--
2 files changed, 19 insertions(+), 27 deletions(-)
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index 6724703d573b..cc304ff09a7b 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -541,6 +541,7 @@ struct dmar_domain {
u8 force_snooping : 1; /* Create IOPTEs with snoop control */
u8 set_pte_snp:1;
+ spinlock_t lock;/* Protect device tracking lists */
struct list_head devices; /* all devices' list */
struct dma_pte *pgd; /* virtual address */
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index aa3dea1c9f13..60e70682a190 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -310,7 +310,6 @@ static int iommu_skip_te_disable;
#define IDENTMAP_GFX 2
#define IDENTMAP_AZALIA4
-static DEFINE_SPINLOCK(device_domain_lock);
const struct iommu_ops intel_iommu_ops;
static bool translation_pre_enabled(struct intel_iommu *iommu)
@@ -534,9 +533,8 @@ static int domain_update_device_node(struct dmar_domain
*domain)
{
struct device_domain_info *info;
int nid = NUMA_NO_NODE;
- unsigned long flags;
- spin_lock_irqsave(_domain_lock, flags);
+ spin_lock(>lock);
list_for_each_entry(info, >devices, link) {
/*
* There could possibly be multiple device numa nodes as devices
@@ -548,7 +546,7 @@ static int domain_update_device_node(struct dmar_domain
*domain)
if (nid != NUMA_NO_NODE)
break;
}
- spin_unlock_irqrestore(_domain_lock, flags);
+ spin_unlock(>lock);
return nid;
}
@@ -1375,12 +1373,11 @@ iommu_support_dev_iotlb(struct dmar_domain *domain,
struct intel_iommu *iommu,
u8 bus, u8 devfn)
{
struct device_domain_info *info = NULL, *tmp;
- unsigned long flags;
if (!iommu->qi)
return NULL;
- spin_lock_irqsave(_domain_lock, flags);
+ spin_lock(>lock);
list_for_each_entry(tmp, >devices, link) {
if (tmp->iommu == iommu && tmp->bus == bus &&
tmp->devfn == devfn) {
@@ -1389,7 +1386,7 @@ iommu_support_dev_iotlb(struct dmar_domain *domain,
struct intel_iommu *iommu,
break;
}
}
- spin_unlock_irqrestore(_domain_lock, flags);
+ spin_unlock(>lock);
return info;
}
@@ -1398,9 +1395,8 @@ static void domain_update_iotlb(struct dmar_domain
*domain)
{
struct device_domain_info *info;
bool has_iotlb_device = false;
- unsigned long flags;
- spin_lock_irqsave(_domain_lock, flags);
+ spin_lock(>lock);
list_for_each_entry(info, >devices, link) {
if (info->ats_enabled) {
has_iotlb_device = true;
@@ -1408,7 +1404,7 @@ static void domain_update_iotlb(struct dmar_domain
*domain)
}
}
domain->has_iotlb_device = has_iotlb_device;
- spin_unlock_irqrestore(_domain_lock, flags);
+ spin_unlock(>lock);
}
static void iommu_enable_dev_iotlb(struct device_domain_info *info)
@@ -1499,17 +1495,15 @@ 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)
{
- unsigned long flags;
struct device_domain_info *info;
if (!domain->has_iotlb_device)
return;
- spin_lock_irqsave(_domain_lock, flags);
+ spin_lock(>lock);
list_for_each_entry(info, >devices, link)
__iommu_flush_dev_iotlb(info, addr, mask);
-
- spin_unlock_irqrestore(_domain_lock, flags);
+ spin_unlock(>lock);
}
static void iommu_flush_iotlb_psi(struct intel_iommu *iommu,
@@ -1769,6 +1763,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(>devices);
+ spin_lock_init(>lock);
return domain;
}
@@ -2442,7 +2437,6 @@ static int domain_add_dev_info(struct dmar_domain
*domain, struct device *dev)
{
struct device_domain_info *info = dev_iommu_priv_get(dev);
[PATCH v2 11/12] iommu/vt-d: Use device_domain_lock accurately
The device_domain_lock is used to protect the device tracking list of
a domain. Remove unnecessary spin_lock/unlock()'s and move the necessary
ones around the list access.
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/iommu.c | 68 +++--
1 file changed, 27 insertions(+), 41 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 8345e0c0824c..aa3dea1c9f13 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -534,16 +534,10 @@ static int domain_update_device_node(struct dmar_domain
*domain)
{
struct device_domain_info *info;
int nid = NUMA_NO_NODE;
+ unsigned long flags;
- assert_spin_locked(_domain_lock);
-
- if (list_empty(>devices))
- return NUMA_NO_NODE;
-
+ spin_lock_irqsave(_domain_lock, flags);
list_for_each_entry(info, >devices, link) {
- if (!info->dev)
- continue;
-
/*
* There could possibly be multiple device numa nodes as devices
* within the same domain may sit behind different IOMMUs. There
@@ -554,6 +548,7 @@ static int domain_update_device_node(struct dmar_domain
*domain)
if (nid != NUMA_NO_NODE)
break;
}
+ spin_unlock_irqrestore(_domain_lock, flags);
return nid;
}
@@ -1376,49 +1371,50 @@ static void __iommu_flush_iotlb(struct intel_iommu
*iommu, u16 did,
}
static struct device_domain_info *
-iommu_support_dev_iotlb (struct dmar_domain *domain, struct intel_iommu *iommu,
-u8 bus, u8 devfn)
+iommu_support_dev_iotlb(struct dmar_domain *domain, struct intel_iommu *iommu,
+ u8 bus, u8 devfn)
{
- struct device_domain_info *info;
-
- assert_spin_locked(_domain_lock);
+ struct device_domain_info *info = NULL, *tmp;
+ unsigned long flags;
if (!iommu->qi)
return NULL;
- list_for_each_entry(info, >devices, link)
- if (info->iommu == iommu && info->bus == bus &&
- info->devfn == devfn) {
- if (info->ats_supported && info->dev)
- return info;
+ spin_lock_irqsave(_domain_lock, flags);
+ list_for_each_entry(tmp, >devices, link) {
+ if (tmp->iommu == iommu && tmp->bus == bus &&
+ tmp->devfn == devfn) {
+ if (tmp->ats_supported)
+ info = tmp;
break;
}
+ }
+ spin_unlock_irqrestore(_domain_lock, flags);
- return NULL;
+ return info;
}
static void domain_update_iotlb(struct dmar_domain *domain)
{
struct device_domain_info *info;
bool has_iotlb_device = false;
+ unsigned long flags;
- assert_spin_locked(_domain_lock);
-
- list_for_each_entry(info, >devices, link)
+ spin_lock_irqsave(_domain_lock, flags);
+ list_for_each_entry(info, >devices, link) {
if (info->ats_enabled) {
has_iotlb_device = true;
break;
}
-
+ }
domain->has_iotlb_device = has_iotlb_device;
+ spin_unlock_irqrestore(_domain_lock, flags);
}
static void iommu_enable_dev_iotlb(struct device_domain_info *info)
{
struct pci_dev *pdev;
- assert_spin_locked(_domain_lock);
-
if (!info || !dev_is_pci(info->dev))
return;
@@ -1464,8 +1460,6 @@ static void iommu_disable_dev_iotlb(struct
device_domain_info *info)
{
struct pci_dev *pdev;
- assert_spin_locked(_domain_lock);
-
if (!dev_is_pci(info->dev))
return;
@@ -1908,11 +1902,11 @@ static int domain_context_mapping_one(struct
dmar_domain *domain,
struct pasid_table *table,
u8 bus, u8 devfn)
{
+ struct device_domain_info *info =
+ iommu_support_dev_iotlb(domain, iommu, bus, devfn);
u16 did = domain->iommu_did[iommu->seq_id];
int translation = CONTEXT_TT_MULTI_LEVEL;
- struct device_domain_info *info = NULL;
struct context_entry *context;
- unsigned long flags;
int ret;
WARN_ON(did == 0);
@@ -1925,7 +1919,6 @@ static int domain_context_mapping_one(struct dmar_domain
*domain,
BUG_ON(!domain->pgd);
- spin_lock_irqsave(_domain_lock, flags);
spin_lock(>lock);
ret = -ENOMEM;
@@ -1978,7 +1971,6 @@ static int domain_context_mapping_one(struct dmar_domain
*domain,
* Setup the Device-TLB enable bit and Page request
* Enable bit:
*/
- info = iommu_support_dev_iotlb(domain, iommu, bus, devfn);
if (info && info->ats_supported)
[PATCH v2 10/12] iommu/vt-d: Fold __dmar_remove_one_dev_info() into its caller
Fold __dmar_remove_one_dev_info() into dmar_remove_one_dev_info() which
is its only caller. Make the spin lock critical range only cover the
device list change code and remove some unnecessary checks.
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/iommu.c | 34 +-
1 file changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index af22690f44c8..8345e0c0824c 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -295,7 +295,6 @@ static LIST_HEAD(dmar_satc_units);
static int g_num_of_iommus;
static void dmar_remove_one_dev_info(struct device *dev);
-static void __dmar_remove_one_dev_info(struct device_domain_info *info);
int dmar_disabled = !IS_ENABLED(CONFIG_INTEL_IOMMU_DEFAULT_ON);
int intel_iommu_sm = IS_ENABLED(CONFIG_INTEL_IOMMU_SCALABLE_MODE_DEFAULT_ON);
@@ -4141,20 +4140,14 @@ static void domain_context_clear(struct
device_domain_info *info)
_context_clear_one_cb, info);
}
-static void __dmar_remove_one_dev_info(struct device_domain_info *info)
+static void dmar_remove_one_dev_info(struct device *dev)
{
- struct dmar_domain *domain;
- struct intel_iommu *iommu;
-
- assert_spin_locked(_domain_lock);
-
- if (WARN_ON(!info))
- return;
-
- iommu = info->iommu;
- domain = info->domain;
+ struct device_domain_info *info = dev_iommu_priv_get(dev);
+ struct dmar_domain *domain = info->domain;
+ struct intel_iommu *iommu = info->iommu;
+ unsigned long flags;
- if (info->dev && !dev_is_real_dma_subdevice(info->dev)) {
+ if (!dev_is_real_dma_subdevice(info->dev)) {
if (dev_is_pci(info->dev) && sm_supported(iommu))
intel_pasid_tear_down_entry(iommu, info->dev,
PASID_RID2PASID, false);
@@ -4164,20 +4157,11 @@ static void __dmar_remove_one_dev_info(struct
device_domain_info *info)
intel_pasid_free_table(info->dev);
}
- list_del(>link);
- domain_detach_iommu(domain, iommu);
-}
-
-static void dmar_remove_one_dev_info(struct device *dev)
-{
- struct device_domain_info *info;
- unsigned long flags;
-
spin_lock_irqsave(_domain_lock, flags);
- info = dev_iommu_priv_get(dev);
- if (info)
- __dmar_remove_one_dev_info(info);
+ list_del(>link);
spin_unlock_irqrestore(_domain_lock, flags);
+
+ domain_detach_iommu(domain, iommu);
}
static int md_domain_init(struct dmar_domain *domain, int guest_width)
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 09/12] iommu/vt-d: Check device list of domain in domain free path
When the IOMMU domain is about to be freed, it should not be set on any
device. Instead of silently dealing with some bug cases, it's better to
trigger a warning to report and fix any potential bugs at the first time.
Signed-off-by: Lu Baolu
Reviewed-by: Jason Gunthorpe
---
drivers/iommu/intel/iommu.c | 19 +++
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index aec33599c7f7..af22690f44c8 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -294,7 +294,6 @@ static LIST_HEAD(dmar_satc_units);
/* bitmap for indexing intel_iommus */
static int g_num_of_iommus;
-static void domain_remove_dev_info(struct dmar_domain *domain);
static void dmar_remove_one_dev_info(struct device *dev);
static void __dmar_remove_one_dev_info(struct device_domain_info *info);
@@ -1843,10 +1842,6 @@ static inline int guestwidth_to_adjustwidth(int gaw)
static void domain_exit(struct dmar_domain *domain)
{
-
- /* Remove associated devices and clear attached or cached domains */
- domain_remove_dev_info(domain);
-
if (domain->pgd) {
LIST_HEAD(freelist);
@@ -1854,6 +1849,9 @@ static void domain_exit(struct dmar_domain *domain)
put_pages_list();
}
+ if (WARN_ON(!list_empty(>devices)))
+ return;
+
kfree(domain);
}
@@ -2336,17 +2334,6 @@ static void domain_context_clear_one(struct
device_domain_info *info, u8 bus, u8
__iommu_flush_dev_iotlb(info, 0, MAX_AGAW_PFN_WIDTH);
}
-static void domain_remove_dev_info(struct dmar_domain *domain)
-{
- struct device_domain_info *info, *tmp;
- unsigned long flags;
-
- spin_lock_irqsave(_domain_lock, flags);
- list_for_each_entry_safe(info, tmp, >devices, link)
- __dmar_remove_one_dev_info(info);
- spin_unlock_irqrestore(_domain_lock, flags);
-}
-
static int domain_setup_first_level(struct intel_iommu *iommu,
struct dmar_domain *domain,
struct device *dev,
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 08/12] iommu/vt-d: Replace spin_lock_irqsave() with spin_lock()
The iommu->lock is used to protect changes in root/context/pasid tables
and domain ID allocation. There's no use case to change these resources
in any interrupt context. Hence there's no need to disable interrupts
when helding the spinlock.
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/debugfs.c | 6 ++
drivers/iommu/intel/iommu.c | 17 +++--
2 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
index 5ebfe32265d5..104f9d77b3f0 100644
--- a/drivers/iommu/intel/debugfs.c
+++ b/drivers/iommu/intel/debugfs.c
@@ -263,10 +263,9 @@ static void ctx_tbl_walk(struct seq_file *m, struct
intel_iommu *iommu, u16 bus)
static void root_tbl_walk(struct seq_file *m, struct intel_iommu *iommu)
{
- unsigned long flags;
u16 bus;
- spin_lock_irqsave(>lock, flags);
+ spin_lock(>lock);
seq_printf(m, "IOMMU %s: Root Table Address: 0x%llx\n", iommu->name,
(u64)virt_to_phys(iommu->root_entry));
seq_puts(m,
"B.D.F\tRoot_entry\t\t\t\tContext_entry\t\t\t\tPASID\tPASID_table_entry\n");
@@ -278,8 +277,7 @@ static void root_tbl_walk(struct seq_file *m, struct
intel_iommu *iommu)
*/
for (bus = 0; bus < 256; bus++)
ctx_tbl_walk(m, iommu, bus);
-
- spin_unlock_irqrestore(>lock, flags);
+ spin_unlock(>lock);
}
static int dmar_translation_struct_show(struct seq_file *m, void *unused)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 12cd12fc86a4..aec33599c7f7 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -797,13 +797,12 @@ static int device_context_mapped(struct intel_iommu
*iommu, u8 bus, u8 devfn)
{
struct context_entry *context;
int ret = 0;
- unsigned long flags;
- spin_lock_irqsave(>lock, flags);
+ spin_lock(>lock);
context = iommu_context_addr(iommu, bus, devfn, 0);
if (context)
ret = context_present(context);
- spin_unlock_irqrestore(>lock, flags);
+ spin_unlock(>lock);
return ret;
}
@@ -2295,16 +2294,15 @@ static void domain_context_clear_one(struct
device_domain_info *info, u8 bus, u8
{
struct intel_iommu *iommu = info->iommu;
struct context_entry *context;
- unsigned long flags;
u16 did_old;
if (!iommu)
return;
- spin_lock_irqsave(>lock, flags);
+ spin_lock(>lock);
context = iommu_context_addr(iommu, bus, devfn, 0);
if (!context) {
- spin_unlock_irqrestore(>lock, flags);
+ spin_unlock(>lock);
return;
}
@@ -2319,7 +2317,7 @@ static void domain_context_clear_one(struct
device_domain_info *info, u8 bus, u8
context_clear_entry(context);
__iommu_flush_cache(iommu, context, sizeof(*context));
- spin_unlock_irqrestore(>lock, flags);
+ spin_unlock(>lock);
iommu->flush.flush_context(iommu,
did_old,
(((u16)bus) << 8) | devfn,
@@ -2772,7 +2770,6 @@ static int copy_translation_tables(struct intel_iommu
*iommu)
struct root_entry *old_rt;
phys_addr_t old_rt_phys;
int ctxt_table_entries;
- unsigned long flags;
u64 rtaddr_reg;
int bus, ret;
bool new_ext, ext;
@@ -2815,7 +2812,7 @@ static int copy_translation_tables(struct intel_iommu
*iommu)
}
}
- spin_lock_irqsave(>lock, flags);
+ spin_lock(>lock);
/* Context tables are copied, now write them to the root_entry table */
for (bus = 0; bus < 256; bus++) {
@@ -2834,7 +2831,7 @@ static int copy_translation_tables(struct intel_iommu
*iommu)
iommu->root_entry[bus].hi = val;
}
- spin_unlock_irqrestore(>lock, flags);
+ spin_unlock(>lock);
kfree(ctxt_tbls);
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 07/12] iommu/vt-d: Acquiring lock in pasid manipulation helpers
The iommu->lock is used to protect the per-IOMMU pasid directory table
and pasid table. Move the spinlock acquisition/release into the helpers
to make the code self-contained. Again, the iommu->lock is never used
in interrupt contexts, hence there's no need to disable the interrupts
when holding the lock.
Signed-off-by: Lu Baolu
Reviewed-by: Kevin Tian
---
drivers/iommu/intel/iommu.c | 2 -
drivers/iommu/intel/pasid.c | 108 +++-
drivers/iommu/intel/svm.c | 5 +-
3 files changed, 57 insertions(+), 58 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 8fdaa01ef10d..12cd12fc86a4 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2496,7 +2496,6 @@ static int domain_add_dev_info(struct dmar_domain
*domain, struct device *dev)
}
/* Setup the PASID entry for requests without PASID: */
- spin_lock_irqsave(>lock, flags);
if (hw_pass_through && domain_type_is_si(domain))
ret = intel_pasid_setup_pass_through(iommu, domain,
dev, PASID_RID2PASID);
@@ -2506,7 +2505,6 @@ static int domain_add_dev_info(struct dmar_domain
*domain, struct device *dev)
else
ret = intel_pasid_setup_second_level(iommu, domain,
dev, PASID_RID2PASID);
- spin_unlock_irqrestore(>lock, flags);
if (ret) {
dev_err(dev, "Setup RID2PASID failed\n");
dmar_remove_one_dev_info(dev);
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index 641a4a6eb61e..3276895d7ba7 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -496,17 +496,17 @@ void intel_pasid_tear_down_entry(struct intel_iommu
*iommu, struct device *dev,
struct pasid_entry *pte;
u16 did, pgtt;
+ spin_lock(>lock);
pte = intel_pasid_get_entry(dev, pasid);
- if (WARN_ON(!pte))
- return;
-
- if (!pasid_pte_is_present(pte))
+ if (WARN_ON(!pte) || !pasid_pte_is_present(pte)) {
+ spin_unlock(>lock);
return;
+ }
did = pasid_get_domain_id(pte);
pgtt = pasid_pte_get_pgtt(pte);
-
intel_pasid_clear_entry(dev, pasid, fault_ignore);
+ spin_unlock(>lock);
if (!ecap_coherent(iommu->ecap))
clflush_cache_range(pte, sizeof(*pte));
@@ -542,21 +542,17 @@ static void pasid_flush_caches(struct intel_iommu *iommu,
}
}
-static inline int pasid_enable_wpe(struct pasid_entry *pte)
+static struct pasid_entry *get_non_present_pasid_entry(struct device *dev,
+ u32 pasid)
{
-#ifdef CONFIG_X86
- unsigned long cr0 = read_cr0();
+ struct pasid_entry *pte;
- /* CR0.WP is normally set but just to be sure */
- if (unlikely(!(cr0 & X86_CR0_WP))) {
- pr_err_ratelimited("No CPU write protect!\n");
- return -EINVAL;
- }
-#endif
- pasid_set_wpe(pte);
+ pte = intel_pasid_get_entry(dev, pasid);
+ if (!pte || pasid_pte_is_present(pte))
+ return NULL;
- return 0;
-};
+ return pte;
+}
/*
* Set up the scalable mode pasid table entry for first only
@@ -574,39 +570,47 @@ int intel_pasid_setup_first_level(struct intel_iommu
*iommu,
return -EINVAL;
}
- pte = intel_pasid_get_entry(dev, pasid);
- if (WARN_ON(!pte))
+ if ((flags & PASID_FLAG_SUPERVISOR_MODE)) {
+#ifdef CONFIG_X86
+ unsigned long cr0 = read_cr0();
+
+ /* CR0.WP is normally set but just to be sure */
+ if (unlikely(!(cr0 & X86_CR0_WP))) {
+ pr_err("No CPU write protect!\n");
+ return -EINVAL;
+ }
+#endif
+ if (!ecap_srs(iommu->ecap)) {
+ pr_err("No supervisor request support on %s\n",
+ iommu->name);
+ return -EINVAL;
+ }
+ }
+
+ if ((flags & PASID_FLAG_FL5LP) && !cap_5lp_support(iommu->cap)) {
+ pr_err("No 5-level paging support for first-level on %s\n",
+ iommu->name);
return -EINVAL;
+ }
- /* Caller must ensure PASID entry is not in use. */
- if (pasid_pte_is_present(pte))
+ spin_lock(>lock);
+ pte = get_non_present_pasid_entry(dev, pasid);
+ if (!pte) {
+ spin_unlock(>lock);
return -EBUSY;
+ }
pasid_clear_entry(pte);
/* Setup the first level page table pointer: */
pasid_set_flptr(pte, (u64)__pa(pgd));
if (flags & PASID_FLAG_SUPERVISOR_MODE) {
- if (!ecap_srs(iommu->ecap)) {
-
[PATCH v2 06/12] iommu/vt-d: Acquiring lock in domain ID allocation helpers
The iommu->lock is used to protect the per-IOMMU domain ID resource.
Moving the lock into the ID alloc/free helpers makes the code more
compact. At the same time, the device_domain_lock is irrelevant to
the domain ID resource, remove its assertion as well.
On the other hand, the iommu->lock is never used in interrupt context,
there's no need to use the irqsave variant of the spinlock calls.
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/iommu.c | 25 +
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index b2ef8af0c3f3..8fdaa01ef10d 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1782,16 +1782,13 @@ static struct dmar_domain *alloc_domain(unsigned int
type)
return domain;
}
-/* Must be called with iommu->lock */
static int domain_attach_iommu(struct dmar_domain *domain,
struct intel_iommu *iommu)
{
unsigned long ndomains;
- int num;
-
- assert_spin_locked(_domain_lock);
- assert_spin_locked(>lock);
+ int num, ret = 0;
+ spin_lock(>lock);
domain->iommu_refcnt[iommu->seq_id] += 1;
if (domain->iommu_refcnt[iommu->seq_id] == 1) {
ndomains = cap_ndoms(iommu->cap);
@@ -1800,7 +1797,8 @@ static int domain_attach_iommu(struct dmar_domain *domain,
if (num >= ndomains) {
pr_err("%s: No free domain ids\n", iommu->name);
domain->iommu_refcnt[iommu->seq_id] -= 1;
- return -ENOSPC;
+ ret = -ENOSPC;
+ goto out_unlock;
}
set_bit(num, iommu->domain_ids);
@@ -1809,7 +1807,9 @@ static int domain_attach_iommu(struct dmar_domain *domain,
domain_update_iommu_cap(domain);
}
- return 0;
+out_unlock:
+ spin_unlock(>lock);
+ return ret;
}
static void domain_detach_iommu(struct dmar_domain *domain,
@@ -1817,9 +1817,7 @@ static void domain_detach_iommu(struct dmar_domain
*domain,
{
int num;
- assert_spin_locked(_domain_lock);
- assert_spin_locked(>lock);
-
+ spin_lock(>lock);
domain->iommu_refcnt[iommu->seq_id] -= 1;
if (domain->iommu_refcnt[iommu->seq_id] == 0) {
num = domain->iommu_did[iommu->seq_id];
@@ -1827,6 +1825,7 @@ static void domain_detach_iommu(struct dmar_domain
*domain,
domain_update_iommu_cap(domain);
domain->iommu_did[iommu->seq_id] = 0;
}
+ spin_unlock(>lock);
}
static inline int guestwidth_to_adjustwidth(int gaw)
@@ -2479,9 +2478,7 @@ static int domain_add_dev_info(struct dmar_domain
*domain, struct device *dev)
spin_lock_irqsave(_domain_lock, flags);
info->domain = domain;
- spin_lock(>lock);
ret = domain_attach_iommu(domain, iommu);
- spin_unlock(>lock);
if (ret) {
spin_unlock_irqrestore(_domain_lock, flags);
return ret;
@@ -4166,7 +4163,6 @@ static void __dmar_remove_one_dev_info(struct
device_domain_info *info)
{
struct dmar_domain *domain;
struct intel_iommu *iommu;
- unsigned long flags;
assert_spin_locked(_domain_lock);
@@ -4187,10 +4183,7 @@ static void __dmar_remove_one_dev_info(struct
device_domain_info *info)
}
list_del(>link);
-
- spin_lock_irqsave(>lock, flags);
domain_detach_iommu(domain, iommu);
- spin_unlock_irqrestore(>lock, flags);
}
static void dmar_remove_one_dev_info(struct device *dev)
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 05/12] iommu/vt-d: Unnecessary spinlock for root table alloc and free
The IOMMU root table is allocated and freed in the IOMMU initialization
code in static boot or hot-remove paths. There's no need for a spinlock.
Signed-off-by: Lu Baolu
Reviewed-by: Kevin Tian
---
drivers/iommu/intel/iommu.c | 18 +-
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 839bb5f3e000..b2ef8af0c3f3 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -809,14 +809,12 @@ static int device_context_mapped(struct intel_iommu
*iommu, u8 bus, u8 devfn)
static void free_context_table(struct intel_iommu *iommu)
{
- int i;
- unsigned long flags;
struct context_entry *context;
+ int i;
+
+ if (!iommu->root_entry)
+ return;
- spin_lock_irqsave(>lock, flags);
- if (!iommu->root_entry) {
- goto out;
- }
for (i = 0; i < ROOT_ENTRY_NR; i++) {
context = iommu_context_addr(iommu, i, 0, 0);
if (context)
@@ -828,12 +826,10 @@ static void free_context_table(struct intel_iommu *iommu)
context = iommu_context_addr(iommu, i, 0x80, 0);
if (context)
free_pgtable_page(context);
-
}
+
free_pgtable_page(iommu->root_entry);
iommu->root_entry = NULL;
-out:
- spin_unlock_irqrestore(>lock, flags);
}
#ifdef CONFIG_DMAR_DEBUG
@@ -1232,7 +1228,6 @@ static void domain_unmap(struct dmar_domain *domain,
unsigned long start_pfn,
static int iommu_alloc_root_entry(struct intel_iommu *iommu)
{
struct root_entry *root;
- unsigned long flags;
root = (struct root_entry *)alloc_pgtable_page(iommu->node);
if (!root) {
@@ -1242,10 +1237,7 @@ static int iommu_alloc_root_entry(struct intel_iommu
*iommu)
}
__iommu_flush_cache(iommu, root, ROOT_SIZE);
-
- spin_lock_irqsave(>lock, flags);
iommu->root_entry = root;
- spin_unlock_irqrestore(>lock, flags);
return 0;
}
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 04/12] iommu/vt-d: Use pci_get_domain_bus_and_slot() in pgtable_walk()
Use pci_get_domain_bus_and_slot() instead of searching the global list
to retrieve the pci device pointer. This also removes the global
device_domain_list as there isn't any consumer anymore.
Signed-off-by: Lu Baolu
Reviewed-by: Jason Gunthorpe
Reviewed-by: Kevin Tian
---
drivers/iommu/intel/iommu.h | 1 -
drivers/iommu/intel/iommu.c | 33 ++---
2 files changed, 6 insertions(+), 28 deletions(-)
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index 2f4a5b9509c0..6724703d573b 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -609,7 +609,6 @@ struct intel_iommu {
/* PCI domain-device relationship */
struct device_domain_info {
struct list_head link; /* link to domain siblings */
- struct list_head global; /* link to global list */
struct list_head table; /* link to pasid table */
u32 segment;/* PCI segment number */
u8 bus; /* PCI bus number */
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 695aed474e5d..839bb5f3e000 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -131,8 +131,6 @@ static struct intel_iommu **g_iommus;
static void __init check_tylersburg_isoch(void);
static int rwbf_quirk;
-static inline struct device_domain_info *
-dmar_search_domain_by_dev_info(int segment, int bus, int devfn);
/*
* set to 1 to panic kernel if can't successfully enable VT-d
@@ -315,7 +313,6 @@ static int iommu_skip_te_disable;
#define IDENTMAP_AZALIA4
static DEFINE_SPINLOCK(device_domain_lock);
-static LIST_HEAD(device_domain_list);
const struct iommu_ops intel_iommu_ops;
static bool translation_pre_enabled(struct intel_iommu *iommu)
@@ -845,9 +842,14 @@ static void pgtable_walk(struct intel_iommu *iommu,
unsigned long pfn, u8 bus, u
struct device_domain_info *info;
struct dma_pte *parent, *pte;
struct dmar_domain *domain;
+ struct pci_dev *pdev;
int offset, level;
- info = dmar_search_domain_by_dev_info(iommu->segment, bus, devfn);
+ pdev = pci_get_domain_bus_and_slot(iommu->segment, bus, devfn);
+ if (!pdev)
+ return;
+
+ info = dev_iommu_priv_get(>dev);
if (!info || !info->domain) {
pr_info("device [%02x:%02x.%d] not probed\n",
bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
@@ -2356,19 +2358,6 @@ static void domain_remove_dev_info(struct dmar_domain
*domain)
spin_unlock_irqrestore(_domain_lock, flags);
}
-static inline struct device_domain_info *
-dmar_search_domain_by_dev_info(int segment, int bus, int devfn)
-{
- struct device_domain_info *info;
-
- list_for_each_entry(info, _domain_list, global)
- if (info->segment == segment && info->bus == bus &&
- info->devfn == devfn)
- return info;
-
- return NULL;
-}
-
static int domain_setup_first_level(struct intel_iommu *iommu,
struct dmar_domain *domain,
struct device *dev,
@@ -4572,7 +4561,6 @@ static struct iommu_device
*intel_iommu_probe_device(struct device *dev)
struct pci_dev *pdev = dev_is_pci(dev) ? to_pci_dev(dev) : NULL;
struct device_domain_info *info;
struct intel_iommu *iommu;
- unsigned long flags;
u8 bus, devfn;
iommu = device_to_iommu(dev, , );
@@ -4615,10 +4603,7 @@ static struct iommu_device
*intel_iommu_probe_device(struct device *dev)
}
}
- spin_lock_irqsave(_domain_lock, flags);
- list_add(>global, _domain_list);
dev_iommu_priv_set(dev, info);
- spin_unlock_irqrestore(_domain_lock, flags);
return >iommu;
}
@@ -4626,15 +4611,9 @@ static struct iommu_device
*intel_iommu_probe_device(struct device *dev)
static void intel_iommu_release_device(struct device *dev)
{
struct device_domain_info *info = dev_iommu_priv_get(dev);
- unsigned long flags;
dmar_remove_one_dev_info(dev);
-
- spin_lock_irqsave(_domain_lock, flags);
dev_iommu_priv_set(dev, NULL);
- list_del(>global);
- spin_unlock_irqrestore(_domain_lock, flags);
-
kfree(info);
set_dma_ops(dev, NULL);
}
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 03/12] iommu/vt-d: Remove clearing translation data in disable_dmar_iommu()
The disable_dmar_iommu() is called when IOMMU initialization fails or
the IOMMU is hot-removed from the system. In both cases, there is no
need to clear the IOMMU translation data structures for devices.
On the initialization path, the device probing only happens after the
IOMMU is initialized successfully, hence there're no translation data
structures.
On the hot-remove path, there is no real use case where the IOMMU is
hot-removed, but the devices that it manages are still alive in the
system. The translation data structures were torn down during device
release, hence there's no need to repeat it in IOMMU hot-remove path
either. This removes the unnecessary code and only leaves a check.
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/pasid.h | 1 +
drivers/iommu/intel/iommu.c | 21 +++--
2 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
index 583ea67fc783..2afbb2afe8cc 100644
--- a/drivers/iommu/intel/pasid.h
+++ b/drivers/iommu/intel/pasid.h
@@ -39,6 +39,7 @@
* only and pass-through transfer modes.
*/
#define FLPT_DEFAULT_DID 1
+#define NUM_RESERVED_DID 2
/*
* The SUPERVISOR_MODE flag indicates a first level translation which
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ff6018f746e0..695aed474e5d 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1715,23 +1715,16 @@ static int iommu_init_domains(struct intel_iommu *iommu)
static void disable_dmar_iommu(struct intel_iommu *iommu)
{
- struct device_domain_info *info, *tmp;
- unsigned long flags;
-
if (!iommu->domain_ids)
return;
- spin_lock_irqsave(_domain_lock, flags);
- list_for_each_entry_safe(info, tmp, _domain_list, global) {
- if (info->iommu != iommu)
- continue;
-
- if (!info->dev || !info->domain)
- continue;
-
- __dmar_remove_one_dev_info(info);
- }
- spin_unlock_irqrestore(_domain_lock, flags);
+ /*
+* All iommu domains must have been detached from the devices,
+* hence there should be no domain IDs in use.
+*/
+ if (WARN_ON(bitmap_weight(iommu->domain_ids, cap_ndoms(iommu->cap))
+ != NUM_RESERVED_DID))
+ return;
if (iommu->gcmd & DMA_GCMD_TE)
iommu_disable_translation(iommu);
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 02/12] iommu/vt-d: Remove for_each_device_domain()
The per-device device_domain_info data could be retrieved from the
device itself. There's no need to search a global list.
Signed-off-by: Lu Baolu
Reviewed-by: Jason Gunthorpe
Reviewed-by: Kevin Tian
---
drivers/iommu/intel/iommu.h | 2 --
drivers/iommu/intel/iommu.c | 25 --
drivers/iommu/intel/pasid.c | 41 +++--
3 files changed, 12 insertions(+), 56 deletions(-)
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index 8a6d64d726c0..2f4a5b9509c0 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -727,8 +727,6 @@ extern int dmar_ir_support(void);
void *alloc_pgtable_page(int node);
void free_pgtable_page(void *vaddr);
struct intel_iommu *domain_get_iommu(struct dmar_domain *domain);
-int for_each_device_domain(int (*fn)(struct device_domain_info *info,
-void *data), void *data);
void iommu_flush_write_buffer(struct intel_iommu *iommu);
int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct device *dev);
struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devfn);
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index a39d72a9d1cf..ff6018f746e0 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -316,31 +316,6 @@ static int iommu_skip_te_disable;
static DEFINE_SPINLOCK(device_domain_lock);
static LIST_HEAD(device_domain_list);
-
-/*
- * Iterate over elements in device_domain_list and call the specified
- * callback @fn against each element.
- */
-int for_each_device_domain(int (*fn)(struct device_domain_info *info,
-void *data), void *data)
-{
- int ret = 0;
- unsigned long flags;
- struct device_domain_info *info;
-
- spin_lock_irqsave(_domain_lock, flags);
- list_for_each_entry(info, _domain_list, global) {
- ret = fn(info, data);
- if (ret) {
- spin_unlock_irqrestore(_domain_lock, flags);
- return ret;
- }
- }
- spin_unlock_irqrestore(_domain_lock, flags);
-
- return 0;
-}
-
const struct iommu_ops intel_iommu_ops;
static bool translation_pre_enabled(struct intel_iommu *iommu)
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index b2ac5869286f..641a4a6eb61e 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -103,36 +103,19 @@ device_detach_pasid_table(struct device_domain_info *info,
}
struct pasid_table_opaque {
- struct pasid_table **pasid_table;
- int segment;
- int bus;
- int devfn;
+ struct pasid_table *pasid_table;
};
-static int search_pasid_table(struct device_domain_info *info, void *opaque)
-{
- struct pasid_table_opaque *data = opaque;
-
- if (info->iommu->segment == data->segment &&
- info->bus == data->bus &&
- info->devfn == data->devfn &&
- info->pasid_table) {
- *data->pasid_table = info->pasid_table;
- return 1;
- }
-
- return 0;
-}
-
static int get_alias_pasid_table(struct pci_dev *pdev, u16 alias, void *opaque)
{
struct pasid_table_opaque *data = opaque;
+ struct device_domain_info *info;
- data->segment = pci_domain_nr(pdev->bus);
- data->bus = PCI_BUS_NUM(alias);
- data->devfn = alias & 0xff;
+ info = dev_iommu_priv_get(>dev);
+ if (info && info->pasid_table)
+ data->pasid_table = info->pasid_table;
- return for_each_device_domain(_pasid_table, data);
+ return data->pasid_table != NULL;
}
/*
@@ -141,12 +124,12 @@ static int get_alias_pasid_table(struct pci_dev *pdev,
u16 alias, void *opaque)
*/
int intel_pasid_alloc_table(struct device *dev)
{
+ struct pasid_table_opaque data = { NULL };
struct device_domain_info *info;
struct pasid_table *pasid_table;
- struct pasid_table_opaque data;
struct page *pages;
u32 max_pasid = 0;
- int ret, order;
+ int order;
int size;
might_sleep();
@@ -155,11 +138,11 @@ int intel_pasid_alloc_table(struct device *dev)
return -EINVAL;
/* DMA alias device already has a pasid table, use it: */
- data.pasid_table = _table;
- ret = pci_for_each_dma_alias(to_pci_dev(dev),
-_alias_pasid_table, );
- if (ret)
+ if (pci_for_each_dma_alias(to_pci_dev(dev),
+ get_alias_pasid_table, )) {
+ pasid_table = data.pasid_table;
goto attach_out;
+ }
pasid_table = kzalloc(sizeof(*pasid_table), GFP_KERNEL);
if (!pasid_table)
--
2.25.1
___
iommu mailing list
[PATCH v2 01/12] iommu/vt-d: debugfs: Remove device_domain_lock usage
The domain_translation_struct debugfs node is used to dump the DMAR page
tables for the PCI devices. It potentially races with setting domains to
devices. The existing code uses a global spinlock device_domain_lock to
avoid the races, but this is problematical as this lock is only used to
protect the device tracking lists of each domain.
This replaces device_domain_lock with group->mutex to protect page tables
from setting a new domain. This also makes device_domain_lock static as
it is now only used inside the file.
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/iommu.h | 1 -
drivers/iommu/intel/debugfs.c | 49 +--
drivers/iommu/intel/iommu.c | 2 +-
3 files changed, 36 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index a22adfbdf870..8a6d64d726c0 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -480,7 +480,6 @@ enum {
#define VTD_FLAG_SVM_CAPABLE (1 << 2)
extern int intel_iommu_sm;
-extern spinlock_t device_domain_lock;
#define sm_supported(iommu)(intel_iommu_sm && ecap_smts((iommu)->ecap))
#define pasid_supported(iommu) (sm_supported(iommu) && \
diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
index d927ef10641b..5ebfe32265d5 100644
--- a/drivers/iommu/intel/debugfs.c
+++ b/drivers/iommu/intel/debugfs.c
@@ -342,15 +342,23 @@ static void pgtable_walk_level(struct seq_file *m, struct
dma_pte *pde,
}
}
-static int show_device_domain_translation(struct device *dev, void *data)
+struct show_domain_opaque {
+ struct device *dev;
+ struct seq_file *m;
+};
+
+static int __show_device_domain_translation(struct device *dev, void *data)
{
- struct device_domain_info *info = dev_iommu_priv_get(dev);
- struct dmar_domain *domain = info->domain;
- struct seq_file *m = data;
+ struct show_domain_opaque *opaque = data;
+ struct device_domain_info *info;
+ struct seq_file *m = opaque->m;
+ struct dmar_domain *domain;
u64 path[6] = { 0 };
- if (!domain)
+ if (dev != opaque->dev)
return 0;
+ info = dev_iommu_priv_get(dev);
+ domain = info->domain;
seq_printf(m, "Device %s @0x%llx\n", dev_name(dev),
(u64)virt_to_phys(domain->pgd));
@@ -359,20 +367,33 @@ static int show_device_domain_translation(struct device
*dev, void *data)
pgtable_walk_level(m, domain->pgd, domain->agaw + 2, 0, path);
seq_putc(m, '\n');
- return 0;
+ return 1;
}
-static int domain_translation_struct_show(struct seq_file *m, void *unused)
+static int show_device_domain_translation(struct device *dev, void *data)
{
- unsigned long flags;
- int ret;
+ struct show_domain_opaque opaque = {dev, data};
+ struct iommu_group *group;
- spin_lock_irqsave(_domain_lock, flags);
- ret = bus_for_each_dev(_bus_type, NULL, m,
- show_device_domain_translation);
- spin_unlock_irqrestore(_domain_lock, flags);
+ group = iommu_group_get(dev);
+ if (group) {
+ /*
+* The group->mutex is held across the callback, which will
+* block calls to iommu_attach/detach_group/device. Hence,
+* the domain of the device will not change during traversal.
+*/
+ iommu_group_for_each_dev(group, ,
+__show_device_domain_translation);
+ iommu_group_put(group);
+ }
- return ret;
+ return 0;
+}
+
+static int domain_translation_struct_show(struct seq_file *m, void *unused)
+{
+ return bus_for_each_dev(_bus_type, NULL, m,
+ show_device_domain_translation);
}
DEFINE_SHOW_ATTRIBUTE(domain_translation_struct);
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 19024dc52735..a39d72a9d1cf 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -314,7 +314,7 @@ static int iommu_skip_te_disable;
#define IDENTMAP_GFX 2
#define IDENTMAP_AZALIA4
-DEFINE_SPINLOCK(device_domain_lock);
+static DEFINE_SPINLOCK(device_domain_lock);
static LIST_HEAD(device_domain_list);
/*
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 00/12] iommu/vt-d: Optimize the use of locks
Hi folks, This series tries to optimize the uses of two locks in the Intel IOMMU driver: - The intel_iommu::lock is used to protect the IOMMU resources shared by devices. They include the IOMMU root and context tables, the pasid tables and the domain IDs. - The global device_domain_lock is used to protect the global and the per-domain device tracking lists. The optimization includes: - Remove the unnecessary global device tracking list; - Remove unnecessary locking; - Reduce the scope of the lock as much as possible, that is, use the lock only where necessary; - The global lock is transformed into a local lock to improve the efficiency. This series is also available on github: https://github.com/LuBaolu/intel-iommu/commits/intel-iommu-lock-optimization-v2 Your comments and suggestions are very appreciated. Best regards, baolu Change log: v2: - Split the lock-free page walk issue into a new patch: https://lore.kernel.org/linux-iommu/20220609070811.902868-1-baolu...@linux.intel.com/ - Drop the conversion from spinlock to mutex and make this series cleanup purpose only. - Address several comments received during v1 review. v1: - https://lore.kernel.org/linux-iommu/20220527063019.3112905-1-baolu...@linux.intel.com/ - Initial post. Lu Baolu (12): iommu/vt-d: debugfs: Remove device_domain_lock usage iommu/vt-d: Remove for_each_device_domain() iommu/vt-d: Remove clearing translation data in disable_dmar_iommu() iommu/vt-d: Use pci_get_domain_bus_and_slot() in pgtable_walk() iommu/vt-d: Unnecessary spinlock for root table alloc and free iommu/vt-d: Acquiring lock in domain ID allocation helpers iommu/vt-d: Acquiring lock in pasid manipulation helpers iommu/vt-d: Replace spin_lock_irqsave() with spin_lock() iommu/vt-d: Check device list of domain in domain free path iommu/vt-d: Fold __dmar_remove_one_dev_info() into its caller iommu/vt-d: Use device_domain_lock accurately iommu/vt-d: Convert global spinlock into per domain ones drivers/iommu/intel/iommu.h | 5 +- drivers/iommu/intel/pasid.h | 1 + drivers/iommu/intel/debugfs.c | 55 --- drivers/iommu/intel/iommu.c | 279 ++ drivers/iommu/intel/pasid.c | 149 +- drivers/iommu/intel/svm.c | 5 +- 6 files changed, 185 insertions(+), 309 deletions(-) -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On 2022/6/14 09:54, Jerry Snitselaar wrote: On Mon, Jun 13, 2022 at 6:51 PM Baolu Lu wrote: On 2022/6/14 09:44, Jerry Snitselaar wrote: On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu wrote: On 2022/6/14 04:57, Jerry Snitselaar wrote: On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote: To support up to 64 sockets with 10 DMAR units each (640), make the value of DMAR_UNITS_SUPPORTED adjustable by a config variable, CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is set. If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ remapping doesn't support X2APIC mode x2apic disabled"; and the system fails to boot properly. Signed-off-by: Steve Wahl --- Note that we could not find a reason for connecting DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. Perhaps it seemed like the two would continue to match on earlier processors. There doesn't appear to be kernel code that assumes that the value of one is related to the other. v2: Make this value a config option, rather than a fixed constant. The default values should match previous configuration except in the MAXSMP case. Keeping the value at a power of two was requested by Kevin Tian. drivers/iommu/intel/Kconfig | 6 ++ include/linux/dmar.h| 6 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig index 247d0f2d5fdf..fdbda77ac21e 100644 --- a/drivers/iommu/intel/Kconfig +++ b/drivers/iommu/intel/Kconfig @@ -9,6 +9,12 @@ config DMAR_PERF config DMAR_DEBUG bool +config DMAR_UNITS_SUPPORTED +int "Number of DMA Remapping Units supported" Also, should there be a "depends on (X86 || IA64)" here? Do you have any compilation errors or warnings? Best regards, baolu I think it is probably harmless since it doesn't get used elsewhere, but our tooling was complaining to me because DMAR_UNITS_SUPPORTED was being autogenerated into the configs for the non-x86 architectures we build (aarch64, s390x, ppcle64). We have files corresponding to the config options that it looks at, and I had one for x86 and not the others so it noticed the discrepancy. So with "depends on (X86 || IA64)", that tool doesn't complain anymore, right? Best regards, baolu Yes, with the depends it no longer happens. The dmar code only exists on X86 and IA64 arch's. Adding this depending makes sense to me. I will add it if no objections. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On Mon, Jun 13, 2022 at 6:51 PM Baolu Lu wrote: > > On 2022/6/14 09:44, Jerry Snitselaar wrote: > > On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu wrote: > >> On 2022/6/14 04:57, Jerry Snitselaar wrote: > >>> On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote: > To support up to 64 sockets with 10 DMAR units each (640), make the > value of DMAR_UNITS_SUPPORTED adjustable by a config variable, > CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is > set. > > If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set > to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to > allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ > remapping doesn't support X2APIC mode x2apic disabled"; and the system > fails to boot properly. > > Signed-off-by: Steve Wahl > --- > > Note that we could not find a reason for connecting > DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. Perhaps > it seemed like the two would continue to match on earlier processors. > There doesn't appear to be kernel code that assumes that the value of > one is related to the other. > > v2: Make this value a config option, rather than a fixed constant. The > default > values should match previous configuration except in the MAXSMP case. > Keeping the > value at a power of two was requested by Kevin Tian. > > drivers/iommu/intel/Kconfig | 6 ++ > include/linux/dmar.h| 6 +- > 2 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > index 247d0f2d5fdf..fdbda77ac21e 100644 > --- a/drivers/iommu/intel/Kconfig > +++ b/drivers/iommu/intel/Kconfig > @@ -9,6 +9,12 @@ config DMAR_PERF > config DMAR_DEBUG > bool > > +config DMAR_UNITS_SUPPORTED > +int "Number of DMA Remapping Units supported" > >>> Also, should there be a "depends on (X86 || IA64)" here? > >> Do you have any compilation errors or warnings? > >> > >> Best regards, > >> baolu > >> > > I think it is probably harmless since it doesn't get used elsewhere, > > but our tooling was complaining to me because DMAR_UNITS_SUPPORTED was > > being autogenerated into the configs for the non-x86 architectures we > > build (aarch64, s390x, ppcle64). > > We have files corresponding to the config options that it looks at, > > and I had one for x86 and not the others so it noticed the > > discrepancy. > > So with "depends on (X86 || IA64)", that tool doesn't complain anymore, > right? > > Best regards, > baolu > Yes, with the depends it no longer happens. Regards, Jerry ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On 2022/6/14 09:44, Jerry Snitselaar wrote: On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu wrote: On 2022/6/14 04:57, Jerry Snitselaar wrote: On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote: To support up to 64 sockets with 10 DMAR units each (640), make the value of DMAR_UNITS_SUPPORTED adjustable by a config variable, CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is set. If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ remapping doesn't support X2APIC mode x2apic disabled"; and the system fails to boot properly. Signed-off-by: Steve Wahl --- Note that we could not find a reason for connecting DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. Perhaps it seemed like the two would continue to match on earlier processors. There doesn't appear to be kernel code that assumes that the value of one is related to the other. v2: Make this value a config option, rather than a fixed constant. The default values should match previous configuration except in the MAXSMP case. Keeping the value at a power of two was requested by Kevin Tian. drivers/iommu/intel/Kconfig | 6 ++ include/linux/dmar.h| 6 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig index 247d0f2d5fdf..fdbda77ac21e 100644 --- a/drivers/iommu/intel/Kconfig +++ b/drivers/iommu/intel/Kconfig @@ -9,6 +9,12 @@ config DMAR_PERF config DMAR_DEBUG bool +config DMAR_UNITS_SUPPORTED +int "Number of DMA Remapping Units supported" Also, should there be a "depends on (X86 || IA64)" here? Do you have any compilation errors or warnings? Best regards, baolu I think it is probably harmless since it doesn't get used elsewhere, but our tooling was complaining to me because DMAR_UNITS_SUPPORTED was being autogenerated into the configs for the non-x86 architectures we build (aarch64, s390x, ppcle64). We have files corresponding to the config options that it looks at, and I had one for x86 and not the others so it noticed the discrepancy. So with "depends on (X86 || IA64)", that tool doesn't complain anymore, right? Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On Mon, Jun 13, 2022 at 6:36 PM Baolu Lu wrote: > > On 2022/6/14 04:57, Jerry Snitselaar wrote: > > On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote: > >> To support up to 64 sockets with 10 DMAR units each (640), make the > >> value of DMAR_UNITS_SUPPORTED adjustable by a config variable, > >> CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is > >> set. > >> > >> If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set > >> to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to > >> allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ > >> remapping doesn't support X2APIC mode x2apic disabled"; and the system > >> fails to boot properly. > >> > >> Signed-off-by: Steve Wahl > >> --- > >> > >> Note that we could not find a reason for connecting > >> DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. Perhaps > >> it seemed like the two would continue to match on earlier processors. > >> There doesn't appear to be kernel code that assumes that the value of > >> one is related to the other. > >> > >> v2: Make this value a config option, rather than a fixed constant. The > >> default > >> values should match previous configuration except in the MAXSMP case. > >> Keeping the > >> value at a power of two was requested by Kevin Tian. > >> > >> drivers/iommu/intel/Kconfig | 6 ++ > >> include/linux/dmar.h| 6 +- > >> 2 files changed, 7 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > >> index 247d0f2d5fdf..fdbda77ac21e 100644 > >> --- a/drivers/iommu/intel/Kconfig > >> +++ b/drivers/iommu/intel/Kconfig > >> @@ -9,6 +9,12 @@ config DMAR_PERF > >> config DMAR_DEBUG > >> bool > >> > >> +config DMAR_UNITS_SUPPORTED > >> +int "Number of DMA Remapping Units supported" > > > > Also, should there be a "depends on (X86 || IA64)" here? > > Do you have any compilation errors or warnings? > > Best regards, > baolu > I think it is probably harmless since it doesn't get used elsewhere, but our tooling was complaining to me because DMAR_UNITS_SUPPORTED was being autogenerated into the configs for the non-x86 architectures we build (aarch64, s390x, ppcle64). We have files corresponding to the config options that it looks at, and I had one for x86 and not the others so it noticed the discrepancy. > > > >> +default 1024 if MAXSMP > >> +default 128 if X86_64 > >> +default 64 > >> + > >> config INTEL_IOMMU > >> bool "Support for Intel IOMMU using DMA Remapping Devices" > >> depends on PCI_MSI && ACPI && (X86 || IA64) > >> diff --git a/include/linux/dmar.h b/include/linux/dmar.h > >> index 45e903d84733..0c03c1845c23 100644 > >> --- a/include/linux/dmar.h > >> +++ b/include/linux/dmar.h > >> @@ -18,11 +18,7 @@ > >> > >> struct acpi_dmar_header; > >> > >> -#ifdef CONFIG_X86 > >> -# defineDMAR_UNITS_SUPPORTEDMAX_IO_APICS > >> -#else > >> -# defineDMAR_UNITS_SUPPORTED64 > >> -#endif > >> +#define DMAR_UNITS_SUPPORTEDCONFIG_DMAR_UNITS_SUPPORTED > >> > >> /* DMAR Flags */ > >> #define DMAR_INTR_REMAP0x1 > >> -- > >> 2.26.2 > >> > > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On 2022/6/14 04:57, Jerry Snitselaar wrote: On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote: To support up to 64 sockets with 10 DMAR units each (640), make the value of DMAR_UNITS_SUPPORTED adjustable by a config variable, CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is set. If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ remapping doesn't support X2APIC mode x2apic disabled"; and the system fails to boot properly. Signed-off-by: Steve Wahl --- Note that we could not find a reason for connecting DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. Perhaps it seemed like the two would continue to match on earlier processors. There doesn't appear to be kernel code that assumes that the value of one is related to the other. v2: Make this value a config option, rather than a fixed constant. The default values should match previous configuration except in the MAXSMP case. Keeping the value at a power of two was requested by Kevin Tian. drivers/iommu/intel/Kconfig | 6 ++ include/linux/dmar.h| 6 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig index 247d0f2d5fdf..fdbda77ac21e 100644 --- a/drivers/iommu/intel/Kconfig +++ b/drivers/iommu/intel/Kconfig @@ -9,6 +9,12 @@ config DMAR_PERF config DMAR_DEBUG bool +config DMAR_UNITS_SUPPORTED + int "Number of DMA Remapping Units supported" Also, should there be a "depends on (X86 || IA64)" here? Do you have any compilation errors or warnings? Best regards, baolu + default 1024 if MAXSMP + default 128 if X86_64 + default 64 + config INTEL_IOMMU bool "Support for Intel IOMMU using DMA Remapping Devices" depends on PCI_MSI && ACPI && (X86 || IA64) diff --git a/include/linux/dmar.h b/include/linux/dmar.h index 45e903d84733..0c03c1845c23 100644 --- a/include/linux/dmar.h +++ b/include/linux/dmar.h @@ -18,11 +18,7 @@ struct acpi_dmar_header; -#ifdef CONFIG_X86 -# define DMAR_UNITS_SUPPORTEDMAX_IO_APICS -#else -# define DMAR_UNITS_SUPPORTED64 -#endif +#defineDMAR_UNITS_SUPPORTEDCONFIG_DMAR_UNITS_SUPPORTED /* DMAR Flags */ #define DMAR_INTR_REMAP 0x1 -- 2.26.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On 2022/6/14 04:38, Jerry Snitselaar wrote: On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote: To support up to 64 sockets with 10 DMAR units each (640), make the value of DMAR_UNITS_SUPPORTED adjustable by a config variable, CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is set. If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ remapping doesn't support X2APIC mode x2apic disabled"; and the system fails to boot properly. Signed-off-by: Steve Wahl --- Note that we could not find a reason for connecting DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. Perhaps it seemed like the two would continue to match on earlier processors. There doesn't appear to be kernel code that assumes that the value of one is related to the other. v2: Make this value a config option, rather than a fixed constant. The default values should match previous configuration except in the MAXSMP case. Keeping the value at a power of two was requested by Kevin Tian. drivers/iommu/intel/Kconfig | 6 ++ include/linux/dmar.h| 6 +- 2 files changed, 7 insertions(+), 5 deletions(-) Baolu do you have this queued up for v5.20? Also do you have a public repo where you keep the vt-d changes before sending Joerg the patches for a release? Yes. I have started to queue patches for v5.20. They could be found on github: https://github.com/LuBaolu/intel-iommu/commits/vtd-next-for-v5.20 Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote: > To support up to 64 sockets with 10 DMAR units each (640), make the > value of DMAR_UNITS_SUPPORTED adjustable by a config variable, > CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is > set. > > If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set > to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to > allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ > remapping doesn't support X2APIC mode x2apic disabled"; and the system > fails to boot properly. > > Signed-off-by: Steve Wahl > --- > > Note that we could not find a reason for connecting > DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. Perhaps > it seemed like the two would continue to match on earlier processors. > There doesn't appear to be kernel code that assumes that the value of > one is related to the other. > > v2: Make this value a config option, rather than a fixed constant. The > default > values should match previous configuration except in the MAXSMP case. > Keeping the > value at a power of two was requested by Kevin Tian. > > drivers/iommu/intel/Kconfig | 6 ++ > include/linux/dmar.h| 6 +- > 2 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig > index 247d0f2d5fdf..fdbda77ac21e 100644 > --- a/drivers/iommu/intel/Kconfig > +++ b/drivers/iommu/intel/Kconfig > @@ -9,6 +9,12 @@ config DMAR_PERF > config DMAR_DEBUG > bool > > +config DMAR_UNITS_SUPPORTED > + int "Number of DMA Remapping Units supported" Also, should there be a "depends on (X86 || IA64)" here? > + default 1024 if MAXSMP > + default 128 if X86_64 > + default 64 > + > config INTEL_IOMMU > bool "Support for Intel IOMMU using DMA Remapping Devices" > depends on PCI_MSI && ACPI && (X86 || IA64) > diff --git a/include/linux/dmar.h b/include/linux/dmar.h > index 45e903d84733..0c03c1845c23 100644 > --- a/include/linux/dmar.h > +++ b/include/linux/dmar.h > @@ -18,11 +18,7 @@ > > struct acpi_dmar_header; > > -#ifdef CONFIG_X86 > -# define DMAR_UNITS_SUPPORTEDMAX_IO_APICS > -#else > -# define DMAR_UNITS_SUPPORTED64 > -#endif > +#define DMAR_UNITS_SUPPORTEDCONFIG_DMAR_UNITS_SUPPORTED > > /* DMAR Flags */ > #define DMAR_INTR_REMAP 0x1 > -- > 2.26.2 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting
On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote: > To support up to 64 sockets with 10 DMAR units each (640), make the > value of DMAR_UNITS_SUPPORTED adjustable by a config variable, > CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is > set. > > If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set > to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to > allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ > remapping doesn't support X2APIC mode x2apic disabled"; and the system > fails to boot properly. > > Signed-off-by: Steve Wahl > --- > > Note that we could not find a reason for connecting > DMAR_UNITS_SUPPORTED to MAX_IO_APICS as was done previously. Perhaps > it seemed like the two would continue to match on earlier processors. > There doesn't appear to be kernel code that assumes that the value of > one is related to the other. > > v2: Make this value a config option, rather than a fixed constant. The > default > values should match previous configuration except in the MAXSMP case. > Keeping the > value at a power of two was requested by Kevin Tian. > > drivers/iommu/intel/Kconfig | 6 ++ > include/linux/dmar.h| 6 +- > 2 files changed, 7 insertions(+), 5 deletions(-) > Baolu do you have this queued up for v5.20? Also do you have a public repo where you keep the vt-d changes before sending Joerg the patches for a release? Regards, Jerry ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/7] iommu/amd: Introduce function to check SEV-SNP support
On 6/13/2022 8:24 AM, Suravee Suthikulpanit wrote:
@@ -3543,3 +3537,30 @@ int amd_iommu_pc_set_reg(struct amd_iommu *iommu, u8
bank, u8 cntr, u8 fxn, u64
return iommu_pc_get_set_reg(iommu, bank, cntr, fxn, value, true);
}
+
+bool iommu_sev_snp_supported(void)
+{
+ /*
+* The SEV-SNP support requires that IOMMU must be enabled, and is
+* not configured in the passthrough mode.
+*/
+ if (no_iommu || iommu_default_passthrough()) {
+ pr_err("SEV-SNP: IOMMU is either disabled or configured in
passthrough mode.\n");
+ return false;
+ }
+
+ amd_iommu_snp_en = amd_iommu_snp_sup;
+ if (amd_iommu_snp_en)
+ pr_info("SNP enabled\n");
+
+ /* Enforce IOMMU v1 pagetable when SNP is enabled. */
+ if ((amd_iommu_pgtable != AMD_IOMMU_V1) &&
+amd_iommu_snp_en) {
+ pr_info("Force to using AMD IOMMU v1 page table due to SNP\n");
+ amd_iommu_pgtable = AMD_IOMMU_V1;
+ amd_iommu_ops.pgsize_bitmap = AMD_IOMMU_PGSIZES;
+ }
+
+ return amd_iommu_snp_en;
+}
+EXPORT_SYMBOL_GPL(iommu_sev_snp_supported);
I need to guard this function w/ #ifdef CONFIG_AMD_MEM_ENCRYPT to prevent build
error when CONFIG_AMD_MEM_ENCRYPT=n.
I will send out v2 to fix this.
Best Regards,
Suravee
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 5/7] iommu: Add domain_type_supported() callback in iommu_ops
Robin, On 6/13/2022 4:31 PM, Robin Murphy wrote: On 2022-06-13 02:25, Suravee Suthikulpanit wrote: When user requests to change IOMMU domain to a new type, IOMMU generic layer checks the requested type against the default domain type returned by vendor-specific IOMMU driver. However, there is only one default domain type, and current mechanism does not allow if the requested type does not match the default type. I don't really follow the reasoning here. If a driver's def_domain_type callback returns a specific type, it's saying that the device *has* to have that specific domain type for driver/platform-specific reasons. Agree, and I understand this part. If that's not the case, then the driver shouldn't say so in the first place. Considering the case: 1. Boot w/ default domain = IOMMU_DOMAIN_DMA_FQ 2. User wants to change to IOMMU_DOMAIN_IDENTITY, which is not supported by IOMMU driver. In this case, IOMMU driver can return IOMMU_DOMAIN_DMA_FQ and prevent the mode change. 3. However, if user want to change to IOMMU_DOMAIN_DMA. The driver can support this. However, since the def_domain_type() returns IOMMU_DOMAIN_DMA_FQ, it ends up prevent the mode change. IIUC, we should support step 3 above. Basically, with the newly proposed interface, it allows us to check with IOMMU driver if it can support certain domain types before trying to allocate the domain. Introducing check_domain_type_supported() callback in iommu_ops, which allows IOMMU generic layer to check with vendor-specific IOMMU driver whether the requested type is supported. This allows user to request types other than the default type. Note also that you're only adding this in the sysfs path - what about the "iommu.passthrough=" parameter or CONFIG_IOMMU_DEFAULT_PASSTHROUGH? For SNP case, we cannot enable SNP if iommu=off or iommu=pt or iommu.passthrough=1 or CONFIG_IOMMU_DEFAULT_PASSTHROUGH=y. So, when another driver tries to enable SNP, the IOMMU driver prevents it (see iommu_sev_snp_supported() in patch 3). Instead, if we boot with iommu.passhthrough=0, when another driver tries to enable SNP, the IOMMU driver allows this and switch to SNP enable mode. Subsequently, if user tries to switch a domain (via sysfs) to IOMMU_DOMAIN_IDENTITY, the IOMMU needs to prevent this because it has already switch to SNP-enabled mode. AFAICS there shouldn't need to be any core-level changes to support this. We already have drivers which don't support passthrough at all, so conditionally not supporting it should be no big deal. What should happen currently is that def_domain_type returns 0 for "don't care", then domain_alloc rejects IOMMU_DOMAIN_IDENTITY and and returns NULL, so iommu_group_alloc_default_domain() falls back to IOMMU_DOMAIN_DMA. Technically, we can do it the way you suggest. But isn't this confusing? At first, def_domain_type() returns 0 for "don't care", but then it rejects the request to change to IOMMU_DOMAIN_IDENTITY when trying to call domain_alloc(). Please let me know if I am missing something. Best Regards, Suravee ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 3/3] iommu/mediatek: Allow page table PA up to 35bit
On Sat, 2022-06-11 at 18:26 +0800, yf.w...@mediatek.com wrote:
> From: Yunfei Wang
>
> Single memory zone feature will remove ZONE_DMA32 and ZONE_DMA. So
> add
> the quirk IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT to let level 1 and level
> 2
> pgtable support at most 35bit PA.
>
> Signed-off-by: Ning Li
> Signed-off-by: Yunfei Wang
Reviewed-by: Yong Wu
Thanks very much for this function. All the lastest SoCs like
mt8192/mt8195 support this.
> ---
> drivers/iommu/mtk_iommu.c | 14 +-
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 3d62399e8865..4dbc33758711 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -138,6 +138,7 @@
> /* PM and clock always on. e.g. infra iommu */
> #define PM_CLK_AOBIT(15)
> #define IFA_IOMMU_PCIE_SUPPORT BIT(16)
> +#define PGTABLE_PA_35_EN BIT(17)
>
> #define MTK_IOMMU_HAS_FLAG_MASK(pdata, _x, mask) \
> pdata)->flags) & (mask)) == (_x))
> @@ -240,6 +241,7 @@ struct mtk_iommu_data {
> struct mtk_iommu_domain {
> struct io_pgtable_cfg cfg;
> struct io_pgtable_ops *iop;
> + u32 ttbr;
>
> struct mtk_iommu_bank_data *bank;
> struct iommu_domain domain;
> @@ -596,6 +598,9 @@ static int mtk_iommu_domain_finalise(struct
> mtk_iommu_domain *dom,
> .iommu_dev = data->dev,
> };
>
> + if (MTK_IOMMU_HAS_FLAG(data->plat_data, PGTABLE_PA_35_EN))
> + dom->cfg.quirks |= IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT;
> +
> if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE))
> dom->cfg.oas = data->enable_4GB ? 33 : 32;
> else
> @@ -684,8 +689,8 @@ static int mtk_iommu_attach_device(struct
> iommu_domain *domain,
> goto err_unlock;
> }
> bank->m4u_dom = dom;
> - writel(dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK,
> -bank->base + REG_MMU_PT_BASE_ADDR);
> + bank->m4u_dom->ttbr = MTK_IOMMU_ADDR(dom-
> >cfg.arm_v7s_cfg.ttbr);
> + writel(bank->m4u_dom->ttbr, data->base +
> REG_MMU_PT_BASE_ADDR);
>
> pm_runtime_put(m4udev);
> }
> @@ -1366,8 +1371,7 @@ static int __maybe_unused
> mtk_iommu_runtime_resume(struct device *dev)
> writel_relaxed(reg->int_control[i], base +
> REG_MMU_INT_CONTROL0);
> writel_relaxed(reg->int_main_control[i], base +
> REG_MMU_INT_MAIN_CONTROL);
> writel_relaxed(reg->ivrp_paddr[i], base +
> REG_MMU_IVRP_PADDR);
> - writel(m4u_dom->cfg.arm_v7s_cfg.ttbr &
> MMU_PT_ADDR_MASK,
> -base + REG_MMU_PT_BASE_ADDR);
> + writel(m4u_dom->ttbr, base + REG_MMU_PT_BASE_ADDR);
> } while (++i < data->plat_data->banks_num);
>
> /*
> @@ -1401,7 +1405,7 @@ static const struct mtk_iommu_plat_data
> mt2712_data = {
> static const struct mtk_iommu_plat_data mt6779_data = {
> .m4u_plat = M4U_MT6779,
> .flags = HAS_SUB_COMM_2BITS | OUT_ORDER_WR_EN |
> WR_THROT_EN |
> - MTK_IOMMU_TYPE_MM,
> + MTK_IOMMU_TYPE_MM | PGTABLE_PA_35_EN,
> .inv_sel_reg = REG_MMU_INV_SEL_GEN2,
> .banks_num= 1,
> .banks_enable = {true},
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 2/3] iommu/mediatek: Rename MTK_IOMMU_TLB_ADDR to MTK_IOMMU_ADDR
On Sat, 2022-06-11 at 18:26 +0800, yf.w...@mediatek.com wrote:
> From: Yunfei Wang
>
> Rename MTK_IOMMU_TLB_ADDR to MTK_IOMMU_ADDR, and update
> MTK_IOMMU_ADDR
> definition for better generality.
Comment more about why you need this.
Prepare for supporting TTBR up to 35bit which also need this macro.
Currently it is dma_addr_t while ttbr is phys_addr_t, thus change the
type to "unsigned long long" for generality.
Anyway,
Reviewed-by: Yong Wu
>
> Signed-off-by: Ning Li
> Signed-off-by: Yunfei Wang
> ---
> drivers/iommu/mtk_iommu.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index bb9dd92c9898..3d62399e8865 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -265,8 +265,8 @@ static const struct iommu_ops mtk_iommu_ops;
>
> static int mtk_iommu_hw_init(const struct mtk_iommu_data *data,
> unsigned int bankid);
>
> -#define MTK_IOMMU_TLB_ADDR(iova) ({
> \
> - dma_addr_t _addr = iova;\
> +#define MTK_IOMMU_ADDR(addr) ({
> \
> + unsigned long long _addr = addr;\
> ((lower_32_bits(_addr) & GENMASK(31, 12)) |
> upper_32_bits(_addr));\
> })
>
> @@ -381,8 +381,8 @@ static void
> mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
> writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> base + data->plat_data->inv_sel_reg);
>
> - writel_relaxed(MTK_IOMMU_TLB_ADDR(iova), base +
> REG_MMU_INVLD_START_A);
> - writel_relaxed(MTK_IOMMU_TLB_ADDR(iova + size - 1),
> + writel_relaxed(MTK_IOMMU_ADDR(iova), base +
> REG_MMU_INVLD_START_A);
> + writel_relaxed(MTK_IOMMU_ADDR(iova + size - 1),
> base + REG_MMU_INVLD_END_A);
> writel_relaxed(F_MMU_INV_RANGE, base +
> REG_MMU_INVALIDATE);
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 5/7] iommu: Add domain_type_supported() callback in iommu_ops
On 2022-06-13 02:25, Suravee Suthikulpanit wrote:
When user requests to change IOMMU domain to a new type, IOMMU generic
layer checks the requested type against the default domain type returned
by vendor-specific IOMMU driver.
However, there is only one default domain type, and current mechanism
does not allow if the requested type does not match the default type.
I don't really follow the reasoning here. If a driver's def_domain_type
callback returns a specific type, it's saying that the device *has* to
have that specific domain type for driver/platform-specific reasons. If
that's not the case, then the driver shouldn't say so in the first place.
Introducing check_domain_type_supported() callback in iommu_ops,
which allows IOMMU generic layer to check with vendor-specific IOMMU driver
whether the requested type is supported. This allows user to request
types other than the default type.
Note also that you're only adding this in the sysfs path - what about
the "iommu.passthrough=" parameter or CONFIG_IOMMU_DEFAULT_PASSTHROUGH?
AFAICS there shouldn't need to be any core-level changes to support
this. We already have drivers which don't support passthrough at all, so
conditionally not supporting it should be no big deal. What should
happen currently is that def_domain_type returns 0 for "don't care",
then domain_alloc rejects IOMMU_DOMAIN_IDENTITY and and returns NULL, so
iommu_group_alloc_default_domain() falls back to IOMMU_DOMAIN_DMA.
Thanks,
Robin.
Signed-off-by: Suravee Suthikulpanit
---
drivers/iommu/iommu.c | 13 -
include/linux/iommu.h | 2 ++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f2c45b85b9fc..4afb956ce083 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1521,6 +1521,16 @@ struct iommu_group *fsl_mc_device_group(struct device
*dev)
}
EXPORT_SYMBOL_GPL(fsl_mc_device_group);
+static bool iommu_domain_type_supported(struct device *dev, int type)
+{
+ const struct iommu_ops *ops = dev_iommu_ops(dev);
+
+ if (ops->domain_type_supported)
+ return ops->domain_type_supported(dev, type);
+
+ return true;
+}
+
static int iommu_get_def_domain_type(struct device *dev)
{
const struct iommu_ops *ops = dev_iommu_ops(dev);
@@ -2937,7 +2947,8 @@ static int iommu_change_dev_def_domain(struct iommu_group
*group,
* domain the device was booted with
*/
type = dev_def_dom ? : iommu_def_domain_type;
- } else if (dev_def_dom && type != dev_def_dom) {
+ } else if (!iommu_domain_type_supported(dev, type) ||
+ (dev_def_dom && type != dev_def_dom)) {
dev_err_ratelimited(prev_dev, "Device cannot be in %s domain\n",
iommu_domain_type_str(type));
ret = -EINVAL;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index fecb72e1b11b..40c47ab15005 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -214,6 +214,7 @@ struct iommu_iotlb_gather {
*- IOMMU_DOMAIN_IDENTITY: must use an identity domain
*- IOMMU_DOMAIN_DMA: must use a dma domain
*- 0: use the default setting
+ * @domain_type_supported: check if the specified domain type is supported
* @default_domain_ops: the default ops for domains
* @pgsize_bitmap: bitmap of all possible supported page sizes
* @owner: Driver module providing these ops
@@ -252,6 +253,7 @@ struct iommu_ops {
struct iommu_page_response *msg);
int (*def_domain_type)(struct device *dev);
+ bool (*domain_type_supported)(struct device *dev, int type);
const struct iommu_domain_ops *default_domain_ops;
unsigned long pgsize_bitmap;
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 6/6] iommu: mtk_iommu: Lookup phandle to retrieve syscon to pericfg
Il 13/06/22 07:32, Yong Wu ha scritto: On Thu, 2022-06-09 at 12:08 +0200, AngeloGioacchino Del Regno wrote: On some SoCs (of which only MT8195 is supported at the time of writing), the "R" and "W" (I/O) enable bits for the IOMMUs are in the pericfg_ao register space and not in the IOMMU space: as it happened already with infracfg, it is expected that this list will grow. Currently I don't see the list will grow. As commented before, In the lastest SoC, The IOMMU enable bits for IOMMU will be in ATF, rather than in this pericfg register region. In this case, Is this patch unnecessary? or we could add this patch when there are 2 SoCs use this setting at least? what's your opinion? Perhaps I've misunderstood... besides, can you please check if there's any other SoC (not just chromebooks, also smartphone SoCs) that need this logic? Thanks, Angelo ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 4/4] swiotlb: panic if nslabs is too small
On 6/11/22 1:25 AM, Dongli Zhang wrote:
> Panic on purpose if nslabs is too small, in order to sync with the remap
> retry logic.
>
> In addition, print the number of bytes for tlb alloc failure.
>
> Signed-off-by: Dongli Zhang
> ---
> kernel/dma/swiotlb.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index fd21f4162f4b..1758b724c7a8 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -242,6 +242,9 @@ void __init swiotlb_init_remap(bool addressing_limit,
> unsigned int flags,
> if (swiotlb_force_disable)
> return;
>
> + if (nslabs < IO_TLB_MIN_SLABS)
> + panic("%s: nslabs = %lu too small\n", __func__, nslabs);
> +
> /*
>* By default allocate the bounce buffer memory from low memory, but
>* allow to pick a location everywhere for hypervisors with guest
> @@ -254,7 +257,8 @@ void __init swiotlb_init_remap(bool addressing_limit,
> unsigned int flags,
> else
> tlb = memblock_alloc_low(bytes, PAGE_SIZE);
> if (!tlb) {
> - pr_warn("%s: failed to allocate tlb structure\n", __func__);
> + pr_warn("%s: Failed to allocate %zu bytes tlb structure\n",
> + __func__, bytes);
Indeed I have a question on this pr_warn(). (I was going to send another patch
to retry with nslabs = ALIGN(nslabs >> 1, IO_TLB_SEGSIZE)).
Why not retry with nslabs = ALIGN(nslabs >> 1, IO_TLB_SEGSIZE), or panic here?
If the QEMU machine of my VM is i440fx, the boot is almost failed even here is
pr_warn. Why not sync with the remap failure handling?
1. retry with nslabs = ALIGN(nslabs >> 1, IO_TLB_SEGSIZE))
2. and finally panic if nslabs is too small.
Thank you very much!
Dongli Zhang
> return;
> }
>
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC v1 4/7] swiotlb: to implement io_tlb_high_mem
On Fri, Jun 10, 2022 at 02:56:08PM -0700, Dongli Zhang wrote: > Since this patch file has 200+ lines, would you please help clarify what does > 'this' indicate? This indicates that any choice of a different swiotlb pools needs to be hidden inside of ѕwiotlb. The dma mapping API already provides swiotlb the addressability requirement for the device. Similarly we already have a SWIOTLB_ANY flag that switches to a 64-bit buffer by default, which we can change to, or replace with a flag that allocates an additional buffer that is not addressing limited. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
