Re: [PATCH 0/3] PCI: Expose resource resizing through sysfs

[Alex Williamson]

On Wed, 17 Aug 2022 12:10:44 +0200
Christian König <christian.koenig@xxxxxxx> wrote:

> Am 16.08.22 um 21:39 schrieb Alex Williamson:
> > We have a couple graphics drivers making use of PCIe Resizable BARs
> > now, but I've been trying to figure out how we can make use of such
> > features for devices assigned to a VM.  This is a proposal for a
> > rather basic interface in sysfs such that we have the ability to
> > pre-enable larger BARs before we bind devices to vfio-pci and
> > attach them to a VM.  
> 
> Ah, yes please.
> 
> I was considering doing this myself just for testing while adding the 
> rebar support for the GFX drivers, but then just implementing it on the 
> GFX side was simpler.
> 
> I would just add a warning that resizing BARs can easily crash the 
> system even when no driver directly claimed the resource or PCIe device.
> 
> It literally took me weeks to figure out that I need to kick out the EFI 
> framebuffer driver before trying to resize the BAR or otherwise I just 
> get a hung system.

Good point, I think maybe we can avoid crashing the system though if we
use the new aperture support to remove conflicting drivers from all VGA
class devices, similar to d17378062079 ("vfio/pci: Remove console
drivers").  A note in the ABI documentation about removing console
drivers from the device when resizing resources would definitely be in
order.

> > Along the way I found a double-free in the error path of creating
> > resource attributes, that can certainly be pulled separately (1/).
> >
> > I'm using an RTX6000 for testing, which unexpectedly only supports
> > REBAR with smaller than default sizes, which led me to question
> > why we have such heavy requirements for shrinking resources (2/).  
> 
> Oh, that's easy. You got tons of ARM boards with less than 512MiB of 
> address space per root PCIe complex.
> 
> If you want to get a GPU working on those you need to decrease the
> BAR size or otherwise you won't be able to fit 256MiB VRAM BAR +
> register BAR into the same hole for the PCIe root complex.
> 
> An alternative explanation is that at least AMD produced some boards 
> with a messed up resize configuration word. But on those you only got 
> 256MiB, 512MiB and 1GiB potential BAR sizes IIRC.

An aspect of shrinking BARs that maybe I'm not giving enough
consideration to is that we might be shrinking a BAR on one device in
order to release MMIO space from a bridge aperture, that we might then
use to expand a BAR elsewhere.  The RTX6000 case only frees a rather
modest amount of MMIO space, but I could imagine more substantial
configurations.  Maybe this justifies resizing the bridge aperture even
in the shrinking case?

> Anyway, with an appropriate warning added to the sysfs documentation
> the patch #2 and #3 are Acked-by: Christian König
> <christian.koenig@xxxxxxx>

Thanks!

Alex