[RFC,v2,00/20] efi_loader: more tightly integrate UEFI disks to driver model

# This is a kind of snapshot of my current work.
# I submit this to show my progress on this effort, and then
# I haven't addressed all the issues in this version.
# See the change history below.

The purpose of this RPC 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.

# 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 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 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 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 answer to issue(4) and (5) yet.


Patchs:
=======
For easy understandings, patches may be categorized into seperate groups
of changes.

Patch#1-#10: DM: add device_probe() for later use of events
Patch#11-#12: DM: add new features (tag and event notification)
Patch#13-#16: 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#17-#18: UEFI: use udevice read/write interfaces
Patch#19-#20: 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:
===============
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 (19):
  part: call part_init() in blk_get_device_by_str() only for MMC
  blk: add a helper function, blk_probe_or_unbind()
  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
  dm: fix an 'undefined' error in some macros
  virtio: call device_probe() in scanning
  dm: add 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) 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                | 249 ++++++++++++++++++++++
 disk/part.c                       |   3 +-
 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        |  17 ++
 drivers/block/ide.c               |   4 +
 drivers/core/Makefile             |   2 +-
 drivers/core/device-remove.c      |   9 +
 drivers/core/device.c             |   9 +
 drivers/core/tag.c                | 162 +++++++++++++++
 drivers/mmc/mmc-uclass.c          |  13 ++
 drivers/nvme/nvme.c               |   4 +
 drivers/scsi/scsi.c               |   5 +
 include/asm-generic/global_data.h |   3 +
 include/blk.h                     |  12 ++
 include/dm/device-internal.h      |  10 +
 include/dm/device.h               |   8 +-
 include/dm/tag.h                  |  76 +++++++
 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/test-main.c                  |   5 +
 41 files changed, 1274 insertions(+), 125 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

Hi, Simon,


On Thu, Jan 27, 2022 at 08:05:56AM -0700, Simon Glass wrote:
> Hi Takahiro,
> 
> On Thu, 9 Dec 2021 at 23:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >
> > # This is a kind of snapshot of my current work.
> > # I submit this to show my progress on this effort, and then
> > # I haven't addressed all the issues in this version.
> > # See the change history below.
> >
> > The purpose of this RPC 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.
> 
> Any update on this series please?

Thank you for your concern.
While I have made no progress so far, most of comments I've got
look trivial. I gonna have to double-check, though.

So do you have any update on your part of event notification (patch11/20)?

-Takahiro Akashi

> Regards,
> Simon

On Fri, Jan 28, 2022 at 02:01:41PM +0900, AKASHI Takahiro wrote:
> Hi, Simon,
> 
> 
> On Thu, Jan 27, 2022 at 08:05:56AM -0700, Simon Glass wrote:
> > Hi Takahiro,
> > 
> > On Thu, 9 Dec 2021 at 23:58, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > >
> > > # This is a kind of snapshot of my current work.
> > > # I submit this to show my progress on this effort, and then
> > > # I haven't addressed all the issues in this version.
> > > # See the change history below.
> > >
> > > The purpose of this RPC 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.
> > 
> > Any update on this series please?
> 
> Thank you for your concern.
> While I have made no progress so far, most of comments I've got
> look trivial. I gonna have to double-check, though.
> 
> So do you have any update on your part of event notification (patch11/20)?

I checked all the comments.
If you have no update, I will be able to submit a new version
early next week.

-Takahiro Akashi

> -Takahiro Akashi
> 
> > Regards,
> > Simon