Re: [RFC PATCH 0/3] iommu/arm-smmu-v3: Add debug interfaces for SMMUv3

From: Will Deacon <will@kernel.org>
To: Zhou Wang <wangzhou1@hisilicon.com>
Cc: Rob Herring <robh@kernel.org>,
	iommu@lists.linux-foundation.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH 0/3] iommu/arm-smmu-v3: Add debug interfaces for SMMUv3
Date: Wed, 3 Feb 2021 21:38:51 +0000	[thread overview]
Message-ID: <20210203213851.GA19847@willie-the-truck> (raw)
In-Reply-To: <5001d8b3-ed2c-f3e3-80c5-d0b6b5df634c@hisilicon.com>

On Wed, Feb 03, 2021 at 11:15:18AM +0800, Zhou Wang wrote:
> On 2021/1/29 17:06, Zhou Wang wrote:
> > This RFC series is the followed patch of this discussion:
> > https://www.spinics.net/lists/arm-kernel/msg866187.html. 
> > 
> > Currently there is no debug interface about SMMUv3 driver, which makes it
> > not convenient when we want to dump some information, like the value of
> > CD/STE, S1/S2 page table, SMMU registers or cmd/event/pri queues.
> > 
> > This series tries to add support of dumping CD/STE and page table. The
> > interface design is that user sets device/pasid firstly by sysfs files
> > and then read related sysfs file to get information:
> > 
> >  (currently only support PCI device)
> >  echo <domain>:<bus>:<dev>.<fun> > /sys/kernel/debug/iommu/smmuv3/pci_dev
> >  echo <pasid> > /sys/kernel/debug/iommu/smmuv3/pasid
> >  
> >  Then value in CD and STE can be got by:
> >  cat /sys/kernel/debug/iommu/smmuv3/ste
> >  cat /sys/kernel/debug/iommu/smmuv3/cd
> >  
> >  S1 and S2 page tables can be got by:
> >  cat /sys/kernel/debug/iommu/smmuv3/pt_dump_s1
> >  cat /sys/kernel/debug/iommu/smmuv3/pt_dump_s2
> > 
> > For STE, CD and page table, related device and pasid are set in pci_dev
> > and pasid files as above.
> > 
> > First and second patch export some help functions or macros in arm-smmu-v3
> > and io-pgtable-arm codes, so we can reuse them in debugfs.c. As a RFC, this
> > series does not go further to dump SMMU registers and cmd/event/pri queues.
> > I am not sure this series is in the right way, so let's post it out and have a
> > discussion. Looking forward to any feedback.
> > 
> > Zhou Wang (3):
> >   iommu/arm-smmu-v3: Export cd/ste get functions
> >   iommu/io-pgtable: Export page table walk needed functions and macros
> >   iommu/arm-smmu-v3: Add debug interfaces for SMMUv3
> > 
> >  drivers/iommu/Kconfig                       |  11 +
> >  drivers/iommu/arm/arm-smmu-v3/Makefile      |   1 +
> >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  10 +-
> >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  10 +
> >  drivers/iommu/arm/arm-smmu-v3/debugfs.c     | 398 ++++++++++++++++++++++++++++
> >  drivers/iommu/io-pgtable-arm.c              |  47 +---
> >  drivers/iommu/io-pgtable-arm.h              |  43 +++
> >  7 files changed, 475 insertions(+), 45 deletions(-)
> >  create mode 100644 drivers/iommu/arm/arm-smmu-v3/debugfs.c
> > 
> 
> Any comments about this series?

Truthfully, I don't really see the use in dumping the state of the SMMU
data structures. They're not especially dynamic, and there are higher level
ways to determine how devices map to groups etc.

However, I can see some utility in dumping the page-tables. We have that
functionality for the CPU side via /sys/kernel/debug/kernel_page_tables,
so something similar in the io-pgtable code could be quite neat. In
particular, the logic to expose things in debugfs and drive the dumping
could be agnostic of the page-table format, while the formats themselves
coule implement optional callback(s) to return the data.

Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu