[PATCH 00/19] efi_loader: more tightly integrate UEFI disks to driver model
Heinrich Schuchardt
xypron.glpk at gmx.de
Sat Feb 5 10:39:39 CET 2022
On 2/2/22 02:08, AKASHI Takahiro wrote:
> Background:
> ===========
> The purpose of this patch is to reignite the discussion about how UEFI
> subystem would best be integrated into U-Boot driver model.
> In the past, I poposed a couple of patch series, the latest one[1],
> while Heinrich revealed his idea[2], and the approach taken here is
> something between them, with a focus on block device handlings.
>
> Disks in UEFI world:
> ====================
> In general in UEFI world, accessing to any device is performed through
> a 'protocol' interface which are installed to (or associated with) the device's
> UEFI handle (or an opaque pointer to UEFI object data). Protocols are
> implemented by either the UEFI system itself or UEFI drivers.
>
> For block IO's, it is a device which has EFI_BLOCK_IO_PROTOCOL (efi_disk
> hereafter). Currently, every efi_disk may have one of two origins:
>
> a.U-Boot's block devices or related partitions
> (lib/efi_loader/efi_disk.c)
> b.UEFI objects which are implemented as a block device by UEFI drivers.
> (lib/efi_driver/efi_block_device.c)
>
> All the efi_diskss as (a) will be enumelated and created only once at UEFI
> subsystem initialization (efi_disk_register()), which is triggered by
> first executing one of UEFI-related U-Boot commands, like "bootefi",
> "setenv -e" or "efidebug".
> EFI_BLOCK_IO_PROTOCOL is implemented by UEFI system using blk_desc(->ops)
> in the corresponding udevice(UCLASS_BLK).
>
> On the other hand, efi_disk as (b) will be created each time UEFI boot
> services' connect_controller() is executed in UEFI app which, as a (device)
> controller, gives the method to access the device's data,
> ie. EFI_BLOCK_IO_PROTOCOL.
>
>>>> more details >>>
> Internally, connect_controller() search for UEFI driver that can support
> this controller/protocol, 'efi_block' driver(UCLASS_EFI) in this case,
> then calls the driver's 'bind' interface, which eventually installs
> the controller's EFI_BLOCK_IO_PROTOCOL to efi_disk object.
> 'efi_block' driver also create a corresponding udevice(UCLASS_BLK) for
> * creating additional partitions efi_disk's, and
> * supporting a file system (EFI_SIMPLE_FILE_SYSTEM_PROTOCOL) on it.
> <<< <<<
>
> Issues:
> =======
> 1. While an efi_disk represents a device equally for either a whole disk
> or a partition in UEFI world, the driver model treats only a whole
> disk as a real block device or udevice(UCLASS_BLK).
>
> 2. efi_disk holds and makes use of "blk_desc" data even though blk_desc
> in plat_data is supposed to be private and not to be accessed outside
> the driver model.
> # This issue, though, exists for all the implmenetation of U-Boot
> # file systems as well.
>
> For efi_disk(a),
> 3. A block device can be enumelated dynamically by 'scanning' a device bus
> in U-Boot, but UEFI subsystem is not able to update efi_disks accordingly.
> For examples,
> => scsi rescan; efidebug devices
> => usb start; efidebug devices ... (A)
> (A) doesn't show any usb devices detected.
>
> => scsi rescan; efidebug boot add -b 0 TEST scsi 0:1 ...
> => scsi rescan ... (B)
> => bootefi bootmgr ... (C)
> (C) may de-reference a bogus blk_desc pointer which has been freed by (B).
> (Please note that "scsi rescan" removes all udevices/blk_desc and then
> re-create them even if nothing is changed on a bus.)
>
> For efi_disk(b),
> 4. A "controller (handle)", combined with efi_block driver, has no
> corresponding udevice as a parent of efi_disks in DM tree, unlike,
> say, a scsi controller, even though it provides methods for block io
> operations.
> 5. There is no way supported to remove efi_disk's even after
> disconnect_controller() is called.
>
>
> My approach:
> ============
> Due to functional differences in semantics, it would be difficult
> to identify "udevice" structure as a handle in UEFI world. Instead, we will
> have to somehow maintain a relationship between a udevice and a handle.
>
> 1-1. add a dedicated uclass, UCLASS_PARTITION, for partitions
> Currently, the uclass for paritions is not a UCLASS_BLK.
> It can be possible to define partitions as UCLASS_BLK
> (with IF_TYPE_PARTION?), but
> I'm afraid that it may introduce some chaos since udevice(UCLASS_BLK)
> is tightly coupled with 'struct blk_desc' data which is still used
> as a "structure to a whole disk" in a lot of interfaces.
> (I hope that you understand what it means.)
>
> In DM tree, a UCLASS_PARTITON instance has a UCLASS_BLK parent:
> For instance,
> UCLASS_SCSI --- UCLASS_BLK --- UCLASS_PARTITION
> (IF_TYPE_SCSI) |
> +- struct blk_desc +- struct disk_part
> +- scsi_blk_ops +- blk_part_ops
>
> 1-2. create partition udevices in the context of device_probe()
> part_init() is already called in blk_post_probe(). See the commit
> d0851c893706 ("blk: Call part_init() in the post_probe() method").
> Why not enumelate partitions as well in there.
>
> 2. add new block access interfaces, which takes a *udevice* as a target
> device, in U-Boot and use those functions to implement efi_disk
> operations (i.e. EFI_BLOCK_IO_PROTOCOL).
>
> 3-1. maintain a bi-directional link between a udevice and an efi_disk
> by adding
> - a UEFI handle pointer as a tag for a udevice
> - a udevice pointer in UEFI handle (in fact, in "struct efi_disk_obj")
>
> 3-2. synchronize the lifetime of efi_disk objects in UEFI world with
> the driver model using
> - event notification associated with device's probe/remove.
>
> 4. I have no solution to issue(4) and (5) yet.
>
>
> <<<Example DM tree on qemu-arm64>>>
> => dm tree
> Class Driver Name
> --------------------------------------------
> root root_driver root_driver
> ...
> pci pci_generic_ecam |-- pcie at 10000000
> pci_generi pci_generic_drv | |-- pci_0:0.0
> virtio virtio-pci.l | |-- virtio-pci.l#0
> ethernet virtio-net | | `-- virtio-net#32
> ahci ahci_pci | |-- ahci_pci
> scsi ahci_scsi | | `-- ahci_scsi
> blk scsi_blk | | |-- ahci_scsi.id0lun0
> partition blk_partition | | | |-- ahci_scsi.id0lun0:1
> partition blk_partition | | | `-- ahci_scsi.id0lun0:2
> blk scsi_blk | | `-- ahci_scsi.id1lun0
> partition blk_partition | | |-- ahci_scsi.id1lun0:1
> partition blk_partition | | `-- ahci_scsi.id1lun0:2
> usb xhci_pci | `-- xhci_pci
> usb_hub usb_hub | `-- usb_hub
> usb_dev_ge usb_dev_generic_drv | |-- generic_bus_0_dev_2
> usb_mass_s usb_mass_storage | `-- usb_mass_storage
> blk usb_storage_blk | `-- usb_mass_storage.lun0
> partition blk_partition | |-- usb_mass_storage.lun0:1
> partition blk_partition | `-- usb_mass_storage.lun0:2
> ...
> => efi devices
> Device Device Path
> ================ ====================
> 000000013eeea8d0 /VenHw()
> 000000013eeed810 /VenHw()/MAC(525252525252,1)
> 000000013eefc460 /VenHw()/Scsi(0,0)
> 000000013eefc5a0 /VenHw()/Scsi(0,0)/HD(1,GPT,ce86c5a7-b32a-488f-a346-88fe698e0edc,0x22,0x4c2a)
> 000000013eefe320 /VenHw()/Scsi(0,0)/HD(2,GPT,aa80aab9-33e6-42b6-b5db-def2cb8d7844,0x5000,0x1a800)
> 000000013eeff210 /VenHw()/Scsi(1,0)
> 000000013eeff390 /VenHw()/Scsi(1,0)/HD(1,GPT,ce86c5a7-b32a-488f-a346-88fe698e0edc,0x22,0x4c2a)
> 000000013eeff7d0 /VenHw()/Scsi(1,0)/HD(2,GPT,aa80aab9-33e6-42b6-b5db-def2cb8d7844,0x5000,0x1a800)
> 000000013ef04c20 /VenHw()/UsbClass(0x0,0x0,0x9,0x0,0x3)/UsbClass(0x46f4,0x1,0x0,0x0,0x0)
> 000000013ef04da0 /VenHw()/UsbClass(0x0,0x0,0x9,0x0,0x3)/UsbClass(0x46f4,0x1,0x0,0x0,0x0)/HD(1,0x01,0,0x0,0x99800)
> 000000013ef04f70 /VenHw()/UsbClass(0x0,0x0,0x9,0x0,0x3)/UsbClass(0x46f4,0x1,0x0,0x0,0x0)/HD(2,0x01,0,0x99800,0x1800)
>
>
> Patchs:
> =======
> For easy understandings, patches may be categorized into separate groups
> of changes.
>
> Patch#1-#7: DM: add device_probe() for later use of events
> Patch#8-#11: DM: add new features (tag and event notification)
> Patch#12-#15: UEFI: dynamically create/remove efi_disk's for a raw disk
> and its partitions
> For removal case, we may need more consideration since removing handles
> unconditionally may end up breaking integrity of handles
> (as some may still be held and referenced to by a UEFI app).
> Patch#16-#17: UEFI: use udevice read/write interfaces
> Patch#18-#19: UEFI: fix-up efi_driver, aligning with changes in DM integration
>
>
> [1] https://lists.denx.de/pipermail/u-boot/2019-February/357923.html
> [2] https://lists.denx.de/pipermail/u-boot/2021-June/452297.html
>
>
> Change history:
> ===============
> v1 (Feb 2, 2022)
> * rebased on 2022.04-rc1
> * drop patches that have already been merged
> * modify a tag-range check with "tag >= DM_TAG_COUNT" (patch#9)
> * move dmtag_list to GD (global data) (patch#9)
> * add function descriptions and a document about DM tag feature (patch#9,10)
> * add tests for DM tag support (patch#11)
> * change 'depends on EVENT' to 'select EVENT' for EFI_LOADER (patch#14)
> * migrate IF_TYPE_EFI to IF_TYPE_EFI_LOADER (patch#18)
>
> RFCv2 (Dec 10, 2021)
> * rebased on 2022-rc3
> * re-order and merge some related commits into ones
> * call device_probe() in MMC (not bind, but) probe hook (patch#5)
> * fix a wrong name of variable (patch#7)
> * add patch#9
> * invoke device_probe() for virtio devices (patch#10)
> * add DM event notitication (from Simon) (patch#11)
> * add DM tag support (patch#12)
> * move UCLASS_PARTITION driver under disk/ (patch#13)
> * create partition's dp using its parent's. This change is necessary
> in particular for 'efi_blk' efi_disk (patch#13)
> * modify the code so that we will use new features like tags and
> event notification (patch#13,15,16,20)
> * rename new functions from blk_read/write() to dev_read/write()
> (patch#17,18)
> * isolate changes in efi_driver from the rest (in efi_loader) (patch#19)
> * drop the previous patch#22 ("efi_selftest: block device: adjust dp
> for a test") due to the fix in patch#13
>
> RFC (Nov 16, 2021)
> * initial RFC
>
> AKASHI Takahiro (18):
> scsi: call device_probe() after scanning
> usb: storage: call device_probe() after scanning
> mmc: call device_probe() after scanning
> nvme: call device_probe() after scanning
> sata: call device_probe() after scanning
> block: ide: call device_probe() after scanning
> virtio: call device_probe() in scanning
> dm: add tag support
> dm: tag: add some document
> test: dm: add tests for tag support
> dm: disk: add UCLASS_PARTITION
> dm: blk: add a device-probe hook for scanning disk partitions
> efi_loader: disk: a helper function to create efi_disk objects from
> udevice
> efi_loader: disk: a helper function to delete efi_disk objects
> dm: disk: add read/write interfaces with udevice
> efi_loader: disk: use udevice instead of blk_desc
> efi_loader: disk: not create BLK device for BLK(IF_TYPE_EFI_LOADER)
> devices
> efi_driver: align with efi_disk-dm integration
>
> Simon Glass (1):
> dm: add event notification
>
> cmd/virtio.c | 21 +-
> common/Kconfig | 11 +
> common/Makefile | 2 +
> common/board_f.c | 2 +
> common/event.c | 103 +++++++++
> common/log.c | 1 +
> common/usb_storage.c | 4 +
> disk/Makefile | 3 +
> disk/disk-uclass.c | 247 +++++++++++++++++++++
> doc/develop/driver-model/design.rst | 20 ++
> drivers/ata/dwc_ahsata.c | 5 +
> drivers/ata/fsl_sata.c | 11 +
> drivers/ata/sata_mv.c | 5 +
> drivers/ata/sata_sil.c | 12 +
> drivers/block/blk-uclass.c | 4 +
> drivers/block/ide.c | 4 +
> drivers/core/Makefile | 2 +-
> drivers/core/device-remove.c | 9 +
> drivers/core/device.c | 9 +
> drivers/core/root.c | 2 +
> drivers/core/tag.c | 139 ++++++++++++
> drivers/mmc/mmc-uclass.c | 12 +
> drivers/nvme/nvme.c | 4 +
> drivers/scsi/scsi.c | 5 +
> include/asm-generic/global_data.h | 4 +
> include/dm/device-internal.h | 10 +
> include/dm/tag.h | 110 ++++++++++
> include/dm/uclass-id.h | 1 +
> include/efi_loader.h | 4 +-
> include/event.h | 105 +++++++++
> include/event_internal.h | 34 +++
> include/log.h | 2 +
> include/part.h | 18 ++
> lib/efi_driver/efi_block_device.c | 34 +--
> lib/efi_loader/Kconfig | 2 +
> lib/efi_loader/efi_disk.c | 329 ++++++++++++++++++++--------
> lib/efi_loader/efi_setup.c | 11 +-
> test/common/Makefile | 1 +
> test/common/event.c | 87 ++++++++
> test/dm/Makefile | 1 +
> test/dm/tag.c | 80 +++++++
> test/test-main.c | 5 +
> 42 files changed, 1355 insertions(+), 120 deletions(-)
> create mode 100644 common/event.c
> create mode 100644 disk/disk-uclass.c
> create mode 100644 drivers/core/tag.c
> create mode 100644 include/dm/tag.h
> create mode 100644 include/event.h
> create mode 100644 include/event_internal.h
> create mode 100644 test/common/event.c
> create mode 100644 test/dm/tag.c
This code does not even compile.
https://source.denx.de/u-boot/custodians/u-boot-efi/-/jobs/387157
I will update the whole series to status "changes requested".
Best regards
Heinrich
More information about the U-Boot
mailing list