linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "John Hubbard" <jhubbard@nvidia.com>
Cc: "Alice Ryhl" <aliceryhl@google.com>, "Zhi Wang" <zhiw@nvidia.com>,
	<rust-for-linux@vger.kernel.org>, <linux-pci@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <bhelgaas@google.com>,
	<kwilczynski@kernel.org>, <ojeda@kernel.org>,
	<alex.gaynor@gmail.com>, <boqun.feng@gmail.com>,
	<gary@garyguo.net>, <bjorn3_gh@protonmail.com>,
	<lossin@kernel.org>, <a.hindborg@kernel.org>, <tmgross@umich.edu>,
	<markus.probst@posteo.de>, <helgaas@kernel.org>,
	<cjia@nvidia.com>, <smitra@nvidia.com>, <ankita@nvidia.com>,
	<aniketa@nvidia.com>, <kwankhede@nvidia.com>,
	<targupta@nvidia.com>, <acourbot@nvidia.com>,
	<joelagnelf@nvidia.com>, <zhiwang@kernel.org>
Subject: Re: [PATCH v6 RESEND 4/7] rust: io: factor common I/O helpers into Io trait
Date: Wed, 19 Nov 2025 10:18:27 +1300	[thread overview]
Message-ID: <DEC4TSQBTESW.28F17X9GHCIU7@kernel.org> (raw)
In-Reply-To: <7b30a8a5-ec0b-4cc6-9e9a-2ff2b42ca3cf@nvidia.com>

On Tue Nov 18, 2025 at 11:44 AM NZDT, John Hubbard wrote:
> IO is generally something that can fail, so this whole idea of infallible
> IO is making me uneasy.
>
> I understand that we're trying to wrap it up into a bound device, but
> bound devices are all about whether or not the driver lifetime is OK,
> not so much about IO.

That is correct, device context states are about driver lifetime. However, it is
at least related, see below.

> For PCIe, it is still possible for the device to fall off of the bus, and 
> in that case you'll usually see 0xFFFF_FFFF returned from PCIe reads. The
> Open RM driver has sprinkled around checks for this value (not fun, I
> know), and Danilo hinted elsewhere that bound-ness requires not getting
> these, so maybe that suffices. But it means that Rust will be "interesting"
> here, because falling off the bus means that there will be a time window in
> which the IO is, in fact, fallible.

The PCI configuration space accessors indeed check a flag that is set when the
device falls off the bus. However, it is not sufficient, since you still have a
period of time when the device fell off the bus where the flag isn't set yet and
the I/O accessor may still be used concurrently.

(If you look at C drivers you will note that almost none of the drivers actually
check the return value of the configuration space accessors; needless to say
MMIO ones don't even have the flag.)

Because of that, there is not a point in making all the I/O accessors fallible,
because you'd have to deal with false negatives anyways, i.e. check the read
value for plausibility, because the device could already be gone, while the flag
is not set yet.

Additionally, when the device fell off the bus the driver core will unbind the
driver, so the period where fallability would serve at least some purpose would
be very short anyways.

Instead, drivers have to be designed to be robust enough to deal with broken
data read from the bus.

> Other IO subsystems can also get IO errors, too.
>
> I wonder if we should just provide IoFallible? (It could check for the
> 0xFFFF_FFFF case, for example, which is helpful to simplify the caller.)

For some registers this could be an expected value, plus a device can fall off
the bus during a read was well, leaving you with broken data.

I don't think trying to make all I/O operations fallible is the way to go, it's
just unreliable to detect in the generic layer. Instead, drivers should perform
a plausibility check on the read values (which they have to do anyways).

  reply	other threads:[~2025-11-18 21:18 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-10 20:41 [PATCH v6 RESEND 0/7] rust: pci: add config space read/write support Zhi Wang
2025-11-10 20:41 ` [PATCH v6 RESEND 1/7] samples: rust: rust_driver_pci: use "kernel vertical" style for imports Zhi Wang
2025-11-10 20:41 ` [PATCH v6 RESEND 2/7] rust: devres: " Zhi Wang
2025-11-10 20:41 ` [PATCH v6 RESEND 3/7] rust: io: " Zhi Wang
2025-11-10 20:41 ` [PATCH v6 RESEND 4/7] rust: io: factor common I/O helpers into Io trait Zhi Wang
2025-11-13  7:36   ` Alexandre Courbot
2025-11-14 12:58   ` Alice Ryhl
2025-11-14 17:27     ` Zhi Wang
2025-11-14 18:53       ` Tamir Duberstein
2025-11-17 17:14         ` Zhi Wang
2025-11-14 20:31       ` Danilo Krummrich
2025-11-17 22:44     ` John Hubbard
2025-11-18 21:18       ` Danilo Krummrich [this message]
2025-11-18 23:43         ` John Hubbard
2025-11-10 20:41 ` [PATCH v6 RESEND 5/7] rust: io: factor out MMIO read/write macros Zhi Wang
2025-11-13  7:36   ` Alexandre Courbot
2025-11-14 16:06     ` Zhi Wang
2025-11-10 20:41 ` [PATCH v6 RESEND 6/7] rust: pci: add config space read/write support Zhi Wang
2025-11-13  7:56   ` Alexandre Courbot
2025-11-14 16:59     ` Zhi Wang
2025-11-14  0:20   ` Joel Fernandes
2025-11-17 20:28     ` Zhi Wang
2025-11-17 22:07     ` Danilo Krummrich
2025-11-10 20:41 ` [PATCH v6 RESNED 7/7] sample: rust: pci: add tests for config space routines Zhi Wang
2025-11-11  0:01 ` [PATCH v6 RESEND 0/7] rust: pci: add config space read/write support Joel Fernandes
2025-11-11  8:43   ` Zhi Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DEC4TSQBTESW.28F17X9GHCIU7@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=aniketa@nvidia.com \
    --cc=ankita@nvidia.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=cjia@nvidia.com \
    --cc=gary@garyguo.net \
    --cc=helgaas@kernel.org \
    --cc=jhubbard@nvidia.com \
    --cc=joelagnelf@nvidia.com \
    --cc=kwankhede@nvidia.com \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=markus.probst@posteo.de \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=smitra@nvidia.com \
    --cc=targupta@nvidia.com \
    --cc=tmgross@umich.edu \
    --cc=zhiw@nvidia.com \
    --cc=zhiwang@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).