Re: [PATCH v2 3/8] iommu/amd: Add debugfs support to dump IOMMU Capability registers
From: Srivastava, Dheeraj Kumar
Date: Wed Jan 15 2025 - 00:06:53 EST
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