[U-Boot] [RFC 2/3] efi_loader: associate BLK/PARTITION device to efi_disk

AKASHI Takahiro takahiro.akashi at linaro.org
Wed Feb 6 07:54:18 UTC 2019


On Wed, Feb 06, 2019 at 12:15:11PM +0900, AKASHI Takahiro wrote:
> On Sat, Feb 02, 2019 at 07:15:53AM -0700, Simon Glass wrote:
> > Hi,
> > 
> > On Thu, 31 Jan 2019 at 22:53, AKASHI Takahiro
> > <takahiro.akashi at linaro.org> wrote:
> > >
> > > Hi Simon,
> > >
> > > Thank you for suggestive comments.
> > > I've got no idea of making DM class for EFI protocol.
> > >
> > > On Wed, Jan 30, 2019 at 06:22:47PM -0700, Simon Glass wrote:
> > > > Hi AKASHI,
> > > >
> > > > On Mon, 28 Jan 2019 at 19:59, AKASHI Takahiro
> > > > <takahiro.akashi at linaro.org> wrote:
> > > > >
> > > > > efi_disk_create() will initialize efi_disk attributes for each device,
> > > > > either UCLASS_BLK or UCLASS_PARTITION.
> > > > >
> > > > > Currently (temporarily), efi_disk_obj structure is embedded into
> > > > > blk_desc to hold efi-specific attributes.
> > > > >
> > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > > > > ---
> > > > >  drivers/block/blk-uclass.c |   9 ++
> > > > >  drivers/core/device.c      |   3 +
> > > > >  include/blk.h              |  24 +++++
> > > > >  include/dm/device.h        |   4 +
> > > > >  lib/efi_loader/efi_disk.c  | 192 ++++++++++++++++++++++++++-----------
> > > > >  5 files changed, 174 insertions(+), 58 deletions(-)
> > > > >
> > > >
> > > > I think the objective should be to drop the EFI data structures.
> > >
> > > More or less so, yes.
> > >
> > > > So your approach of just including them in DM structures seems about
> > > > right to me, as a short-term migration measure. Given the large amount
> > > > of code that has built up I don't think it is possible to do it any
> > > > other way.
> > >
> > > Right.
> > >
> > > > It is very unfortunate though.
> > > >
> > > > So basically migration could be something like this:
> > > >
> > > > 1. Move each of the EFI structs into the DM structs one by one
> > > > 2. Drop struct members that are not needed and can be calculated from DM members
> > > > 3. Update DM to have 1:1 uclasses for each EFI protocol
> > > > 4. Move all the protocol structs into the DM uclasses
> > > > 5. Whittle these down until they are just shells used by the API, with
> > > > everything going through normal DM calls
> > > >
> > > > Or would it be better to just start again and rewrite it with the
> > > > existing code as a base?
> > > >
> > > > Looking at it, efi_object is not very useful in DM. It contains two members:
> > > >
> > > > 1. link - linked list link, which devices already have, although we
> > > > don't have a single list of all them. Instead they are linked into
> > > > separate lists for each uclass
> > > >
> > > > 2. protocols - list of protocols. But DM devices support only one
> > > > protocol. Multiple protocols are handled using child devices. E.g a
> > > > UCLASS_PMIC device that supports UCLASS_GPIO, UCLASS_REGULATOR and
> > > > UCLASS_AUDIO_CODEC would have three children, one for each uclass. So
> > > > perhaps with EFI we should create a separate child for each protocol
> > > > in a similar way?
> > > >
> > > > Which comes back to the idea of creating an EFI child device for every
> > > > DM device. Perhaps instead we create one EFI child for each protocol
> > > > supported by the parent device?
> > >
> > > Well, "child device as a EFI protocol" is really workable, but
> > > I have some concerns:
> > > * Can a DM device be an abstract instance with no real hardware?
> > 
> > Yes we do that quite a bit. Even UCLASS_BLK is like this, if you think about it.
> 
> OK
> 
> > > * There may be two different types of "children" mixed for an EFI object
> > >    - some physical hierarchy, say disk partitions for a raw disk
> > >    - these EFI protocols
> > >   That is, for example, one raw disk has
> > >          - partition 1 has
> > >                  - block io protocol
> > >                  - device path protocol
> > >                  - simple file system protocol
> > >          - partition 2 has
> > >                  - block io protocol
> > >                  - device path protocol
> > >                  - simple file system protocol
> > >          - block io protocol
> > >          - device path protocol
> > > * device path protocol can be annoying as almost all devices (visible
> > >   to UEFI) have some sort of device path, while corresponding u-boot
> > >   notion is, say, "scsi 0:1" which only appears on command interfaces.
> > 
> > Yes. We could use the device path from concatenating device names from
> > all parents, perhaps. I've been thinking about adding that to the
> > command line as an option.
> 
> Device path is kinda device hierarchy of DM, so it's easily confusing.
> Please see an example below.
> 
> > >
> > > I'm not sure if those concerns are acceptable.
> > >
> > > > Another point is that the operations supported by EFI should be in DM
> > > > operations structs. For example I think struct
> > > > efi_simple_text_output_protocol should have methods which call into
> > > > the corresponding uclass operations.
> > >
> > > I have no idea on those "console" devices yet.
> > >
> > > > It is confusing that an EFI disk is in fact a partition. Or do I have
> > > > that wrong?
> > >
> > > IMO, EFI disk is any type of EFI object which supports EFI_BLOCK_IO_PROTOCOL.
> > > So a raw disk(UCLASS_BLK) as well as its partitions(UCLASS_PARTITION) are
> > > EFI disks as well.
> > > Is this an answer to you?
> > 
> > Yes OK, I see.
> > 
> > >
> > > Those said, your suggestion is truly worth considering.
> > 
> > OK, good. Certainly an interesting project.
> 
> Conversion of efi protocols to uclass device(UCLASS_PROTOCOL) was done
> and efi_handle_t is now 'struct udevice *'!

