[resent RFC 00/22] efi_loader: more tightly integrate UEFI disks to device model
takahiro.akashi at linaro.org
Tue Oct 5 04:14:06 CEST 2021
On Mon, Oct 04, 2021 at 04:47:53PM +0200, Heinrich Schuchardt wrote:
> On 10/4/21 05:44, AKASHI Takahiro wrote:
> > # Resending the RFC as some of patches were deplicately submitted.
> > # See also
> > https://git.linaro.org/people/takahiro.akashi/u-boot.git efi/dm_disk
> > The purpose of this RPC is to reignite the discussion about how UEFI
> > subystem would best be integrated into U-Boot device model.
> > In the past, I poposed a couple of patch series, the latest one,
> > while Heinrich revealed his idea, and the approach taken here is
> > something between them, with a focus on block device handlings.
> > # The code is a PoC and not well tested yet.
> > 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 device 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 device 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 perations.
> > 5. There is no way supported to remove efi_disk's even after
> > disconnect_controller() is called.
> > My approach in this RFC:
> > ========================
> > 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 "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 by adding
> > - a UEFI handle pointer in "struct udevice"
> > - a udevice pointer in UEFI handle (in fact, in "struct efi_disk_obj")
> An EFI application can create handles with any combination of protocols,
> e.g. a handle with both the block IO protocol and the simple network
> protocol. This means that a udevice cannot be assigned to a handle
> created by an EFI application.
Can you please elaborate more to clarify your point/suggestion here?
> When the EFI application calls ConnectController() for the handle,
> U-Boot can create child controllers. If U-Boot creates a udevice for
> such a child controller, it has to store the udevice pointer.
> lib/efi_driver/efi_block_device.c uses a private data section but you it
> could be preferable to use a field in struct efi_obj.
Before submitting this RFC, I also thought of a possibility of
re-implementing lib/efi_loader/efi_disk.c by defining a "controller"
for each U-Boot's block device (udevice) which is essentially a source
of providing BLOCK_IO_PROTOCOL as "efi_disk" devices and then implementing
"bind" interface of DRIVER_BINDING_PROTOCOL to create a mapping between
udevice(UCLASS_BLK) and efi_disk.
(Then I hoped we could reuse efi_driver framework for the case (1) below.)
Is this similar to what you think of here?
As I mentioned, there are two paths in creating efi_disks:
1) U-Boot's block device => efi_disk
(efi_disk_add_dev() in lib/efi_loader/efi_disk.c is responsible for this.)
2) EFI app/driver -> efi_disk => U-Boot's block device
(efi_bl_bind() in lib/efi_driver/efi_block_device.c)
Those two methods try to establish the relationship in opposite directions.
This is somewhat a cause of confusion/misunderstanding.
> > 3-2. use device model's post_probe/pre_remove hook to synchronize the lifetime
> > of efi_disk objects in UEFI world with the device model.
> > 4. I have no answer to issue(4) and (5) yet.
> 4) A udevice shall only exist for the child controller handle created by
> U-Boot and not for the controller handle created by an EFI application.
I don't know what is a "child" controller, and will think of it.
> 5) The stop() method of the driver binding protocol has to take care of
> destroying the child controllers and the associated udevices.
That is a missing piece of code.
> Best regards
More information about the U-Boot