mbox series

[0/4] introduce EFI_RAM_DISK_PROTOCOL

Message ID 20230707040045.485790-1-masahisa.kojima@linaro.org
Headers show
Series introduce EFI_RAM_DISK_PROTOCOL | expand

Message

Masahisa Kojima July 7, 2023, 4 a.m. UTC
This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
The major purpose of this series is a preparation for EFI HTTP(S) boot.

Now U-Boot can download the distro installer ISO image
via wget or tftpboot commands, but U-Boot can not mount
the downloaded ISO image.
By calling EFI_RAM_DISK_PROTOCOL->register API, user can
mount the ISO image and boot the distro installer.

Note that the installation process may not proceed
after the distro installer calls ExitBootServices()
since there is no hand-off process for the block device of
memory mapped ISO image.

The EFI_RAM_DISK_PROTOCOL was tested with the
debian network installer[1] on qemu_arm64 platform.
The example procedure is as follows.
 => tftpboot 41000000 mini.iso
 => efidebug disk load 41000000 $filesize

After these commands, ISO image is mounted like:

 => efidebug dh

    000000007eec5910 (efiblk#0)
      /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)
      Block IO

    000000007eec6140 (efiblk#0:1)
      /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(1,0x01,0,0x0,0x33800)
      Block IO

    000000007eec62b0 (efiblk#0:2)
      /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(2,0x01,0,0x33800,0x10000)
      Block IO
      System Partition
      Simple File System

  => dm tree

    blk           0  [ + ]   efi_blk               `-- efiblk#0
    partition     0  [ + ]   blk_partition             |-- efiblk#0:1
    partition     1  [ + ]   blk_partition             `-- efiblk#0:2

Debian can be successfully installed with this RAM disk on QEMU.

[TODO]
- udevices created in ./lib/efi_driver/efi_block_device.c::efi_bl_bind()
  are not removed when the efi_handle is removed.
  So after unload the RAM disk, udevices still exist.
  I plan to add a udevice removal process in ./lib/efi_driver/efi_uclass.c::efi_uc_stop().
  In addition, I also plan to add unbind() callback in struct efi_driver_ops.


[1] https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/mini.iso

Masahisa Kojima (4):
  efi_loader: add RAM disk device path
  efi_loader: add EFI_RAM_DISK_PROTOCOL implementation
  cmd: efidebug: add RAM disk mount command
  efi_selftest: add EFI_RAM_DISK_PROTOCOL selftest

 cmd/efidebug.c                           | 117 ++++++
 include/efi_api.h                        |  32 ++
 include/efi_loader.h                     |   4 +
 lib/efi_driver/efi_uclass.c              |   7 +-
 lib/efi_loader/Kconfig                   |   6 +
 lib/efi_loader/Makefile                  |   1 +
 lib/efi_loader/efi_device_path_to_text.c |  14 +
 lib/efi_loader/efi_ram_disk.c            | 334 +++++++++++++++
 lib/efi_loader/efi_setup.c               |   6 +
 lib/efi_selftest/Makefile                |   1 +
 lib/efi_selftest/efi_selftest_ram_disk.c | 511 +++++++++++++++++++++++
 lib/uuid.c                               |   4 +
 12 files changed, 1035 insertions(+), 2 deletions(-)
 create mode 100644 lib/efi_loader/efi_ram_disk.c
 create mode 100644 lib/efi_selftest/efi_selftest_ram_disk.c


base-commit: e2e2aea5733f0d23cd9593bbefe5c803c552dcb9

Comments

AKASHI Takahiro July 7, 2023, 5:17 a.m. UTC | #1
On Fri, Jul 07, 2023 at 01:00:40PM +0900, Masahisa Kojima wrote:
> This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
> The major purpose of this series is a preparation for EFI HTTP(S) boot.
> 
> Now U-Boot can download the distro installer ISO image
> via wget or tftpboot commands, but U-Boot can not mount
> the downloaded ISO image.
> By calling EFI_RAM_DISK_PROTOCOL->register API, user can
> mount the ISO image and boot the distro installer.
> 
> Note that the installation process may not proceed
> after the distro installer calls ExitBootServices()
> since there is no hand-off process for the block device of
> memory mapped ISO image.
> 
> The EFI_RAM_DISK_PROTOCOL was tested with the
> debian network installer[1] on qemu_arm64 platform.
> The example procedure is as follows.
>  => tftpboot 41000000 mini.iso
>  => efidebug disk load 41000000 $filesize

Can a disk created here be used natively on U-Boot
with commands like (fs)ls or (fs)load?
If so, it would be nice to mention the fact.

-Takahiro Akashi


> After these commands, ISO image is mounted like:
> 
>  => efidebug dh
> 
>     000000007eec5910 (efiblk#0)
>       /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)
>       Block IO
> 
>     000000007eec6140 (efiblk#0:1)
>       /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(1,0x01,0,0x0,0x33800)
>       Block IO
> 
>     000000007eec62b0 (efiblk#0:2)
>       /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(2,0x01,0,0x33800,0x10000)
>       Block IO
>       System Partition
>       Simple File System
> 
>   => dm tree
> 
>     blk           0  [ + ]   efi_blk               `-- efiblk#0
>     partition     0  [ + ]   blk_partition             |-- efiblk#0:1
>     partition     1  [ + ]   blk_partition             `-- efiblk#0:2
> 
> Debian can be successfully installed with this RAM disk on QEMU.
> 
> [TODO]
> - udevices created in ./lib/efi_driver/efi_block_device.c::efi_bl_bind()
>   are not removed when the efi_handle is removed.
>   So after unload the RAM disk, udevices still exist.
>   I plan to add a udevice removal process in ./lib/efi_driver/efi_uclass.c::efi_uc_stop().
>   In addition, I also plan to add unbind() callback in struct efi_driver_ops.
> 
> 
> [1] https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/mini.iso
> 
> Masahisa Kojima (4):
>   efi_loader: add RAM disk device path
>   efi_loader: add EFI_RAM_DISK_PROTOCOL implementation
>   cmd: efidebug: add RAM disk mount command
>   efi_selftest: add EFI_RAM_DISK_PROTOCOL selftest
> 
>  cmd/efidebug.c                           | 117 ++++++
>  include/efi_api.h                        |  32 ++
>  include/efi_loader.h                     |   4 +
>  lib/efi_driver/efi_uclass.c              |   7 +-
>  lib/efi_loader/Kconfig                   |   6 +
>  lib/efi_loader/Makefile                  |   1 +
>  lib/efi_loader/efi_device_path_to_text.c |  14 +
>  lib/efi_loader/efi_ram_disk.c            | 334 +++++++++++++++
>  lib/efi_loader/efi_setup.c               |   6 +
>  lib/efi_selftest/Makefile                |   1 +
>  lib/efi_selftest/efi_selftest_ram_disk.c | 511 +++++++++++++++++++++++
>  lib/uuid.c                               |   4 +
>  12 files changed, 1035 insertions(+), 2 deletions(-)
>  create mode 100644 lib/efi_loader/efi_ram_disk.c
>  create mode 100644 lib/efi_selftest/efi_selftest_ram_disk.c
> 
> 
> base-commit: e2e2aea5733f0d23cd9593bbefe5c803c552dcb9
> -- 
> 2.34.1
>
Heinrich Schuchardt July 7, 2023, 6:29 a.m. UTC | #2
On 7/7/23 06:00, Masahisa Kojima wrote:
> This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
> The major purpose of this series is a preparation for EFI HTTP(S) boot.
>
> Now U-Boot can download the distro installer ISO image
> via wget or tftpboot commands, but U-Boot can not mount
> the downloaded ISO image.
> By calling EFI_RAM_DISK_PROTOCOL->register API, user can
> mount the ISO image and boot the distro installer.

If I understand you correctly, with your design RAM disks will only
exist in the EFI sub-system.

We strive to synchronize what is happening on the driver model level and
on the UEFI level. I would prefer a design where we have a UCLASS_BLK
driver ram disks and the EFI_RAM_DISK_PROTOCOL is a one method of
managing ram disk devices.

A big issue we have is RAM management. malloc() can only assign limited
amount of memory which is probably too small for the RAM disk you are
looking at. We manage page sized memory in the EFI sub-system but this
is not integrated with the LMB memory checks.

The necessary sequence of development is probably:

* Rework memory management
* Implement ramdisks as UCLASS_BLK driver
* Implement the EFI_RAM_DISK_PROTOCOL based on the UCLASS_BLK driver.

Best regards

Heinrich

>
> Note that the installation process may not proceed
> after the distro installer calls ExitBootServices()
> since there is no hand-off process for the block device of
> memory mapped ISO image.
>
> The EFI_RAM_DISK_PROTOCOL was tested with the
> debian network installer[1] on qemu_arm64 platform.
> The example procedure is as follows.
>   => tftpboot 41000000 mini.iso
>   => efidebug disk load 41000000 $filesize
>
> After these commands, ISO image is mounted like:
>
>   => efidebug dh
>
>      000000007eec5910 (efiblk#0)
>        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)
>        Block IO
>
>      000000007eec6140 (efiblk#0:1)
>        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(1,0x01,0,0x0,0x33800)
>        Block IO
>
>      000000007eec62b0 (efiblk#0:2)
>        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(2,0x01,0,0x33800,0x10000)
>        Block IO
>        System Partition
>        Simple File System
>
>    => dm tree
>
>      blk           0  [ + ]   efi_blk               `-- efiblk#0
>      partition     0  [ + ]   blk_partition             |-- efiblk#0:1
>      partition     1  [ + ]   blk_partition             `-- efiblk#0:2
>
> Debian can be successfully installed with this RAM disk on QEMU.
>
> [TODO]
> - udevices created in ./lib/efi_driver/efi_block_device.c::efi_bl_bind()
>    are not removed when the efi_handle is removed.
>    So after unload the RAM disk, udevices still exist.
>    I plan to add a udevice removal process in ./lib/efi_driver/efi_uclass.c::efi_uc_stop().
>    In addition, I also plan to add unbind() callback in struct efi_driver_ops.
>
>
> [1] https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/mini.iso
>
> Masahisa Kojima (4):
>    efi_loader: add RAM disk device path
>    efi_loader: add EFI_RAM_DISK_PROTOCOL implementation
>    cmd: efidebug: add RAM disk mount command
>    efi_selftest: add EFI_RAM_DISK_PROTOCOL selftest
>
>   cmd/efidebug.c                           | 117 ++++++
>   include/efi_api.h                        |  32 ++
>   include/efi_loader.h                     |   4 +
>   lib/efi_driver/efi_uclass.c              |   7 +-
>   lib/efi_loader/Kconfig                   |   6 +
>   lib/efi_loader/Makefile                  |   1 +
>   lib/efi_loader/efi_device_path_to_text.c |  14 +
>   lib/efi_loader/efi_ram_disk.c            | 334 +++++++++++++++
>   lib/efi_loader/efi_setup.c               |   6 +
>   lib/efi_selftest/Makefile                |   1 +
>   lib/efi_selftest/efi_selftest_ram_disk.c | 511 +++++++++++++++++++++++
>   lib/uuid.c                               |   4 +
>   12 files changed, 1035 insertions(+), 2 deletions(-)
>   create mode 100644 lib/efi_loader/efi_ram_disk.c
>   create mode 100644 lib/efi_selftest/efi_selftest_ram_disk.c
>
>
> base-commit: e2e2aea5733f0d23cd9593bbefe5c803c552dcb9
AKASHI Takahiro July 7, 2023, 7:16 a.m. UTC | #3
On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote:
> On 7/7/23 06:00, Masahisa Kojima wrote:
> > This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
> > The major purpose of this series is a preparation for EFI HTTP(S) boot.
> > 
> > Now U-Boot can download the distro installer ISO image
> > via wget or tftpboot commands, but U-Boot can not mount
> > the downloaded ISO image.
> > By calling EFI_RAM_DISK_PROTOCOL->register API, user can
> > mount the ISO image and boot the distro installer.
> 
> If I understand you correctly, with your design RAM disks will only
> exist in the EFI sub-system.

Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot
block device (and associated partition devices) thanks to your
efi_driver framework.

> We strive to synchronize what is happening on the driver model level and
> on the UEFI level. I would prefer a design where we have a UCLASS_BLK
> driver ram disks and the EFI_RAM_DISK_PROTOCOL is a one method of
> managing ram disk devices.

That said, I agree to starting with U-Boot block device implementation,
UEFI_DISK comes automatically. It benefits both U-Boot proper and
UEFI subsystem equally as well.
(That is also what I meant to say in my first response.)

> A big issue we have is RAM management. malloc() can only assign limited
> amount of memory which is probably too small for the RAM disk you are
> looking at. We manage page sized memory in the EFI sub-system but this
> is not integrated with the LMB memory checks.

Not sure, is it enough simply to add some restrictions on the start size
and the size when a memory region is specified for a raw disk?

-Takahiro Akashi

> The necessary sequence of development is probably:
> 
> * Rework memory management
> * Implement ramdisks as UCLASS_BLK driver
> * Implement the EFI_RAM_DISK_PROTOCOL based on the UCLASS_BLK driver.
> 
> Best regards
> 
> Heinrich
> 
> > 
> > Note that the installation process may not proceed
> > after the distro installer calls ExitBootServices()
> > since there is no hand-off process for the block device of
> > memory mapped ISO image.
> > 
> > The EFI_RAM_DISK_PROTOCOL was tested with the
> > debian network installer[1] on qemu_arm64 platform.
> > The example procedure is as follows.
> >   => tftpboot 41000000 mini.iso
> >   => efidebug disk load 41000000 $filesize
> > 
> > After these commands, ISO image is mounted like:
> > 
> >   => efidebug dh
> > 
> >      000000007eec5910 (efiblk#0)
> >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)
> >        Block IO
> > 
> >      000000007eec6140 (efiblk#0:1)
> >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(1,0x01,0,0x0,0x33800)
> >        Block IO
> > 
> >      000000007eec62b0 (efiblk#0:2)
> >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(2,0x01,0,0x33800,0x10000)
> >        Block IO
> >        System Partition
> >        Simple File System
> > 
> >    => dm tree
> > 
> >      blk           0  [ + ]   efi_blk               `-- efiblk#0
> >      partition     0  [ + ]   blk_partition             |-- efiblk#0:1
> >      partition     1  [ + ]   blk_partition             `-- efiblk#0:2
> > 
> > Debian can be successfully installed with this RAM disk on QEMU.
> > 
> > [TODO]
> > - udevices created in ./lib/efi_driver/efi_block_device.c::efi_bl_bind()
> >    are not removed when the efi_handle is removed.
> >    So after unload the RAM disk, udevices still exist.
> >    I plan to add a udevice removal process in ./lib/efi_driver/efi_uclass.c::efi_uc_stop().
> >    In addition, I also plan to add unbind() callback in struct efi_driver_ops.
> > 
> > 
> > [1] https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/mini.iso
> > 
> > Masahisa Kojima (4):
> >    efi_loader: add RAM disk device path
> >    efi_loader: add EFI_RAM_DISK_PROTOCOL implementation
> >    cmd: efidebug: add RAM disk mount command
> >    efi_selftest: add EFI_RAM_DISK_PROTOCOL selftest
> > 
> >   cmd/efidebug.c                           | 117 ++++++
> >   include/efi_api.h                        |  32 ++
> >   include/efi_loader.h                     |   4 +
> >   lib/efi_driver/efi_uclass.c              |   7 +-
> >   lib/efi_loader/Kconfig                   |   6 +
> >   lib/efi_loader/Makefile                  |   1 +
> >   lib/efi_loader/efi_device_path_to_text.c |  14 +
> >   lib/efi_loader/efi_ram_disk.c            | 334 +++++++++++++++
> >   lib/efi_loader/efi_setup.c               |   6 +
> >   lib/efi_selftest/Makefile                |   1 +
> >   lib/efi_selftest/efi_selftest_ram_disk.c | 511 +++++++++++++++++++++++
> >   lib/uuid.c                               |   4 +
> >   12 files changed, 1035 insertions(+), 2 deletions(-)
> >   create mode 100644 lib/efi_loader/efi_ram_disk.c
> >   create mode 100644 lib/efi_selftest/efi_selftest_ram_disk.c
> > 
> > 
> > base-commit: e2e2aea5733f0d23cd9593bbefe5c803c552dcb9
>
Masahisa Kojima July 7, 2023, 8:10 a.m. UTC | #4
Hi Heinrich,

On Fri, 7 Jul 2023 at 15:34, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 7/7/23 06:00, Masahisa Kojima wrote:
> > This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
> > The major purpose of this series is a preparation for EFI HTTP(S) boot.
> >
> > Now U-Boot can download the distro installer ISO image
> > via wget or tftpboot commands, but U-Boot can not mount
> > the downloaded ISO image.
> > By calling EFI_RAM_DISK_PROTOCOL->register API, user can
> > mount the ISO image and boot the distro installer.
>
> If I understand you correctly, with your design RAM disks will only
> exist in the EFI sub-system.

Yes, RAM disk can be accessed through the EFI sub-system.

>
> We strive to synchronize what is happening on the driver model level and
> on the UEFI level. I would prefer a design where we have a UCLASS_BLK
> driver ram disks and the EFI_RAM_DISK_PROTOCOL is a one method of
> managing ram disk devices.

The current EFI_RAM_DIISK_PROTOCOL implementation is the same as what
EDK2 reference
implementation does, and I'm not yet fully convinced how much the
native U-Boot gets the benefit
of RAM disk support.

>
> A big issue we have is RAM management. malloc() can only assign limited
> amount of memory which is probably too small for the RAM disk you are
> looking at. We manage page sized memory in the EFI sub-system but this
> is not integrated with the LMB memory checks.

OK, when we think about EFI HTTP boot and hand-off the RAM disk to the kernel,
we need to consider the memory allocation for the big ISO image.
But as far as EFI_RAM_DISK_PROTOCOL is concerned, can we leave
the memory management issue?

Thanks,
Masahisa Kojima

>
> The necessary sequence of development is probably:
>
> * Rework memory management
> * Implement ramdisks as UCLASS_BLK driver
> * Implement the EFI_RAM_DISK_PROTOCOL based on the UCLASS_BLK driver.
>
> Best regards
>
> Heinrich
>
> >
> > Note that the installation process may not proceed
> > after the distro installer calls ExitBootServices()
> > since there is no hand-off process for the block device of
> > memory mapped ISO image.
> >
> > The EFI_RAM_DISK_PROTOCOL was tested with the
> > debian network installer[1] on qemu_arm64 platform.
> > The example procedure is as follows.
> >   => tftpboot 41000000 mini.iso
> >   => efidebug disk load 41000000 $filesize
> >
> > After these commands, ISO image is mounted like:
> >
> >   => efidebug dh
> >
> >      000000007eec5910 (efiblk#0)
> >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)
> >        Block IO
> >
> >      000000007eec6140 (efiblk#0:1)
> >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(1,0x01,0,0x0,0x33800)
> >        Block IO
> >
> >      000000007eec62b0 (efiblk#0:2)
> >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(2,0x01,0,0x33800,0x10000)
> >        Block IO
> >        System Partition
> >        Simple File System
> >
> >    => dm tree
> >
> >      blk           0  [ + ]   efi_blk               `-- efiblk#0
> >      partition     0  [ + ]   blk_partition             |-- efiblk#0:1
> >      partition     1  [ + ]   blk_partition             `-- efiblk#0:2
> >
> > Debian can be successfully installed with this RAM disk on QEMU.
> >
> > [TODO]
> > - udevices created in ./lib/efi_driver/efi_block_device.c::efi_bl_bind()
> >    are not removed when the efi_handle is removed.
> >    So after unload the RAM disk, udevices still exist.
> >    I plan to add a udevice removal process in ./lib/efi_driver/efi_uclass.c::efi_uc_stop().
> >    In addition, I also plan to add unbind() callback in struct efi_driver_ops.
> >
> >
> > [1] https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/mini.iso
> >
> > Masahisa Kojima (4):
> >    efi_loader: add RAM disk device path
> >    efi_loader: add EFI_RAM_DISK_PROTOCOL implementation
> >    cmd: efidebug: add RAM disk mount command
> >    efi_selftest: add EFI_RAM_DISK_PROTOCOL selftest
> >
> >   cmd/efidebug.c                           | 117 ++++++
> >   include/efi_api.h                        |  32 ++
> >   include/efi_loader.h                     |   4 +
> >   lib/efi_driver/efi_uclass.c              |   7 +-
> >   lib/efi_loader/Kconfig                   |   6 +
> >   lib/efi_loader/Makefile                  |   1 +
> >   lib/efi_loader/efi_device_path_to_text.c |  14 +
> >   lib/efi_loader/efi_ram_disk.c            | 334 +++++++++++++++
> >   lib/efi_loader/efi_setup.c               |   6 +
> >   lib/efi_selftest/Makefile                |   1 +
> >   lib/efi_selftest/efi_selftest_ram_disk.c | 511 +++++++++++++++++++++++
> >   lib/uuid.c                               |   4 +
> >   12 files changed, 1035 insertions(+), 2 deletions(-)
> >   create mode 100644 lib/efi_loader/efi_ram_disk.c
> >   create mode 100644 lib/efi_selftest/efi_selftest_ram_disk.c
> >
> >
> > base-commit: e2e2aea5733f0d23cd9593bbefe5c803c552dcb9
>
Masahisa Kojima July 7, 2023, 8:19 a.m. UTC | #5
Hi Akashi-san,

On Fri, 7 Jul 2023 at 16:16, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote:
> > On 7/7/23 06:00, Masahisa Kojima wrote:
> > > This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
> > > The major purpose of this series is a preparation for EFI HTTP(S) boot.
> > >
> > > Now U-Boot can download the distro installer ISO image
> > > via wget or tftpboot commands, but U-Boot can not mount
> > > the downloaded ISO image.
> > > By calling EFI_RAM_DISK_PROTOCOL->register API, user can
> > > mount the ISO image and boot the distro installer.
> >
> > If I understand you correctly, with your design RAM disks will only
> > exist in the EFI sub-system.
>
> Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot
> block device (and associated partition devices) thanks to your
> efi_driver framework.

Read/Write the RAM disk is expected to be called from the EFI context, so
native U-Boot can not access the RAM disk.
 # Honestly speaking, I'm not sure how U-Boot prohibits the access to
the EFI RAM disk
    because the udevices are created for the RAM disk.

Thanks,
Masahisa Kojima

>
> > We strive to synchronize what is happening on the driver model level and
> > on the UEFI level. I would prefer a design where we have a UCLASS_BLK
> > driver ram disks and the EFI_RAM_DISK_PROTOCOL is a one method of
> > managing ram disk devices.
>
> That said, I agree to starting with U-Boot block device implementation,
> UEFI_DISK comes automatically. It benefits both U-Boot proper and
> UEFI subsystem equally as well.
> (That is also what I meant to say in my first response.)
>
> > A big issue we have is RAM management. malloc() can only assign limited
> > amount of memory which is probably too small for the RAM disk you are
> > looking at. We manage page sized memory in the EFI sub-system but this
> > is not integrated with the LMB memory checks.
>
> Not sure, is it enough simply to add some restrictions on the start size
> and the size when a memory region is specified for a raw disk?
>
> -Takahiro Akashi
>
> > The necessary sequence of development is probably:
> >
> > * Rework memory management
> > * Implement ramdisks as UCLASS_BLK driver
> > * Implement the EFI_RAM_DISK_PROTOCOL based on the UCLASS_BLK driver.
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > > Note that the installation process may not proceed
> > > after the distro installer calls ExitBootServices()
> > > since there is no hand-off process for the block device of
> > > memory mapped ISO image.
> > >
> > > The EFI_RAM_DISK_PROTOCOL was tested with the
> > > debian network installer[1] on qemu_arm64 platform.
> > > The example procedure is as follows.
> > >   => tftpboot 41000000 mini.iso
> > >   => efidebug disk load 41000000 $filesize
> > >
> > > After these commands, ISO image is mounted like:
> > >
> > >   => efidebug dh
> > >
> > >      000000007eec5910 (efiblk#0)
> > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)
> > >        Block IO
> > >
> > >      000000007eec6140 (efiblk#0:1)
> > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(1,0x01,0,0x0,0x33800)
> > >        Block IO
> > >
> > >      000000007eec62b0 (efiblk#0:2)
> > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(2,0x01,0,0x33800,0x10000)
> > >        Block IO
> > >        System Partition
> > >        Simple File System
> > >
> > >    => dm tree
> > >
> > >      blk           0  [ + ]   efi_blk               `-- efiblk#0
> > >      partition     0  [ + ]   blk_partition             |-- efiblk#0:1
> > >      partition     1  [ + ]   blk_partition             `-- efiblk#0:2
> > >
> > > Debian can be successfully installed with this RAM disk on QEMU.
> > >
> > > [TODO]
> > > - udevices created in ./lib/efi_driver/efi_block_device.c::efi_bl_bind()
> > >    are not removed when the efi_handle is removed.
> > >    So after unload the RAM disk, udevices still exist.
> > >    I plan to add a udevice removal process in ./lib/efi_driver/efi_uclass.c::efi_uc_stop().
> > >    In addition, I also plan to add unbind() callback in struct efi_driver_ops.
> > >
> > >
> > > [1] https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/mini.iso
> > >
> > > Masahisa Kojima (4):
> > >    efi_loader: add RAM disk device path
> > >    efi_loader: add EFI_RAM_DISK_PROTOCOL implementation
> > >    cmd: efidebug: add RAM disk mount command
> > >    efi_selftest: add EFI_RAM_DISK_PROTOCOL selftest
> > >
> > >   cmd/efidebug.c                           | 117 ++++++
> > >   include/efi_api.h                        |  32 ++
> > >   include/efi_loader.h                     |   4 +
> > >   lib/efi_driver/efi_uclass.c              |   7 +-
> > >   lib/efi_loader/Kconfig                   |   6 +
> > >   lib/efi_loader/Makefile                  |   1 +
> > >   lib/efi_loader/efi_device_path_to_text.c |  14 +
> > >   lib/efi_loader/efi_ram_disk.c            | 334 +++++++++++++++
> > >   lib/efi_loader/efi_setup.c               |   6 +
> > >   lib/efi_selftest/Makefile                |   1 +
> > >   lib/efi_selftest/efi_selftest_ram_disk.c | 511 +++++++++++++++++++++++
> > >   lib/uuid.c                               |   4 +
> > >   12 files changed, 1035 insertions(+), 2 deletions(-)
> > >   create mode 100644 lib/efi_loader/efi_ram_disk.c
> > >   create mode 100644 lib/efi_selftest/efi_selftest_ram_disk.c
> > >
> > >
> > > base-commit: e2e2aea5733f0d23cd9593bbefe5c803c552dcb9
> >
AKASHI Takahiro July 7, 2023, 9:12 a.m. UTC | #6
On Fri, Jul 07, 2023 at 05:19:33PM +0900, Masahisa Kojima wrote:
> Hi Akashi-san,
> 
> On Fri, 7 Jul 2023 at 16:16, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >
> > On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote:
> > > On 7/7/23 06:00, Masahisa Kojima wrote:
> > > > This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
> > > > The major purpose of this series is a preparation for EFI HTTP(S) boot.
> > > >
> > > > Now U-Boot can download the distro installer ISO image
> > > > via wget or tftpboot commands, but U-Boot can not mount
> > > > the downloaded ISO image.
> > > > By calling EFI_RAM_DISK_PROTOCOL->register API, user can
> > > > mount the ISO image and boot the distro installer.
> > >
> > > If I understand you correctly, with your design RAM disks will only
> > > exist in the EFI sub-system.
> >
> > Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot
> > block device (and associated partition devices) thanks to your
> > efi_driver framework.
> 
> Read/Write the RAM disk is expected to be called from the EFI context, so

An associated U-Boot block device should work as well on top of your
RAW_DISK_PROTOCOL via a generated RAM disk object (efi_disk).
That is why SYMPLE_FILE_SYSTEM_PROTOCOL, which solely relies on U-Boot's proper
filesystem subsystem, is installed to the RAM disk object.

> native U-Boot can not access the RAM disk.

I believe that, in theory, "fatls efidisk 0 1" (or efi_disk as an interface?)
should work even under your current implementation.

-Takahiro Akashi

>  # Honestly speaking, I'm not sure how U-Boot prohibits the access to
> the EFI RAM disk
>     because the udevices are created for the RAM disk.
> 
> Thanks,
> Masahisa Kojima
> 
> >
> > > We strive to synchronize what is happening on the driver model level and
> > > on the UEFI level. I would prefer a design where we have a UCLASS_BLK
> > > driver ram disks and the EFI_RAM_DISK_PROTOCOL is a one method of
> > > managing ram disk devices.
> >
> > That said, I agree to starting with U-Boot block device implementation,
> > UEFI_DISK comes automatically. It benefits both U-Boot proper and
> > UEFI subsystem equally as well.
> > (That is also what I meant to say in my first response.)
> >
> > > A big issue we have is RAM management. malloc() can only assign limited
> > > amount of memory which is probably too small for the RAM disk you are
> > > looking at. We manage page sized memory in the EFI sub-system but this
> > > is not integrated with the LMB memory checks.
> >
> > Not sure, is it enough simply to add some restrictions on the start size
> > and the size when a memory region is specified for a raw disk?
> >
> > -Takahiro Akashi
> >
> > > The necessary sequence of development is probably:
> > >
> > > * Rework memory management
> > > * Implement ramdisks as UCLASS_BLK driver
> > > * Implement the EFI_RAM_DISK_PROTOCOL based on the UCLASS_BLK driver.
> > >
> > > Best regards
> > >
> > > Heinrich
> > >
> > > >
> > > > Note that the installation process may not proceed
> > > > after the distro installer calls ExitBootServices()
> > > > since there is no hand-off process for the block device of
> > > > memory mapped ISO image.
> > > >
> > > > The EFI_RAM_DISK_PROTOCOL was tested with the
> > > > debian network installer[1] on qemu_arm64 platform.
> > > > The example procedure is as follows.
> > > >   => tftpboot 41000000 mini.iso
> > > >   => efidebug disk load 41000000 $filesize
> > > >
> > > > After these commands, ISO image is mounted like:
> > > >
> > > >   => efidebug dh
> > > >
> > > >      000000007eec5910 (efiblk#0)
> > > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)
> > > >        Block IO
> > > >
> > > >      000000007eec6140 (efiblk#0:1)
> > > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(1,0x01,0,0x0,0x33800)
> > > >        Block IO
> > > >
> > > >      000000007eec62b0 (efiblk#0:2)
> > > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(2,0x01,0,0x33800,0x10000)
> > > >        Block IO
> > > >        System Partition
> > > >        Simple File System
> > > >
> > > >    => dm tree
> > > >
> > > >      blk           0  [ + ]   efi_blk               `-- efiblk#0
> > > >      partition     0  [ + ]   blk_partition             |-- efiblk#0:1
> > > >      partition     1  [ + ]   blk_partition             `-- efiblk#0:2
> > > >
> > > > Debian can be successfully installed with this RAM disk on QEMU.
> > > >
> > > > [TODO]
> > > > - udevices created in ./lib/efi_driver/efi_block_device.c::efi_bl_bind()
> > > >    are not removed when the efi_handle is removed.
> > > >    So after unload the RAM disk, udevices still exist.
> > > >    I plan to add a udevice removal process in ./lib/efi_driver/efi_uclass.c::efi_uc_stop().
> > > >    In addition, I also plan to add unbind() callback in struct efi_driver_ops.
> > > >
> > > >
> > > > [1] https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/mini.iso
> > > >
> > > > Masahisa Kojima (4):
> > > >    efi_loader: add RAM disk device path
> > > >    efi_loader: add EFI_RAM_DISK_PROTOCOL implementation
> > > >    cmd: efidebug: add RAM disk mount command
> > > >    efi_selftest: add EFI_RAM_DISK_PROTOCOL selftest
> > > >
> > > >   cmd/efidebug.c                           | 117 ++++++
> > > >   include/efi_api.h                        |  32 ++
> > > >   include/efi_loader.h                     |   4 +
> > > >   lib/efi_driver/efi_uclass.c              |   7 +-
> > > >   lib/efi_loader/Kconfig                   |   6 +
> > > >   lib/efi_loader/Makefile                  |   1 +
> > > >   lib/efi_loader/efi_device_path_to_text.c |  14 +
> > > >   lib/efi_loader/efi_ram_disk.c            | 334 +++++++++++++++
> > > >   lib/efi_loader/efi_setup.c               |   6 +
> > > >   lib/efi_selftest/Makefile                |   1 +
> > > >   lib/efi_selftest/efi_selftest_ram_disk.c | 511 +++++++++++++++++++++++
> > > >   lib/uuid.c                               |   4 +
> > > >   12 files changed, 1035 insertions(+), 2 deletions(-)
> > > >   create mode 100644 lib/efi_loader/efi_ram_disk.c
> > > >   create mode 100644 lib/efi_selftest/efi_selftest_ram_disk.c
> > > >
> > > >
> > > > base-commit: e2e2aea5733f0d23cd9593bbefe5c803c552dcb9
> > >
Masahisa Kojima July 10, 2023, 2:13 a.m. UTC | #7
On Fri, 7 Jul 2023 at 18:12, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>
> On Fri, Jul 07, 2023 at 05:19:33PM +0900, Masahisa Kojima wrote:
> > Hi Akashi-san,
> >
> > On Fri, 7 Jul 2023 at 16:16, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > >
> > > On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote:
> > > > On 7/7/23 06:00, Masahisa Kojima wrote:
> > > > > This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
> > > > > The major purpose of this series is a preparation for EFI HTTP(S) boot.
> > > > >
> > > > > Now U-Boot can download the distro installer ISO image
> > > > > via wget or tftpboot commands, but U-Boot can not mount
> > > > > the downloaded ISO image.
> > > > > By calling EFI_RAM_DISK_PROTOCOL->register API, user can
> > > > > mount the ISO image and boot the distro installer.
> > > >
> > > > If I understand you correctly, with your design RAM disks will only
> > > > exist in the EFI sub-system.
> > >
> > > Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot
> > > block device (and associated partition devices) thanks to your
> > > efi_driver framework.
> >
> > Read/Write the RAM disk is expected to be called from the EFI context, so
>
> An associated U-Boot block device should work as well on top of your
> RAW_DISK_PROTOCOL via a generated RAM disk object (efi_disk).
> That is why SYMPLE_FILE_SYSTEM_PROTOCOL, which solely relies on U-Boot's proper
> filesystem subsystem, is installed to the RAM disk object.

I now realize that my RAM_DISK_PROTOCOL implementation happens to work
thanks to the block cache.
When I disable CONFIG_BLOCK_CACHE and load some EFI application(it
does set  'app_gd')
before calling RAM_DISK_PROTOCOL service, RAM_DISK_PROTOCOL does not work.
ConnectController fails.

>
> > native U-Boot can not access the RAM disk.
>
> I believe that, in theory, "fatls efidisk 0 1" (or efi_disk as an interface?)
> should work even under your current implementation.

"fatls efiloader 0:2" does not work.
I think "efiloader" device is designed to be accessed from EFI application only
even if it is listed by "dm tree".

I understand that ramdisk as UCLASS_BLK driver is required as Heinrich said.

Thanks,
Masahisa Kojima

>
> -Takahiro Akashi
>
> >  # Honestly speaking, I'm not sure how U-Boot prohibits the access to
> > the EFI RAM disk
> >     because the udevices are created for the RAM disk.
> >
> > Thanks,
> > Masahisa Kojima
> >
> > >
> > > > We strive to synchronize what is happening on the driver model level and
> > > > on the UEFI level. I would prefer a design where we have a UCLASS_BLK
> > > > driver ram disks and the EFI_RAM_DISK_PROTOCOL is a one method of
> > > > managing ram disk devices.
> > >
> > > That said, I agree to starting with U-Boot block device implementation,
> > > UEFI_DISK comes automatically. It benefits both U-Boot proper and
> > > UEFI subsystem equally as well.
> > > (That is also what I meant to say in my first response.)
> > >
> > > > A big issue we have is RAM management. malloc() can only assign limited
> > > > amount of memory which is probably too small for the RAM disk you are
> > > > looking at. We manage page sized memory in the EFI sub-system but this
> > > > is not integrated with the LMB memory checks.
> > >
> > > Not sure, is it enough simply to add some restrictions on the start size
> > > and the size when a memory region is specified for a raw disk?
> > >
> > > -Takahiro Akashi
> > >
> > > > The necessary sequence of development is probably:
> > > >
> > > > * Rework memory management
> > > > * Implement ramdisks as UCLASS_BLK driver
> > > > * Implement the EFI_RAM_DISK_PROTOCOL based on the UCLASS_BLK driver.
> > > >
> > > > Best regards
> > > >
> > > > Heinrich
> > > >
> > > > >
> > > > > Note that the installation process may not proceed
> > > > > after the distro installer calls ExitBootServices()
> > > > > since there is no hand-off process for the block device of
> > > > > memory mapped ISO image.
> > > > >
> > > > > The EFI_RAM_DISK_PROTOCOL was tested with the
> > > > > debian network installer[1] on qemu_arm64 platform.
> > > > > The example procedure is as follows.
> > > > >   => tftpboot 41000000 mini.iso
> > > > >   => efidebug disk load 41000000 $filesize
> > > > >
> > > > > After these commands, ISO image is mounted like:
> > > > >
> > > > >   => efidebug dh
> > > > >
> > > > >      000000007eec5910 (efiblk#0)
> > > > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)
> > > > >        Block IO
> > > > >
> > > > >      000000007eec6140 (efiblk#0:1)
> > > > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(1,0x01,0,0x0,0x33800)
> > > > >        Block IO
> > > > >
> > > > >      000000007eec62b0 (efiblk#0:2)
> > > > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(2,0x01,0,0x33800,0x10000)
> > > > >        Block IO
> > > > >        System Partition
> > > > >        Simple File System
> > > > >
> > > > >    => dm tree
> > > > >
> > > > >      blk           0  [ + ]   efi_blk               `-- efiblk#0
> > > > >      partition     0  [ + ]   blk_partition             |-- efiblk#0:1
> > > > >      partition     1  [ + ]   blk_partition             `-- efiblk#0:2
> > > > >
> > > > > Debian can be successfully installed with this RAM disk on QEMU.
> > > > >
> > > > > [TODO]
> > > > > - udevices created in ./lib/efi_driver/efi_block_device.c::efi_bl_bind()
> > > > >    are not removed when the efi_handle is removed.
> > > > >    So after unload the RAM disk, udevices still exist.
> > > > >    I plan to add a udevice removal process in ./lib/efi_driver/efi_uclass.c::efi_uc_stop().
> > > > >    In addition, I also plan to add unbind() callback in struct efi_driver_ops.
> > > > >
> > > > >
> > > > > [1] https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/mini.iso
> > > > >
> > > > > Masahisa Kojima (4):
> > > > >    efi_loader: add RAM disk device path
> > > > >    efi_loader: add EFI_RAM_DISK_PROTOCOL implementation
> > > > >    cmd: efidebug: add RAM disk mount command
> > > > >    efi_selftest: add EFI_RAM_DISK_PROTOCOL selftest
> > > > >
> > > > >   cmd/efidebug.c                           | 117 ++++++
> > > > >   include/efi_api.h                        |  32 ++
> > > > >   include/efi_loader.h                     |   4 +
> > > > >   lib/efi_driver/efi_uclass.c              |   7 +-
> > > > >   lib/efi_loader/Kconfig                   |   6 +
> > > > >   lib/efi_loader/Makefile                  |   1 +
> > > > >   lib/efi_loader/efi_device_path_to_text.c |  14 +
> > > > >   lib/efi_loader/efi_ram_disk.c            | 334 +++++++++++++++
> > > > >   lib/efi_loader/efi_setup.c               |   6 +
> > > > >   lib/efi_selftest/Makefile                |   1 +
> > > > >   lib/efi_selftest/efi_selftest_ram_disk.c | 511 +++++++++++++++++++++++
> > > > >   lib/uuid.c                               |   4 +
> > > > >   12 files changed, 1035 insertions(+), 2 deletions(-)
> > > > >   create mode 100644 lib/efi_loader/efi_ram_disk.c
> > > > >   create mode 100644 lib/efi_selftest/efi_selftest_ram_disk.c
> > > > >
> > > > >
> > > > > base-commit: e2e2aea5733f0d23cd9593bbefe5c803c552dcb9
> > > >
AKASHI Takahiro July 10, 2023, 2:28 a.m. UTC | #8
On Mon, Jul 10, 2023 at 11:13:12AM +0900, Masahisa Kojima wrote:
> On Fri, 7 Jul 2023 at 18:12, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >
> > On Fri, Jul 07, 2023 at 05:19:33PM +0900, Masahisa Kojima wrote:
> > > Hi Akashi-san,
> > >
> > > On Fri, 7 Jul 2023 at 16:16, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > > >
> > > > On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote:
> > > > > On 7/7/23 06:00, Masahisa Kojima wrote:
> > > > > > This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
> > > > > > The major purpose of this series is a preparation for EFI HTTP(S) boot.
> > > > > >
> > > > > > Now U-Boot can download the distro installer ISO image
> > > > > > via wget or tftpboot commands, but U-Boot can not mount
> > > > > > the downloaded ISO image.
> > > > > > By calling EFI_RAM_DISK_PROTOCOL->register API, user can
> > > > > > mount the ISO image and boot the distro installer.
> > > > >
> > > > > If I understand you correctly, with your design RAM disks will only
> > > > > exist in the EFI sub-system.
> > > >
> > > > Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot
> > > > block device (and associated partition devices) thanks to your
> > > > efi_driver framework.
> > >
> > > Read/Write the RAM disk is expected to be called from the EFI context, so
> >
> > An associated U-Boot block device should work as well on top of your
> > RAW_DISK_PROTOCOL via a generated RAM disk object (efi_disk).
> > That is why SYMPLE_FILE_SYSTEM_PROTOCOL, which solely relies on U-Boot's proper
> > filesystem subsystem, is installed to the RAM disk object.
> 
> I now realize that my RAM_DISK_PROTOCOL implementation happens to work
> thanks to the block cache.
> When I disable CONFIG_BLOCK_CACHE and load some EFI application(it
> does set  'app_gd')
> before calling RAM_DISK_PROTOCOL service, RAM_DISK_PROTOCOL does not work.
> ConnectController fails.
> 
> >
> > > native U-Boot can not access the RAM disk.
> >
> > I believe that, in theory, "fatls efidisk 0 1" (or efi_disk as an interface?)
> > should work even under your current implementation.
> 
> "fatls efiloader 0:2" does not work.
> I think "efiloader" device is designed to be accessed from EFI application only
> even if it is listed by "dm tree".

I don't believe so.
(the interface name may be "efi_blk" or "efiblk", though.)

If the command fails, it's a bug. Must be fixed.

> I understand that ramdisk as UCLASS_BLK driver is required as Heinrich said.

That said, I agree to starting with UCLASS_BLK from the viewpoint of
U-Boot driver model/UEFI integration.

-Takahiro Akashi

> 
> Thanks,
> Masahisa Kojima
> 
> >
> > -Takahiro Akashi
> >
> > >  # Honestly speaking, I'm not sure how U-Boot prohibits the access to
> > > the EFI RAM disk
> > >     because the udevices are created for the RAM disk.
> > >
> > > Thanks,
> > > Masahisa Kojima
> > >
> > > >
> > > > > We strive to synchronize what is happening on the driver model level and
> > > > > on the UEFI level. I would prefer a design where we have a UCLASS_BLK
> > > > > driver ram disks and the EFI_RAM_DISK_PROTOCOL is a one method of
> > > > > managing ram disk devices.
> > > >
> > > > That said, I agree to starting with U-Boot block device implementation,
> > > > UEFI_DISK comes automatically. It benefits both U-Boot proper and
> > > > UEFI subsystem equally as well.
> > > > (That is also what I meant to say in my first response.)
> > > >
> > > > > A big issue we have is RAM management. malloc() can only assign limited
> > > > > amount of memory which is probably too small for the RAM disk you are
> > > > > looking at. We manage page sized memory in the EFI sub-system but this
> > > > > is not integrated with the LMB memory checks.
> > > >
> > > > Not sure, is it enough simply to add some restrictions on the start size
> > > > and the size when a memory region is specified for a raw disk?
> > > >
> > > > -Takahiro Akashi
> > > >
> > > > > The necessary sequence of development is probably:
> > > > >
> > > > > * Rework memory management
> > > > > * Implement ramdisks as UCLASS_BLK driver
> > > > > * Implement the EFI_RAM_DISK_PROTOCOL based on the UCLASS_BLK driver.
> > > > >
> > > > > Best regards
> > > > >
> > > > > Heinrich
> > > > >
> > > > > >
> > > > > > Note that the installation process may not proceed
> > > > > > after the distro installer calls ExitBootServices()
> > > > > > since there is no hand-off process for the block device of
> > > > > > memory mapped ISO image.
> > > > > >
> > > > > > The EFI_RAM_DISK_PROTOCOL was tested with the
> > > > > > debian network installer[1] on qemu_arm64 platform.
> > > > > > The example procedure is as follows.
> > > > > >   => tftpboot 41000000 mini.iso
> > > > > >   => efidebug disk load 41000000 $filesize
> > > > > >
> > > > > > After these commands, ISO image is mounted like:
> > > > > >
> > > > > >   => efidebug dh
> > > > > >
> > > > > >      000000007eec5910 (efiblk#0)
> > > > > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)
> > > > > >        Block IO
> > > > > >
> > > > > >      000000007eec6140 (efiblk#0:1)
> > > > > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(1,0x01,0,0x0,0x33800)
> > > > > >        Block IO
> > > > > >
> > > > > >      000000007eec62b0 (efiblk#0:2)
> > > > > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(2,0x01,0,0x33800,0x10000)
> > > > > >        Block IO
> > > > > >        System Partition
> > > > > >        Simple File System
> > > > > >
> > > > > >    => dm tree
> > > > > >
> > > > > >      blk           0  [ + ]   efi_blk               `-- efiblk#0
> > > > > >      partition     0  [ + ]   blk_partition             |-- efiblk#0:1
> > > > > >      partition     1  [ + ]   blk_partition             `-- efiblk#0:2
> > > > > >
> > > > > > Debian can be successfully installed with this RAM disk on QEMU.
> > > > > >
> > > > > > [TODO]
> > > > > > - udevices created in ./lib/efi_driver/efi_block_device.c::efi_bl_bind()
> > > > > >    are not removed when the efi_handle is removed.
> > > > > >    So after unload the RAM disk, udevices still exist.
> > > > > >    I plan to add a udevice removal process in ./lib/efi_driver/efi_uclass.c::efi_uc_stop().
> > > > > >    In addition, I also plan to add unbind() callback in struct efi_driver_ops.
> > > > > >
> > > > > >
> > > > > > [1] https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/mini.iso
> > > > > >
> > > > > > Masahisa Kojima (4):
> > > > > >    efi_loader: add RAM disk device path
> > > > > >    efi_loader: add EFI_RAM_DISK_PROTOCOL implementation
> > > > > >    cmd: efidebug: add RAM disk mount command
> > > > > >    efi_selftest: add EFI_RAM_DISK_PROTOCOL selftest
> > > > > >
> > > > > >   cmd/efidebug.c                           | 117 ++++++
> > > > > >   include/efi_api.h                        |  32 ++
> > > > > >   include/efi_loader.h                     |   4 +
> > > > > >   lib/efi_driver/efi_uclass.c              |   7 +-
> > > > > >   lib/efi_loader/Kconfig                   |   6 +
> > > > > >   lib/efi_loader/Makefile                  |   1 +
> > > > > >   lib/efi_loader/efi_device_path_to_text.c |  14 +
> > > > > >   lib/efi_loader/efi_ram_disk.c            | 334 +++++++++++++++
> > > > > >   lib/efi_loader/efi_setup.c               |   6 +
> > > > > >   lib/efi_selftest/Makefile                |   1 +
> > > > > >   lib/efi_selftest/efi_selftest_ram_disk.c | 511 +++++++++++++++++++++++
> > > > > >   lib/uuid.c                               |   4 +
> > > > > >   12 files changed, 1035 insertions(+), 2 deletions(-)
> > > > > >   create mode 100644 lib/efi_loader/efi_ram_disk.c
> > > > > >   create mode 100644 lib/efi_selftest/efi_selftest_ram_disk.c
> > > > > >
> > > > > >
> > > > > > base-commit: e2e2aea5733f0d23cd9593bbefe5c803c552dcb9
> > > > >
Masahisa Kojima July 11, 2023, 6:23 a.m. UTC | #9
On Mon, 10 Jul 2023 at 11:28, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> On Mon, Jul 10, 2023 at 11:13:12AM +0900, Masahisa Kojima wrote:
> > On Fri, 7 Jul 2023 at 18:12, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > >
> > > On Fri, Jul 07, 2023 at 05:19:33PM +0900, Masahisa Kojima wrote:
> > > > Hi Akashi-san,
> > > >
> > > > On Fri, 7 Jul 2023 at 16:16, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > > > >
> > > > > On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote:
> > > > > > On 7/7/23 06:00, Masahisa Kojima wrote:
> > > > > > > This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
> > > > > > > The major purpose of this series is a preparation for EFI HTTP(S) boot.
> > > > > > >
> > > > > > > Now U-Boot can download the distro installer ISO image
> > > > > > > via wget or tftpboot commands, but U-Boot can not mount
> > > > > > > the downloaded ISO image.
> > > > > > > By calling EFI_RAM_DISK_PROTOCOL->register API, user can
> > > > > > > mount the ISO image and boot the distro installer.
> > > > > >
> > > > > > If I understand you correctly, with your design RAM disks will only
> > > > > > exist in the EFI sub-system.
> > > > >
> > > > > Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot
> > > > > block device (and associated partition devices) thanks to your
> > > > > efi_driver framework.
> > > >
> > > > Read/Write the RAM disk is expected to be called from the EFI context, so
> > >
> > > An associated U-Boot block device should work as well on top of your
> > > RAW_DISK_PROTOCOL via a generated RAM disk object (efi_disk).
> > > That is why SYMPLE_FILE_SYSTEM_PROTOCOL, which solely relies on U-Boot's proper
> > > filesystem subsystem, is installed to the RAM disk object.
> >
> > I now realize that my RAM_DISK_PROTOCOL implementation happens to work
> > thanks to the block cache.
> > When I disable CONFIG_BLOCK_CACHE and load some EFI application(it
> > does set  'app_gd')
> > before calling RAM_DISK_PROTOCOL service, RAM_DISK_PROTOCOL does not work.
> > ConnectController fails.
> >
> > >
> > > > native U-Boot can not access the RAM disk.
> > >
> > > I believe that, in theory, "fatls efidisk 0 1" (or efi_disk as an interface?)
> > > should work even under your current implementation.
> >
> > "fatls efiloader 0:2" does not work.
> > I think "efiloader" device is designed to be accessed from EFI application only
> > even if it is listed by "dm tree".
>
> I don't believe so.
> (the interface name may be "efi_blk" or "efiblk", though.)
>
> If the command fails, it's a bug. Must be fixed.

"fatls efiloader 0:2" failed here:
https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/block/blk-uclass.c#L139

This is because the parent is 'dm_root' for the device created by
lib/efi_driver/efi_block_device.c,
so uclass_id is different.

Thanks,
Masahisa Kojima

>
> > I understand that ramdisk as UCLASS_BLK driver is required as Heinrich said.
>
> That said, I agree to starting with UCLASS_BLK from the viewpoint of
> U-Boot driver model/UEFI integration.
>
> -Takahiro Akashi
>
> >
> > Thanks,
> > Masahisa Kojima
> >
> > >
> > > -Takahiro Akashi
> > >
> > > >  # Honestly speaking, I'm not sure how U-Boot prohibits the access to
> > > > the EFI RAM disk
> > > >     because the udevices are created for the RAM disk.
> > > >
> > > > Thanks,
> > > > Masahisa Kojima
> > > >
> > > > >
> > > > > > We strive to synchronize what is happening on the driver model level and
> > > > > > on the UEFI level. I would prefer a design where we have a UCLASS_BLK
> > > > > > driver ram disks and the EFI_RAM_DISK_PROTOCOL is a one method of
> > > > > > managing ram disk devices.
> > > > >
> > > > > That said, I agree to starting with U-Boot block device implementation,
> > > > > UEFI_DISK comes automatically. It benefits both U-Boot proper and
> > > > > UEFI subsystem equally as well.
> > > > > (That is also what I meant to say in my first response.)
> > > > >
> > > > > > A big issue we have is RAM management. malloc() can only assign limited
> > > > > > amount of memory which is probably too small for the RAM disk you are
> > > > > > looking at. We manage page sized memory in the EFI sub-system but this
> > > > > > is not integrated with the LMB memory checks.
> > > > >
> > > > > Not sure, is it enough simply to add some restrictions on the start size
> > > > > and the size when a memory region is specified for a raw disk?
> > > > >
> > > > > -Takahiro Akashi
> > > > >
> > > > > > The necessary sequence of development is probably:
> > > > > >
> > > > > > * Rework memory management
> > > > > > * Implement ramdisks as UCLASS_BLK driver
> > > > > > * Implement the EFI_RAM_DISK_PROTOCOL based on the UCLASS_BLK driver.
> > > > > >
> > > > > > Best regards
> > > > > >
> > > > > > Heinrich
> > > > > >
> > > > > > >
> > > > > > > Note that the installation process may not proceed
> > > > > > > after the distro installer calls ExitBootServices()
> > > > > > > since there is no hand-off process for the block device of
> > > > > > > memory mapped ISO image.
> > > > > > >
> > > > > > > The EFI_RAM_DISK_PROTOCOL was tested with the
> > > > > > > debian network installer[1] on qemu_arm64 platform.
> > > > > > > The example procedure is as follows.
> > > > > > >   => tftpboot 41000000 mini.iso
> > > > > > >   => efidebug disk load 41000000 $filesize
> > > > > > >
> > > > > > > After these commands, ISO image is mounted like:
> > > > > > >
> > > > > > >   => efidebug dh
> > > > > > >
> > > > > > >      000000007eec5910 (efiblk#0)
> > > > > > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)
> > > > > > >        Block IO
> > > > > > >
> > > > > > >      000000007eec6140 (efiblk#0:1)
> > > > > > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(1,0x01,0,0x0,0x33800)
> > > > > > >        Block IO
> > > > > > >
> > > > > > >      000000007eec62b0 (efiblk#0:2)
> > > > > > >        /RamDisk(0x41000000,4974afff,3d5abd30-4175-87ce-6d64-d2ade523c4bb,0x0)/HD(2,0x01,0,0x33800,0x10000)
> > > > > > >        Block IO
> > > > > > >        System Partition
> > > > > > >        Simple File System
> > > > > > >
> > > > > > >    => dm tree
> > > > > > >
> > > > > > >      blk           0  [ + ]   efi_blk               `-- efiblk#0
> > > > > > >      partition     0  [ + ]   blk_partition             |-- efiblk#0:1
> > > > > > >      partition     1  [ + ]   blk_partition             `-- efiblk#0:2
> > > > > > >
> > > > > > > Debian can be successfully installed with this RAM disk on QEMU.
> > > > > > >
> > > > > > > [TODO]
> > > > > > > - udevices created in ./lib/efi_driver/efi_block_device.c::efi_bl_bind()
> > > > > > >    are not removed when the efi_handle is removed.
> > > > > > >    So after unload the RAM disk, udevices still exist.
> > > > > > >    I plan to add a udevice removal process in ./lib/efi_driver/efi_uclass.c::efi_uc_stop().
> > > > > > >    In addition, I also plan to add unbind() callback in struct efi_driver_ops.
> > > > > > >
> > > > > > >
> > > > > > > [1] https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/mini.iso
> > > > > > >
> > > > > > > Masahisa Kojima (4):
> > > > > > >    efi_loader: add RAM disk device path
> > > > > > >    efi_loader: add EFI_RAM_DISK_PROTOCOL implementation
> > > > > > >    cmd: efidebug: add RAM disk mount command
> > > > > > >    efi_selftest: add EFI_RAM_DISK_PROTOCOL selftest
> > > > > > >
> > > > > > >   cmd/efidebug.c                           | 117 ++++++
> > > > > > >   include/efi_api.h                        |  32 ++
> > > > > > >   include/efi_loader.h                     |   4 +
> > > > > > >   lib/efi_driver/efi_uclass.c              |   7 +-
> > > > > > >   lib/efi_loader/Kconfig                   |   6 +
> > > > > > >   lib/efi_loader/Makefile                  |   1 +
> > > > > > >   lib/efi_loader/efi_device_path_to_text.c |  14 +
> > > > > > >   lib/efi_loader/efi_ram_disk.c            | 334 +++++++++++++++
> > > > > > >   lib/efi_loader/efi_setup.c               |   6 +
> > > > > > >   lib/efi_selftest/Makefile                |   1 +
> > > > > > >   lib/efi_selftest/efi_selftest_ram_disk.c | 511 +++++++++++++++++++++++
> > > > > > >   lib/uuid.c                               |   4 +
> > > > > > >   12 files changed, 1035 insertions(+), 2 deletions(-)
> > > > > > >   create mode 100644 lib/efi_loader/efi_ram_disk.c
> > > > > > >   create mode 100644 lib/efi_selftest/efi_selftest_ram_disk.c
> > > > > > >
> > > > > > >
> > > > > > > base-commit: e2e2aea5733f0d23cd9593bbefe5c803c552dcb9
> > > > > >
Simon Glass July 11, 2023, 7:13 p.m. UTC | #10
Hi,

On Tue, 11 Jul 2023 at 00:23, Masahisa Kojima
<masahisa.kojima@linaro.org> wrote:
>
> On Mon, 10 Jul 2023 at 11:28, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > On Mon, Jul 10, 2023 at 11:13:12AM +0900, Masahisa Kojima wrote:
> > > On Fri, 7 Jul 2023 at 18:12, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > > >
> > > > On Fri, Jul 07, 2023 at 05:19:33PM +0900, Masahisa Kojima wrote:
> > > > > Hi Akashi-san,
> > > > >
> > > > > On Fri, 7 Jul 2023 at 16:16, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> > > > > >
> > > > > > On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote:
> > > > > > > On 7/7/23 06:00, Masahisa Kojima wrote:
> > > > > > > > This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
> > > > > > > > The major purpose of this series is a preparation for EFI HTTP(S) boot.
> > > > > > > >
> > > > > > > > Now U-Boot can download the distro installer ISO image
> > > > > > > > via wget or tftpboot commands, but U-Boot can not mount
> > > > > > > > the downloaded ISO image.
> > > > > > > > By calling EFI_RAM_DISK_PROTOCOL->register API, user can
> > > > > > > > mount the ISO image and boot the distro installer.
> > > > > > >
> > > > > > > If I understand you correctly, with your design RAM disks will only
> > > > > > > exist in the EFI sub-system.
> > > > > >
> > > > > > Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot
> > > > > > block device (and associated partition devices) thanks to your
> > > > > > efi_driver framework.
> > > > >
> > > > > Read/Write the RAM disk is expected to be called from the EFI context, so
> > > >
> > > > An associated U-Boot block device should work as well on top of your
> > > > RAW_DISK_PROTOCOL via a generated RAM disk object (efi_disk).
> > > > That is why SYMPLE_FILE_SYSTEM_PROTOCOL, which solely relies on U-Boot's proper
> > > > filesystem subsystem, is installed to the RAM disk object.
> > >
> > > I now realize that my RAM_DISK_PROTOCOL implementation happens to work
> > > thanks to the block cache.
> > > When I disable CONFIG_BLOCK_CACHE and load some EFI application(it
> > > does set  'app_gd')
> > > before calling RAM_DISK_PROTOCOL service, RAM_DISK_PROTOCOL does not work.
> > > ConnectController fails.
> > >
> > > >
> > > > > native U-Boot can not access the RAM disk.
> > > >
> > > > I believe that, in theory, "fatls efidisk 0 1" (or efi_disk as an interface?)
> > > > should work even under your current implementation.
> > >
> > > "fatls efiloader 0:2" does not work.
> > > I think "efiloader" device is designed to be accessed from EFI application only
> > > even if it is listed by "dm tree".
> >
> > I don't believe so.
> > (the interface name may be "efi_blk" or "efiblk", though.)
> >
> > If the command fails, it's a bug. Must be fixed.
>
> "fatls efiloader 0:2" failed here:
> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/block/blk-uclass.c#L139
>
> This is because the parent is 'dm_root' for the device created by
> lib/efi_driver/efi_block_device.c,
> so uclass_id is different.

The parent device of a UCLASS_BLK device MUST be a media uclass. It
cannot be root. I believe EFI has some suitable classes.

At some point we will remove the uclass_id from struct blk_desc and
just use the parent's uclass ID.

Regards,
Simon
Heinrich Schuchardt July 12, 2023, 6:41 a.m. UTC | #11
On 7/11/23 21:13, Simon Glass wrote:
> Hi,
>
> On Tue, 11 Jul 2023 at 00:23, Masahisa Kojima
> <masahisa.kojima@linaro.org> wrote:
>>
>> On Mon, 10 Jul 2023 at 11:28, AKASHI Takahiro
>> <takahiro.akashi@linaro.org> wrote:
>>>
>>> On Mon, Jul 10, 2023 at 11:13:12AM +0900, Masahisa Kojima wrote:
>>>> On Fri, 7 Jul 2023 at 18:12, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>>>>>
>>>>> On Fri, Jul 07, 2023 at 05:19:33PM +0900, Masahisa Kojima wrote:
>>>>>> Hi Akashi-san,
>>>>>>
>>>>>> On Fri, 7 Jul 2023 at 16:16, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
>>>>>>>
>>>>>>> On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote:
>>>>>>>> On 7/7/23 06:00, Masahisa Kojima wrote:
>>>>>>>>> This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
>>>>>>>>> The major purpose of this series is a preparation for EFI HTTP(S) boot.
>>>>>>>>>
>>>>>>>>> Now U-Boot can download the distro installer ISO image
>>>>>>>>> via wget or tftpboot commands, but U-Boot can not mount
>>>>>>>>> the downloaded ISO image.
>>>>>>>>> By calling EFI_RAM_DISK_PROTOCOL->register API, user can
>>>>>>>>> mount the ISO image and boot the distro installer.
>>>>>>>>
>>>>>>>> If I understand you correctly, with your design RAM disks will only
>>>>>>>> exist in the EFI sub-system.
>>>>>>>
>>>>>>> Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot
>>>>>>> block device (and associated partition devices) thanks to your
>>>>>>> efi_driver framework.
>>>>>>
>>>>>> Read/Write the RAM disk is expected to be called from the EFI context, so
>>>>>
>>>>> An associated U-Boot block device should work as well on top of your
>>>>> RAW_DISK_PROTOCOL via a generated RAM disk object (efi_disk).
>>>>> That is why SYMPLE_FILE_SYSTEM_PROTOCOL, which solely relies on U-Boot's proper
>>>>> filesystem subsystem, is installed to the RAM disk object.
>>>>
>>>> I now realize that my RAM_DISK_PROTOCOL implementation happens to work
>>>> thanks to the block cache.
>>>> When I disable CONFIG_BLOCK_CACHE and load some EFI application(it
>>>> does set  'app_gd')
>>>> before calling RAM_DISK_PROTOCOL service, RAM_DISK_PROTOCOL does not work.
>>>> ConnectController fails.
>>>>
>>>>>
>>>>>> native U-Boot can not access the RAM disk.
>>>>>
>>>>> I believe that, in theory, "fatls efidisk 0 1" (or efi_disk as an interface?)
>>>>> should work even under your current implementation.
>>>>
>>>> "fatls efiloader 0:2" does not work.
>>>> I think "efiloader" device is designed to be accessed from EFI application only
>>>> even if it is listed by "dm tree".
>>>
>>> I don't believe so.
>>> (the interface name may be "efi_blk" or "efiblk", though.)
>>>
>>> If the command fails, it's a bug. Must be fixed.
>>
>> "fatls efiloader 0:2" failed here:
>> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/block/blk-uclass.c#L139
>>
>> This is because the parent is 'dm_root' for the device created by
>> lib/efi_driver/efi_block_device.c,
>> so uclass_id is different.
>
> The parent device of a UCLASS_BLK device MUST be a media uclass. It
> cannot be root. I believe EFI has some suitable classes.
>
> At some point we will remove the uclass_id from struct blk_desc and
> just use the parent's uclass ID.

Hello Simon,

we want to more tightly integrate EFI and DM.

If an EFI program like iPXE creates a handle for an iSCSI drive with the
block IO protocol the parent of that handle will be a network device and
nothing having to do with a block device.

Introducing dummy devices like you did for the sandbox host block
devices will only make integration of EFI and DM more complicated.

As I indicated years before you have to use blk_desc->uclass_id instead
of the parent uclass_id to identify block devices in
blk_get_devnum_by_uclass_idname() [1][2].

Removing class_id from struct blk_desc is a step against better future
integration of EFI and DM.

Best regards

Heinrich

[1]
https://lore.kernel.org/all/20221003093550.106304-1-heinrich.schuchardt@canonical.com/
[2]
https://lore.kernel.org/all/20220802094933.69170-1-heinrich.schuchardt@canonical.com/
Simon Glass July 12, 2023, 2 p.m. UTC | #12
Hi Heinrich,

On Wed, 12 Jul 2023 at 00:41, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 7/11/23 21:13, Simon Glass wrote:
> > Hi,
> >
> > On Tue, 11 Jul 2023 at 00:23, Masahisa Kojima
> > <masahisa.kojima@linaro.org> wrote:
> >>
> >> On Mon, 10 Jul 2023 at 11:28, AKASHI Takahiro
> >> <takahiro.akashi@linaro.org> wrote:
> >>>
> >>> On Mon, Jul 10, 2023 at 11:13:12AM +0900, Masahisa Kojima wrote:
> >>>> On Fri, 7 Jul 2023 at 18:12, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >>>>>
> >>>>> On Fri, Jul 07, 2023 at 05:19:33PM +0900, Masahisa Kojima wrote:
> >>>>>> Hi Akashi-san,
> >>>>>>
> >>>>>> On Fri, 7 Jul 2023 at 16:16, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >>>>>>>
> >>>>>>> On Fri, Jul 07, 2023 at 08:29:12AM +0200, Heinrich Schuchardt wrote:
> >>>>>>>> On 7/7/23 06:00, Masahisa Kojima wrote:
> >>>>>>>>> This series introduces the EFI_RAM_DISK_PROTOCOL implementation.
> >>>>>>>>> The major purpose of this series is a preparation for EFI HTTP(S) boot.
> >>>>>>>>>
> >>>>>>>>> Now U-Boot can download the distro installer ISO image
> >>>>>>>>> via wget or tftpboot commands, but U-Boot can not mount
> >>>>>>>>> the downloaded ISO image.
> >>>>>>>>> By calling EFI_RAM_DISK_PROTOCOL->register API, user can
> >>>>>>>>> mount the ISO image and boot the distro installer.
> >>>>>>>>
> >>>>>>>> If I understand you correctly, with your design RAM disks will only
> >>>>>>>> exist in the EFI sub-system.
> >>>>>>>
> >>>>>>> Probably, not. As Kojima-san's "dm tree" shows, there is a U-Boot
> >>>>>>> block device (and associated partition devices) thanks to your
> >>>>>>> efi_driver framework.
> >>>>>>
> >>>>>> Read/Write the RAM disk is expected to be called from the EFI context, so
> >>>>>
> >>>>> An associated U-Boot block device should work as well on top of your
> >>>>> RAW_DISK_PROTOCOL via a generated RAM disk object (efi_disk).
> >>>>> That is why SYMPLE_FILE_SYSTEM_PROTOCOL, which solely relies on U-Boot's proper
> >>>>> filesystem subsystem, is installed to the RAM disk object.
> >>>>
> >>>> I now realize that my RAM_DISK_PROTOCOL implementation happens to work
> >>>> thanks to the block cache.
> >>>> When I disable CONFIG_BLOCK_CACHE and load some EFI application(it
> >>>> does set  'app_gd')
> >>>> before calling RAM_DISK_PROTOCOL service, RAM_DISK_PROTOCOL does not work.
> >>>> ConnectController fails.
> >>>>
> >>>>>
> >>>>>> native U-Boot can not access the RAM disk.
> >>>>>
> >>>>> I believe that, in theory, "fatls efidisk 0 1" (or efi_disk as an interface?)
> >>>>> should work even under your current implementation.
> >>>>
> >>>> "fatls efiloader 0:2" does not work.
> >>>> I think "efiloader" device is designed to be accessed from EFI application only
> >>>> even if it is listed by "dm tree".
> >>>
> >>> I don't believe so.
> >>> (the interface name may be "efi_blk" or "efiblk", though.)
> >>>
> >>> If the command fails, it's a bug. Must be fixed.
> >>
> >> "fatls efiloader 0:2" failed here:
> >> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/block/blk-uclass.c#L139
> >>
> >> This is because the parent is 'dm_root' for the device created by
> >> lib/efi_driver/efi_block_device.c,
> >> so uclass_id is different.
> >
> > The parent device of a UCLASS_BLK device MUST be a media uclass. It
> > cannot be root. I believe EFI has some suitable classes.
> >
> > At some point we will remove the uclass_id from struct blk_desc and
> > just use the parent's uclass ID.
>
> Hello Simon,
>
> we want to more tightly integrate EFI and DM.

Who is working on this? I have not seen a series for a while.

>
> If an EFI program like iPXE creates a handle for an iSCSI drive with the
> block IO protocol the parent of that handle will be a network device and
> nothing having to do with a block device.
>
> Introducing dummy devices like you did for the sandbox host block
> devices will only make integration of EFI and DM more complicated.
>
> As I indicated years before you have to use blk_desc->uclass_id instead
> of the parent uclass_id to identify block devices in
> blk_get_devnum_by_uclass_idname() [1][2].
>
> Removing class_id from struct blk_desc is a step against better future
> integration of EFI and DM.

I thought we discussed this before, but this his my understanding:
devices cannot exist in EFI that are not known by U-Boot. So if there
is a an iSCSI drive it should exist in U-Boot too, under DM.

Anyway, I hope this important work can restart soon.

Regards,
Simon


>
> Best regards
>
> Heinrich
>
> [1]
> https://lore.kernel.org/all/20221003093550.106304-1-heinrich.schuchardt@canonical.com/
> [2]
> https://lore.kernel.org/all/20220802094933.69170-1-heinrich.schuchardt@canonical.com/