linux-kernel - Re: [PATCH 7/8] rust: pci: add physfn(), to return PF device for VF device

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: <20251122161615.GN233636@ziepe.ca>
Date: Sat, 22 Nov 2025 12:16:15 -0400
From: Jason Gunthorpe <jgg@...pe.ca>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Peter Colberg <pcolberg@...hat.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Krzysztof Wilczyński <kwilczynski@...nel.org>,
	Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <lossin@...nel.org>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	Abdiel Janulgue <abdiel.janulgue@...il.com>,
	Daniel Almeida <daniel.almeida@...labora.com>,
	Robin Murphy <robin.murphy@....com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Dave Ertman <david.m.ertman@...el.com>,
	Ira Weiny <ira.weiny@...el.com>, Leon Romanovsky <leon@...nel.org>,
	linux-pci@...r.kernel.org, rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Alexandre Courbot <acourbot@...dia.com>,
	Alistair Popple <apopple@...dia.com>,
	Joel Fernandes <joelagnelf@...dia.com>,
	John Hubbard <jhubbard@...dia.com>, Zhi Wang <zhiw@...dia.com>
Subject: Re: [PATCH 7/8] rust: pci: add physfn(), to return PF device for VF
 device

On Sat, Nov 22, 2025 at 11:23:16PM +1300, Danilo Krummrich wrote:
> So far I haven't heard a convincing reason for not providing this guarantee. The
> only reason not to guarantee this I have heard is that some PF drivers only
> enable SR-IOV and hence could be unloaded afterwards. However, I think there is
> no strong reason to do so.

I know some people have work flows around this. I think they are
wrong. When we "fixed" mlx5 a while back there was some pushback and
some weird things did stop working.

So while I support this goal, I don't know if enough people will
agree..

> With this, the above code will be correct and a driver can use the generic
> infrastructure to:
> 
>   1) Call pci::Device<Bound>::physfn() returning a Result<pci::Device<Bound>>
>   2) Grab the driver's device private data from the returned Device<Bound>
> 
> Note that 2) (i.e. accessing the driver's device private data with
> Device::drvdata() [1]) ensures that the device is actually bound (drvdata() is
> only implemented for Device<Bound>) and that the returned type actually matches
> the type of the object that has been stored.

This is what the core helper does, with the twist that it "validates"
the PF driver is working right by checking its driver binding..

> I suggest to have a look at [2] for an example with how this turns out with the
> auxiliary bus [2][3], since in the end it's the same problem, i.e. an auxiliary
> driver calling into its parent, except that the auxiliary bus already guarantees
> that the parent is bound when the child is bound.

Aux bus is a little different because it should be used in a way where
there are stronger guarantees about what the parent is. Ie the aux bus
names should be unique and limited to the type of parent.

> So, if we'd provide a Rust accessor for the PF's device driver data, we'd
> implement it like above, because Device::drvdata() is already safe. If we want
> pci::Device::pf_drvdata() to be safe, we'd otherwise need to do all the checks
> Device::drvdata() already does again before we call into
> pci_iov_get_pf_drvdata().

I think to make progress along this line you need to still somehow
validate that the PF driver is working right, either by checking that
the driver is bound to a rust driver somehow or using the same
approach as the core helper.

I'm not sure the idea to force all drivers to do disable sriov is
going to be easy, and I'd rather see rust bindings progress without
opening such a topic..

Jason