[U-Boot] [RFC v2 00/15] dm, efi: integrate efi objects into DM

Heinrich Schuchardt xypron.glpk at gmx.de
Tue Feb 12 09:47:44 UTC 2019



On 2/12/19 8:24 AM, AKASHI Takahiro wrote:
> Hi Heinrich, Simon,
> 
> On Sat, Feb 09, 2019 at 05:00:33PM -0600, Simon Glass wrote:
>> Hi Heinrich,
>>
>> On Fri, 8 Feb 2019 at 03:36, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>>
>>>
>>>
>>> On 2/8/19 9:15 AM, AKASHI Takahiro wrote:
>>>> # bootefi doesn't work with this patch set yet
>>>>
>>>> This patch set came from the past discussion[1] on my "removable device
>>>> support" patch and is intended to be an attempt to integrate efi objects
>>>>  into u-boot's Driver Model as much seamlessly as possible.
>>>>
>>>> [1] https://lists.denx.de/pipermail/u-boot/2019-January/354010.html
>>>>
>>>> Since this patch is a prototype (or POC, Proof-Of-Concept), the aim here
>>>> is to discuss further about how in a better shape we will be able to
>>>> merge the two worlds.
>>>>
>>>> After RFC, Simon suggested that efi protocols could be also presented
>>>> as DM devices. This is a major change in RFC v2.
>>>>
>>> Hello Takahiro,
>>>
>>> thanks a lot for laying out your thoughts about a possible integration of
>>> the EFI subsystem and the driver model. Thanks also for providing a first
>>> implementation.
>>
>> Yes indeed. It is very clever what you have been able to do Takahiro.
> 
> I think that I'm going to extremes here :)

The other extreme is what EDK2 does. They live with an EFI only model.

I think moving the DM model in this direction would be feasible and
would be much more consistent than trying to map EFI objects onto DM
structures with different semantics. But I do not see the man power to
do such a change.

Between the two extremes is:

- link the worlds via pointers
- move protocol implementations into the respective uclasses
- endow these uclasses with an implementation of the driver
  binding protocol

This is feasible with the available capacity and sufficient to cover
your use case of plugable devices.

> 
> I wonder what the EFI world will look like if all the handles
> and protocols are also DM devices.
> I don't expect that my patch will be upstreamed any time soon
> (or possibly forever not) So, instead of claiming the change
> would be meaningless, I'd welcome any suggestions, like what will
> happen if we merge/integrate EFI's A with U-Boot's B?
> 
> I'm willing to make best efforts to give such an idea a reality
> if possible. Then choices come after that.
> 
>>>
>>>> Basic idea is
>>>> * efi_root is a DM device
>>>> * Any efi object, refered to by efi_handle_t in UEFI world,
>>>>   has a corresponding DM device.
>>>
>>> EFI applications and drivers will create handles having no relation to
>>> the U-Boot world.
>>
>> I suggest that we change that, i.e. that all devices in existence have
>> a struct udevice. That way DM knows about everything and we don't have
>> the strange parallel 'EFI' world. I don't see any need for it.
> 
> Simply, it would be nice that we can list all the applications
> and drivers loaded at one place, akin to linux's
>   * ls /proc/
>   * cat /proc/modules
> 
> (From the viewpoint of API, we can do that just by calling
> locate_handle(BY_PROTOCOL, EFI_LOADED_IMAGE, ...) though.)
> 
>>>
>>>>   - define efi_handle_t as "struct udevice *"
>>>
>>> An EFI handle does not necessarily relate to any U-Boot device. Why
>>> should a handle which has not backing device carry the extra fields of
>>> struct udevice?
>>
>> Because this is the U-Boot driver model. We should not have an EFI
>> parallel to DM and certainly not just to save a few dozen bytes of
>> data space. If you were trying to save data space, you would not use
>> EFI :-)
> 
> Ah, thank you.
>>From a viewpoint of implementation, the situation where some handles
> are DM devices and some are not could make the efi code, particularly
> boottime.c, quite ugly and complicated.
> 
>>>
>>>>   - for efi_disk,
>>>>     * add "struct efi_disk_obj" to blk_desc
>>>
>>> struct efi_disk_obj * is currently the handle of the block device. So
>>> you only some fields may be moved to blk_desc. But I guess I will find
>>> that in one of the patches.
> 
> This is definitely a future work item.
> In this case, however, blk_desc should also be able to represent
> a partition.
> 
>>>>   - for the objects below, there is only one instance for each and so
>>>>     they are currently global data:
>>>>       efi_gop_obj,
>>>>       efi_net_obj,
>>>
>>> efi_net_obj * is the handle of the network device. In future we should
>>> support multiple network devices.
> 
> It will be a natural extension.
> 
>>>>       simple_text_output_mode
>>>>   - for loaded_image,
>>>>     * link efi_loaded_image_obj to device's platdata
>>>
>>> An EFI application can create an image out of "nothing". Just create a
>>> handle with InstallProtocolInterface() and then call LoadImage() with a
>>> pointer to some place in memory.
>>>
>>> Many images loaded from the same device may be present at the same time,
>>> e.g. iPXE, GRUB, and Linux.
> 
> I don't get your point here, but please notice that a "loaded image"
> is more or less portion of main memory with loaded code.
> iPXE, GRUB and Linux are the same in this respect.

