Switch BcmGenetDxe, DisplayDxe and DwUsbHostDxe to use gEdkiiNonDiscoverableDeviceProtocolGuid

[samerhaj]

This includes the following

• [x] BcmGenetDxe
• [ ] DisplayDxe
• [ ] DwUsbHostDxe

We need to use the proper mechanisim in EDK2 for driver model drivers of non-discoverable devices using gEdkiiNonDiscoverableDeviceProtocolGuid and the helper library NonDiscoverableDeviceRegistrationLib

Explanation summary courtesy of @ardbiesheuvel :

The idea is to install gEdkiiNonDiscoverableDeviceProtocolGuid instances in platform DXE driver that does not have any knowledge about the driver hardware itself, other than the resources needed for the platform (such as the MMIO base address of the IP block, and other device specific properties that are needed like PHY address). This installation should happen in an event notification callback that is registered for EndOfDxe. This ensures that the protocols are installed at the right time, but not any earlier.

Then the actual device driver that implements the UEFI Driver Model checks in its Supported() for existence of gEdkiiNonDiscoverableDeviceProtocolGuid on the input handle, and if so, checks the GUID to match the supported devices.

See for example implementations in

• Socionext NetsecDxe driver code , and platform code

• Socionext SynQuacerI2cDxe (driver code and platform code)

• NXP I2cDxe (driver code and platform code)

• DesignWare DwEmacSnpDxe (driver code , no sample platform code)

[ardbiesheuvel]

So the idea is that we can take advantage of the UEFI driver model for the following reasons:

• natural split between platform and device drivers
• drivers can be (re)connected and/or (un)loaded at will
• drivers are only connected automatically if they are needed to connect the device path of the boot target

The latter point is especially significant: you don't want to 'connect' 4 NICs using DHCP requests etc if you are booting from the SATA drive.

The I2C examples may be useful, although it is not entirely clear why the I2C subsystem was coerced to fit the driver model in the first place, given that you don't usually boot from a I2C device. So let's disregard those for now.

The NetSec driver is a better example, although I just realized it is not 100% complete. In particular, what is missing is the device path for the non-discoverable node, and so we are currently relying on an obscure (mis)feature that connects handles that are installed in the entry point of a DXE driver (this is related to UEFI 1.02 compatibility). For instance, the HTTPv4 boot device path on the SynQuacer currently looks like this

MAC(408d5cb1e6be,1)/IPv4(0.0.0.00.0.0.0,0,0)

while it should really look something like

VenHw(<guid>,xxx)/MAC(408d5cb1e6be,1)/IPv4(0.0.0.00.0.0.0,0,0)

where the first part is the device path that gets installed onto the handle that carries the non-discoverable protocol as well.

The difference is that, given the full device path above, the UEFI driver model subsystem knows that, in order to instantiate the whole device path, it has to connect the first part, which will result in the non-discoverable node handle to be passed to each registered Supported() method, including the one implemented by the NetSec NIC driver.

I'll take this as an action to fix the SynQuacer platform, and please take this into account for future upstream contributions to RPix as well.

[andreiw]

Also added DisplayDxe to the set of drivers that need fixing

[ardbiesheuvel]

I fixed the SynQuacer, but I went with the slightly different approach than I suggested above: the PlatformDxe driver instantiates a handle with the non-discoverable device protocol installed, using driver specific GUID and a descriptor array containing the MMIO base address, and an instance of the device path protocol equivalent to MAC(408d5cb1e6be,1)/

Since the associated driver model SNP driver is not a bus driver, it does not instantiate new handles, and there is no need to disambiguate them using device paths. Now it longer installs a device path at all, it simply installs the SNP protocol onto the same handle.

The end result is that, at EndOfDxe, only MAC(408d5cb1e6be,1)/ exists in the handle database, but any attempt on the part of the BDS to connect a device path that starts with MAC(408d5cb1e6be,1)/ will recursively connect all the required drivers.

[andreiw]

BcmGenetDxe is not in the upstream yet. Do we have consensus is that it needs to be fixed in this fashion before it is submitted?

[samerhaj]

First pass of this (for GENET) is in this branch: https://github.com/pftf/edk2-platforms/tree/pi4_genet_samer

This installs gEdkiiNonDiscoverableDeviceProtocolGuid (as well as DevicePath protocol) in ConfigDxe (in an event notification callback that is registered for EndOfDxe).

The GENET driver then uses that protocol (and looks for GUID match of gBcmNetNonDiscoverableDeviceGuid) to bind to the controller handle.

We still need to pass useful "platform" information from ConfigDxe to BcmGenetDxe using the NON_DISCOVERABLE_DEVICE handle

[ardbiesheuvel]

On Sun, 19 Apr 2020 at 19:21, Samer El-Haj-Mahmoud [email protected] wrote:

First pass of this (for GENET) is in this branch: https://github.com/pftf/edk2-platforms/tree/pi4_genet_samer

