[U-Boot] [RFC v2 00/15] dm, efi: integrate efi objects into DM
AKASHI Takahiro
takahiro.akashi at linaro.org
Tue Feb 12 07:24:56 UTC 2019
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 :)
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.
> >
> > >
> > > * 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.
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.
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.)
> >
> > 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.
> >
> > > - 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?
> > > * 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.
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