rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Zhi Wang" <zhiw@nvidia.com>
Cc: <rust-for-linux@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>,
	<aliceryhl@google.com>, <tmgross@umich.edu>,
	<linux-pci@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<cjia@nvidia.com>, <smitra@nvidia.com>, <ankita@nvidia.com>,
	<aniketa@nvidia.com>, <kwankhede@nvidia.com>,
	<targupta@nvidia.com>, <zhiwang@kernel.org>,
	<acourbot@nvidia.com>, <joelagnelf@nvidia.com>,
	<jhubbard@nvidia.com>, <markus.probst@posteo.de>
Subject: Re: [RFC 0/6] rust: pci: add config space read/write support
Date: Mon, 13 Oct 2025 22:02:55 +0200	[thread overview]
Message-ID: <DDHGOCNZJRND.129VXJYMXMCZW@kernel.org> (raw)
In-Reply-To: <20251013212518.555a19ad.zhiw@nvidia.com>

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.

  reply	other threads:[~2025-10-13 20:03 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-10  8:03 [RFC 0/6] rust: pci: add config space read/write support Zhi Wang
2025-10-10  8:03 ` [RFC 1/6] rust: io: refactor Io<SIZE> helpers into IoRegion trait Zhi Wang
2025-10-10  8:03 ` [RFC 2/6] rust: io: factor out MMIO read/write macros Zhi Wang
2025-10-10  8:03 ` [RFC 3/6] rust: pci: add a helper to query configuration space size Zhi Wang
2025-10-10  8:03 ` [RFC 4/6] rust: pci: add config space read/write support Zhi Wang
2025-10-10  8:03 ` [RFC 5/6] rust: pci: add helper to find extended capability Zhi Wang
2025-10-10  8:03 ` [RFC 6/6] [!UPSTREAM] nova-core: test configuration routine Zhi Wang
2025-10-13 15:39 ` [RFC 0/6] rust: pci: add config space read/write support Danilo Krummrich
2025-10-13 18:25   ` Zhi Wang
2025-10-13 20:02     ` Danilo Krummrich [this message]
2025-10-15 10:44       ` 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=DDHGOCNZJRND.129VXJYMXMCZW@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=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).