On 2/26/24 16:13, Albecki, Mateusz wrote: > 1. Using SerialDxe instead of PciSioSerialDxe - from our perspective > the main idea is to avoid maintaining our own implementation of > functions that actually communicate with UART controller. To use > SerialDxe we would have to still maintain our own SerialPortLib that > actually goes and sends data over UART. Also a note on LPSS UART - in > terms of registers it is just a standard UART controller, the only > quirk here is how to access it hence the custom PCI_IO_PROTOCOL > implementation.
Is it possible to factor out a common base library class (with just one instance) for accessing an LPSS UART? I guess the PCI B/D/F numbers would have to be passed to the individual functions. Then that library could be used in PciSioSerialDxe, and also in a (thin wrapper) SerialPortLib instance. > > 2. Using ConnectController before BDS - I noticed that the section I > quoted says that BDS will use ConnectController however it doesn't say > that it can't be used outside of that context. I did search the UEFI > spec to see whether it provides additional restrictions and the only > section that elaborates on this is the following: > /"Under the UEFI Driver Model , the act of connecting and > disconnecting drivers from controllers in a platform is under the > platform firmware’s control. This will typically be implemented as > part of the UEFI Boot Manager, but other implementations are possible. > " - from Section 2.5.6 Platform components/ It seems to be rather > permissive when it comes to who and when can call it. > > 3. How to make sure dispatch is early enough and not too late - this > will depend on the overall platform implementation. For our part - we > simply put it into flash as early as we can get away with. Even > apriori section isn't strictly necessary if the platform is > comfortable with relying on the fact that DXE core in MdeModulePkg > dispatches modules in order of their placement in flash(that's not > architectural as far as I know). Other platforms might choose to > introduce explicit depex on gEfiSerialIoProtocolGuid. To reiterate > this is the implementation that works for us in a sense that we get > logs from all modules that change frequently from generation to > generation, I understand that the same might not be true for platforms > other than Intel however I think majority of platforms could still > make at least some use of early UART debugging. > > 4. When exactly do we connect LPSS UART and start logging - we try to > be as early as possible for this interface. We miss all of DXE_CORE > logs(obviously), Pcd.inf and a couple of modules that implement some > of the architectural protocols(from PciSio perspective metronome is > the only actually required as far as we can tell since stall has to > work). > > I also want to note that I get why this is a controversial change. I > didn't realize it earlier but it would be the first DXE_DRIVER in EDK2 > tree that implements driver binding and in general it is strange to > have PCI device driver that could potentially dispatch before PCI bus > driver(however it is worth noting here that PciSioSerialDxe is not > just a PCI driver, it is a combo driver capable of supporting PCI and > SIO). That said I still think EDK2 in general needs a way to support > early device drivers and using DXE_DRIVER seems like the least > invasive idea. We need early drivers not just for debug(although that > is one of the most important use cases I would say) other important > use case is platform management through SMBUS/I2C/other serial > interface which might be required to even be able to enumerate full > PCI hierarchy(for instance some of the PCI slots might be powered down > and you need to power them on sending commands over I2C) or maybe > flash access to EFI variable storage(nothing says that you can't have > it connected to PCI SPI controller) or maybe GPIO control to do any > number of platform specific things. To my eyes, this series saves a bit of refactoring now and the maintenance of a thin wrapper library instance later, in exchange for a very unconventional solution in MdeModulePkg that is not easily reusable by other platforms. I don't like that trade-off; it makes me uncomfortable. If Liming and Mike accept this solution, I won't try to block it. In that case however, please document the design (all we've discussed thus far) in the "PciSioSerialDxeEarly.inf" file, in a (long) series of comments. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115971): https://edk2.groups.io/g/devel/message/115971 Mute This Topic: https://groups.io/mt/104469297/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
