On Mon, May 23, 2022 at 11:24 PM, Gerd Hoffmann wrote:
On Mon, May 23, 2022 at 04:48:05AM -0700, Albecki, Mateusz wrote:You mention that devices with IO bar fail to initialize but that is contrary to what I would expect from code review. I've run an experiment with your change in which I am telling
EDK2 that no IO space is available on the system by not updating the IO range in PciHostBridgeLib. Sure enough Devices that need an IO are still enumerated, device path and PciIo are installed
and in general everything works as it used to. If I had an UHCI controller on that system UHCI driver would be loaded and it could potentially result in some strange behavior since that driver isn't smart enough to check
if IO space has been allocated for the device.
To make things worse I see that if we return success there EDK2 will actually go ahead and start assigning trash addresses to the device and enable IO space decoding in case of the PCI root port which means that device will try to decode
invalid IO ranges. Not an issue for a system without an IO but for a system in which we have run out of the IO and we have entered this code branch this new behavior is potentially more dangerous then simply not enumerating the device.
PciBus: Resource Map for Root Bridge PciRoot(0x0)
Type = Io16; Base = 0xFFFFFFFFFFFFFFFF; Length = 0x1000; Alignment = 0xFFF
Base = 0x0; Length = 0x4; Alignment = 0x3; Owner = PPB [00|06|04:14]
Base = 0x0; Length = 0x4; Alignment = 0x3; Owner = PPB [00|06|00:14]
Base = 0xFFFFFFFC; Length = 0x4; Alignment = 0x3; Owner = PCI [00|04|00:24]
Base = 0xFFFFFFFC; Length = 0x4; Alignment = 0x3; Owner = PCI [00|04|00:20]
Base = 0xFFFFFFFC; Length = 0x4; Alignment = 0x3; Owner = PCI [00|04|00:1C]
Base = 0xFFFFFFFC; Length = 0x4; Alignment = 0x3; Owner = PCI [00|04|00:18]
It seems to me like it does. Specifically the error scenario where the system has run out of IO space will not be handled properly I think.Even with those bad device drivers I would agree that taking thisAlso note that for pcie root bridges which do support io address space
Ok I misunderstood previous mailsThat said I think we are missing a larger issue here - why are weI don't think so. I see the *linux kernel* hand out io address space to
I think to really handle it we would have to have a more involved change. Specifically in the PciHostBridge.c:NoitfyPhase function we need to have a way to tell the PciBus driver which resources specifically failed to allocate and treat this as a condition we need to handle instead of panicking and asserting the system or dropping the entire host bridge. When PciBus driver sees that IO failed to allocate it would skip IO bars and would not allow to set the IO space enable bit in the root bridge. We also would need to change the logic in PciResourceSupport.c:ProgramBar because that function is very optimistic and assumes that if we were able to program one BAR then surely all resources for the device are allocated:
// Indicate pci bus driver has allocated
// resource for this device
// It might be a temporary solution here since
// pci device could have multiple bar
Node->PciDev->Allocated = TRUE;
Which simply isn't the case.