[PATCH v4 09/34] efi: Add EFI uclass for media
Simon Glass
sjg at chromium.org
Mon Nov 8 16:58:42 CET 2021
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.
Regards,
Simon
More information about the U-Boot
mailing list