Why do you want to treat such a memory area as a separate device?

> 
>>>
>>>>
>>>> * Any efi protocol has a corresponding DM device.
>>>
>>> Protocol implementations are not only provided by U-Boot but also by EFI
>>> applications and driver binaries. And of cause the EFI binaries will
>>> implement a lot of protocols completely unknown to U-Boot. So what
>>> should be the meaning of the above sentence in this context?
>>
>> Can we instead add a uclass for each EFI protocol? Then U-Boot does
>> know about them.
> 
> Yeah, I thought about defining an uclass for each EFI protocol.
> Given that a protocol defines a protocol-specific set of function
> interfaces in most cases, it will be natural to define a separate
> uclass.

An EFI binary can implement any EFI protocol that only exists for this
special application. E.g. somebody could write an EFI driver
implementing NVME over TCP.

Requiring that U-Boot has a uclass for every protocol would mean that at
U-Boot compile time you would already have to define which EFI binaries
a user is able to load. E.g. if a uclass for NVME over TCP does not
exist a user would not be able to run above hypothetical binary.

> 
> On the other hand, this will make it a bit complicated to determine
> whether a given handle is an efi object or efi protocol in DM tree.
> 

In EFI terminology a protocol is not a handle but an abstract interface.
The implementation of a protocol is called protocol interface.

A protocol interface may be installed on one or more handles. But it
should not be enumerated by LocateHandles(SearchType = AllHandles)

