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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115887): https://edk2.groups.io/g/devel/message/115887
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]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to