On Wed, Sep 11, 2019 at 07:31:31PM +0200, Heinrich Schuchardt wrote: > On 9/11/19 8:16 AM, AKASHI Takahiro wrote: > >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.aka...@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. > > I did not mean this as a requirement but as a source of errors. > > My idea is that could have an MBR which by chance looks like the start > of a file system. When you now expose this non-existent file system the > UEFI client may create havoc.
See below. > > > >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. > > That sounds like a bug. If a partition isn't formatted we would not even > know whether to call the FAT or the EXT4 driver. Yes, the issue does exist in the current implementation. So a good approach is 1. Check if a partition table is installed or not 2. If yes, 2-1 If a partition has a file system, install FILE_SYSTEM_PROTOCOL to *that* partition. 2-2 not install FILE_SYSTEM_PROTOCOL to a whole disk 3. If no, and only if a file system exists, install FILE_SYSTEM_PROTOCOL to a whole disk (2.1) and additional check at (3) should be added. -Takahiro Akashi > Regards > > Heinrich > > >> > >>Thanks, > >>-Takahiro Akashi > >> > >> > >>>Best regards > >>> > >>>Heinrich > >>> > >>>> > >>>>>Signed-off-by: AKASHI Takahiro <takahiro.aka...@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@lists.denx.de > >>>>>https://lists.denx.de/listinfo/u-boot > >>>> > >>> > > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot