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. > 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. > - 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? > - 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? 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. > - 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. > - 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 - non-DM drivers and devices in U-Boot 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. - provide a pointer serving as EFI handle in struct udevice 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(-) > _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
