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

Alexander Graf
2019-01-25T09:31:20Z

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

Alex