Re: [PATCH 13/35] efi: Add a media/block driver for EFI block devices

From: Heinrich Schuchardt <xypron.glpk@gmx.de>
To: Simon Glass <sjg@chromium.org>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Bin Meng <bmeng.cn@gmail.com>, Tom Rini <trini@konsulko.com>,
	Christian Melki <christian.melki@t2data.com>,
	Alexander Graf <agraf@csgraf.de>,
	U-Boot Mailing List <u-boot@lists.denx.de>
Subject: Re: [PATCH 13/35] efi: Add a media/block driver for EFI block devices
Date: Thu, 9 Sep 2021 12:35:27 +0200	[thread overview]
Message-ID: <b084701b-fd1d-bfba-cd7d-0b19414ad269@gmx.de> (raw)
In-Reply-To: <CAPnjgZ3QSaGwDLSgQ4chyNKGdyt+HfeyEixZYPEj0yk8t-4bxA@mail.gmail.com>

On 9/9/21 10:57 AM, Simon Glass wrote:
> Hi Heinrich,
>
> On Wed, 8 Sept 2021 at 12:04, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>>
>>
>> On 9/8/21 3:33 PM, Simon Glass wrote:
>>> Add a block driver which handles read/write for EFI block devices. This
>>> driver actually already exists ('efi_block') but is not really suitable
>>> for use as a real U-Boot driver:
>>>
>>> - The operations do not provide a udevice
>>
>> efi_bl_bind() creates a udevice by calling blk_create_device() when an
>> EFI application calls ConnectController() for a handle with an
>> EFI_BLOCK_IO_PROTOCOL.
>>
>> Please, explain in some detail what you think is wrong with the existing
>> code.
>
> See below:
>
>>
>>> - The code is designed for running as part of EFI loader, so uses
>>>       EFI_PRINT() and EFI_CALL().
>>> - It creates block devices for all the partitions too, which is not
>>>     somthing we want to support in this way

No, it creates a block device for the handle that was passed to
ConnectController(). The handles for the partitions are created by
U-Boot and these are not backed by separate block devices.

>>> - The bind method probes the device, which is not permitted
>>> - It uses 'EFI' as its parent device

All devices that the UEFI sub-system uses must be probed.

>>>
>>> The new driver is more 'normal', just requiring its platform data be set
>>> up in advance.

No clue what you mean by "in advance". It is a software like iPXE that
produces a handle at runtime and then calls ConnectController(). When
that call is done we must create a block device which is in status
probed, create handles for the partitions and install protocol
interfaces on these partitions.

Best regards

Heinrich

>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>
>> Please, separate this series in two. One for U-Boot on EFI and one for
>> U-Boot's UEFI implementation.
>
> Again I'm not sure what you mean here. Please point to something you
> don't want in this series, which is focussed on the app.
>
>>
>> Best regardss
>>
>> Heinrich
>>
>>> ---
>>>
>>>    drivers/block/Kconfig   |  10 ++++
>>>    drivers/block/Makefile  |   1 +
>>>    drivers/block/efi_blk.c | 115 ++++++++++++++++++++++++++++++++++++++++
>>>    include/efi.h           |  11 ++++
>>>    4 files changed, 137 insertions(+)
>>>    create mode 100644 drivers/block/efi_blk.c
>>>
>
> Regards,
> Simon
>