Re: [edk2-devel] [RFC PATCH v2 0/5] hw/pci, hw/arm/virt: fixed PCI BAR placement

[Gerd Hoffmann <kraxel@redhat.com>]

  Hi,

> > Is this the reason we cannot rely on the Enhanced Allocation (EA)
> > capability here?
> 
> AFAICT, this was explored on RFC v1.

> EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL was suggested as the
> shorter path at the time, which is what this series uses.

Essentially we have *two* problems to solve here.  The first is how do
we get the fixed bar information from qemu to the firmware, and the
second is how we integrate that into edk2.

So we could have a driver which parses EA and passes along the
information found to PciDxe using
EFI_INCOMPATIBLE_PCI_DEVICE_SUPPORT_PROTOCOL.

Not sure how much of a win that would be compared to adding EA support
to PciDxe directly given that the PciDxe bridge window logic needs
enhancements to properly handle fixed bars (as discussed below).

> > Agreed - if the bridge windows are not programmed correctly on the first
> > pass, there is something in the code that needs to be fixed. I don't think
> > papering over it like this is the right approach.
> 
> That's a fair point. I did the window-sizing after PciBusDxe because I
> didn't want to touch existing PciBusDxe code too much.
> 
> As per my understanding, PciBusDxe's enumeration splits into three phases:
> 
> Phase 1 (PciHostBridgeEnumerator) walks the whole tree and calls
> CheckDevice() per device — by the time this phase finishes, every fixed
> BAR address is already known and cached on the device
> (PciBar[Bar].FixedBaseAddress).
> 
> Phase 2 (PciHostBridgeResourceAllocator) prepares an address for every
> resource node in the tree — both individual BARs and bridge windows
> alike — purely from size and alignment; PCI_RESOURCE_NODE has no address
> field at all, so this is entirely blind to whether a fixed address was
> already required.
> 
> Phase 3 (ProgramResource) then writes the actual PCI config-space
> registers, and by default it just writes whatever address Phase 2
> prepared, for both BARs and bridge windows. The one exception is
> ProgramBar() — it specifically checks whether that particular BAR was
> marked fixed back in Phase 1, and if so, overrides Phase 2's prepared
> base address with the real fixed one. However, ProgramPpbApperture(),
> which writes the bridge's own window registers, has no equivalent
> override — it always writes whatever Phase 2 prepared, with no awareness
> of a fixed BAR anywhere underneath it. And that needs fixing, and for
> that I have to change the existing code.
> 
> I think the fix would be to extend Phase 2's own sizing step
> (CalculateResourceAperture() in PciResourceSupport.c) to check for the
> already-known fixed address on each child and, when present, size and
> position the window as the exact union of those addresses instead of the
> blind size-only sum. Does that match the direction you had in mind, or
> is there a different integration point you'd suggest?

Sounds about right, when propagating resource requirements up from
devices to bridges looking only at the size is not enough if we want
properly support pci bars at fixed locations.

take care,
  Gerd