[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