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

[Alexander Graf]

> Am 28.01.2019 um 09:56 schrieb AKASHI Takahiro <takahiro.akashi at linaro.org>:
> 
>> On Fri, Jan 25, 2019 at 10:31:20AM +0100, Alexander Graf wrote:
>> 
>> 
>>> On 25.01.19 10:18, AKASHI Takahiro wrote:
>>>> On Fri, Jan 25, 2019 at 09:52:31AM +0100, Alexander Graf wrote:
>>>> 
>>>> 
>>>>> On 25.01.19 09:27, AKASHI Takahiro wrote:
>>>>> Alex,
>>>>> 
>>>>>> On Wed, Jan 23, 2019 at 10:51:29AM +0100, Alexander Graf wrote:
>>>>>>> On 01/22/2019 08: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
>>>>>> 
>>>>>> ... and then we need to create our own sub object model around the
>>>>>> UCLASS_EFI devices again. I' not convinced that's a great idea yet :). I
>>>>>> really see little reason not to just expose every dm device as EFI handle.
>>>>>> Things would plug in quite naturally I think.
>>>>> 
>>>>> You said that the ultimate goal is to remove all efi_object data.
>>>>> Do you think that all the existing efi_object can be mapped to
>>>>> one of existing u-boot uclass devices?
>>>>> 
>>>>> If so, what would be an real entity of a UEFI handle?
>>>>> struct udevice *?
>>>>> 
>>>>> But Simon seems not to agree to adding any UEFI-specific members
>>>>> in struct udevice.
>>>> 
>>>> I think we'll have to experiment with both approaches. I personally
>>>> would like to have struct udevice * be the UEFI handle, yes.
>>>> 
>>>>> 
>>>>>> But either way, someone would need to sit down and prototype things to be
>>>>>> sure.
>>>>>> 
>>>>> 
>>>>> The most simplest prototype would include
>>>>> * event mechanism (just registration and execution of hook/handler)
>>>>>    event: udevice creation (and deletion)
>>>>> * efi_disk hook for udevice(UCLASS_BLK) creation
>>>>> * modified block device's enumeration code, say, scsi_scan(),
>>>>>  to add an event hook at udevice creation
>>>>> * removing efi_disk_register() from efi_init_obj_list()
>>>>> * Optionally(?) UCLASS_PARTITION
>>>>>  (Partition udevices would be created in part_init().)
>>>> 
>>>> Almost.
>>>> 
>>>> The simplest prototype would be to add a struct efi_object into struct
>>>> udevice. Then whenever we're looping over efi_obj_list in the code, we
>>>> additionally loop over all udevices to find the handle.
>>> 
>>> Ah, yes. You're going further :)
>>> 
>>>> Then, we could slowly give the uclasses explicit knowledge of uefi
>>>> protocols. So most of the logic of efi_disk_register() would move into
>>>> (or get called by) drivers/block/blk-uclass.c:blk_create_device().
>>> 
>>> Via event? Otherwise, we cannot decouple u-boot and UEFI world.
>> 
>> For a prototype, just make it explicit and see how far that gets us.
>> 
>>>> Instead of creating diskobj and adding calling efi_add_handle(), we
>>>> could then just use existing data structure from the udevice (and its
>>>> platdata).
>>> 
>>> I don't have good confidence that we can remove struct efi_disk_obj,
>>> at least, for the time being as some of its members are quite UEFI-specific.
>> 
>> Maybe we can move them into struct blk_desc? It's a matter of
>> experimenting I guess.
>> 
>>> 
>>>> 
>>>> Does this make sense? Less events, more implicity :).
>>> 
>>> I'll go for it.
>> 
>> Thanks a lot :). Feel free to pick an easier target for starters too if
>> you prefer.
> 
> Prototyping is done :)
> Since it was so easy and simple, now I'm thinking of implementing
> UCLASS_PARTITION. But it is not so straightforward as I expected,
> and it won't bring us lots of advantages.
> (I think that blk_desc should also support a partition in its own.)
> 
> Once it gets working, may I send out a patch?

Feel free to even just send a patch of what you have now as RFC, so that we can see if this looks like the right direction. Let's make use of our time zone differences :).


Alex