linux-kernel - Re: [RFC 0/6] rust: pci: add config space read/write support

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: <20251013212518.555a19ad.zhiw@nvidia.com>
Date: Mon, 13 Oct 2025 21:25:18 +0300
From: Zhi Wang <zhiw@...dia.com>
To: Danilo Krummrich <dakr@...nel.org>
CC: <rust-for-linux@...r.kernel.org>, <bhelgaas@...gle.com>,
	<kwilczynski@...nel.org>, <ojeda@...nel.org>, <alex.gaynor@...il.com>,
	<boqun.feng@...il.com>, <gary@...yguo.net>, <bjorn3_gh@...tonmail.com>,
	<lossin@...nel.org>, <a.hindborg@...nel.org>, <aliceryhl@...gle.com>,
	<tmgross@...ch.edu>, <linux-pci@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <cjia@...dia.com>, <smitra@...dia.com>,
	<ankita@...dia.com>, <aniketa@...dia.com>, <kwankhede@...dia.com>,
	<targupta@...dia.com>, <zhiwang@...nel.org>, <acourbot@...dia.com>,
	<joelagnelf@...dia.com>, <jhubbard@...dia.com>, <markus.probst@...teo.de>
Subject: Re: [RFC 0/6] rust: pci: add config space read/write support

On Mon, 13 Oct 2025 17:39:41 +0200
"Danilo Krummrich" <dakr@...nel.org> wrote:

> Hi Zhi,
> 
> (Cc: Alex, Joel, John, Markus)
> 
> On Fri Oct 10, 2025 at 10:03 AM CEST, Zhi Wang wrote:
> > This ideas of this series are:
> >
> > - Factor out a common trait IoRegion for other accessors to share
> > the same compiling/runtime check like before.  
> 
> Yes, this is something we want to have in general:
> 
> Currently, we have a single I/O backend (struct Io) which is used for
> generic MMIO. However, we should make Io a trait instead and require
> a new MMIO type to implement the trait, where the trait methods would
> remain to be {try_}{read,write}{8,16,..}().
> 

I was considering the same when writing this series. The concern is
mostly about having to change the drivers' MMIO code to adapt to the
re-factor.

IMHO, if we are seeing the necessity of this re-factor, we should do it
before it got more usage. This could be the part 1 of the next spin.

and adding pci::Device<Bound>::config_space() could be part 2 and
register! marco could be part 3.

ditto

> 
> > The current kernel::Io MMIO read/write doesn't return a failure,
> > because {read, write}{b, w, l}() are always successful. This is not
> > true in pci_{read, write}_config{byte, word, dword}() because a PCI
> > device can be disconnected from the bus. Thus a failure is
> > returned.  
> 
> This is in fact also true for the PCI configuration space. The PCI
> configuration space has a minimum size that is known at compile time.
> All registers within this minimum size can be access in an infallible
> way with the non try_*() methods.
> 
> The main idea behind the fallible and infallible accessors is that
> you can assert a minimum expected size of an I/O backend (e.g. a PCI
> bar). I.e. drivers know their minimum requirements of the size of the
> I/O region. If the I/O backend can fulfill the request we can be sure
> about the minimum size and hence accesses with offsets that are known
> at compile time can be infallible (because we know the minimum
> accepted size of the I/O backend at compile time as well).
> 

For PCI configuration space. Standard configuration space should be
readable and to access the extended configuration space, the MCFG
should be enabled beforehand and the enabling is system-wide.

I think the size of standard configuration space falls in "falliable
accessors", and the extended configuration space falls in "infalliable"
parts

But for the "infallible" part in PCI configuration space, the device
can be disconnected from the PCI bus. E.g. unresponsive device. In that
case, the current PCI core will mark the device as "disconnected" before
they causes more problems and any access to the configuration space
will fail with an error code. This can also happen on access to
"infalliable" part.

How should we handle this case in "infallible" accessors of PCI
configuration space? Returning Result<> seems doesn't fit the concept
of "infallible", but causing a rust panic seems overkill...

Z.