* [PATH v5 1/3] iommu/vt-d: debugfs: Dump entry pointing to huge page
2023-10-13 13:58 [PATH v5 0/3] iommu/vt-d: debugfs: Enhancements to IOMMU debugfs Jingqi Liu
@ 2023-10-13 13:58 ` Jingqi Liu
2023-10-13 13:58 ` [PATH v5 2/3] iommu/vt-d: debugfs: Create/remove debugfs file per {device, pasid} Jingqi Liu
2023-10-13 13:58 ` [PATH v5 3/3] iommu/vt-d: debugfs: Support dumping a specified page table Jingqi Liu
2 siblings, 0 replies; 10+ messages in thread
From: Jingqi Liu @ 2023-10-13 13:58 UTC (permalink / raw)
To: iommu, Lu Baolu, Tian Kevin, Joerg Roedel, Will Deacon,
Robin Murphy
Cc: linux-kernel, Jingqi Liu
For the page table entry pointing to a huge page, the data below the
level of the huge page is meaningless and does not need to be dumped.
Signed-off-by: Jingqi Liu <Jingqi.liu@intel.com>
---
drivers/iommu/intel/debugfs.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
index 1f925285104e..497c1561d3d2 100644
--- a/drivers/iommu/intel/debugfs.c
+++ b/drivers/iommu/intel/debugfs.c
@@ -311,9 +311,14 @@ static inline unsigned long level_to_directory_size(int level)
static inline void
dump_page_info(struct seq_file *m, unsigned long iova, u64 *path)
{
- seq_printf(m, "0x%013lx |\t0x%016llx\t0x%016llx\t0x%016llx\t0x%016llx\t0x%016llx\n",
- iova >> VTD_PAGE_SHIFT, path[5], path[4],
- path[3], path[2], path[1]);
+ seq_printf(m, "0x%013lx |\t0x%016llx\t0x%016llx\t0x%016llx",
+ iova >> VTD_PAGE_SHIFT, path[5], path[4], path[3]);
+ if (path[2]) {
+ seq_printf(m, "\t0x%016llx", path[2]);
+ if (path[1])
+ seq_printf(m, "\t0x%016llx", path[1]);
+ }
+ seq_putc(m, '\n');
}
static void pgtable_walk_level(struct seq_file *m, struct dma_pte *pde,
--
2.21.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATH v5 2/3] iommu/vt-d: debugfs: Create/remove debugfs file per {device, pasid}
2023-10-13 13:58 [PATH v5 0/3] iommu/vt-d: debugfs: Enhancements to IOMMU debugfs Jingqi Liu
2023-10-13 13:58 ` [PATH v5 1/3] iommu/vt-d: debugfs: Dump entry pointing to huge page Jingqi Liu
@ 2023-10-13 13:58 ` Jingqi Liu
2024-11-12 21:22 ` Kees Bakker
2023-10-13 13:58 ` [PATH v5 3/3] iommu/vt-d: debugfs: Support dumping a specified page table Jingqi Liu
2 siblings, 1 reply; 10+ messages in thread
From: Jingqi Liu @ 2023-10-13 13:58 UTC (permalink / raw)
To: iommu, Lu Baolu, Tian Kevin, Joerg Roedel, Will Deacon,
Robin Murphy
Cc: linux-kernel, Jingqi Liu
Add a debugfs directory per pair of {device, pasid} if the mappings of
its page table are created and destroyed by the iommu_map/unmap()
interfaces. i.e. /sys/kernel/debug/iommu/intel/<device source id>/<pasid>.
Create a debugfs file in the directory for users to dump the page
table corresponding to {device, pasid}. e.g.
/sys/kernel/debug/iommu/intel/0000:00:02.0/1/domain_translation_struct.
For the default domain without pasid, it creates a debugfs file in the
debugfs device directory for users to dump its page table. e.g.
/sys/kernel/debug/iommu/intel/0000:00:02.0/domain_translation_struct.
When setting a domain to a PASID of device, create a debugfs file in
the pasid debugfs directory for users to dump the page table of the
specified pasid. Remove the debugfs device directory of the device
when releasing a device. e.g.
/sys/kernel/debug/iommu/intel/0000:00:01.0
Signed-off-by: Jingqi Liu <Jingqi.liu@intel.com>
---
drivers/iommu/intel/debugfs.c | 53 +++++++++++++++++++++++++++++++----
drivers/iommu/intel/iommu.c | 7 +++++
drivers/iommu/intel/iommu.h | 14 +++++++++
3 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
index 497c1561d3d2..8a18a7be5215 100644
--- a/drivers/iommu/intel/debugfs.c
+++ b/drivers/iommu/intel/debugfs.c
@@ -111,6 +111,8 @@ static const struct iommu_regset iommu_regs_64[] = {
IOMMU_REGSET_ENTRY(VCRSP),
};
+static struct dentry *intel_iommu_debug;
+
static int iommu_regset_show(struct seq_file *m, void *unused)
{
struct dmar_drhd_unit *drhd;
@@ -671,16 +673,12 @@ static const struct file_operations dmar_perf_latency_fops = {
void __init intel_iommu_debugfs_init(void)
{
- struct dentry *intel_iommu_debug = debugfs_create_dir("intel",
- iommu_debugfs_dir);
+ intel_iommu_debug = debugfs_create_dir("intel", iommu_debugfs_dir);
debugfs_create_file("iommu_regset", 0444, intel_iommu_debug, NULL,
&iommu_regset_fops);
debugfs_create_file("dmar_translation_struct", 0444, intel_iommu_debug,
NULL, &dmar_translation_struct_fops);
- debugfs_create_file("domain_translation_struct", 0444,
- intel_iommu_debug, NULL,
- &domain_translation_struct_fops);
debugfs_create_file("invalidation_queue", 0444, intel_iommu_debug,
NULL, &invalidation_queue_fops);
#ifdef CONFIG_IRQ_REMAP
@@ -690,3 +688,48 @@ void __init intel_iommu_debugfs_init(void)
debugfs_create_file("dmar_perf_latency", 0644, intel_iommu_debug,
NULL, &dmar_perf_latency_fops);
}
+
+/*
+ * Create a debugfs directory for each device, and then create a
+ * debugfs file in this directory for users to dump the page table
+ * of the default domain. e.g.
+ * /sys/kernel/debug/iommu/intel/0000:00:01.0/domain_translation_struct
+ */
+void intel_iommu_debugfs_create_dev(struct device_domain_info *info)
+{
+ info->debugfs_dentry = debugfs_create_dir(dev_name(info->dev), intel_iommu_debug);
+
+ debugfs_create_file("domain_translation_struct", 0444, info->debugfs_dentry,
+ NULL, &domain_translation_struct_fops);
+}
+
+/* Remove the device debugfs directory. */
+void intel_iommu_debugfs_remove_dev(struct device_domain_info *info)
+{
+ debugfs_remove_recursive(info->debugfs_dentry);
+}
+
+/*
+ * Create a debugfs directory per pair of {device, pasid}, then create the
+ * corresponding debugfs file in this directory for users to dump its page
+ * table. e.g.
+ * /sys/kernel/debug/iommu/intel/0000:00:01.0/1/domain_translation_struct
+ *
+ * The debugfs only dumps the page tables whose mappings are created and
+ * destroyed by the iommu_map/unmap() interfaces. Check the mapping type
+ * of the domain before creating debugfs directory.
+ */
+void intel_iommu_debugfs_create_dev_pasid(struct dev_pasid_info *dev_pasid)
+{
+ struct device_domain_info *info = dev_iommu_priv_get(dev_pasid->dev);
+ char dir_name[10];
+
+ sprintf(dir_name, "%x", dev_pasid->pasid);
+ dev_pasid->debugfs_dentry = debugfs_create_dir(dir_name, info->debugfs_dentry);
+}
+
+/* Remove the device pasid debugfs directory. */
+void intel_iommu_debugfs_remove_dev_pasid(struct dev_pasid_info *dev_pasid)
+{
+ debugfs_remove_recursive(dev_pasid->debugfs_dentry);
+}
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 3685ba90ec88..d77d60bc418f 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4409,6 +4409,8 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
}
}
+ intel_iommu_debugfs_create_dev(info);
+
return &iommu->iommu;
}
@@ -4418,6 +4420,7 @@ static void intel_iommu_release_device(struct device *dev)
dmar_remove_one_dev_info(dev);
intel_pasid_free_table(dev);
+ intel_iommu_debugfs_remove_dev(info);
dev_iommu_priv_set(dev, NULL);
kfree(info);
set_dma_ops(dev, NULL);
@@ -4710,6 +4713,7 @@ static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
spin_unlock_irqrestore(&dmar_domain->lock, flags);
domain_detach_iommu(dmar_domain, iommu);
+ intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
kfree(dev_pasid);
out_tear_down:
intel_pasid_tear_down_entry(iommu, dev, pasid, false);
@@ -4762,6 +4766,9 @@ static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
list_add(&dev_pasid->link_domain, &dmar_domain->dev_pasids);
spin_unlock_irqrestore(&dmar_domain->lock, flags);
+ if (domain->type & __IOMMU_DOMAIN_PAGING)
+ intel_iommu_debugfs_create_dev_pasid(dev_pasid);
+
return 0;
out_detach_iommu:
domain_detach_iommu(dmar_domain, iommu);
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index 7dac94f62b4e..f9c1dd1ccff8 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -716,12 +716,18 @@ struct device_domain_info {
struct intel_iommu *iommu; /* IOMMU used by this device */
struct dmar_domain *domain; /* pointer to domain */
struct pasid_table *pasid_table; /* pasid table */
+#ifdef CONFIG_INTEL_IOMMU_DEBUGFS
+ struct dentry *debugfs_dentry; /* pointer to device directory dentry */
+#endif
};
struct dev_pasid_info {
struct list_head link_domain; /* link to domain siblings */
struct device *dev;
ioasid_t pasid;
+#ifdef CONFIG_INTEL_IOMMU_DEBUGFS
+ struct dentry *debugfs_dentry; /* pointer to pasid directory dentry */
+#endif
};
static inline void __iommu_flush_cache(
@@ -883,8 +889,16 @@ static inline void intel_svm_remove_dev_pasid(struct device *dev, ioasid_t pasid
#ifdef CONFIG_INTEL_IOMMU_DEBUGFS
void intel_iommu_debugfs_init(void);
+void intel_iommu_debugfs_create_dev(struct device_domain_info *info);
+void intel_iommu_debugfs_remove_dev(struct device_domain_info *info);
+void intel_iommu_debugfs_create_dev_pasid(struct dev_pasid_info *dev_pasid);
+void intel_iommu_debugfs_remove_dev_pasid(struct dev_pasid_info *dev_pasid);
#else
static inline void intel_iommu_debugfs_init(void) {}
+static inline void intel_iommu_debugfs_create_dev(struct device_domain_info *info) {}
+static inline void intel_iommu_debugfs_remove_dev(struct device_domain_info *info) {}
+static inline void intel_iommu_debugfs_create_dev_pasid(struct dev_pasid_info *dev_pasid) {}
+static inline void intel_iommu_debugfs_remove_dev_pasid(struct dev_pasid_info *dev_pasid) {}
#endif /* CONFIG_INTEL_IOMMU_DEBUGFS */
extern const struct attribute_group *intel_iommu_groups[];
--
2.21.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATH v5 2/3] iommu/vt-d: debugfs: Create/remove debugfs file per {device, pasid}
2023-10-13 13:58 ` [PATH v5 2/3] iommu/vt-d: debugfs: Create/remove debugfs file per {device, pasid} Jingqi Liu
@ 2024-11-12 21:22 ` Kees Bakker
2024-11-13 2:13 ` Baolu Lu
0 siblings, 1 reply; 10+ messages in thread
From: Kees Bakker @ 2024-11-12 21:22 UTC (permalink / raw)
To: Jingqi Liu, iommu, Lu Baolu, Tian Kevin, Joerg Roedel,
Will Deacon, Robin Murphy
Cc: linux-kernel
Op 13-10-2023 om 15:58 schreef Jingqi Liu:
> Add a debugfs directory per pair of {device, pasid} if the mappings of
> its page table are created and destroyed by the iommu_map/unmap()
> interfaces. i.e. /sys/kernel/debug/iommu/intel/<device source id>/<pasid>.
> Create a debugfs file in the directory for users to dump the page
> table corresponding to {device, pasid}. e.g.
> /sys/kernel/debug/iommu/intel/0000:00:02.0/1/domain_translation_struct.
> For the default domain without pasid, it creates a debugfs file in the
> debugfs device directory for users to dump its page table. e.g.
> /sys/kernel/debug/iommu/intel/0000:00:02.0/domain_translation_struct.
>
> When setting a domain to a PASID of device, create a debugfs file in
> the pasid debugfs directory for users to dump the page table of the
> specified pasid. Remove the debugfs device directory of the device
> when releasing a device. e.g.
> /sys/kernel/debug/iommu/intel/0000:00:01.0
>
> Signed-off-by: Jingqi Liu <Jingqi.liu@intel.com>
> ---
> drivers/iommu/intel/debugfs.c | 53 +++++++++++++++++++++++++++++++----
> drivers/iommu/intel/iommu.c | 7 +++++
> drivers/iommu/intel/iommu.h | 14 +++++++++
> 3 files changed, 69 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
> [...]
> +/* Remove the device pasid debugfs directory. */
> +void intel_iommu_debugfs_remove_dev_pasid(struct dev_pasid_info *dev_pasid)
> +{
> + debugfs_remove_recursive(dev_pasid->debugfs_dentry);
> +}
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> [...]
> @@ -4710,6 +4713,7 @@ static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
> spin_unlock_irqrestore(&dmar_domain->lock, flags);
>
> domain_detach_iommu(dmar_domain, iommu);
> + intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
> kfree(dev_pasid);
> out_tear_down:
> intel_pasid_tear_down_entry(iommu, dev, pasid, false);
>
So, a call to intel_iommu_debugfs_remove_dev_pasid() was added.
There is a potential problem that dev_pasid can be NULL.
The diff doesn't show the whole context so let me give that here.
Today that piece of the code looks like this
list_for_each_entry(curr, &dmar_domain->dev_pasids, link_domain) {
if (curr->dev == dev && curr->pasid == pasid) {
list_del(&curr->link_domain);
dev_pasid = curr;
break;
}
}
WARN_ON_ONCE(!dev_pasid);
spin_unlock_irqrestore(&dmar_domain->lock, flags);
cache_tag_unassign_domain(dmar_domain, dev, pasid);
domain_detach_iommu(dmar_domain, iommu);
intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
kfree(dev_pasid);
The for_each loop can exit without finding an entry.
The WARN_ON_ONCE also suggests that there can be a NULL.
After that the new function is called (see above) and it will
dereference the NULL pointer.
Can you have a closer look?
--
Kees Bakker
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATH v5 2/3] iommu/vt-d: debugfs: Create/remove debugfs file per {device, pasid}
2024-11-12 21:22 ` Kees Bakker
@ 2024-11-13 2:13 ` Baolu Lu
2024-11-13 20:35 ` Kees Bakker
0 siblings, 1 reply; 10+ messages in thread
From: Baolu Lu @ 2024-11-13 2:13 UTC (permalink / raw)
To: Kees Bakker, Jingqi Liu, iommu, Tian Kevin, Joerg Roedel,
Will Deacon, Robin Murphy
Cc: linux-kernel
On 11/13/24 05:22, Kees Bakker wrote:
> Op 13-10-2023 om 15:58 schreef Jingqi Liu:
>> Add a debugfs directory per pair of {device, pasid} if the mappings of
>> its page table are created and destroyed by the iommu_map/unmap()
>> interfaces. i.e. /sys/kernel/debug/iommu/intel/<device source id>/
>> <pasid>.
>> Create a debugfs file in the directory for users to dump the page
>> table corresponding to {device, pasid}. e.g.
>> /sys/kernel/debug/iommu/intel/0000:00:02.0/1/domain_translation_struct.
>> For the default domain without pasid, it creates a debugfs file in the
>> debugfs device directory for users to dump its page table. e.g.
>> /sys/kernel/debug/iommu/intel/0000:00:02.0/domain_translation_struct.
>>
>> When setting a domain to a PASID of device, create a debugfs file in
>> the pasid debugfs directory for users to dump the page table of the
>> specified pasid. Remove the debugfs device directory of the device
>> when releasing a device. e.g.
>> /sys/kernel/debug/iommu/intel/0000:00:01.0
>>
>> Signed-off-by: Jingqi Liu <Jingqi.liu@intel.com>
>> ---
>> drivers/iommu/intel/debugfs.c | 53 +++++++++++++++++++++++++++++++----
>> drivers/iommu/intel/iommu.c | 7 +++++
>> drivers/iommu/intel/iommu.h | 14 +++++++++
>> 3 files changed, 69 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/
>> debugfs.c
>> [...]
>> +/* Remove the device pasid debugfs directory. */
>> +void intel_iommu_debugfs_remove_dev_pasid(struct dev_pasid_info
>> *dev_pasid)
>> +{
>> + debugfs_remove_recursive(dev_pasid->debugfs_dentry);
>> +}
>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>> [...]
>> @@ -4710,6 +4713,7 @@ static void intel_iommu_remove_dev_pasid(struct
>> device *dev, ioasid_t pasid)
>> spin_unlock_irqrestore(&dmar_domain->lock, flags);
>> domain_detach_iommu(dmar_domain, iommu);
>> + intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
>> kfree(dev_pasid);
>> out_tear_down:
>> intel_pasid_tear_down_entry(iommu, dev, pasid, false);
>>
>
> So, a call to intel_iommu_debugfs_remove_dev_pasid() was added.
> There is a potential problem that dev_pasid can be NULL.
> The diff doesn't show the whole context so let me give that here.
> Today that piece of the code looks like this
>
> list_for_each_entry(curr, &dmar_domain->dev_pasids, link_domain) {
> if (curr->dev == dev && curr->pasid == pasid) {
> list_del(&curr->link_domain);
> dev_pasid = curr;
> break;
> }
> }
> WARN_ON_ONCE(!dev_pasid);
> spin_unlock_irqrestore(&dmar_domain->lock, flags);
>
> cache_tag_unassign_domain(dmar_domain, dev, pasid);
> domain_detach_iommu(dmar_domain, iommu);
> intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
> kfree(dev_pasid);
>
> The for_each loop can exit without finding an entry.
> The WARN_ON_ONCE also suggests that there can be a NULL.
> After that the new function is called (see above) and it will
> dereference the NULL pointer.
>
> Can you have a closer look?
It's already a kernel bug if dev_pasid is NULL when it reaches here. If
that happens, we should fix the bug, not avoid using the NULL pointer.
--
baolu
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATH v5 2/3] iommu/vt-d: debugfs: Create/remove debugfs file per {device, pasid}
2024-11-13 2:13 ` Baolu Lu
@ 2024-11-13 20:35 ` Kees Bakker
2024-11-14 1:29 ` Baolu Lu
0 siblings, 1 reply; 10+ messages in thread
From: Kees Bakker @ 2024-11-13 20:35 UTC (permalink / raw)
To: Baolu Lu, Jingqi Liu, iommu, Tian Kevin, Joerg Roedel,
Will Deacon, Robin Murphy
Cc: linux-kernel
Op 13-11-2024 om 03:13 schreef Baolu Lu:
> On 11/13/24 05:22, Kees Bakker wrote:
>> Op 13-10-2023 om 15:58 schreef Jingqi Liu:
>>> Add a debugfs directory per pair of {device, pasid} if the mappings of
>>> its page table are created and destroyed by the iommu_map/unmap()
>>> interfaces. i.e. /sys/kernel/debug/iommu/intel/<device source id>/
>>> <pasid>.
>>> Create a debugfs file in the directory for users to dump the page
>>> table corresponding to {device, pasid}. e.g.
>>> /sys/kernel/debug/iommu/intel/0000:00:02.0/1/domain_translation_struct.
>>> For the default domain without pasid, it creates a debugfs file in the
>>> debugfs device directory for users to dump its page table. e.g.
>>> /sys/kernel/debug/iommu/intel/0000:00:02.0/domain_translation_struct.
>>>
>>> When setting a domain to a PASID of device, create a debugfs file in
>>> the pasid debugfs directory for users to dump the page table of the
>>> specified pasid. Remove the debugfs device directory of the device
>>> when releasing a device. e.g.
>>> /sys/kernel/debug/iommu/intel/0000:00:01.0
>>>
>>> Signed-off-by: Jingqi Liu <Jingqi.liu@intel.com>
>>> ---
>>> drivers/iommu/intel/debugfs.c | 53
>>> +++++++++++++++++++++++++++++++----
>>> drivers/iommu/intel/iommu.c | 7 +++++
>>> drivers/iommu/intel/iommu.h | 14 +++++++++
>>> 3 files changed, 69 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/
>>> debugfs.c
>>> [...]
>>> +/* Remove the device pasid debugfs directory. */
>>> +void intel_iommu_debugfs_remove_dev_pasid(struct dev_pasid_info
>>> *dev_pasid)
>>> +{
>>> + debugfs_remove_recursive(dev_pasid->debugfs_dentry);
>>> +}
>>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>>> [...]
>>> @@ -4710,6 +4713,7 @@ static void
>>> intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
>>> spin_unlock_irqrestore(&dmar_domain->lock, flags);
>>> domain_detach_iommu(dmar_domain, iommu);
>>> + intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
>>> kfree(dev_pasid);
>>> out_tear_down:
>>> intel_pasid_tear_down_entry(iommu, dev, pasid, false);
>>>
>>
>> So, a call to intel_iommu_debugfs_remove_dev_pasid() was added.
>> There is a potential problem that dev_pasid can be NULL.
>> The diff doesn't show the whole context so let me give that here.
>> Today that piece of the code looks like this
>>
>> list_for_each_entry(curr, &dmar_domain->dev_pasids,
>> link_domain) {
>> if (curr->dev == dev && curr->pasid == pasid) {
>> list_del(&curr->link_domain);
>> dev_pasid = curr;
>> break;
>> }
>> }
>> WARN_ON_ONCE(!dev_pasid);
>> spin_unlock_irqrestore(&dmar_domain->lock, flags);
>>
>> cache_tag_unassign_domain(dmar_domain, dev, pasid);
>> domain_detach_iommu(dmar_domain, iommu);
>> intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
>> kfree(dev_pasid);
>>
>> The for_each loop can exit without finding an entry.
>> The WARN_ON_ONCE also suggests that there can be a NULL.
>> After that the new function is called (see above) and it will
>> dereference the NULL pointer.
>>
>> Can you have a closer look?
>
> It's already a kernel bug if dev_pasid is NULL when it reaches here. If
> that happens, we should fix the bug, not avoid using the NULL pointer.
How about moving the WARN_ON_ONCE down a bit and use its return value?
Like so
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 527f6f89d8a1..204873976ef3 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4096,13 +4096,14 @@ void domain_remove_dev_pasid(struct iommu_domain
*domain,
break;
}
}
- WARN_ON_ONCE(!dev_pasid);
spin_unlock_irqrestore(&dmar_domain->lock, flags);
cache_tag_unassign_domain(dmar_domain, dev, pasid);
domain_detach_iommu(dmar_domain, iommu);
- intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
- kfree(dev_pasid);
+ if (!WARN_ON_ONCE(!dev_pasid)) {
+ intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
+ kfree(dev_pasid);
+ }
}
static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t
pasid,
Would you accept a patch like that? Or do you want to change it yourself?
--
Kees
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATH v5 2/3] iommu/vt-d: debugfs: Create/remove debugfs file per {device, pasid}
2024-11-13 20:35 ` Kees Bakker
@ 2024-11-14 1:29 ` Baolu Lu
0 siblings, 0 replies; 10+ messages in thread
From: Baolu Lu @ 2024-11-14 1:29 UTC (permalink / raw)
To: Kees Bakker, Jingqi Liu, iommu, Tian Kevin, Joerg Roedel,
Will Deacon, Robin Murphy
Cc: linux-kernel
On 11/14/24 04:35, Kees Bakker wrote:
> Op 13-11-2024 om 03:13 schreef Baolu Lu:
>> On 11/13/24 05:22, Kees Bakker wrote:
>>> Op 13-10-2023 om 15:58 schreef Jingqi Liu:
>>>> Add a debugfs directory per pair of {device, pasid} if the mappings of
>>>> its page table are created and destroyed by the iommu_map/unmap()
>>>> interfaces. i.e. /sys/kernel/debug/iommu/intel/<device source id>/
>>>> <pasid>.
>>>> Create a debugfs file in the directory for users to dump the page
>>>> table corresponding to {device, pasid}. e.g.
>>>> /sys/kernel/debug/iommu/intel/0000:00:02.0/1/domain_translation_struct.
>>>> For the default domain without pasid, it creates a debugfs file in the
>>>> debugfs device directory for users to dump its page table. e.g.
>>>> /sys/kernel/debug/iommu/intel/0000:00:02.0/domain_translation_struct.
>>>>
>>>> When setting a domain to a PASID of device, create a debugfs file in
>>>> the pasid debugfs directory for users to dump the page table of the
>>>> specified pasid. Remove the debugfs device directory of the device
>>>> when releasing a device. e.g.
>>>> /sys/kernel/debug/iommu/intel/0000:00:01.0
>>>>
>>>> Signed-off-by: Jingqi Liu <Jingqi.liu@intel.com>
>>>> ---
>>>> drivers/iommu/intel/debugfs.c | 53 ++++++++++++++++++++++++++++++
>>>> +----
>>>> drivers/iommu/intel/iommu.c | 7 +++++
>>>> drivers/iommu/intel/iommu.h | 14 +++++++++
>>>> 3 files changed, 69 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/
>>>> debugfs.c
>>>> [...]
>>>> +/* Remove the device pasid debugfs directory. */
>>>> +void intel_iommu_debugfs_remove_dev_pasid(struct dev_pasid_info
>>>> *dev_pasid)
>>>> +{
>>>> + debugfs_remove_recursive(dev_pasid->debugfs_dentry);
>>>> +}
>>>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>>>> [...]
>>>> @@ -4710,6 +4713,7 @@ static void
>>>> intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
>>>> spin_unlock_irqrestore(&dmar_domain->lock, flags);
>>>> domain_detach_iommu(dmar_domain, iommu);
>>>> + intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
>>>> kfree(dev_pasid);
>>>> out_tear_down:
>>>> intel_pasid_tear_down_entry(iommu, dev, pasid, false);
>>>>
>>>
>>> So, a call to intel_iommu_debugfs_remove_dev_pasid() was added.
>>> There is a potential problem that dev_pasid can be NULL.
>>> The diff doesn't show the whole context so let me give that here.
>>> Today that piece of the code looks like this
>>>
>>> list_for_each_entry(curr, &dmar_domain->dev_pasids,
>>> link_domain) {
>>> if (curr->dev == dev && curr->pasid == pasid) {
>>> list_del(&curr->link_domain);
>>> dev_pasid = curr;
>>> break;
>>> }
>>> }
>>> WARN_ON_ONCE(!dev_pasid);
>>> spin_unlock_irqrestore(&dmar_domain->lock, flags);
>>>
>>> cache_tag_unassign_domain(dmar_domain, dev, pasid);
>>> domain_detach_iommu(dmar_domain, iommu);
>>> intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
>>> kfree(dev_pasid);
>>>
>>> The for_each loop can exit without finding an entry.
>>> The WARN_ON_ONCE also suggests that there can be a NULL.
>>> After that the new function is called (see above) and it will
>>> dereference the NULL pointer.
>>>
>>> Can you have a closer look?
>>
>> It's already a kernel bug if dev_pasid is NULL when it reaches here. If
>> that happens, we should fix the bug, not avoid using the NULL pointer.
>
> How about moving the WARN_ON_ONCE down a bit and use its return value?
> Like so
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 527f6f89d8a1..204873976ef3 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4096,13 +4096,14 @@ void domain_remove_dev_pasid(struct iommu_domain
> *domain,
> break;
> }
> }
> - WARN_ON_ONCE(!dev_pasid);
> spin_unlock_irqrestore(&dmar_domain->lock, flags);
>
> cache_tag_unassign_domain(dmar_domain, dev, pasid);
> domain_detach_iommu(dmar_domain, iommu);
> - intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
> - kfree(dev_pasid);
> + if (!WARN_ON_ONCE(!dev_pasid)) {
> + intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
> + kfree(dev_pasid);
> + }
> }
>
> static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t
> pasid,
>
>
> Would you accept a patch like that? Or do you want to change it yourself?
Yes. That looks better. Please post a patch. Thanks!
--
baolu
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATH v5 3/3] iommu/vt-d: debugfs: Support dumping a specified page table
2023-10-13 13:58 [PATH v5 0/3] iommu/vt-d: debugfs: Enhancements to IOMMU debugfs Jingqi Liu
2023-10-13 13:58 ` [PATH v5 1/3] iommu/vt-d: debugfs: Dump entry pointing to huge page Jingqi Liu
2023-10-13 13:58 ` [PATH v5 2/3] iommu/vt-d: debugfs: Create/remove debugfs file per {device, pasid} Jingqi Liu
@ 2023-10-13 13:58 ` Jingqi Liu
2023-10-16 3:04 ` Baolu Lu
2 siblings, 1 reply; 10+ messages in thread
From: Jingqi Liu @ 2023-10-13 13:58 UTC (permalink / raw)
To: iommu, Lu Baolu, Tian Kevin, Joerg Roedel, Will Deacon,
Robin Murphy
Cc: linux-kernel, Jingqi Liu
The original debugfs only dumps all page tables without pasid. With
pasid supported, the page table with pasid also needs to be dumped.
This patch supports dumping a specified page table in legacy mode or
scalable mode with or without a specified pasid.
For legacy mode, according to bus number and DEVFN, traverse the root
table and context table to get the pointer of page table in the
context table entry, then dump the specified page table.
For scalable mode, according to bus number, DEVFN and pasid, traverse
the root table, context table, pasid directory and pasid table to get
the pointer of page table in the pasid table entry, then dump the
specified page table..
Examples are as follows:
1) Dump the page table of device "0000:00:1f.0" that only supports
legacy mode.
$ sudo cat
/sys/kernel/debug/iommu/intel/0000:00:1f.0/domain_translation_struct
2) Dump the page table of device "0000:00:0a.0" with PASID "1" that
supports scalable mode.
$ sudo cat
/sys/kernel/debug/iommu/intel/0000:00:0a.0/1/domain_translation_struct
Suggested-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Jingqi Liu <Jingqi.liu@intel.com>
---
drivers/iommu/intel/debugfs.c | 154 ++++++++++++++++++++++++++--------
1 file changed, 120 insertions(+), 34 deletions(-)
diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
index 8a18a7be5215..239136727ac4 100644
--- a/drivers/iommu/intel/debugfs.c
+++ b/drivers/iommu/intel/debugfs.c
@@ -347,58 +347,141 @@ 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 domain_translation_struct_show(struct seq_file *m,
+ struct device_domain_info *info,
+ ioasid_t pasid)
{
- 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;
+ bool scalable, found = false;
+ struct dmar_drhd_unit *drhd;
+ struct intel_iommu *iommu;
+ u16 devfn, bus, seg;
- seq_printf(m, "Device %s @0x%llx\n", dev_name(dev),
- (u64)virt_to_phys(domain->pgd));
- seq_puts(m, "IOVA_PFN\t\tPML5E\t\t\tPML4E\t\t\tPDPE\t\t\tPDE\t\t\tPTE\n");
+ bus = info->bus;
+ devfn = info->devfn;
+ seg = info->segment;
- pgtable_walk_level(m, domain->pgd, domain->agaw + 2, 0, path);
- seq_putc(m, '\n');
+ rcu_read_lock();
+ for_each_active_iommu(iommu, drhd) {
+ struct context_entry *context;
+ u64 pgd, path[6] = { 0 };
+ u32 sts, agaw;
- /* Don't iterate */
- return 1;
-}
+ if (seg != iommu->segment)
+ continue;
-static int show_device_domain_translation(struct device *dev, void *data)
-{
- struct iommu_group *group;
+ sts = dmar_readl(iommu->reg + DMAR_GSTS_REG);
+ if (!(sts & DMA_GSTS_TES)) {
+ seq_printf(m, "DMA Remapping is not enabled on %s\n",
+ iommu->name);
+ continue;
+ }
+ if (dmar_readq(iommu->reg + DMAR_RTADDR_REG) & DMA_RTADDR_SMT)
+ scalable = true;
+ else
+ scalable = false;
- 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 iommu->lock is held across the callback, which will
+ * block calls to domain_attach/domain_detach. 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()
+ * Traversing page table 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);
+ spin_lock(&iommu->lock);
+
+ context = iommu_context_addr(iommu, bus, devfn, 0);
+ if (!context || !context_present(context))
+ goto iommu_unlock;
+
+ if (scalable) { /* scalable mode */
+ struct pasid_dir_entry *dir_tbl, *dir_entry;
+ struct pasid_entry *pasid_tbl, *pasid_tbl_entry;
+ u16 pasid_dir_size, dir_idx, tbl_idx, pgtt;
+ u64 pasid_dir_ptr;
+
+ pasid_dir_ptr = context->lo & VTD_PAGE_MASK;
+ pasid_dir_size = get_pasid_dir_size(context);
+
+ /* Dump specified device domain mappings with PASID. */
+ dir_idx = pasid >> PASID_PDE_SHIFT;
+ tbl_idx = pasid & PASID_PTE_MASK;
+
+ dir_tbl = phys_to_virt(pasid_dir_ptr);
+ dir_entry = &dir_tbl[dir_idx];
+
+ pasid_tbl = get_pasid_table_from_pde(dir_entry);
+ if (!pasid_tbl)
+ goto iommu_unlock;
+
+ pasid_tbl_entry = &pasid_tbl[tbl_idx];
+ if (!pasid_pte_is_present(pasid_tbl_entry))
+ goto iommu_unlock;
+
+ /*
+ * According to PASID Granular Translation Type(PGTT),
+ * get the page table pointer.
+ */
+ pgtt = (u16)(pasid_tbl_entry->val[0] & GENMASK_ULL(8, 6)) >> 6;
+ agaw = (u8)(pasid_tbl_entry->val[0] & GENMASK_ULL(4, 2)) >> 2;
+
+ switch (pgtt) {
+ case PASID_ENTRY_PGTT_FL_ONLY:
+ pgd = pasid_tbl_entry->val[2];
+ break;
+ case PASID_ENTRY_PGTT_SL_ONLY:
+ case PASID_ENTRY_PGTT_NESTED:
+ pgd = pasid_tbl_entry->val[0];
+ break;
+ default:
+ goto iommu_unlock;
+ }
+ pgd &= VTD_PAGE_MASK;
+ } else { /* legacy mode */
+ pgd = context->lo & VTD_PAGE_MASK;
+ agaw = context->hi & 7;
+ }
+
+ seq_printf(m, "Device %04x:%02x:%02x.%x ",
+ iommu->segment, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+
+ if (scalable)
+ seq_printf(m, "with pasid %x @0x%llx\n", pasid, pgd);
+ else
+ seq_printf(m, "@0x%llx\n", pgd);
+
+ seq_printf(m, "%-17s\t%-18s\t%-18s\t%-18s\t%-18s\t%-s\n",
+ "IOVA_PFN", "PML5E", "PML4E", "PDPE", "PDE", "PTE");
+ pgtable_walk_level(m, phys_to_virt(pgd), agaw + 2, 0, path);
+
+ found = true;
+iommu_unlock:
+ spin_unlock(&iommu->lock);
+ if (found)
+ break;
}
+ rcu_read_unlock();
return 0;
}
-static int domain_translation_struct_show(struct seq_file *m, void *unused)
+static int dev_domain_translation_struct_show(struct seq_file *m, void *unused)
{
- return bus_for_each_dev(&pci_bus_type, NULL, m,
- show_device_domain_translation);
+ struct device_domain_info *info = (struct device_domain_info *)m->private;
+
+ return domain_translation_struct_show(m, info, IOMMU_NO_PASID);
}
-DEFINE_SHOW_ATTRIBUTE(domain_translation_struct);
+DEFINE_SHOW_ATTRIBUTE(dev_domain_translation_struct);
+
+static int pasid_domain_translation_struct_show(struct seq_file *m, void *unused)
+{
+ struct dev_pasid_info *dev_pasid = (struct dev_pasid_info *)m->private;
+ struct device_domain_info *info = dev_iommu_priv_get(dev_pasid->dev);
+
+ return domain_translation_struct_show(m, info, dev_pasid->pasid);
+}
+DEFINE_SHOW_ATTRIBUTE(pasid_domain_translation_struct);
static void invalidation_queue_entry_show(struct seq_file *m,
struct intel_iommu *iommu)
@@ -700,7 +783,7 @@ void intel_iommu_debugfs_create_dev(struct device_domain_info *info)
info->debugfs_dentry = debugfs_create_dir(dev_name(info->dev), intel_iommu_debug);
debugfs_create_file("domain_translation_struct", 0444, info->debugfs_dentry,
- NULL, &domain_translation_struct_fops);
+ info, &dev_domain_translation_struct_fops);
}
/* Remove the device debugfs directory. */
@@ -726,6 +809,9 @@ void intel_iommu_debugfs_create_dev_pasid(struct dev_pasid_info *dev_pasid)
sprintf(dir_name, "%x", dev_pasid->pasid);
dev_pasid->debugfs_dentry = debugfs_create_dir(dir_name, info->debugfs_dentry);
+
+ debugfs_create_file("domain_translation_struct", 0444, dev_pasid->debugfs_dentry,
+ dev_pasid, &pasid_domain_translation_struct_fops);
}
/* Remove the device pasid debugfs directory. */
--
2.21.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATH v5 3/3] iommu/vt-d: debugfs: Support dumping a specified page table
2023-10-13 13:58 ` [PATH v5 3/3] iommu/vt-d: debugfs: Support dumping a specified page table Jingqi Liu
@ 2023-10-16 3:04 ` Baolu Lu
2023-10-17 3:02 ` Liu, Jingqi
0 siblings, 1 reply; 10+ messages in thread
From: Baolu Lu @ 2023-10-16 3:04 UTC (permalink / raw)
To: Jingqi Liu, iommu, Tian Kevin, Joerg Roedel, Will Deacon,
Robin Murphy
Cc: baolu.lu, linux-kernel
On 10/13/23 9:58 PM, Jingqi Liu wrote:
> The original debugfs only dumps all page tables without pasid. With
> pasid supported, the page table with pasid also needs to be dumped.
>
> This patch supports dumping a specified page table in legacy mode or
> scalable mode with or without a specified pasid.
>
> For legacy mode, according to bus number and DEVFN, traverse the root
> table and context table to get the pointer of page table in the
> context table entry, then dump the specified page table.
>
> For scalable mode, according to bus number, DEVFN and pasid, traverse
> the root table, context table, pasid directory and pasid table to get
> the pointer of page table in the pasid table entry, then dump the
> specified page table..
>
> Examples are as follows:
> 1) Dump the page table of device "0000:00:1f.0" that only supports
> legacy mode.
> $ sudo cat
> /sys/kernel/debug/iommu/intel/0000:00:1f.0/domain_translation_struct
>
> 2) Dump the page table of device "0000:00:0a.0" with PASID "1" that
> supports scalable mode.
> $ sudo cat
> /sys/kernel/debug/iommu/intel/0000:00:0a.0/1/domain_translation_struct
>
> Suggested-by: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Jingqi Liu <Jingqi.liu@intel.com>
> ---
> drivers/iommu/intel/debugfs.c | 154 ++++++++++++++++++++++++++--------
> 1 file changed, 120 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
> index 8a18a7be5215..239136727ac4 100644
> --- a/drivers/iommu/intel/debugfs.c
> +++ b/drivers/iommu/intel/debugfs.c
> @@ -347,58 +347,141 @@ 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 domain_translation_struct_show(struct seq_file *m,
> + struct device_domain_info *info,
> + ioasid_t pasid)
> {
> - 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;
> + bool scalable, found = false;
> + struct dmar_drhd_unit *drhd;
> + struct intel_iommu *iommu;
> + u16 devfn, bus, seg;
>
> - seq_printf(m, "Device %s @0x%llx\n", dev_name(dev),
> - (u64)virt_to_phys(domain->pgd));
> - seq_puts(m, "IOVA_PFN\t\tPML5E\t\t\tPML4E\t\t\tPDPE\t\t\tPDE\t\t\tPTE\n");
> + bus = info->bus;
> + devfn = info->devfn;
> + seg = info->segment;
>
> - pgtable_walk_level(m, domain->pgd, domain->agaw + 2, 0, path);
> - seq_putc(m, '\n');
> + rcu_read_lock();
> + for_each_active_iommu(iommu, drhd) {
> + struct context_entry *context;
> + u64 pgd, path[6] = { 0 };
> + u32 sts, agaw;
>
> - /* Don't iterate */
> - return 1;
> -}
> + if (seg != iommu->segment)
> + continue;
>
> -static int show_device_domain_translation(struct device *dev, void *data)
> -{
> - struct iommu_group *group;
> + sts = dmar_readl(iommu->reg + DMAR_GSTS_REG);
> + if (!(sts & DMA_GSTS_TES)) {
> + seq_printf(m, "DMA Remapping is not enabled on %s\n",
> + iommu->name);
> + continue;
> + }
> + if (dmar_readq(iommu->reg + DMAR_RTADDR_REG) & DMA_RTADDR_SMT)
> + scalable = true;
> + else
> + scalable = false;
>
> - 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 iommu->lock is held across the callback, which will
> + * block calls to domain_attach/domain_detach. 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()
> + * Traversing page table 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);
> + spin_lock(&iommu->lock);
> +
> + context = iommu_context_addr(iommu, bus, devfn, 0);
> + if (!context || !context_present(context))
> + goto iommu_unlock;
> +
> + if (scalable) { /* scalable mode */
> + struct pasid_dir_entry *dir_tbl, *dir_entry;
> + struct pasid_entry *pasid_tbl, *pasid_tbl_entry;
> + u16 pasid_dir_size, dir_idx, tbl_idx, pgtt;
> + u64 pasid_dir_ptr;
0day robot complained:
drivers/iommu/intel/debugfs.c: In function
'domain_translation_struct_show':
>> drivers/iommu/intel/debugfs.c:401:29: warning: variable
'pasid_dir_size' set but not used [-Wunused-but-set-variable]
401 | u16 pasid_dir_size, dir_idx,
tbl_idx, pgtt;
| ^~~~~~~~~~~~~~
I have removed pasid_dir_size and queue the whole series for v6.7.
Thank you!
Best regards,
baolu
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATH v5 3/3] iommu/vt-d: debugfs: Support dumping a specified page table
2023-10-16 3:04 ` Baolu Lu
@ 2023-10-17 3:02 ` Liu, Jingqi
0 siblings, 0 replies; 10+ messages in thread
From: Liu, Jingqi @ 2023-10-17 3:02 UTC (permalink / raw)
To: Baolu Lu, iommu, Tian Kevin, Joerg Roedel, Will Deacon,
Robin Murphy
Cc: linux-kernel
On 10/16/2023 11:04 AM, Baolu Lu wrote:
> On 10/13/23 9:58 PM, Jingqi Liu wrote:
>>
......
>> + if (scalable) { /* scalable mode */
>> + struct pasid_dir_entry *dir_tbl, *dir_entry;
>> + struct pasid_entry *pasid_tbl, *pasid_tbl_entry;
>> + u16 pasid_dir_size, dir_idx, tbl_idx, pgtt;
>> + u64 pasid_dir_ptr;
>
> 0day robot complained:
>
> drivers/iommu/intel/debugfs.c: In function
> 'domain_translation_struct_show':
> >> drivers/iommu/intel/debugfs.c:401:29: warning: variable
> 'pasid_dir_size' set but not used [-Wunused-but-set-variable]
> 401 | u16 pasid_dir_size, dir_idx,
> tbl_idx, pgtt;
> | ^~~~~~~~~~~~~~
>
> I have removed pasid_dir_size and queue the whole series for v6.7.
>
Thanks Baolu.
Indeed. It's an unused variable.
BR,
Jingqi
> Thank you!
>
> Best regards,
> baolu
^ permalink raw reply [flat|nested] 10+ messages in thread