> Regarding a protocol "unknown to U-Boot," it is kinda headache
> as we can invent a totally *original* protocol which is unknown at
> compile time of U-Boot.
> That is one of reasons why all the protocols have the same type
> of uclass in my current implementation.
> (I don't think there is any way to define uclass dynamically.)

What is the benefit of protocol interface pointing to a uclass that
doesn't implement the protocol?

> 
>>>
>>> Above you suggested that struct udevice * would be used as a handle.
>>> So on which handle is the protocol now installed in your model?
> 
> "efi_add_protocol" take a handle as a first argument, which is
> set to a parent of that protocol.
> If a handle is NULL here, a generated protocol handle will be
> temporarily attached to efi_root.
> 
>>> For a protocol like the device path protocol which is only a data
>>> structure with no related function modules I do not understand the
>>> benefit of creating a separate sub-device.
>>
>> I think it is only a matter of convenience and to keep things regular.
> 
> Unifying device path hierarchy to DM tree is another challenge. 

As an application may change the device path protocol of a handle at any
time and we do not want to mess up the DM tree I would not see that this
is possible.

> 
>>>
>>>>   - link "struct efi_handler" to device's uclass_platdata
>>>
>>> struct efi_handler is an item in the list of protocols installed on a
>>> handle. For some of the protocols installed by an EFI application there
>>> will not be any corresponding uclass.
>>
>> As above, perhaps we should fix that?
> 
> As I said above, I recognize that this is an issue.
> 
>>>
>>>>   - be a child of a efi object (hence DM device) in DM device hierarchy
>>>>     so that enumerating protocols belonging to efi object is done by
>>>>     traversing the tree.
>>>>
>>>
>>> Above you said a struct udevice * would be a handle. So here you imply
>>> that for each protocol interface you will create an extra handle. That
>>> does not fit the EFI model.
> 
> Please don't interpret the concept to such an extent.
> While, say, a GPIO has a DM device (and efi handle in this sense),
> is it also an efi object? No.
> 
>>>
>>>> * Any efi object which has a backing DM device should be created
>>>>   when that DM device is detected (and probed).
>>>
>>> Detected or probed? This is not the same.
> 
> So what is your suggestion here?

U-Boot follows the idea of late probing. So I guess we want an EFI
handle to be created when the DM device is detected and probe it when
the installed protocol is used for the first time.

> 
>>>> * For efi_disk (or any object with EFI_BLOCK_IO_PROTOCOL),
>>>>   - add UCLASS_PARTITION
>>>>   - put partitions under a raw block device
>>>>   - partitions as well as raw devices can be efi_disk
>>>
>>> Agreed.
>>>
>>>>
>>>> With those extensive changes, there still exists plenty of
>>>> "wrapper" code. Do you have any idea to reduce it?
>>>>
>>>
>>> The concept presented does not cover:
>>>
>>> - device, drivers, protocols created by EFI applications and
>>>   driver binaries
> 
> I definitely want to see a good example so that I will investigate.
> 
>> Can we create uclasses for each 'protocol'? Is there any reason why we cannot?
> 
> I think that I partly answered to you above.
> 
>>> - non-DM drivers and devices in U-Boot
>>
>> This doesn't really matter as they will be gone soon. At the risk of
>> repeating myself, EFI support should never have supported non-DM in
>> the first place. It was not the right decision, in my view.
> 
> OK
> 
>>>
>>> It creates extra handles per installed protocol interface which should
>>> not exist in the EFI world.
>>>
>>> So some rework of the concept is needed.
>>>
>>> I suggest to start smaller:
>>>
>>> - convert partitions to the DM model.
>>
>> This is in later patches from what I can tell.

I would put it at the beginning as it is required for your use case of
plugable devices.

Best regards

Heinrich

> 
> Yeah, to some extent.
> As I said above, blk_desc should be extended to support disk
> partitions on its own.
> 
>>> - provide a pointer serving as EFI handle in struct udevice
>>
>> I actually feel that the approach here, while admittedly bold, seems
>> to be a good step forward.
>>
>> Regards,
>> Simon
>>
>>>
>>> Best regards
>>>
>>> Heinrich
> 
> Thank both of you for valuable comments.
> 
> -Takahiro Akashi
> 
>>>
>>>>
>>>> ***** Example operation ******
>>>> (Two scsi disks, one with no partition, one with two partitions)
>>>>
>>>> => efi dev
>>>> EFI: Initializing UCLASS_EFI_DRIVER
>>>> Device           Device Path
>>>> ================ ====================
>>>> 000000007eef9470 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
>>>> => scsi rescan
>>>>
>>>> Reset SCSI
>>>> scanning bus for devices...
>>>> Target spinup took 0 ms.
>>>> Target spinup took 0 ms.
>>>> SATA link 2 timeout.
>>>> SATA link 3 timeout.
>>>> SATA link 4 timeout.
>>>> SATA link 5 timeout.
>>>> AHCI 0001.0000 32 slots 6 ports 1.5 Gbps 0x3f impl SATA mode
>>>> flags: 64bit ncq only
>>>>   Device 0: (0:0) Vendor: ATA Prod.: QEMU HARDDISK Rev: 2.5+
>>>>             Type: Hard Disk
>>>>             Capacity: 16.0 MB = 0.0 GB (32768 x 512)
>>>>   Device 0: (1:0) Vendor: ATA Prod.: QEMU HARDDISK Rev: 2.5+
>>>>             Type: Hard Disk
>>>>             Capacity: 256.0 MB = 0.2 GB (524288 x 512)
>>>> => efi dev
>>>> Device           Device Path
>>>> ================ ====================
>>>> 000000007eef9470 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
>>>> 000000007ef01c90 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)
>>>> 000000007ef04910 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)
>>>> 000000007ef04ee0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(335544330,MBR,0x086246ba,0x17ff4f1a0,0x7eee3770)
>>>> 000000007ef055a0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(335544330,MBR,0x086246ba,0x17ff4f1a0,0x7eee3770)
>>>> => dm tree
>>>>  Class    index  Probed  Driver                Name
>>>> -----------------------------------------------------------
>>>>  root        0  [ + ]   root_driver           root_driver
>>>>  simple_bus  0  [   ]   generic_simple_bus    |-- platform at c000000
>>>>  virtio      0  [ + ]   virtio-mmio           |-- virtio_mmio at a000000
>>>>
>>>>  [snip]
>>>>
>>>>  pci         0  [ + ]   pci_generic_ecam      |-- pcie at 10000000
>>>>  pci_generi  0  [   ]   pci_generic_drv       |   |-- pci_0:0.0
>>>>  virtio      32  [   ]   virtio-pci.l          |   |-- virtio-pci.l#0
>>>>  ahci        0  [ + ]   ahci_pci              |   `-- ahci_pci
>>>>  scsi        0  [ + ]   ahci_scsi             |       `-- ahci_scsi
>>>>  blk         0  [ + ]   scsi_blk              |           |-- ahci_scsi.id0lun0
>>>>  efi_protoc  8  [ + ]   efi_disk              |           |   |-- BLOCK_IO
>>>>  efi_protoc  9  [ + ]   efi_device_path       |           |   `-- Scsi(0,0)
>>>>  blk         1  [ + ]   scsi_blk              |           `-- ahci_scsi.id1lun0
>>>>  efi_protoc  10  [ + ]   efi_disk              |               |-- BLOCK_IO
>>>>  efi_protoc  11  [ + ]   efi_device_path       |               |-- Scsi(1,0)
>>>>  partition   0  [ + ]   blk_partition         |               |-- ahci_scsi.id1lun0:1
>>>>  efi_protoc  12  [ + ]   efi_disk              |               |   |-- BLOCK_IO
>>>>  efi_protoc  13  [ + ]   efi_device_path       |               |   |-- HD(335544330,MBR,0x086246ba,0x17ff4f1a0,0x7eee3770)
>>>>  efi_protoc  14  [ + ]   efi_simple_file_syst  |               |   `-- SIMPLE_FILE_SYSTEM
>>>>  partition   1  [ + ]   blk_partition         |               `-- ahci_scsi.id1lun0:2
>>>>  efi_protoc  15  [ + ]   efi_disk              |                   |-- BLOCK_IO
>>>>  efi_protoc  16  [ + ]   efi_device_path       |                   |-- HD(335544330,MBR,0x086246ba,0x17ff4f1a0,0x7eee3770)
>>>>  efi_protoc  17  [ + ]   efi_simple_file_syst  |                   `-- SIMPLE_FILE_SYSTEM
>>>>  rtc         0  [   ]   rtc-pl031             |-- pl031 at 9010000
>>>>  serial      0  [   ]   serial_pl01x          |-- pl011 at 9050000
>>>>  serial      1  [ + ]   serial_pl01x          |-- pl011 at 9000000
>>>>  efi_protoc  0  [ + ]   efi_simple_text_outp  |   |-- SIMPLE_TEXT_OUTPUT
>>>>  efi_protoc  1  [ + ]   efi_simple_text_inpu  |   |-- SIMPLE_TEXT_INPUT
>>>>  efi_protoc  2  [ + ]   efi_simple_text_inpu  |   `-- SIMPLE_TEXT_INPUT_EX
>>>>  mtd         0  [ + ]   cfi_flash             |-- flash at 0
>>>>  firmware    0  [ + ]   psci                  |-- psci
>>>>  sysreset    0  [   ]   psci-sysreset         |   `-- psci-sysreset
>>>>  efi         0  [ + ]   efi_root              `-- UEFI sub system
>>>>  efi_protoc  3  [ + ]   efi_device_path           |-- VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
>>>>  efi_protoc  4  [ + ]   efi_device_path_to_t      |-- DEVICE_PATH_TO_TEXT
>>>>  efi_protoc  5  [ + ]   efi_device_path_util      |-- DEVICE_PATH_UTILITIES
>>>>  efi_protoc  6  [ + ]   efi_unicode_collatio      |-- en
>>>>  efi_driver  0  [ + ]   EFI block driver          `-- EFI block driver
>>>>  efi_protoc  7  [ + ]   efi_driver_binding            `-- DRIVER_BINDING
>>>>
>>>>
>>>>
>>>> Thanks,
>>>> -Takahiro Akashi
>>>>
>>>> AKASHI Takahiro (15):
>>>>   efi_loader: efi objects and protocols as DM devices
>>>>   efi_loader: boottime: convert efi_loaded_image_obj to DM
>>>>   efi_loader: image_loader: aligned with DM
>>>>   efi_driver: rename UCLASS_EFI to UCLASS_EFI_DRIVER
>>>>   efi_loader: convert efi_root_node to DM
>>>>   efi_loader: device path: convert efi_device_path to DM
>>>>   efi_loader: unicode_collation: converted to DM
>>>>   efi_loader: console: convert efi console input/output to DM
>>>>   efi_loader: net: convert efi_net_obj to DM
>>>>   efi_loader: gop: convert efi_gop_obj to DM
>>>>   dm: blk: add UCLASS_PARTITION
>>>>   efi_loader: disk: convert efi_disk_obj to DM
>>>>   drivers: align block device drivers with DM-efi integration
>>>>   efi_driver: converted to DM
>>>>   cmd: efidebug: aligned with DM-efi integration
>>>>
>>>>  cmd/bootefi.c                              |  61 +--
>>>>  cmd/efidebug.c                             |   5 +-
>>>>  common/usb_storage.c                       |  27 +-
>>>>  drivers/block/blk-uclass.c                 |  61 +++
>>>>  drivers/scsi/scsi.c                        |  22 +
>>>>  drivers/serial/serial-uclass.c             |   6 +
>>>>  drivers/video/video-uclass.c               |   9 +
>>>>  include/blk.h                              |  24 +
>>>>  include/dm/device.h                        |   3 +
>>>>  include/dm/uclass-id.h                     |   6 +-
>>>>  include/efi.h                              |   4 +-
>>>>  include/efi_loader.h                       |  50 +-
>>>>  lib/efi_driver/efi_block_device.c          |  36 +-
>>>>  lib/efi_driver/efi_uclass.c                |  37 +-
>>>>  lib/efi_loader/efi_boottime.c              | 605 ++++++++++++++-------
>>>>  lib/efi_loader/efi_console.c               |  64 ++-
>>>>  lib/efi_loader/efi_device_path.c           | 136 +++--
>>>>  lib/efi_loader/efi_device_path_to_text.c   |  55 ++
>>>>  lib/efi_loader/efi_device_path_utilities.c |  14 +
>>>>  lib/efi_loader/efi_disk.c                  | 216 +++++---
>>>>  lib/efi_loader/efi_file.c                  |  14 +
>>>>  lib/efi_loader/efi_gop.c                   |  28 +-
>>>>  lib/efi_loader/efi_image_loader.c          |  61 ++-
>>>>  lib/efi_loader/efi_net.c                   |  50 +-
>>>>  lib/efi_loader/efi_root_node.c             |  14 +-
>>>>  lib/efi_loader/efi_setup.c                 |  60 +-
>>>>  lib/efi_loader/efi_unicode_collation.c     |  19 +
>>>>  net/eth-uclass.c                           |   5 +
>>>>  28 files changed, 1226 insertions(+), 466 deletions(-)
>>>>
> 


More information about the U-Boot mailing list