linux-kernel - Re: [PATCH v2 3/8] iommu/amd: Add debugfs support to dump IOMMU Capability registers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250107201847.GA179670@bhelgaas>
Date: Tue, 7 Jan 2025 14:18:47 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: "Srivastava, Dheeraj Kumar" <dheerajkumar.srivastava@....com>
Cc: joro@...tes.org, suravee.suthikulpanit@....com, will@...nel.org,
	robin.murphy@....com, linux-kernel@...r.kernel.org,
	iommu@...ts.linux.dev, vasant.hegde@....com
Subject: Re: [PATCH v2 3/8] iommu/amd: Add debugfs support to dump IOMMU
 Capability registers

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