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

From: Christian König
Date: Thu Aug 18 2022 - 07:16:22 EST


Am 17.08.22 um 16:02 schrieb 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?

That was the original idea why I've kept that in there, yes.

But I never really seen a case where that really mattered.

So far making BARs smaller was something only the BIOS does.

Christian.


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