Re: [PATCH v2 1/1] PCI: Fix BAR resizing when VF BARs are assigned

[Michał Winiarski]

On Fri, Mar 14, 2025 at 08:56:49AM -0600, Alex Williamson wrote:
> On Fri,  7 Mar 2025 16:03:49 +0200
> Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote:
> 
> > __resource_resize_store() attempts to release all resources of the
> > device before attempting the resize. The loop, however, only covers
> > standard BARs (< PCI_STD_NUM_BARS). If a device has VF BARs that are
> > assigned, pci_reassign_bridge_resources() finds the bridge window still
> > has some assigned child resources and returns -NOENT which makes
> > pci_resize_resource() to detect an error and abort the resize.
> > 
> > Change the release loop to cover all resources up to VF BARs which
> > allows the resize operation to release the bridge windows and attempt
> > to assigned them again with the different size.
> > 
> > As __resource_resize_store() checks first that no driver is bound to
> > the PCI device before resizing is allowed, SR-IOV cannot be enabled
> > during resize so it is safe to release also the IOV resources.
> 
> Is this true?  pci-pf-stub doesn't teardown SR-IOV on release, which I
> understand is done intentionally.  Thanks,

Is that really intentional?
PCI warns when that scenario occurs:
https://elixir.bootlin.com/linux/v6.13.7/source/drivers/pci/iov.c#L936

I thought that the usecase is binding pci-pf-stub, creating VFs, and
letting the driver be.
But unbinding after creating VFs? What's the goal of that?
Perhaps we're just missing .remove() in pci-pf-stub?

Thanks,
-Michał

> 
> Alex
>  
> > Fixes: 91fa127794ac ("PCI: Expose PCIe Resizable BAR support via sysfs")
> > Reported-by: Michał Winiarski <michal.winiarski@xxxxxxxxx>
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> > ---
> > 
> > v2:
> > - Removed language about expansion ROMs
> > 
> >  drivers/pci/pci-sysfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> > index b46ce1a2c554..0c16751bab40 100644
> > --- a/drivers/pci/pci-sysfs.c
> > +++ b/drivers/pci/pci-sysfs.c
> > @@ -1578,7 +1578,7 @@ static ssize_t __resource_resize_store(struct device *dev, int n,
> >  
> >  	pci_remove_resource_files(pdev);
> >  
> > -	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> > +	for (i = 0; i < PCI_BRIDGE_RESOURCES; i++) {
> >  		if (pci_resource_len(pdev, i) &&
> >  		    pci_resource_flags(pdev, i) == flags)
> >  			pci_release_resource(pdev, i);
> > 
> > base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
>