[SeaBIOS] [RFC PATCH v2 0/4] Allow RedHat PCI bridges reserve more buses than necessary during init
Laszlo Ersek
lersek at redhat.com
Tue Jul 25 17:46:07 CEST 2017
On 07/23/17 00:11, Aleksandr Bezzubikov wrote:
> Now PCI bridges get a bus range number on a system init, basing on
> currently plugged devices. That's why when one wants to hotplug
> another bridge, it needs his child bus, which the parent is unable to
> provide (speaking about virtual device). The suggested workaround is
> to have vendor-specific capability in Red Hat PCI bridges that
> contains number of additional bus to reserve on BIOS PCI init. So this
> capability is intented only for pure QEMU->SeaBIOS usage.
>
> Considering all aforesaid, this series is directly connected with
> QEMU RFC series (v2) "Generic PCIE-PCI Bridge".
>
> Although the new PCI capability is supposed to contain various limits
> along with bus number to reserve, now only its full layout is
> proposed, but only bus_reserve field is used in QEMU and BIOS. Limits
> usage is still a subject for implementation as now the main goal of
> this series to provide necessary support from the firmware side to
> PCIE-PCI bridge hotplug.
>
> Changes v1->v2:
> 1. New #define for Red Hat vendor added (addresses Konrad's comment).
> 2. Refactored pci_find_capability function (addresses Marcel's
> comment).
> 3. Capability reworked:
> - data type added;
> - reserve space in a structure for IO, memory and
> prefetchable memory limits.
>
>
> Aleksandr Bezzubikov (4):
> pci: refactor pci_find_capapibilty to get bdf as the first argument
> instead of the whole pci_device
> pci: add RedHat vendor ID
> pci: add QEMU-specific PCI capability structure
> pci: enable RedHat PCI bridges to reserve additional buses on PCI
> init
>
> src/fw/pciinit.c | 18 ++++++++++++++----
> src/hw/pci_cap.h | 23 +++++++++++++++++++++++
> src/hw/pci_ids.h | 2 ++
> src/hw/pcidevice.c | 12 ++++++------
> src/hw/pcidevice.h | 2 +-
> src/hw/virtio-pci.c | 4 ++--
> 6 files changed, 48 insertions(+), 13 deletions(-)
> create mode 100644 src/hw/pci_cap.h
>
Coming back from PTO, it's hard for me to follow up on all the comments
that have been made across the v1 and v2 of this RFC series, so I'll
just provide a brain dump here:
(1) Mentioned by Michael: documentation. That's the most important part.
I haven't seen the QEMU patches, so perhaps they already include
documentation. If not, please start this work with adding a detailed
description do QEMU's docs/ or docs/specs/.
There are a number of preexistent documents that might be related, just
search docs/ for filenames with "pci" in them.
(2) Bus range reservation, and hotplugging bridges. What's the
motivation? Our recommendations in "docs/pcie.txt" suggest flat
hierarchies.
If this use case is really necessary, I think it should be covered in
"docs/pcie.txt". In particular it has a consequence for PXB as well
(search "pcie.txt" for "bus_nr") -- if users employ extra root buses,
then the bus number partitions that they specify must account for any
bridges that they plan to hot-plug (and for the bus range reservations
on the cold-plugged bridges behind those extra root buses).
(3) Regarding the contents and the format of the capability structure, I
wrote up my thoughts earlier in
https://bugzilla.redhat.com/show_bug.cgi?id=1434747#c8
Let me quote it here for ease of commenting:
> (In reply to Gerd Hoffmann from comment #7)
> > So, now that the generic ports are there we can go on figure how to
> > handle this best. I still think the best way to communicate window
> > size hints would be to use a vendor specific pci capability (instead
> > of setting the desired size on reset). The information will always
> > be available then and we don't run into initialization order issues.
>
> This seems good to me -- I can't promise 100% without actually trying,
> but I think I should be able to parse the capability list in config
> space for this hint, in the GetResourcePadding() callback.
>
> I propose that we try to handle this issue "holistically", together
> with bug 1434740. We need a method that provides controls for both IO
> and MMIO:
>
> - For IO, we need a mechanism that can prevent *both* firmware *and*
> Linux from reserving IO for PCI Express ports. I think Marcel's
> approach in bug 1344299 is sufficient, i.e., set the IO base/limit
> registers of the bridge to 0 for disabling IO support. And, if not
> disabled, just go with the default 4KB IO reservation (for both PCI
> Express ports and legacy PCI bridges, as the latter is documented in
> the guidelines).
>
> - For MMIO, the vendor specific capability structure should work
> something like this:
> - if the capability is missing, reserve 2MB, 32-bit,
> non-prefetchable,
>
> - otherwise, the capability structure should consist of 3 fields
> (reservation sizes):
> - uint32_t non_prefetchable_32,
> - uint32_t prefetchable_32,
> - uint64_t prefetchable_64,
>
> - of prefetchable_32 and prefetchable_64, at most one may be
> nonzero (they are mutually exclusive, and they can be both
> zero),
>
> - whenever a field is 0, that kind of reservation is not needed.
This would now have to be extended with bus range reservation.
(4) Whether the reservation size should be absolute or relative (raised
by Gerd). IIUC, Gerd suggests that the absolute aperture size should be
specified (as a minimum), not the increment / reservation for hotplug
purposes.
The Platform Initialization Specification, v1.6, downloadable at
<http://www.uefi.org/specs>, writes the following under
EFI_PCI_HOT_PLUG_INIT_PROTOCOL.GetResourcePadding()
in whose implementation I will have to parse the values from the
capability structure, and return the appropriate representation to the
platform-independent PciBusDxe driver (i.e., the enumeration /
allocation agent):
> The padding is returned in the form of ACPI (2.0 & 3.0) resource
> descriptors. The exact definition of each of the fields is the same as
> in the
> EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL.SubmitResources()
> function. See the section 10.8 for the definition of this function.
>
> The PCI bus driver is responsible for adding this resource request to
> the resource requests by the physical PCI devices. If Attributes is
> EfiPaddingPciBus, the padding takes effect at the PCI bus level. If
> Attributes is EfiPaddingPciRootBridge, the required padding takes
> effect at the root bridge level. For details, see the definition of
> EFI_HPC_PADDING_ATTRIBUTES in "Related Definitions" below.
Emphasis on "*adding* this resource request to the resource requests by
the physical PCI devices".
However... After checking some OVMF logs, it seems that in practice,
PciBusDxe does exactly what Gerd suggests.
Currently OVMF returns a constant 2MB MMIO padding and a constant 512B
IO padding for all hotplug-capable bridges (including PCI Express root
ports), from GetResourcePadding(). For example, for the following QEMU
command line fragment:
-device pxb-pcie,bus_nr=64,id=rootbr1,numa_node=1,bus=pcie.0,addr=0x3 \
\
-device ioh3420,id=rootbr1-port1,bus=rootbr1,addr=0x1,slot=3 \
-device e1000e,bus=rootbr1-port1,netdev=net0 \
\
-device ioh3420,id=rootbr1-port2,bus=rootbr1,addr=0x2,slot=4 \
-device e1000e,bus=rootbr1-port2,netdev=net1 \
PciBusDxe produces the following log (extract):
> PciBus: Resource Map for Root Bridge PciRoot(0x40)
> Type = Io16; Base = 0x8000; Length = 0x2000; Alignment = 0xFFF
> Base = 0x8000; Length = 0x1000; Alignment = 0xFFF; Owner = PPB [40|02|00:**]
> Base = 0x9000; Length = 0x1000; Alignment = 0xFFF; Owner = PPB [40|01|00:**]
> Type = Mem32; Base = 0x98600000; Length = 0x400000; Alignment = 0x1FFFFF
> Base = 0x98600000; Length = 0x200000; Alignment = 0x1FFFFF; Owner = PPB [40|02|00:**]
> Base = 0x98800000; Length = 0x200000; Alignment = 0x1FFFFF; Owner = PPB [40|01|00:**]
>
> PciBus: Resource Map for Bridge [40|01|00]
> Type = Io16; Base = 0x9000; Length = 0x1000; Alignment = 0xFFF
> Base = Padding; Length = 0x200; Alignment = 0x1FF
> Base = 0x9000; Length = 0x20; Alignment = 0x1F; Owner = PCI [41|00|00:18]
> Type = Mem32; Base = 0x98800000; Length = 0x200000; Alignment = 0x1FFFFF
> Base = Padding; Length = 0x200000; Alignment = 0x1FFFFF
> Base = 0x98800000; Length = 0x20000; Alignment = 0x1FFFF; Owner = PCI [41|00|00:14]
> Base = 0x98820000; Length = 0x20000; Alignment = 0x1FFFF; Owner = PCI [41|00|00:10]
> Base = 0x98840000; Length = 0x4000; Alignment = 0x3FFF; Owner = PCI [41|00|00:1C]
>
> PciBus: Resource Map for Bridge [40|02|00]
> Type = Io16; Base = 0x8000; Length = 0x1000; Alignment = 0xFFF
> Base = Padding; Length = 0x200; Alignment = 0x1FF
> Base = 0x8000; Length = 0x20; Alignment = 0x1F; Owner = PCI [42|00|00:18]
> Type = Mem32; Base = 0x98600000; Length = 0x200000; Alignment = 0x1FFFFF
> Base = Padding; Length = 0x200000; Alignment = 0x1FFFFF
> Base = 0x98600000; Length = 0x20000; Alignment = 0x1FFFF; Owner = PCI [42|00|00:14]
> Base = 0x98620000; Length = 0x20000; Alignment = 0x1FFFF; Owner = PCI [42|00|00:10]
> Base = 0x98640000; Length = 0x4000; Alignment = 0x3FFF; Owner = PCI [42|00|00:1C]
This means that
- both ioh3420 root ports get a padding of 512B for IO and 2MB fo MMIO,
- the actual resources for each e1000e NIC on the respective ioh3420
root port are allocated *within* the padding,
- because each MMIO padding is larger than the actual resource needs of
the respective e1000e NIC, it is the MMIO paddings that are ultimately
considered on the root bridge,
- additionally, the 0x200 IO paddings are rounded up to 0x1000 on the
root bridge.
So, it seems that PciBusDxe
- conflicts with the Platform Initialization spec, but
- at the same time it seems to do what Gerd is suggesting.
(5) Returning to bus range reservation... This looks problematic with
edk2.
* Some background first:
In the Platform Init spec, two kinds of hotplug controllers are
distinguished: "root" and "non-root". This naming is confusing because
in this context, "root" simply means
cannot be found by normal enumeration, or needs special initialization
when found before recursing it
whereas "non-root" means
can be found by normal enumeration and needs no special initialization
before recursing it
In this context, QEMU does not provide any "root" hotplug controllers,
because all the controllers that support hotplug -- i.e., can *accept* a
hot-plugged device -- *can* be found with enumeration, and need no
platform-specific initialization callback:
- PCI Express root ports
- PCI Express downstream ports
- PCI-PCI Bridges
Now, if a platform provides "root" (in the above context) hotplug
controllers, then the platform's
EFI_PCI_HOT_PLUG_INIT_PROTOCOL.GetRootHpcList()
method has to return them. (Importantly, this function has to return the
list of "root HPCs" *without* accessing PCI config space, using
"apriori" knowledge.) Accordingly to the QEMU situation, OVMF returns an
empty list here.
* Why this is a problem:
For "non-root" (in the above sense) hotplug controllers, PciBusDxe
considers the IO and MMIO paddings, but it does *not* consider bus
number paddings, even if
EFI_PCI_HOT_PLUG_INIT_PROTOCOL.GetResourcePadding() returns some. (As
far as I can see from the source -- I have never tested it, obviously.)
For QEMU, this means all of the hotplug controllers.
For "root" (in the above sense) hotplug controllers, PciBusDxe considers
bus number paddings. However, on QEMU, this set of controllers is empty.
The PI spec says,
> [...] For all the root HPCs and the nonroot HPCs, call
> EFI_PCI_HOT_PLUG_INIT_PROTOCOL.GetResourcePadding() to obtain the
> amount of overallocation and add that amount to the requests from the
> physical devices. Reprogram the bus numbers by taking into account the
> bus resource padding information. [...]
However, according to my interpretation of the source code, PciBusDxe
does not consider bus number padding for non-root HPCs (which are "all"
HPCs on QEMU).
So, I agree that we can add a bus number range field to the capability,
but I'm fairly sure it won't work with edk2's PciBusDxe without major
surgery. (Given that we haven't ever considered hot-plugging entire
bridges until now, i.e., anything that would require bus number
reservation, I think we can live with such a limitation when using OVMF,
for an indefinite time to come.)
Thanks
Laszlo
More information about the SeaBIOS
mailing list