Date
1 - 7 of 7
[PATCH v7 1/6] MdeModulePkg/PciHostBridge: io range is not mandatory
|
Gerd Hoffmann
io range is not mandatory according to pcie spec, so allow
pcie host bridge configurations without io window in case there are no io reservations. Signed-off-by: Gerd Hoffmann <kraxel@...> Reviewed-by: Ard Biesheuvel <ardb@...> --- MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c index b20bcd310ad5..354be6dbb313 100644 --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c @@ -1085,6 +1085,12 @@ NotifyPhase ( RootBridge->ResAllocNode[Index].Base = BaseAddress; RootBridge->ResAllocNode[Index].Status = ResAllocated; DEBUG ((DEBUG_INFO, "Success\n")); + } else if ((Index == TypeIo) && + (RootBridge->Io.Base == MAX_UINT64) && + (RootBridge->ResAllocNode[Index].Length == 0)) + { + /* I/O is optional on PCIe */ + DEBUG ((DEBUG_INFO, "Success (PCIe NoIO)\n")); } else { ReturnStatus = EFI_OUT_OF_RESOURCES; DEBUG ((DEBUG_ERROR, "Out Of Resource!\n")); -- 2.36.1
|
|
|
|
Ni, Ray
Gerd,
toggle quoted messageShow quoted text
The fix should work. But I am curious why the if in blow is not hit. https://github.com/TianoCore/edk2/blob/master/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c#L922 The RootBridge->ResAllocNode[TypeIo].Status is assigned as ResNone in: https://github.com/TianoCore/edk2/blob/master/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c#L263 Or maybe your OVMF platform doesn't execute the above code path? Then I guess maybe the Base/Limit of Aperture is not set properly so that below if is hit? https://github.com/tianocore/edk2/blob/64706ef761273ba403f9cb3b7a986bfb804c0a87/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c#L249 Thanks, Ray
-----Original Message-----
|
|
|
|
Ard Biesheuvel
On Thu, 2 Jun 2022 at 12:14, Ni, Ray <ray.ni@...> wrote:
I did a quick test with both ArmVirtQemu and microvm (using this series but omitting the MdeModulePkg), and I can confirm that not having a I/O resource window at all seems to work fine if none of the PCI devices have I/O BARs. Gerd, do you remember why exactly this patch is needed? Is it related to devices that have I/O BARs but don't actually require them to function correctly?
|
|
|
|
Gerd Hoffmann
Hi,
I did a quick test with both ArmVirtQemu and microvm (using thisWell, the difference seem to be pcie root ports. When plugging my virtio device into the root bus everything is fine: PCI Bus First Scanning PciBus: Discovered PCI @ [00|00|00] PciBus: Discovered PCI @ [00|01|00] BAR[1]: Type = Mem32; Alignment = 0xFFF; Length = 0x1000; Offset = 0x14 BAR[4]: Type = PMem64; Alignment = 0x3FFF; Length = 0x4000; Offset = 0x20 [ ... ] PciHostBridge: NotifyPhase (AllocateResources) RootBridge: PciRoot(0x0) Mem64: Base/Length/Alignment = 6000000000/100000/FFFFF - Success Mem: Base/Length/Alignment = C0000000/100000/FFFFF - Success PciBus: HostBridge->NotifyPhase(AllocateResources) - Success When plugging the virtio device into a pcie root port it doesn't work and the log looks like this: PCI Bus First Scanning PciBus: Discovered PCI @ [00|00|00] PciBus: Discovered PPB @ [00|08|00] Padding: Type = Mem32; Alignment = 0x1FFFFF; Length = 0x200000 Padding: Type = Io; Alignment = 0x1FF; Length = 0x200 BAR[0]: Type = Mem32; Alignment = 0xFFF; Length = 0x1000; Offset = 0x10 PciBus: Discovered PCI @ [01|00|00] BAR[1]: Type = Mem32; Alignment = 0xFFF; Length = 0x1000; Offset = 0x14 BAR[4]: Type = PMem64; Alignment = 0x3FFF; Length = 0x4000; Offset = 0x20 [ ... ] PciHostBridge: NotifyPhase (AllocateResources) RootBridge: PciRoot(0x0) Mem: Base/Length/Alignment = C0000000/300000/1FFFFF - Success Mem64: Base/Length/Alignment = 6000000000/100000/FFFFF - Success I/O: Base/Length/Alignment = FFFFFFFFFFFFFFFF/1000/FFF - Out Of Resource! [ ... ] PciHostBridge: NotifyPhase (AllocateResources) RootBridge: PciRoot(0x0) Mem64: Base/Length/Alignment = 6000000000/100000/FFFFF - Success Mem: Base/Length/Alignment = C0000000/200000/FFFFF - Success I/O: Base/Length/Alignment = FFFFFFFFFFFFFFFF/0/FFF - Out Of Resource! So, it's apparently the io window of the pcie root port which causes edk2 try allocate io resources. take care, Gerd
|
|
|
|
Ard Biesheuvel
On Thu, 2 Jun 2022 at 15:15, Gerd Hoffmann <kraxel@...> wrote:
This seems to be related to the padding logic, i.e., we are trying to preserve some extra I/O space for the root port in case we hotplug something that might need it. The hack below gets around this - we'll need something suitable check here that avoids I/O padding when the root port has not I/O resource window in the first place. Care to cook something up? --- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c +++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c @@ -733,7 +733,7 @@ GetResourcePadding ( } } - if (DefaultIo) { + if (DefaultIo && FALSE) { // // Request defaults. //
|
|
|
|
Gerd Hoffmann
Hi,
This seems to be related to the padding logic, i.e., we are trying to --- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.cOh, *there* it comes from. Given this is configurable already we can fix that one in qemu with a microvm tweak: diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c index 4b3b1dd262f1..f01d972f5d28 100644 --- a/hw/i386/microvm.c +++ b/hw/i386/microvm.c @@ -757,6 +757,12 @@ static void microvm_class_init(ObjectClass *oc, void *data) "Set off to disable adding virtio-mmio devices to the kernel cmdline"); machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE); + + /* + * pcie host bridge (gpex) on microvm has no io address window, + * so reserving io space is not going to work. Turn it off. + */ + object_register_sugar_prop("pcie-root-port", "io-reserve", "0", true); } static const TypeInfo microvm_machine_info = { So, I think we can drop patch #1. Want me respin the series, or can you simply drop the patch on merge? thanks, Gerd
|
|
|
|
Ard Biesheuvel
On Fri, 3 Jun 2022 at 10:29, Gerd Hoffmann <kraxel@...> wrote:
I've already queued it up.
|
|
|