[U-Boot] [PATCH] efi_loader: disk: install file system protocol to a whole disk

[AKASHI Takahiro]

Heinrich,

On Fri, Aug 23, 2019 at 09:04:21AM +0900, AKASHI Takahiro wrote:
> On Thu, Aug 22, 2019 at 12:52:41PM +0200, Heinrich Schuchardt wrote:
> > On 8/22/19 11:11 AM, Mark Kettenis wrote:
> > >>From: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > >>Date: Thu, 22 Aug 2019 17:06:25 +0900
> > >>
> > >>Currently, a whole disk without any partitions is not associated
> > >>with EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. So even if it houses FAT
> > >>file system, there is a chance that we may not be able to access
> > >>it, particularly, when accesses are to be attempted after searching
> > >>that protocol against a device handle.
> > >>
> > >>With this patch, EFI_SIMPLE_FILE_SYSTEM_PROTOCOL is installed
> > >>to such a disk if part_get_info() shows there is not partition
> > >>table installed on it.
> > >
> > >Do other UEFI implementations support this?
> > 
> > What use cases exist that come without partition table?
> 
> I didn't find any *explicit* description in UEFI specification
> that mandates that any block device should have a partition table.
> It may be mandatory for boot(able) disks, but for others?
> 
> > You can create an MBR with partition table that is a valid start of a
> > file system.
> 
> Obviously we can do that, but if this is not a mandatory requirement,
> we'd better support no-partitioned cases.

Any further comments?

-Takahiro Akashi

> 
> > So you should first check if a partition table exists. Only if none
> > exists you can test for a possible file system.
> 
> I don't get your point. Are you saying that we should support a file system
> for a disk only if it has a file system?
> This is not true even for existing partitions as FILE_SYSTEM PROTOCOL
> is always installed to every partition whether or not it really
> houses a file system under the current implementation.
> 
> Thanks,
> -Takahiro Akashi
> 
> 
> > Best regards
> > 
> > Heinrich
> > 
> > >
> > >>Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > >>---
> > >>  lib/efi_loader/efi_disk.c | 4 +++-
> > >>  1 file changed, 3 insertions(+), 1 deletion(-)
> > >>
> > >>diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > >>index 7a6b06821a47..548fe667e6f8 100644
> > >>--- a/lib/efi_loader/efi_disk.c
> > >>+++ b/lib/efi_loader/efi_disk.c
> > >>@@ -239,6 +239,7 @@ static efi_status_t efi_disk_add_dev(
> > >>  				struct efi_disk_obj **disk)
> > >>  {
> > >>  	struct efi_disk_obj *diskobj;
> > >>+	disk_partition_t info;
> > >>  	efi_status_t ret;
> > >>
> > >>  	/* Don't add empty devices */
> > >>@@ -270,7 +271,8 @@ static efi_status_t efi_disk_add_dev(
> > >>  			       diskobj->dp);
> > >>  	if (ret != EFI_SUCCESS)
> > >>  		return ret;
> > >>-	if (part >= 1) {
> > >>+	/* partitions or whole disk without partitions */
> > >>+	if (part >= 1 || part_get_info(desc, part, &info)) {
> > >>  		diskobj->volume = efi_simple_file_system(desc, part,
> > >>  							 diskobj->dp);
> > >>  		ret = efi_add_protocol(&diskobj->header,
> > >>--
> > >>2.21.0
> > >>
> > >>_______________________________________________
> > >>U-Boot mailing list
> > >>U-Boot at lists.denx.de
> > >>https://lists.denx.de/listinfo/u-boot
> > >
> >