Re: [PATCH 3/3] PCI/IOV: Restore initial VF ReBAR sizes on PF release

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unsafe hardware configuration writes during kobject .release callback and incorrect lifecycle hook.
--

commit b1af88a89dd13b3ede54e65628a75befe1b43cf4
Author: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>

PCI/IOV: Restore initial VF ReBAR sizes on PF release

This commit attempts to restore the per-VF BAR sizes to their initial
values when sriov_release() is called. It aims to handle the case where
VF BARs were resized but SR-IOV was not enabled when the PF driver is
unbound.

>     sriov_disable() and sriov_enable() error paths restore the per-VF BAR
>     sizes saved at probe time. They do not cover the case where the PF
>     driver is unbound while VF BARs were resized but SR-IOV was never
>     enabled (or was already disabled).
>     
>     On the next bind, sriov_init() re-reads the SR-IOV capability and
>     captures the current (inflated) BAR registers as the new initial sizes
>     in iov->barsz_orig[]. The restore logic becomes a no-op for that boot
>     and the kernel may reserve MMIO for the inflated VF aperture multiplied
>     by TotalVFs.

Is sriov_release() the correct place for this restoration?

The commit message states it covers when the PF driver is unbound and mentions
the next bind, but sriov_release() is called by pci_release_dev() which is
the kobject .release callback.

This executes when the pci_dev reference count drops to 0 (such as during
device destruction), not during a driver unbind via sysfs. Furthermore,
sriov_init() is called during device enumeration, not driver bind.

> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index a05c1be388d2f..794f956910886 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -969,6 +969,8 @@ static void sriov_release(struct pci_dev *dev)
>  {
>  	BUG_ON(dev->sriov->num_VFs);
>  
> +	sriov_restore_vf_rebar_initial_sizes(dev);

Is it safe to access the hardware configuration space here?

Since sriov_release() executes as a .release callback, it can run at an
arbitrary time, such as being delayed indefinitely by an open sysfs file
descriptor.

If the device has been surprise-removed or placed in D3cold (suspended), won't
PCIe config space reads return ~0 (0xFFFF)?

In that case, pci_iov_is_memory_decoding_enabled() would evaluate
0xFFFF & PCI_SRIOV_CTRL_MSE as true, causing pci_iov_vf_bar_set_size() to
return -EBUSY.

This could trigger a pci_warn() in sriov_restore_vf_rebar_initial_sizes() for
every resized VF BAR, spamming the kernel log with false-positive warnings, and
potentially causing PCIe Unsupported Requests or bus errors.

> +
>  	if (dev != dev->sriov->dev)
>  		pci_dev_put(dev->sriov->dev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260505170010.3414074-1-marcin.bernatowicz@linux.intel.com?part=3