[U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time

[AKASHI Takahiro]

Simon,

On Thu, Jan 24, 2019 at 10:58:53AM +1300, Simon Glass wrote:
> Hi Heinrich,
> 
> On Wed, 23 Jan 2019 at 10:05, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >
> > On 1/22/19 8:39 PM, Simon Glass wrote:
> > > Hi Alex,
> > >
> > > On Tue, 22 Jan 2019 at 22:08, Alexander Graf <agraf at suse.de> wrote:
> > >>
> > >>
> > >>
> > >> On 22.01.19 09:29, AKASHI Takahiro wrote:
> > >>> Alex, Simon,
> > >>>
> > >>> Apologies for my slow response on this matter,
> > >>>
> > >>> On Fri, Jan 11, 2019 at 08:57:05AM +0100, Alexander Graf wrote:
> > >>>>
> > >>>>
> > >>>> On 11.01.19 05:29, AKASHI Takahiro wrote:
> > >>>>> Alex, Heinrich and Simon,
> > >>>>>
> > >>>>> Thank you for your comments, they are all valuable but also make me
> > >>>>> confused as different people have different requirements :)
> > >>>>> I'm not sure that all of us share the same *ultimate* goal here.
> > >>>>
> > >>>> The shared ultimate goal is to "merge" (as Simon put it) dm and efi objects.
> > >>>
> > >>> I don't still understand what "merge" means very well.
> > >>
> > >> It basically means that "struct efi_object" moves into "struct udevice".
> > >> Every udevice instance of type UCLASS_BLK would expose the block and
> > >> device_path protocols.
> > >>
> > >> This will be a slightly bigger rework, but eventually allows us to
> > >> basically get rid of efi_init_obj_list() I think.
> > >
> > > I envisaged something like:
> > >
> > > - EFI objects have their own UCLASS_EFI uclass
> > > - DM uclasses which support EFI would create a child EFI device (e.g.
> > > a UCLASS_EFI child of each UCLASS_BLK device)
> > > - EFI-uclass devices would thus be bound as needed
> > > - Probing an EFI device would causes its parents to be probed
> > > - We can use all the existing DM hooks (probe, remove, parent/child
> > > data, operations), to implement EFI things
> > >
> > > I'm assuming that a small percentage of devices would have EFI
> > > children, so that this is more efficient than trying to merge the data
> > > structures. It also allows EFI to maintain some separate from the core
> > > DM code.
> >
> > Dear Simon,
> >
> > thanks to your suggestions.
> >
> > I am not yet convinced that an UCLASS_EFI child device will be helpful.
> > It is not an EFI object.
> >
> > A DM uclass is the equivalent to an EFI driver, i.e. a handle with the
> > EFI_DRIVER_BINDING_PROTOCOL installed on it [1]. So the natural thing
> > for a uclass supporting EFI would be to provide such a handle.
> >
> > For the actual devices we also need handles.
> 
> Yes but I understand why this is problematic?
> 
> >
> > In the EFI world partitions are devices. How does this fit into your
> > picture?
> 
> Perhaps we could consider adding a UCLASS_PARTITION? The rework of the
> FS layer may not be too much - at least once we drop the non-BLK code
> (as you say at [1]).

IMO, instead of considering UCLASS_PARTITION,
1) add IF_TYPE_BLK_PARTITION and set its type_class_id to UCLASS_BLK
   So any partition has a parent node (I don't know the correct language
   in DM world) that is a real raw disk device, and yet is seen
   as a UCLASS_BLK device, or

2) create a block device (blk_desc) for each partition, either in bind/probe
   or in enumerating devices, as I mentioned in the previous e-mail

What's different between (1) and (2),
we may enumerate block devices and identify the given one by scanning
a UCLASS_BLK list with (1), while by scanning a blk_desc list with (2)
at do_load()/fs_set_blk_dev().

# In any way, we will need
a) a bi-directional link between
     UCLASS_BLK          efi_obj
         or        and       or
     blk_desc            efi_disk_obj, and
b) a event notification mechanism, in your language, as as to maintain
   (create/delete) those links

If you agree to approach (1) or (2), I will be able to start a prototyping.

-Takahiro Akashi

> >
> > [1] https://lists.denx.de/pipermail/u-boot/2019-January/354359.html
> > [RFC] Device model for block devices - integration with EFI subsystem
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > >>
> > >>>
> > >>>> But we have this annoying interim state where we would lose a few boards
> > >>>> because they haven't been converted to DM. That's what keeps us from it.
> > >>>>
> > >>>> I think what this discussion boils down to is that someone needs to
> > >>>> start prototyping the DM/EFI integration. Start off with a simple
> > >>>> subsystem, like BLK.
> > >>>
> > >>> Even in the current implementation,
> > >>> * UEFI disk is implemented using UCLASS_BLK devices
> > >>>   (We can ignore !CONFIG_BLK case now as we have agreed.)
> > >>> * UEFI-specific block device can be seen as UCLASS_BLK/IF_TYPE_EFI
> > >>>
> > >>> So how essentially different is the *ultimate* goal from the current form
> > >>> regarding block devices?
> > >>
> > >> The ultimate goal is that efi_disk_register() and efi_obj_list disappear.
> > >>
> > >> Functionality wise we should be 100% identical to today, so all test
> > >> cases would still apply the same way as they do now. This is purely
> > >> internal rework, nothing visible to UEFI payloads.
> > >
> > > Yes.
> > >
> > >>
> > >>> In order to identify UEFI disks with u-boot devices transparently, we will
> > >>> have to have some sort of *hook* (or hotplug in Alex's language?), either
> > >>> in create_block_devices or bind/probe?  I don't know, but Simon seems
> > >>> to be in denial about this idea.
> > >>
> > >> Well, if a udevice *is* an efi device, we no longer need hooks. The
> > >> object list would simply change.
> > >>
> > >> We may still need to have event notifications at that stage, but that's
> > >> a different story.
> > >
> > > Yes, it's something that I think will need to be added to DM. I
> > > haven't got to this as I have not run into an important use case yet.
> > > Maybe something like:
> > >
> > > Controlled by CONFIG_EVENT
> > >
> > > - int dev_ev_register(struct udevice *dev, enum event_t type,
> > > event_handler_func_t handler, void *userdata)
> > >
> > > which calls handler(struct udevice *dev, void *userdata) when an event is fired
> > >
> > > - int dev_ev_unregister() to unregister
> > >
> > > - int dev_ev_send(struct udevice *dev, enum struct event_info *info)
> > >
> > > which sends events to registered listeners.
> > >
> > > struct event_info {
> > >   enum event_t type;
> > >   union {
> > >      struct ev_data_probed probed;
> > >      struct ev_data_removed removed;
> > >      ...
> > >  } d;
> > > };
> > >
> > >>
> > >> As transitioning period, we could probably implement 2 efi object roots:
> > >> efi_obj_list as well as the udevice based one. Every piece of code that
> > >> iterates through devices then just iterates over both. That way we
> > >> should be able to slowly move devices from the old object model to the
> > >> new one.
> > >
> > > Will the uclass I mentioned above you can iterate through UCLASS_EFI
> > > and thus you have a list of EFI devices.
> > >
> > > [...]
> > >
> > > Regards,
> > > Simon
> > >
> >