[PATCH v4 09/34] efi: Add EFI uclass for media
Simon Glass
sjg at chromium.org
Sat Nov 13 04:24:31 CET 2021
Hi Heinrich,
On Mon, 8 Nov 2021 at 08:58, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Heinrich,
>
> On Mon, 8 Nov 2021 at 08:39, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >
> >
> >
> > On 11/8/21 06:29, AKASHI Takahiro wrote:
> > > On Sun, Nov 07, 2021 at 09:43:22AM -0700, Simon Glass wrote:
> > >> Hi Heinrich,
> > >>
> > >> On Sun, 7 Nov 2021 at 01:21, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> > >>>
> > >>>
> > >>>
> > >>> On 11/4/21 04:09, Simon Glass wrote:
> > >>>> At present UCLASS_EFI is used to represent an EFI filesystem among other
> > >>>> things. The description of this uclass is "EFI managed devices" which is
> > >>>> pretty vague. The only driver that uses this uclass is in fact not a real
> > >>>> U-Boot driver, since its operations do not include a struct udevice.
> > >>>>
> > >>>> Rather than mess with this, create a new UCLASS_EFI_MEDIA uclass to handle
> > >>>> EFI media such as disks. Make this the uclass to use for EFI media so that
> > >>>> it can be used with 'part list', for example.
> > >>>>
> > >>>> The existing implementation using UCLASS_EFI remains as is, for
> > >>>> discussion.
> > >>>>
> > >>>> Signed-off-by: Simon Glass <sjg at chromium.org>
> > >>>> ---
> > >>>>
> > >>>> (no changes since v2)
> > >>>>
> > >>>> Changes in v2:
> > >>>> - Add MAINTAINERS entry
> > >>>> - Show the correct interface type with 'part list'
> > >>>> - Update the commit message to explain things better
> > >>>>
> > >>>> MAINTAINERS | 3 +++
> > >>>> arch/sandbox/dts/test.dts | 4 ++++
> > >>>> disk/part.c | 5 ++++-
> > >>>> drivers/block/Kconfig | 23 +++++++++++++++++++++++
> > >>>> drivers/block/Makefile | 3 +++
> > >>>> drivers/block/blk-uclass.c | 2 +-
> > >>>> drivers/block/efi-media-uclass.c | 15 +++++++++++++++
> > >>>> drivers/block/sb_efi_media.c | 20 ++++++++++++++++++++
> > >>>> include/dm/uclass-id.h | 1 +
> > >>>> test/dm/Makefile | 1 +
> > >>>> test/dm/efi_media.c | 24 ++++++++++++++++++++++++
> > >>>> 11 files changed, 99 insertions(+), 2 deletions(-)
> > >>>> create mode 100644 drivers/block/efi-media-uclass.c
> > >>>> create mode 100644 drivers/block/sb_efi_media.c
> > >>>> create mode 100644 test/dm/efi_media.c
> > >>>>
> > >>>> diff --git a/MAINTAINERS b/MAINTAINERS
> > >>>> index 00ff572d4d2..3e8f10c72fa 100644
> > >>>> --- a/MAINTAINERS
> > >>>> +++ b/MAINTAINERS
> > >>>> @@ -712,8 +712,11 @@ W: https://u-boot.readthedocs.io/en/latest/develop/uefi/u-boot_on_efi.html
> > >>>> F: board/efi/efi-x86_app
> > >>>> F: configs/efi-x86_app*
> > >>>> F: doc/develop/uefi/u-boot_on_efi.rst
> > >>>> +F: drivers/block/efi-media-uclass.c
> > >>>> +F: drivers/block/sb_efi_media.c
> > >>>> F: lib/efi/efi_app.c
> > >>>> F: scripts/build-efi.sh
> > >>>> +F: test/dm/efi_media.c
> > >>>>
> > >>>> EFI PAYLOAD
> > >>>> M: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > >>>> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> > >>>> index 8cd688e8d26..2cea4a43c87 100644
> > >>>> --- a/arch/sandbox/dts/test.dts
> > >>>> +++ b/arch/sandbox/dts/test.dts
> > >>>> @@ -498,6 +498,10 @@
> > >>>> compatible = "sandbox,clk-ccf";
> > >>>> };
> > >>>>
> > >>>> + efi-media {
> > >>>> + compatible = "sandbox,efi-media";
> > >>>> + };
> > >>>> +
> > >>>> eth at 10002000 {
> > >>>> compatible = "sandbox,eth";
> > >>>> reg = <0x10002000 0x1000>;
> > >>>> diff --git a/disk/part.c b/disk/part.c
> > >>>> index a6a8f7052bd..2560f6a50bc 100644
> > >>>> --- a/disk/part.c
> > >>>> +++ b/disk/part.c
> > >>>> @@ -296,8 +296,11 @@ static void print_part_header(const char *type, struct blk_desc *dev_desc)
> > >>>> case IF_TYPE_VIRTIO:
> > >>>> puts("VirtIO");
> > >>>> break;
> > >>>> + case IF_TYPE_EFI:
> > >>>> + puts("EFI");
> > >>>> + break;
> > >>>> default:
> > >>>> - puts ("UNKNOWN");
> > >>>> + puts("UNKNOWN");
> > >>>> break;
> > >>>> }
> > >>>> printf (" device %d -- Partition Type: %s\n\n",
> > >>>> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> > >>>> index 56a4eec05ac..755fdccb574 100644
> > >>>> --- a/drivers/block/Kconfig
> > >>>> +++ b/drivers/block/Kconfig
> > >>>> @@ -61,6 +61,29 @@ config TPL_BLOCK_CACHE
> > >>>> help
> > >>>> This option enables the disk-block cache in TPL
> > >>>>
> > >>>> +config EFI_MEDIA
> > >>>> + bool "Support EFI media drivers"
> > >>>> + default y if EFI || SANDBOX
> > >>>> + help
> > >>>> + Enable this to support media devices on top of UEFI. This enables
> > >>>> + just the uclass so you also need a specific driver to make this do
> > >>>> + anything.
> > >>>> +
> > >>>> + For sandbox there is a test driver.
> > >>>> +
> > >>>> +if EFI_MEDIA
> > >>>> +
> > >>>> +config EFI_MEDIA_SANDBOX
> > >>>> + bool "Sandbox EFI media driver"
> > >>>> + depends on SANDBOX
> > >>>> + default y
> > >>>> + help
> > >>>> + Enables a simple sandbox media driver, used for testing just the
> > >>>> + EFI_MEDIA uclass. It does not do anything useful, since sandbox does
> > >>>> + not actually support running on top of UEFI.
> > >>>> +
> > >>>> +endif # EFI_MEDIA
> > >>>> +
> > >>>> config IDE
> > >>>> bool "Support IDE controllers"
> > >>>> select HAVE_BLOCK_DEVICE
> > >>>> diff --git a/drivers/block/Makefile b/drivers/block/Makefile
> > >>>> index 94ab5c6f906..3778633da1d 100644
> > >>>> --- a/drivers/block/Makefile
> > >>>> +++ b/drivers/block/Makefile
> > >>>> @@ -14,3 +14,6 @@ obj-$(CONFIG_IDE) += ide.o
> > >>>> endif
> > >>>> obj-$(CONFIG_SANDBOX) += sandbox.o
> > >>>> obj-$(CONFIG_$(SPL_TPL_)BLOCK_CACHE) += blkcache.o
> > >>>> +
> > >>>> +obj-$(CONFIG_EFI_MEDIA) += efi-media-uclass.o
> > >>>> +obj-$(CONFIG_EFI_MEDIA_SANDBOX) += sb_efi_media.o
> > >>>> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> > >>>> index 83682dcc181..2db7ce5de20 100644
> > >>>> --- a/drivers/block/blk-uclass.c
> > >>>> +++ b/drivers/block/blk-uclass.c
> > >>>> @@ -44,7 +44,7 @@ static enum uclass_id if_type_uclass_id[IF_TYPE_COUNT] = {
> > >>>> [IF_TYPE_SATA] = UCLASS_AHCI,
> > >>>> [IF_TYPE_HOST] = UCLASS_ROOT,
> > >>>> [IF_TYPE_NVME] = UCLASS_NVME,
> > >>>> - [IF_TYPE_EFI] = UCLASS_EFI,
> > >>>> + [IF_TYPE_EFI] = UCLASS_EFI_MEDIA,
> > >>>
> > >>> Don't break existing code. Create a new if_type here.
> > >>
> > >> My understanding is that this is not actually used at present. The
> > >> tests appear to pass without trouble.
> >
> > None of these tests uses the 'load efi 0:1' command!
> >
> > The iftype-uclass relationship is used during the look up. Please, stick
> > to separate if_types per uclass.
> >
> > Best regards
> >
> > Heinrich
> >
> > >>
> > >> What problem are you seeing?
> > >
> > > I can agree with Simon's claim that the notion of UCLASS_EFI sounds vague.
> > > In my understanding, it is expected to represent any UEFI driver/application
> > > who will convert "UEFI protocol" to "U-Boot device", the only example
> > > right now is "efi_block" device.
> > >
> > > One of issues that I can see is that any instance of UCLASS_EFI doesn't
> > > appear in DM tree. I have made an experimental patch which makes UCLASS_EFI
> > > a pseudo U-Boot device. DM tree would look like:
> > > root
> > > |-- 'EFI block driver' (CLASS_EFI for block_io_protocol)
> > > |-- 'efi_blk' (CLASS_BLK with TYPE_EFI)
> > >
> > > This way, the meaning of UCLASS_EFI should be much clearer?
> > >
> > > # In fact, the actual implementer of BLOCK_IO_PROTOCOL is
> > > # a loaded UEFI application, though.
>
> In which case, do we need it? Or should I rename the existing
> UCLASS_EFI to UCLASS_EFI_LOADER ? At some point I hope we can clean it
> up, but this might allow us to make progress.
I'd really like to get this series in. Can you please take a look at
this? I don't understand what test fails or what the problem is here.
Please unblock this series and let's get things moving. RC1 has
already come out and this series was sent out over 2 months ago now.
Loads of other things have been applied in the meantime.
If you don't want to be the maintainer for this part of EFI, that's
fine with me, but please let me know.
Regards,
Simon
More information about the U-Boot
mailing list