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

Jason Gunthorpe
2025-11-21T23:26:47Z

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