| Message ID | 20190208081542.2813-1-takahiro.akashi@linaro.org |
|---|---|
| Headers | show |
| Series | dm, efi: integrate efi objects into DM | expand |
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(-) >
Hi, On Fri, 8 Feb 2019 at 01:14, AKASHI Takahiro <takahiro.akashi@linaro.org> 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 Is this pushed to a tree somewhere? If not, what commit does this series apply on top of? Regards, Simon
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(-) > >
Hi Takahiro, On Fri, 8 Feb 2019 at 02:14, AKASHI Takahiro <takahiro.akashi@linaro.org> 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 Some general comments: protocol_list: Can you use DM_GET_DRIVER? It should be more efficient efi_open_protocol_information: - rename of protocol to protocol_guid should be in a separate patch u-boot - please use 'U-Boot' consistently Your patch to rename UCLASS_EFI -> UCLASS_EFI_DRIVER still leaves UCLASS_EFI remaining. Can you mention why> It says efi_root is for backward compatibility. Just temporary? I could not quite figure that out. Use if (IS_ENABLED()) instead of #ifdef where you can. I am very encouraged by this series as it genuinely unifies EFI with DM. Re your comment about wrapper code, I suspect that might become clearer once the data structures are unified. Regards, Simon
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@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@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(-) > > >
Simon, On Sat, Feb 09, 2019 at 01:08:10PM -0700, Simon Glass wrote: > Hi, > > On Fri, 8 Feb 2019 at 01:14, AKASHI Takahiro <takahiro.akashi@linaro.org> 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 > > Is this pushed to a tree somewhere? If not, what commit does this > series apply on top of? Please take a look at: https://git.linaro.org/people/takahiro.akashi/u-boot.git/log/?h=efi/dm I don't think we need any prerequisite patch although I added my own patches like efidebug. Thanks, -Takahiro Akashi > Regards, > Simon
On Sat, Feb 09, 2019 at 05:04:19PM -0600, Simon Glass wrote: > Hi Takahiro, > > On Fri, 8 Feb 2019 at 02:14, AKASHI Takahiro <takahiro.akashi@linaro.org> 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 > > Some general comments: > > protocol_list: Can you use DM_GET_DRIVER? It should be more efficient Okay. > efi_open_protocol_information: > - rename of protocol to protocol_guid should be in a separate patch Okay, but I may will rename other argument names instead. > u-boot - please use 'U-Boot' consistently Sure. > Your patch to rename UCLASS_EFI -> UCLASS_EFI_DRIVER still leaves > UCLASS_EFI remaining. Can you mention why> > > It says efi_root is for backward compatibility. Just temporary? I > could not quite figure that out. The concept of "efi_root" is a discussion. > Use if (IS_ENABLED()) instead of #ifdef where you can. Okay > I am very encouraged by this series as it genuinely unifies EFI with > DM. Re your comment about wrapper code, I suspect that might become > clearer once the data structures are unified. Your comments also encourage me very much. Thanks! -Takahiro Akashi > Regards, > Simon
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@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@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(-) >>>> >
Hi Heinrich, On Tue, 12 Feb 2019 at 17:53, Heinrich Schuchardt <xypron.glpk@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@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@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(-) > >>>> > >
# 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. 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. - define efi_handle_t as "struct udevice *" - for efi_disk, * add "struct efi_disk_obj" to blk_desc - for the objects below, there is only one instance for each and so they are currently global data: efi_gop_obj, efi_net_obj, simple_text_output_mode - for loaded_image, * link efi_loaded_image_obj to device's platdata * Any efi protocol has a corresponding DM device. - link "struct efi_handler" to device's uclass_platdata - 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. * Any efi object which has a backing DM device should be created when that DM device is detected (and probed). * 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 With those extensive changes, there still exists plenty of "wrapper" code. Do you have any idea to reduce it? ***** 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(-)