Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: Load Serial driver earlier in DXE

["Albecki, Mateusz" <mateusz.albecki@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3921 bytes --]

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.

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.

Thanks,
Mateusz


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115968): https://edk2.groups.io/g/devel/message/115968
Mute This Topic: https://groups.io/mt/104469297/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 4402 bytes --]