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

[Simon Glass]

Hi Takahiro,

On Thu, 24 Jan 2019 at 13:53, AKASHI Takahiro
<takahiro.akashi at linaro.org> wrote:
>
> 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

Well 'parent node' is used in device tree. For driver model, we say
'parent device'.

In fact enum if_type is not so necessary in the DM world. We could
instead use the uclass of the parent. For example, we could use
UCLASS_MMC instead of IF_TYPE_MMC.

So I'm not sure that it is right to set the if_type to UCLASS_BLK.
There are many U-Boot commands which number the different 'interface
types' separately. For example 'mmc' devices create a block device
with blk_desc containing devnum values numbered from 0, as do 'sata'
devices, etc. So what does it mean to have a generic 'block' interface
type?

>
> 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

Here you are really just creating a device of UCLASS_BLK, since there
is a 1:1 correspondence between a UCLASS_BLK device and blk_desc.

struct blk_desc *desc = dev_get_uclass_platdata(blk_dev);

>
> 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().

As above these are really the same, in that a blk_desc can only exist
as an attachment to a UCLASS_BLK / block device.

>
> # 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.

I think those two are actually the same, but I suspect I misunderstand
something.

If you look at how Unix handles devices versus partitions:

/dev/sdb - whole block device
/dev/sdb1 - first partition
/dev/sdb2 - second partition

So having the partitions as separate devices at least has precedent. I
do think it would work pretty well in U-Boot too.

Another thought might be that we make the block devices be siblings of
the 'whole' block device instead of children. Then a SATA device may
have 4 children, for example, all UCLASS_BLK, with the first one
containing the whole device contents and the other three containing
subsets of the first.

But it seems to me that having the partitions be children of a parent
BLK device makes more sense, since if you write to the parent you are
implicitly writing from one of the children. Also, if you write the
partition table, you do that with the parent BLK device, but that will
cause the children to be deleted and recreated. Also the operations on
a partition are not quite the same as those on a whole device: they
can only see a window into the whole device, so will likely add their
offset and then call the parent-device operations.

[..]

Regards,
Simon