Hi,
I was originally responsible for suggesting this change internally(more specifically - to switch from Intel specific LPSS UART driver to EDK2 driver which on inspection seemed to cover all relevant modes of operations of LPSS UART).
First I would like to explain how exactly we are using this driver when LPSS UART is used for debug messages:
1. LPSS UART is a PCI device(although it can be put into hidden mode where standard PCI enumerator doesn't see it, this is the mode we use for debug) integrated in a chipset.
2. PciSioSerialDxe driver will be placed in apriori section of the DXE FW to ensure it is loaded early the same goes for a special platform specific driver.
3. During EarlyDxe platform specific driver setup LPSS UART and install PCI_IO_PROTOCOL and DEVICE_PATH_PROTOCOL instance for LPSS UART. Setup includes assigning MMIO, enabling memory path to LPSS UART etc.
4. Next platform specific module will call connect controller on a handle with installed PCI_IO_PROTOCOL and DEVICE_PATH_PROTOCOL. Note that it will only connect LPSS UART(and any other critical device). It will not connect entire system. In fact connecting entire system is not possible as majority of other PCI drivers(including PciBus) are not loaded at this point.
5. PciSioSerialDxe performs normal binding flow at this point and produces SERIAL_IO_PROTOCOL
6. When other modules call DEBUG() macro our DebugLib will try to locate that SERIAL_IO_PROTOCOL and if located print over it. NOTE: there is no additional depex introduced to modules that use debug lib. If protocol is not present debug will simply not happen.
Next I want to go broadly over concerns raised in this thread:
1. UEFI driver model violation
To be honest this is the first time I am hearing that DXE_DRIVER shouldn't or can't install EFI_DRIVER_BINDING_PROTOCOL or use PCI_IO_PROTOCOL. I checked in PI specification to see whether it places any restrictions on DXE_DRIVERS and the protocols it can consume/produce and it seems like it doesn't. In fact it explicitly states that DXE drivers can be UEFI driver model compliant.
from: https://uefi.org/specs/PI/1.8/V2_DXE_Drivers.html#dxe-drivers
As for UEFI driver writes guide I really don't see any restrictions placed on DXE drivers in this spec, it seems to be mostly focused on describing types of UEFI drivers and their typical behavior.
2. Rewrite the change so that it doesn't use PCI_IO_PROTOCOL and driver binding
While possible I think this is a really bad idea that doesn't provide much value and only introduces additional interfaces for edk2 community to maintain.
- Replacing PCI_IO_PROTOCOL with PciSegment and friends would still require to pass information such as BDF, MMIO and mechanism to enable memory decode. So we still need some protocol interface. We could potentially drop PCI_IO entirely and instead say if you are in early DXE use EFI_SIO_PROTOCOL, but that one is less robust than PCI_IO. Note that in our implementation we do not require full PCI enumeration since LPSS UART is placed outside of the main PCI tree(situation is somewhat similar to IncompatiblePciDeviceSupport.c in EDK2).
- Rewriting driver binding introduces additional cost without any real benefit I believe. Having driver binding allows the platform to call ConnectController when it is finished initializing enough of the HW so that UART can work. Dropping driver binding and instead doing everything in entry point would require depending on driver dispatch order or on some additional protocol which would be placed in depex section of PciSioSerialDxe and installed by platform driver. It would also complicate the way in which we would support multiple UART devices which can be ready to operate on different stages in boot.
- Other drawbacks - if we have completely separate entry point for DXE driver we need to keep 2 EFI modules in flash - 1 DXE only UART driver and 2nd UEFI only UART driver. As it is now we can simply keep the DXE_DRIVER instance in flash and it supports both early UARTS that are used for debug and late UARTS that are used for console redirection.
Thanks,
Mateusz