From: Jason Gunthorpe <jgg@ziepe.ca>
To: Peter Colberg <pcolberg@redhat.com>
Cc: "Danilo Krummrich" <dakr@kernel.org>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Alex Gaynor" <alex.gaynor@gmail.com>,
"Boqun Feng" <boqun.feng@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Trevor Gross" <tmgross@umich.edu>,
"Abdiel Janulgue" <abdiel.janulgue@gmail.com>,
"Daniel Almeida" <daniel.almeida@collabora.com>,
"Robin Murphy" <robin.murphy@arm.com>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Dave Ertman" <david.m.ertman@intel.com>,
"Ira Weiny" <ira.weiny@intel.com>,
"Leon Romanovsky" <leon@kernel.org>,
linux-pci@vger.kernel.org, rust-for-linux@vger.kernel.org,
linux-kernel@vger.kernel.org,
"Alexandre Courbot" <acourbot@nvidia.com>,
"Alistair Popple" <apopple@nvidia.com>,
"Joel Fernandes" <joelagnelf@nvidia.com>,
"John Hubbard" <jhubbard@nvidia.com>,
"Zhi Wang" <zhiw@nvidia.com>
Subject: Re: [PATCH 7/8] rust: pci: add physfn(), to return PF device for VF device
Date: Fri, 21 Nov 2025 19:26:42 -0400 [thread overview]
Message-ID: <20251121232642.GG233636@ziepe.ca> (raw)
In-Reply-To: <20251119-rust-pci-sriov-v1-7-883a94599a97@redhat.com>
On Wed, Nov 19, 2025 at 05:19:11PM -0500, Peter Colberg wrote:
> Add a method to return the Physical Function (PF) device for a Virtual
> Function (VF) device in the bound device context.
>
> Unlike for a PCI driver written in C, guarantee that when a VF device is
> bound to a driver, the underlying PF device is bound to a driver, too.
You can't do this as an absolutely statement from rust code alone,
this statement is confused.
> When a device with enabled VFs is unbound from a driver, invoke the
> sriov_configure() callback to disable SR-IOV before the unbind()
> callback. To ensure the guarantee is upheld, call disable_sriov()
> to remove all VF devices if the driver has not done so already.
This doesn't seem like it should be in this patch.
Good drivers using the PCI APIs should be calling pci_disable_sriov()
during their unbind.
The prior patch #3 should not have added "safe" bindings for
enable_sriov that allow this lifetime rule to be violated.
> + #[cfg(CONFIG_PCI_IOV)]
> + pub fn physfn(&self) -> Result<&Device<device::Bound>> {
> + if !self.is_virtfn() {
> + return Err(EINVAL);
> + }
> + // SAFETY:
> + // `self.as_raw` returns a valid pointer to a `struct pci_dev`.
> + //
> + // `physfn` is a valid pointer to a `struct pci_dev` since self.is_virtfn() is `true`.
> + //
> + // `physfn` may be cast to a `Device<device::Bound>` since `pci::Driver::remove()` calls
> + // `disable_sriov()` to remove all VF devices, which guarantees that the underlying
> + // PF device is always bound to a driver when the VF device is bound to a driver.
Wrong safety statement. There are drivers that don't call
disable_sriov(). You need to also check that the driver attached to
the PF is actually working properly.
I do not like to see this attempt to open code the tricky login of
pci_iov_get_pf_drvdata() in rust without understanding the issues :(
Jason
next prev parent reply other threads:[~2025-11-21 23:26 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-19 22:19 [PATCH 0/8] rust: pci: add abstractions for SR-IOV capability Peter Colberg
2025-11-19 22:19 ` [PATCH 1/8] rust: pci: add is_virtfn(), to check for VFs Peter Colberg
2025-11-21 3:00 ` kernel test robot
2025-11-21 18:27 ` Peter Colberg
2025-11-19 22:19 ` [PATCH 2/8] rust: pci: add is_physfn(), to check for PFs Peter Colberg
2025-11-21 4:35 ` kernel test robot
2025-11-19 22:19 ` [PATCH 3/8] rust: pci: add {enable,disable}_sriov(), to control SR-IOV capability Peter Colberg
2025-11-21 23:28 ` Jason Gunthorpe
2025-11-19 22:19 ` [PATCH 4/8] rust: pci: add num_vf(), to return number of VFs Peter Colberg
2025-11-19 22:19 ` [PATCH 5/8] rust: pci: add vtable attribute to pci::Driver trait Peter Colberg
2025-11-19 22:19 ` [PATCH 6/8] rust: pci: add bus callback sriov_configure(), to control SR-IOV from sysfs Peter Colberg
2025-11-21 6:00 ` kernel test robot
2025-11-19 22:19 ` [PATCH 7/8] rust: pci: add physfn(), to return PF device for VF device Peter Colberg
2025-11-21 7:57 ` kernel test robot
2025-11-21 23:26 ` Jason Gunthorpe [this message]
2025-11-22 10:23 ` Danilo Krummrich
2025-11-22 16:16 ` Jason Gunthorpe
2025-11-22 18:57 ` Leon Romanovsky
2025-11-22 22:26 ` Danilo Krummrich
2025-11-23 6:34 ` Leon Romanovsky
2025-11-23 10:07 ` Danilo Krummrich
2025-11-23 11:18 ` Leon Romanovsky
2025-11-22 22:43 ` Danilo Krummrich
2025-11-19 22:19 ` [PATCH 8/8] samples: rust: add SR-IOV driver sample Peter Colberg
2025-11-20 6:41 ` Zhi Wang
2025-11-20 15:49 ` Peter Colberg
2025-11-20 6:32 ` [PATCH 0/8] rust: pci: add abstractions for SR-IOV capability Zhi Wang
2025-11-20 15:03 ` Peter Colberg
2025-11-20 18:34 ` Zhi Wang
2025-11-20 21:16 ` Zhi Wang
2025-11-21 17:05 ` Peter Colberg
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=20251121232642.GG233636@ziepe.ca \
--to=jgg@ziepe.ca \
--cc=a.hindborg@kernel.org \
--cc=abdiel.janulgue@gmail.com \
--cc=acourbot@nvidia.com \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=apopple@nvidia.com \
--cc=bhelgaas@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=david.m.ertman@intel.com \
--cc=gary@garyguo.net \
--cc=gregkh@linuxfoundation.org \
--cc=ira.weiny@intel.com \
--cc=jhubbard@nvidia.com \
--cc=joelagnelf@nvidia.com \
--cc=kwilczynski@kernel.org \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=ojeda@kernel.org \
--cc=pcolberg@redhat.com \
--cc=robin.murphy@arm.com \
--cc=rust-for-linux@vger.kernel.org \
--cc=tmgross@umich.edu \
--cc=zhiw@nvidia.com \
/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).