Re: [PATCH v2 01/12] iommu/vt-d: debugfs: Remove device_domain_lock usage
On 2022/6/15 14:13, Tian, Kevin wrote:
From: Baolu Lu
Sent: Wednesday, June 15, 2022 9:54 AM
On 2022/6/14 14:43, Tian, Kevin wrote:
From: Lu Baolu
Sent: Tuesday, June 14, 2022 10:51 AM
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.
is it really problematic at this point? Before following patches are applied
using device_domain_lock should have similar effect as holding the group
lock.
Here it might make more sense to just focus on removing the use of
device_domain_lock outside of iommu.c. Just that using group lock is
cleaner and more compatible to following cleanups.
and it's worth mentioning that racing with page table updates is out
of the scope of this series. Probably also add a comment in the code
to clarify this point.
Hi Kevin,
How do you like below updated patch?
Yes, this is better.
From cecc9a0623780a11c4ea4d0a15aa6187f01541c4 Mon Sep 17 00:00:00
2001
From: Lu Baolu
Date: Sun, 29 May 2022 10:18:56 +0800
Subject: [PATCH 1/1] 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 the global spinlock device_domain_lock to
avoid the races.
This removes the use of device_domain_lock outside of iommu.c by replacing
it with the group mutex lock. Using the group mutex lock is cleaner and
more compatible to following cleanups.
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/debugfs.c | 42 +--
drivers/iommu/intel/iommu.c | 2 +-
drivers/iommu/intel/iommu.h | 1 -
3 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
index d927ef10641b..f4acd8993f60 100644
--- a/drivers/iommu/intel/debugfs.c
+++ b/drivers/iommu/intel/debugfs.c
@@ -342,13 +342,13 @@ static void pgtable_walk_level(struct seq_file *m,
struct dma_pte *pde,
}
}
-static int show_device_domain_translation(struct device *dev, void *data)
+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 dmar_domain *domain;
struct seq_file *m = data;
u64 path[6] = { 0 };
+ domain = to_dmar_domain(iommu_get_domain_for_dev(dev));
if (!domain)
return 0;
@@ -359,20 +359,38 @@ 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 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.
+*
+* All devices in an iommu group share a single domain,
hence
+* we only dump the domain of the first device. Even though,
bus_for_each_dev() will still lead to duplicated dump in the same group
but probably we can leave with it for a debug interface.
Yes. This is what it was. Ideally we could walk the iommu groups and
dump the device names belonging to the group and it's domain mappings,
but I was not willing to add any helpers in the iommu core just for a
debugfs use.
---
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v2 01/12] iommu/vt-d: debugfs: Remove device_domain_lock usage
> From: Baolu Lu
> Sent: Wednesday, June 15, 2022 9:54 AM
>
> On 2022/6/14 14:43, Tian, Kevin wrote:
> >> From: Lu Baolu
> >> Sent: Tuesday, June 14, 2022 10:51 AM
> >>
> >> 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.
> > is it really problematic at this point? Before following patches are applied
> > using device_domain_lock should have similar effect as holding the group
> > lock.
> >
> > Here it might make more sense to just focus on removing the use of
> > device_domain_lock outside of iommu.c. Just that using group lock is
> > cleaner and more compatible to following cleanups.
> >
> > and it's worth mentioning that racing with page table updates is out
> > of the scope of this series. Probably also add a comment in the code
> > to clarify this point.
> >
>
> Hi Kevin,
>
> How do you like below updated patch?
Yes, this is better.
>
> From cecc9a0623780a11c4ea4d0a15aa6187f01541c4 Mon Sep 17 00:00:00
> 2001
> From: Lu Baolu
> Date: Sun, 29 May 2022 10:18:56 +0800
> Subject: [PATCH 1/1] 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 the global spinlock device_domain_lock to
> avoid the races.
>
> This removes the use of device_domain_lock outside of iommu.c by replacing
> it with the group mutex lock. Using the group mutex lock is cleaner and
> more compatible to following cleanups.
>
> Signed-off-by: Lu Baolu
> ---
> drivers/iommu/intel/debugfs.c | 42 +--
> drivers/iommu/intel/iommu.c | 2 +-
> drivers/iommu/intel/iommu.h | 1 -
> 3 files changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
> index d927ef10641b..f4acd8993f60 100644
> --- a/drivers/iommu/intel/debugfs.c
> +++ b/drivers/iommu/intel/debugfs.c
> @@ -342,13 +342,13 @@ static void pgtable_walk_level(struct seq_file *m,
> struct dma_pte *pde,
> }
> }
>
> -static int show_device_domain_translation(struct device *dev, void *data)
> +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 dmar_domain *domain;
> struct seq_file *m = data;
> u64 path[6] = { 0 };
>
> + domain = to_dmar_domain(iommu_get_domain_for_dev(dev));
> if (!domain)
> return 0;
>
> @@ -359,20 +359,38 @@ 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 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.
> + *
> + * All devices in an iommu group share a single domain,
> hence
> + * we only dump the domain of the first device. Even though,
bus_for_each_dev() will still lead to duplicated dump in the same group
but probably we can leave with it for a debug interface.
> + * this code still possibly races with the iommu_unmap()
> + * interface. This could be solved by RCU-freeing the page
> + * table pages in the iommu_unmap() path.
> + */
> + iommu_group_for_each_dev(group, data,
> + __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
Re: [PATCH v2 01/12] iommu/vt-d: debugfs: Remove device_domain_lock usage
On 2022/6/14 14:43, Tian, Kevin wrote:
From: Lu Baolu
Sent: Tuesday, June 14, 2022 10:51 AM
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.
is it really problematic at this point? Before following patches are applied
using device_domain_lock should have similar effect as holding the group
lock.
Here it might make more sense to just focus on removing the use of
device_domain_lock outside of iommu.c. Just that using group lock is
cleaner and more compatible to following cleanups.
and it's worth mentioning that racing with page table updates is out
of the scope of this series. Probably also add a comment in the code
to clarify this point.
Hi Kevin,
How do you like below updated patch?
From cecc9a0623780a11c4ea4d0a15aa6187f01541c4 Mon Sep 17 00:00:00 2001
From: Lu Baolu
Date: Sun, 29 May 2022 10:18:56 +0800
Subject: [PATCH 1/1] 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 the global spinlock device_domain_lock to
avoid the races.
This removes the use of device_domain_lock outside of iommu.c by replacing
it with the group mutex lock. Using the group mutex lock is cleaner and
more compatible to following cleanups.
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/debugfs.c | 42 +--
drivers/iommu/intel/iommu.c | 2 +-
drivers/iommu/intel/iommu.h | 1 -
3 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
index d927ef10641b..f4acd8993f60 100644
--- a/drivers/iommu/intel/debugfs.c
+++ b/drivers/iommu/intel/debugfs.c
@@ -342,13 +342,13 @@ static void pgtable_walk_level(struct seq_file *m,
struct dma_pte *pde,
}
}
-static int show_device_domain_translation(struct device *dev, void *data)
+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 dmar_domain *domain;
struct seq_file *m = data;
u64 path[6] = { 0 };
+ domain = to_dmar_domain(iommu_get_domain_for_dev(dev));
if (!domain)
return 0;
@@ -359,20 +359,38 @@ 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 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.
+*
+* All devices in an iommu group share a single domain, hence
+* we only dump the domain of the first device. Even though,
+* this code still possibly races with the iommu_unmap()
+* interface. This could be solved by RCU-freeing the page
+* table pages in the iommu_unmap() path.
+*/
+ iommu_group_for_each_dev(group, data,
+__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);
/*
diff --git a/drivers/iommu/intel/iommu.h
Re: [PATCH v2 01/12] iommu/vt-d: debugfs: Remove device_domain_lock usage
Hi Kevin,
Thanks for your reviewing.
On 2022/6/14 14:43, Tian, Kevin wrote:
From: Lu Baolu
Sent: Tuesday, June 14, 2022 10:51 AM
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.
is it really problematic at this point? Before following patches are applied
using device_domain_lock should have similar effect as holding the group
lock.
The device_domain_lock only protects the device tracking list of the
domain, it doesn't include the domain pointer stored in the dev_info
structure. That's really protected by the group->mutex.
Here it might make more sense to just focus on removing the use of
device_domain_lock outside of iommu.c. Just that using group lock is
cleaner and more compatible to following cleanups.
Fair enough. I will update the commit message with above statement.
and it's worth mentioning that racing with page table updates is out
of the scope of this series. Probably also add a comment in the code
to clarify this point.
Sure.
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.
s/the file/iommu.c/
Sure.
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;
+};
Sounds redundant as both bus_for_each_dev() and
iommu_group_for_each_dev() declares the same fn type which
accepts a device pointer and void *data.
+
+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;
not required.
Together with above comment.
The iommu group might have other devices. I only want to dump the domain
of the secific @opaque->dev. It reads a bit confusing, but it's the
only helper I can use outside of drivers/iommu/iommu.c.
Or, since all devices in the iommu group share the same domain, hence
only dump once?
+ 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, ,
+
RE: [PATCH v2 01/12] iommu/vt-d: debugfs: Remove device_domain_lock usage
> From: Lu Baolu
> Sent: Tuesday, June 14, 2022 10:51 AM
>
> 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.
is it really problematic at this point? Before following patches are applied
using device_domain_lock should have similar effect as holding the group
lock.
Here it might make more sense to just focus on removing the use of
device_domain_lock outside of iommu.c. Just that using group lock is
cleaner and more compatible to following cleanups.
and it's worth mentioning that racing with page table updates is out
of the scope of this series. Probably also add a comment in the code
to clarify this point.
>
> 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.
s/the file/iommu.c/
>
> 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;
> +};
Sounds redundant as both bus_for_each_dev() and
iommu_group_for_each_dev() declares the same fn type which
accepts a device pointer and void *data.
> +
> +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;
not required.
> + 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
> +++
[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
