Hi Heinrich,
On Fri, 8 Feb 2019 at 03:36, Heinrich Schuchardt xypron.glpk@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.
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.
- 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 :-)
- 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.
- 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.
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.
- 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.
Above you suggested that struct udevice * would be used as a handle. So on which handle is the protocol now installed in your model?
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.
- 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?
- 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.
- 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.
- 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
Can we create uclasses for each 'protocol'? Is there any reason why we cannot?
- 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.
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.
- 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
***** 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@c000000 virtio 0 [ + ] virtio-mmio |-- virtio_mmio@a000000
[snip]
pci 0 [ + ] pci_generic_ecam |-- pcie@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@9010000 serial 0 [ ] serial_pl01x |-- pl011@9050000 serial 1 [ + ] serial_pl01x |-- pl011@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@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(-)