This installs gEdkiiNonDiscoverableDeviceProtocolGuid (as well as DevicePath protocol) in ConfigDxe (in an event notification callback that is registered for EndOfDxe).

The GENET driver then uses that protocol (and looks for GUID match of gBcmNetNonDiscoverableDeviceGuid) to bind to the controller handle.

We still need to pass useful "platform" information from ConfigDxe to BcmGenetDxe using the NON_DISCOVERABLE_DEVICE handle

Nice. I suppose the only platform information is the MMIO base address, right?

[benjaminmordaunt]

Hi, @ardbiesheuvel, I'm fairly new to EDK2 so apologies if there are some daft questions. I'm also coming at this from an Arm background instead of an Intel one.

I'm still struggling to understand the value of installing gEdkiiNonDiscoverableDeviceProtocolGuid to handles. Going over your points:

natural split between platform and device drivers

From what I can see, the only current platform information being conveyed to device driver is the MMIO base/range information, and whatever other information EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR provides. Why can't this be simply described with platform/silicon-specific header files containing such data/Pcds etc., instead of adding this plumbing?

drivers can be (re)connected and/or (un)loaded at will

Sorry I don't understand how this protocol helps here. Why does a device driver need this protocol to load/unload at will? What does "at will" mean specifically?

drivers are only connected automatically if they are needed to connect the device path of the boot target

This sounds like a nice feature, but I don't understand the whole "vendor device path" idea? What is it and how does it work? How does it ensure that a controller is only initialised if needed? Thanks.

[ardbiesheuvel]

Hi, @ardbiesheuvel, I'm fairly new to EDK2 so apologies if there are some daft questions. I'm also coming at this from an Arm background instead of an Intel one.

I take it you mean embedded, right?

I'm still struggling to understand the value of installing gEdkiiNonDiscoverableDeviceProtocolGuid to handles. Going over your points:

natural split between platform and device drivers

From what I can see, the only current platform information being conveyed to device driver is the MMIO base/range information, and whatever other information EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR provides. Why can't this be simply described with platform/silicon-specific header files containing such data/Pcds etc., instead of adding this plumbing?

That would compile this into the driver, rather that into the the platform, making the driver binary (which is a separate PE/COFF executable) specific to this platform build. This is not necessarily a problem, but generally, it is nice to put the abstractions where they belong, i.e., in the description of the platform that incorporates the IP rather than the driver for the IP itself.

drivers can be (re)connected and/or (un)loaded at will

Sorry I don't understand how this protocol helps here. Why does a device driver need this protocol to load/unload at will? What does "at will" mean specifically?

It means it will get listed as a driver when you run the 'drivers' command in the UEFI shell, and you can do 'unload XX' to unload it, and load a different version on the fly. This means you could, e.g., distribute a DEBUG version of the driver and plug it in on a deployed system.

drivers are only connected automatically if they are needed to connect the device path of the boot target

This sounds like a nice feature, but I don't understand the whole "vendor device path" idea? What is it and how does it work? How does it ensure that a controller is only initialised if needed? Thanks.

Bringing up a NIC takes time, in the order of seconds when DHCP queries are involved.

So when not using the driver model and using PCDs etc, you always take this penalty, given that the driver is always loaded, unless you add special bespoke handling to your driver to avoid it.

If you do use the driver module, the driver core will only call into the driver if the current boot path is a network URL, and if you are booting from disk, it will only load the block device drivers.

In general, the boot path approach is intended to do the least amount of work to get to the boot image as soon as possible, instead of discovering and initializing every peripheral and never use them.

This also means, though, that implementing this for the USB host controller is not as important, given that we need to connect it to decide whether or not a USB keyboard is attached, in order for the ESC / F1 key detection to work. But not calling into the NIC driver unless needed is a nice win for boot speed.

[benjaminmordaunt]

Thanks, points 1 & 2 understood.

If you do use the driver module, the driver core will only call into the driver if the current boot path is a network URL, and if you are booting from disk, it will only load the block device drivers.

In general, the boot path approach is intended to do the least amount of work to get to the boot image as soon as possible, instead of discovering and initializing every peripheral and never use them.

Yes, I understand the concept. So I'm presuming that the VenHw (hardware vendor device path) node is the piece that maps to a specific platform, with the non-discoverable device protocol attached. This piece is then forwarded to generic device drivers down the chain. OK, I think I understand. Thanks.

[ardbiesheuvel]

So I'm presuming that the VenHw (hardware vendor device path) node is the piece that maps to a specific platform, with the non-discoverable device protocol attached. This piece is then forwarded to generic device drivers down the chain.

Indeed. This is what allows you to describe a network boot source.

For instance, when passing MAC(00001a1bb328,1)/IPv4(0.0.0.00.0.0.0,0,0)/Uri() to ConnectController(), it will use LocateDevicePath() to retrieve the associated protocol handle of each component, and pass it to each available device driver in turn, until it finds one that can consume any of the other protocols installed on the same handle, and produce a new protocol, e.g., SNP, which will be consumed by the driver that implements support for IPv4().