[U-Boot] [RFC 2/3] efi_loader: associate BLK/PARTITION device to efi_disk
AKASHI Takahiro
takahiro.akashi at linaro.org
Wed Feb 6 03:15:11 UTC 2019
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 *'!
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
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)
As you can see, I have only one efi protocol "driver" and so
there's no specific name given for each protocol now.
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