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

Message-Id: <DDHGOCNZJRND.129VXJYMXMCZW@kernel.org>
Date: Mon, 13 Oct 2025 22:02:55 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Zhi Wang" <zhiw@...dia.com>
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 Oct 13, 2025 at 8:25 PM CEST, Zhi Wang wrote:
> 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.

For this you need to adjust the register macro to take something that
dereferences to `T: Io` instead of something that dereferences to `Io`.

This change should be trivial.

> 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.

Yes, you can do it in a separete series. But I'd also be fine if you do both in
a single one. The required code changes shouldn't be crazy.

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

Part 3 has to happen with part 1 anyways, otherwise it breaks compilation.

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

Both can be infallible. The standard configuration space size is constant, hence
all accesses to the standard configuration space with constant offsets can be
infallible.

For the extended space it depends what a driver can assert, just like for any
MMIO space.

However, you seem to talk about whether a physical device is still present.

> 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...

Panics are for the "the machine is unrecoverably dead" case, this clearly isn't
one of them. :)

I think we should do the same as with "normal" MMIO and just return the value,
i.e. all bits set (PCI_ERROR_RESPONSE).

The window between physical unplug and the driver core unbinds the driver should
be pretty small and drivers have to be able to deal with garbage values read
from registers anyways.

If we really want to handle it, you can only implement the try_*() methods and
for the non-try_*() methods throw a compile time error, but I don't see a reason
for that.