IOMMU Capability registers defines capabilities of IOMMU and information
needed for initialising MMIO registers and device table. This is useful
to dump these registers for debugging IOMMU related issues.
e.g.To get capability registers value for iommu<x>
# echo "0x10" > /sys/kernel/debug/iommu/amd/iommu00/capability
# cat /sys/kernel/debug/iommu/amd/iommu00/capability_dump
Signed-off-by: Dheeraj Kumar Srivastava <dheerajkumar.srivastava@amd.com>
---
drivers/iommu/amd/debugfs.c | 60 +++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/drivers/iommu/amd/debugfs.c b/drivers/iommu/amd/debugfs.c
index e56c050eb7c8..25a4e738d067 100644
--- a/drivers/iommu/amd/debugfs.c
+++ b/drivers/iommu/amd/debugfs.c
@@ -18,6 +18,7 @@ static struct dentry *amd_iommu_debugfs;
#define OFS_IN_SZ 8
static int mmio_offset = -1;
+static int cap_offset = -1;
static ssize_t iommu_mmio_write(struct file *filp, const char __user *ubuf,
size_t cnt, loff_t *ppos)
@@ -69,6 +70,61 @@ static int iommu_mmio_dump_show(struct seq_file *m, void *unused)
}
DEFINE_SHOW_ATTRIBUTE(iommu_mmio_dump);
+static ssize_t iommu_capability_write(struct file *filp, const char __user *ubuf,
+ size_t cnt, loff_t *ppos)
+{
+ int ret;
+
+ if (cnt > OFS_IN_SZ)
+ return -EINVAL;
+
+ ret = kstrtou32_from_user(ubuf, cnt, 0, &cap_offset);
+ if (ret)
+ return ret;
+
+ /* Capability register at offset 0x14 is the last IOMMU capability register. */
+ if (cap_offset > 0x14) {
+ cap_offset = -1;
+ return -EINVAL;
+ }
+
+ return cnt;
+}
+
+static int iommu_capability_show(struct seq_file *m, void *unused)
+{
+ if (cap_offset >= 0)
+ seq_printf(m, "0x%x\n", cap_offset);
+ else
+ seq_puts(m, "No or invalid input provided\n");
+
+ return 0;
+}
+DEFINE_SHOW_STORE_ATTRIBUTE(iommu_capability);
+
+static int iommu_capability_dump_show(struct seq_file *m, void *unused)
+{
+ struct amd_iommu *iommu = m->private;
+ u32 value;
+ int err;
+
+ if (cap_offset < 0) {
+ seq_puts(m, "Please provide capability register's offset\n");
+ return 0;
+ }
+
+ err = pci_read_config_dword(iommu->dev, iommu->cap_ptr + cap_offset, &value);
+ if (err) {
+ seq_printf(m, "Not able to read capability register at 0x%x\n", cap_offset);
+ return 0;
+ }
+
+ seq_printf(m, "0x%08x\n", value);
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(iommu_capability_dump);
+
void amd_iommu_debugfs_setup(void)
{
struct amd_iommu *iommu;
@@ -84,5 +140,9 @@ void amd_iommu_debugfs_setup(void)
&iommu_mmio_fops);
debugfs_create_file("mmio_dump", 0444, iommu->debugfs, iommu,
&iommu_mmio_dump_fops);
+ debugfs_create_file("capability", 0644, iommu->debugfs, iommu,
+ &iommu_capability_fops);
+ debugfs_create_file("capability_dump", 0444, iommu->debugfs,
+ iommu, &iommu_capability_dump_fops);
}
}
--
2.25.1On Wed, Nov 06, 2024 at 01:16:34PM +0530, Dheeraj Kumar Srivastava wrote: > IOMMU Capability registers defines capabilities of IOMMU and information > needed for initialising MMIO registers and device table. This is useful > to dump these registers for debugging IOMMU related issues. > > e.g.To get capability registers value for iommu<x> > # echo "0x10" > /sys/kernel/debug/iommu/amd/iommu00/capability > # cat /sys/kernel/debug/iommu/amd/iommu00/capability_dump Same comment here. Why does this need to be so complicated to use? Can't you make a single read-only file that contains all the registers of interest? Bjorn
Hi, On 11/27/2024 2:29 AM, Bjorn Helgaas wrote: > On Wed, Nov 06, 2024 at 01:16:34PM +0530, Dheeraj Kumar Srivastava wrote: >> IOMMU Capability registers defines capabilities of IOMMU and information >> needed for initialising MMIO registers and device table. This is useful >> to dump these registers for debugging IOMMU related issues. >> >> e.g.To get capability registers value for iommu<x> >> # echo "0x10" > /sys/kernel/debug/iommu/amd/iommu00/capability >> # cat /sys/kernel/debug/iommu/amd/iommu00/capability_dump > > Same comment here. Why does this need to be so complicated to use? > Can't you make a single read-only file that contains all the registers > of interest? > Please do let me know your concerns and views on my comments in the previous patch. With the implemented approach we do need separate files for mmio registers and capability registers input/output files as to understand if user input is mmio's offset or capability register's offset. Thanks Dheeraj > Bjorn
On Tue, Jan 07, 2025 at 11:33:16AM +0530, Srivastava, Dheeraj Kumar wrote: > On 11/27/2024 2:29 AM, Bjorn Helgaas wrote: > > On Wed, Nov 06, 2024 at 01:16:34PM +0530, Dheeraj Kumar Srivastava wrote: > > > IOMMU Capability registers defines capabilities of IOMMU and information > > > needed for initialising MMIO registers and device table. This is useful > > > to dump these registers for debugging IOMMU related issues. > > > > > > e.g.To get capability registers value for iommu<x> > > > # echo "0x10" > /sys/kernel/debug/iommu/amd/iommu00/capability > > > # cat /sys/kernel/debug/iommu/amd/iommu00/capability_dump > > > > Same comment here. Why does this need to be so complicated to use? > > Can't you make a single read-only file that contains all the registers > > of interest? > > Please do let me know your concerns and views on my comments in the > previous patch. > > With the implemented approach we do need separate files for mmio > registers and capability registers input/output files as to > understand if user input is mmio's offset or capability register's > offset. My comment is not about the difference between MMIO and config space registers. My concern is using two separate files to read the same register. That's inherently racy: UserA# echo "0x10" > /sys/kernel/debug/iommu/amd/iommu00/capability UserB# echo "0x20" > /sys/kernel/debug/iommu/amd/iommu00/capability UserA# cat /sys/kernel/debug/iommu/amd/iommu00/capability_dump UserA expected to see the register at 0x10, but sees the one at 0x20 instead. I think there's value in using a strategy similar to other IOMMU drivers, e.g., intel. But I'm not an IOMMU maintainer, so I'm just kibbitzing here, and maybe your strategy is better. Bjorn
Hi, On 1/8/2025 1:48 AM, Bjorn Helgaas wrote: > On Tue, Jan 07, 2025 at 11:33:16AM +0530, Srivastava, Dheeraj Kumar wrote: >> On 11/27/2024 2:29 AM, Bjorn Helgaas wrote: >>> On Wed, Nov 06, 2024 at 01:16:34PM +0530, Dheeraj Kumar Srivastava wrote: >>>> IOMMU Capability registers defines capabilities of IOMMU and information >>>> needed for initialising MMIO registers and device table. This is useful >>>> to dump these registers for debugging IOMMU related issues. >>>> >>>> e.g.To get capability registers value for iommu<x> >>>> # echo "0x10" > /sys/kernel/debug/iommu/amd/iommu00/capability >>>> # cat /sys/kernel/debug/iommu/amd/iommu00/capability_dump >>> >>> Same comment here. Why does this need to be so complicated to use? >>> Can't you make a single read-only file that contains all the registers >>> of interest? >> >> Please do let me know your concerns and views on my comments in the >> previous patch. >> >> With the implemented approach we do need separate files for mmio >> registers and capability registers input/output files as to >> understand if user input is mmio's offset or capability register's >> offset. > > My comment is not about the difference between MMIO and config space > registers. My concern is using two separate files to read the same > register. That's inherently racy: > > UserA# echo "0x10" > /sys/kernel/debug/iommu/amd/iommu00/capability > UserB# echo "0x20" > /sys/kernel/debug/iommu/amd/iommu00/capability > UserA# cat /sys/kernel/debug/iommu/amd/iommu00/capability_dump > > UserA expected to see the register at 0x10, but sees the one at 0x20 > instead. > > I think there's value in using a strategy similar to other IOMMU > drivers, e.g., intel. But I'm not an IOMMU maintainer, so I'm just > kibbitzing here, and maybe your strategy is better. Thanks for clarifying. This makes sense. Will update in the next series. Regards Dheeraj > > Bjorn
© 2016 - 2026 Red Hat, Inc.