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] [thread-next>] [day] [month] [year] [list]

Message-ID: <abe84488-9ad2-4ff4-9499-7d5c3b361cf5@amd.com>
Date: Wed, 15 Jan 2025 10:36:32 +0530
From: "Srivastava, Dheeraj Kumar" <dheerajkumar.srivastava@....com>
To: Bjorn Helgaas <helgaas@...nel.org>
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

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