To be clear, what is converted to DM device here is "struct efi_handler,"
not "protocol interface" structure in UEFI world. The latter is
a well-defined data structure in UEFI spec and cannot be treated
as a DM device.

> I could do this almost "systematically," but the code size doesn't decrease
> much. This is no surprise because we rely on udevice only for traversing
> efi handles and protocols. We still need lots of "wrapper" layers
> on top of block devices, file systems and so on due to differences
> of APIs and their semantics.
> (Because of this, I don't have good confidence yet that efi protocol
> should also be a device in DM.)
> 
> Here is a sample operation:
> 
> => efi dev
> Device           Device Path
> ================ ====================
> 000000007eef9590 /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 devices
> Device           Device Path
> ================ ====================
> 000000007eef9590 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> 000000007ef04c50 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)
> 000000007ef02d10 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)
> 000000007ef05c70 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(335544330,MBR,0x086246ba,0x17ff501a0,0x7eee4770)
> 000000007ef06270 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(335544330,MBR,0x086246ba,0x17ff501a0,0x7eee4770)
> => efi dh
> Handle           Protocols
> ================ ====================
> 000000007eef9590 Device Path, Device Path To Text, Device Path Utilities, Unicode Collation 2
> 000000007ef00970 Driver Binding
> 000000007eef7b80 Simple Text Output, Simple Text Input, Simple Text Input Ex
> 000000007ef04c50 Block IO, Device Path
> 000000007ef02d10 Block IO, Device Path
> 000000007ef05c70 Block IO, Device Path, Simple File System
> 000000007ef06270 Block IO, Device Path, Simple File System
> => dm tree
>  Class    index  Probed  Driver                Name
> -----------------------------------------------------------
>  root        0  [ + ]   root_driver           root_driver
>  simple_bus  0  [   ]   generic_simple_bus    |-- platform at c000000
>  virtio      0  [ + ]   virtio-mmio           |-- virtio_mmio at a000000
> 
>  ...
> 
>  pci         0  [ + ]   pci_generic_ecam      |-- pcie at 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_protocol          |           |   |-- (PROTO)
>  efi_protoc  9  [   ]   efi_protocol          |           |   `-- (PROTO)
>  blk         1  [   ]   scsi_blk              |           `-- ahci_scsi.id1lun0
>  efi_protoc  10  [   ]   efi_protocol          |               |-- (PROTO)
>  efi_protoc  11  [   ]   efi_protocol          |               |-- (PROTO)
>  partition   0  [   ]   blk_partition         |               |-- ahci_scsi.id1lun0:1
>  efi_protoc  12  [   ]   efi_protocol          |               |   |-- (PROTO)
>  efi_protoc  13  [   ]   efi_protocol          |               |   |-- (PROTO)
>  efi_protoc  14  [   ]   efi_protocol          |               |   `-- (PROTO)
>  partition   1  [   ]   blk_partition         |               `-- ahci_scsi.id1lun0:2
>  efi_protoc  15  [   ]   efi_protocol          |                   |-- (PROTO)
>  efi_protoc  16  [   ]   efi_protocol          |                   |-- (PROTO)
>  efi_protoc  17  [   ]   efi_protocol          |                   `-- (PROTO)
>  rtc         0  [   ]   rtc-pl031             |-- pl031 at 9010000
>  serial      0  [   ]   serial_pl01x          |-- pl011 at 9050000
>  serial      1  [ + ]   serial_pl01x          |-- pl011 at 9000000
>  efi_protoc  5  [   ]   efi_protocol          |   |-- (PROTO)
>  efi_protoc  6  [   ]   efi_protocol          |   |-- (PROTO)
>  efi_protoc  7  [   ]   efi_protocol          |   `-- (PROTO)
>  mtd         0  [ + ]   cfi_flash             |-- flash at 0
>  firmware    0  [ + ]   psci                  |-- psci
>  sysreset    0  [   ]   psci-sysreset         |   `-- psci-sysreset

I changed this part of output:

>  efi_object  0  [   ]   efi_dumb_object       |-- efi_root
>  efi_protoc  0  [   ]   efi_protocol          |   |-- (PROTO)
>  efi_protoc  1  [   ]   efi_protocol          |   |-- (PROTO)
>  efi_protoc  2  [   ]   efi_protocol          |   |-- (PROTO)
>  efi_protoc  3  [   ]   efi_protocol          |   `-- (PROTO)
>  efi_object  1  [   ]   efi_dumb_object       `-- EFI block driver
>  efi_protoc  4  [   ]   efi_protocol              `-- (PROTO)

to

 efi_object  0  [   ]   efi_dumb_object       `-- efi_root
 efi_protoc  0  [   ]   efi_protocol              |-- (PROTO)
 efi_protoc  1  [   ]   efi_protocol              |-- (PROTO)
 efi_protoc  2  [   ]   efi_protocol              |-- (PROTO)
 efi_protoc  3  [   ]   efi_protocol              |-- (PROTO)
 efi         0  [   ]   EFI block driver          `-- EFI block driver
 efi_protoc  4  [   ]   efi_protocol                  `-- (PROTO)

> As you can see, I have only one efi protocol "driver" and so
> there's no specific name given for each protocol now.

More specifically, there seem to be a few options:
1-1. to keep one uclass, distinguishing protocols with an internal id,
     something like IF_TYPE_* for blk_desc
1-2. to keep one uclass, having one driver for one protocol,
(Either way, there's not common "operation" btw protocols here.)

2. to give each protocol an unique uclass id

Thanks,
-Takahiro Akashi

> As I'm still debugging my code, I will re-post my patch
> once it gets back working.
> 
> Thanks,
> -Takahiro Akashi
> 
> > Regards,
> > Simon


More information about the U-Boot mailing list