[U-Boot] [RFC v2 00/15] dm, efi: integrate efi objects into DM
Simon Glass
sjg at chromium.org
Tue Mar 19 01:25:37 UTC 2019
Hi Heinrich,
On Tue, 12 Feb 2019 at 17:53, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
>
>
> 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.
I think you might be describing some other bootloader :-) I think it's
fine to add EFI support to U-Boot but I don't like the idea of moving
the internal structure to that. Overall I find EFI confusing and
overly complicated.
>
> 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.
That sounds good.
>
> >
> > 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.
Yes that's right, but I think that makes sense to have that support in
U-Boot itself rather than out on a limb with EFI. It is natural
consequence of fully using DM for EFI.
>
> >
> > 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)
I need to make time to read the EFI spec another 5 times :-) As I
understand it the protocol corresponds to the operations in a U-Boot
uclass.
>
> > 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?
I don't know about that, it doesn't seem useful.
>
> >
> >>>
> >>> 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.
What is the use case to changing the device path protocol?
>
> >
> >>>
> >>>> - 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.
That sound right to me.
>
> >
> >>>> * 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.
Regards,
Simon
>
> 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