* [U-Boot] [PATCH v4 0/5] efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk
@ 2019-10-07 5:59 AKASHI Takahiro
2019-10-07 5:59 ` [U-Boot] [PATCH v4 1/5] fs: export fs_close() AKASHI Takahiro
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: AKASHI Takahiro @ 2019-10-07 5:59 UTC (permalink / raw)
To: u-boot
Changes in v4 (Oct 7, 2019)
* modify commit message of patch#1 after Heinrich's suggestion
* add patch#2 after Heinrich's suggestion
* add a function description to efi_fs_exists() after Heinrich's
suggestion (patch#4)
Changes in v3 (Oct 4, 2019)
* add patch#1
* use newly-added fs_get_type() instead fo fs_get_type_name()
* check for FS_TYPE_ANY to confirm if a file system exists or not
* remove a redundant check on whether a device is a partition
or a whole disk
Changes in v2 (Sept 12, 2019)
* add patch#1 and #2
* install the protocol only if a file system does exist
AKASHI Takahiro (5):
fs: export fs_close()
fs: clean up around fs_type
fs: add fs_get_type() for current filesystem type
efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available
efi_loader: disk: install file system protocol to a whole disk
fs/fs.c | 18 ++++++++++++++----
include/fs.h | 17 +++++++++++++++++
lib/efi_loader/efi_disk.c | 25 ++++++++++++++++++++++++-
3 files changed, 55 insertions(+), 5 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [U-Boot] [PATCH v4 1/5] fs: export fs_close() 2019-10-07 5:59 [U-Boot] [PATCH v4 0/5] efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk AKASHI Takahiro @ 2019-10-07 5:59 ` AKASHI Takahiro 2019-10-12 21:24 ` Heinrich Schuchardt 2019-10-07 5:59 ` [U-Boot] [PATCH v4 2/5] fs: clean up around fs_type AKASHI Takahiro ` (3 subsequent siblings) 4 siblings, 1 reply; 14+ messages in thread From: AKASHI Takahiro @ 2019-10-07 5:59 UTC (permalink / raw) To: u-boot fs_close() closes the connection to a file system which opened with either fs_set_blk_dev() or fs_set_dev_with_part(). Many file system functions implicitly call fs_close(), e.g. fs_closedir(), fs_exist(), fs_ln(), fs_ls(), fs_mkdir(), fs_read(), fs_size(), fs_write() and fs_unlink(). So just export it. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- fs/fs.c | 2 +- include/fs.h | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/fs/fs.c b/fs/fs.c index d8a4ced4698e..64ba25fea8bf 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -389,7 +389,7 @@ int fs_set_blk_dev_with_part(struct blk_desc *desc, int part) return -1; } -static void fs_close(void) +void fs_close(void) { struct fstype_info *info = fs_get_info(fs_type); diff --git a/include/fs.h b/include/fs.h index 7601b0343bcd..5a1244d57fd2 100644 --- a/include/fs.h +++ b/include/fs.h @@ -37,6 +37,13 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype); */ int fs_set_blk_dev_with_part(struct blk_desc *desc, int part); +/** + * fs_close() - Unset current block device and partition + * + * Should be paired with either fs_set_blk_dev() or fs_set_dev_with_part() + */ +void fs_close(void); + /** * fs_get_type_name() - Get type of current filesystem * -- 2.21.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v4 1/5] fs: export fs_close() 2019-10-07 5:59 ` [U-Boot] [PATCH v4 1/5] fs: export fs_close() AKASHI Takahiro @ 2019-10-12 21:24 ` Heinrich Schuchardt 0 siblings, 0 replies; 14+ messages in thread From: Heinrich Schuchardt @ 2019-10-12 21:24 UTC (permalink / raw) To: u-boot On 10/7/19 7:59 AM, AKASHI Takahiro wrote: > fs_close() closes the connection to a file system which opened with > either fs_set_blk_dev() or fs_set_dev_with_part(). Many file system > functions implicitly call fs_close(), e.g. fs_closedir(), fs_exist(), > fs_ln(), fs_ls(), fs_mkdir(), fs_read(), fs_size(), fs_write() > and fs_unlink(). > So just export it. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > fs/fs.c | 2 +- > include/fs.h | 7 +++++++ > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/fs/fs.c b/fs/fs.c > index d8a4ced4698e..64ba25fea8bf 100644 > --- a/fs/fs.c > +++ b/fs/fs.c > @@ -389,7 +389,7 @@ int fs_set_blk_dev_with_part(struct blk_desc *desc, int part) > return -1; > } > > -static void fs_close(void) > +void fs_close(void) > { > struct fstype_info *info = fs_get_info(fs_type); > > diff --git a/include/fs.h b/include/fs.h > index 7601b0343bcd..5a1244d57fd2 100644 > --- a/include/fs.h > +++ b/include/fs.h > @@ -37,6 +37,13 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype); > */ > int fs_set_blk_dev_with_part(struct blk_desc *desc, int part); > > +/** > + * fs_close() - Unset current block device and partition > + * > + * Should be paired with either fs_set_blk_dev() or fs_set_dev_with_part() > + */ > +void fs_close(void); > + > /** > * fs_get_type_name() - Get type of current filesystem > * > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v4 2/5] fs: clean up around fs_type 2019-10-07 5:59 [U-Boot] [PATCH v4 0/5] efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk AKASHI Takahiro 2019-10-07 5:59 ` [U-Boot] [PATCH v4 1/5] fs: export fs_close() AKASHI Takahiro @ 2019-10-07 5:59 ` AKASHI Takahiro 2019-10-12 21:23 ` Heinrich Schuchardt 2019-10-07 5:59 ` [U-Boot] [PATCH v4 3/5] fs: add fs_get_type() for current filesystem type AKASHI Takahiro ` (2 subsequent siblings) 4 siblings, 1 reply; 14+ messages in thread From: AKASHI Takahiro @ 2019-10-07 5:59 UTC (permalink / raw) To: u-boot fs_ls(), fs_mkdir() and fs_unlink() sets fs_type to FS_TYPE_ANY explicitly, but it is redundant as they call fs_close(). So just remove those lines. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- fs/fs.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/fs/fs.c b/fs/fs.c index 64ba25fea8bf..b17b76a46ae0 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -413,7 +413,6 @@ int fs_ls(const char *dirname) ret = info->ls(dirname); - fs_type = FS_TYPE_ANY; fs_close(); return ret; @@ -597,7 +596,6 @@ int fs_unlink(const char *filename) ret = info->unlink(filename); - fs_type = FS_TYPE_ANY; fs_close(); return ret; @@ -611,7 +609,6 @@ int fs_mkdir(const char *dirname) ret = info->mkdir(dirname); - fs_type = FS_TYPE_ANY; fs_close(); return ret; -- 2.21.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v4 2/5] fs: clean up around fs_type 2019-10-07 5:59 ` [U-Boot] [PATCH v4 2/5] fs: clean up around fs_type AKASHI Takahiro @ 2019-10-12 21:23 ` Heinrich Schuchardt 0 siblings, 0 replies; 14+ messages in thread From: Heinrich Schuchardt @ 2019-10-12 21:23 UTC (permalink / raw) To: u-boot On 10/7/19 7:59 AM, AKASHI Takahiro wrote: > fs_ls(), fs_mkdir() and fs_unlink() sets fs_type to FS_TYPE_ANY > explicitly, but it is redundant as they call fs_close(). > So just remove those lines. > > Signed-off-by: AKASHI Takahiro<takahiro.akashi@linaro.org> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v4 3/5] fs: add fs_get_type() for current filesystem type 2019-10-07 5:59 [U-Boot] [PATCH v4 0/5] efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk AKASHI Takahiro 2019-10-07 5:59 ` [U-Boot] [PATCH v4 1/5] fs: export fs_close() AKASHI Takahiro 2019-10-07 5:59 ` [U-Boot] [PATCH v4 2/5] fs: clean up around fs_type AKASHI Takahiro @ 2019-10-07 5:59 ` AKASHI Takahiro 2019-10-07 5:59 ` [U-Boot] [PATCH v4 4/5] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available AKASHI Takahiro 2019-10-07 5:59 ` [U-Boot] [PATCH v4 5/5] efi_loader: disk: install file system protocol to a whole disk AKASHI Takahiro 4 siblings, 0 replies; 14+ messages in thread From: AKASHI Takahiro @ 2019-10-07 5:59 UTC (permalink / raw) To: u-boot This function is a variant of fs_get_type_name() and returns a filesystem type with which the current device is associated. We don't want to export fs_type variable directly because we have to take care of it consistently within fs.c. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- fs/fs.c | 13 +++++++++++++ include/fs.h | 10 ++++++++++ 2 files changed, 23 insertions(+) diff --git a/fs/fs.c b/fs/fs.c index b17b76a46ae0..0c66d6047703 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -307,6 +307,19 @@ static struct fstype_info *fs_get_info(int fstype) return info; } +/** + * fs_get_type() - Get type of current filesystem + * + * Return: filesystem type + * + * Returns filesystem type representing the current filesystem, or + * FS_TYPE_ANY for any unrecognised filesystem. + */ +int fs_get_type(void) +{ + return fs_type; +} + /** * fs_get_type_name() - Get type of current filesystem * diff --git a/include/fs.h b/include/fs.h index 5a1244d57fd2..6dfdb5c5307a 100644 --- a/include/fs.h +++ b/include/fs.h @@ -44,6 +44,16 @@ int fs_set_blk_dev_with_part(struct blk_desc *desc, int part); */ void fs_close(void); +/** + * fs_get_type() - Get type of current filesystem + * + * Return: filesystem type + * + * Returns filesystem type representing the current filesystem, or + * FS_TYPE_ANY for any unrecognised filesystem. + */ +int fs_get_type(void); + /** * fs_get_type_name() - Get type of current filesystem * -- 2.21.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v4 4/5] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available 2019-10-07 5:59 [U-Boot] [PATCH v4 0/5] efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk AKASHI Takahiro ` (2 preceding siblings ...) 2019-10-07 5:59 ` [U-Boot] [PATCH v4 3/5] fs: add fs_get_type() for current filesystem type AKASHI Takahiro @ 2019-10-07 5:59 ` AKASHI Takahiro 2019-10-22 20:29 ` [U-Boot] [BUG] efi_driver: crash while reading from iSCSI drive Heinrich Schuchardt 2019-10-07 5:59 ` [U-Boot] [PATCH v4 5/5] efi_loader: disk: install file system protocol to a whole disk AKASHI Takahiro 4 siblings, 1 reply; 14+ messages in thread From: AKASHI Takahiro @ 2019-10-07 5:59 UTC (permalink / raw) To: u-boot In the current implementation, EFI_SIMPLEFILE_SYSTEM_PROTOCOL is always installed to all the partitions even if some of them may house no file system. With this patch, that protocol will be installed only if any file system exists. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- lib/efi_loader/efi_disk.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 9007a5f77f3d..861fcaf3747f 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -9,6 +9,7 @@ #include <blk.h> #include <dm.h> #include <efi_loader.h> +#include <fs.h> #include <part.h> #include <malloc.h> @@ -262,6 +263,27 @@ efi_fs_from_path(struct efi_device_path *full_path) return handler->protocol_interface; } +/** + * efi_fs_exists() - check if a partition bears a file system + * + * @desc: block device descriptor + * @part: partition number + * Return: 1 if a file system exists on the partition + * 0 otherwise + */ +static int efi_fs_exists(struct blk_desc *desc, int part) +{ + if (fs_set_blk_dev_with_part(desc, part)) + return 0; + + if (fs_get_type() == FS_TYPE_ANY) + return 0; + + fs_close(); + + return 1; +} + /* * Create a handle for a partition or disk * @@ -315,7 +337,7 @@ static efi_status_t efi_disk_add_dev( diskobj->dp); if (ret != EFI_SUCCESS) return ret; - if (part >= 1) { + if (part >= 1 && efi_fs_exists(desc, part)) { diskobj->volume = efi_simple_file_system(desc, part, diskobj->dp); ret = efi_add_protocol(&diskobj->header, -- 2.21.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [U-Boot] [BUG] efi_driver: crash while reading from iSCSI drive 2019-10-07 5:59 ` [U-Boot] [PATCH v4 4/5] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available AKASHI Takahiro @ 2019-10-22 20:29 ` Heinrich Schuchardt 2019-10-23 10:30 ` AKASHI Takahiro 0 siblings, 1 reply; 14+ messages in thread From: Heinrich Schuchardt @ 2019-10-22 20:29 UTC (permalink / raw) To: u-boot The patch commit 867400677cda0fac4a411f1549fe3a61bb5ed172 efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available breaks booting my Pine A64 LTS board via iPXE and GRUB. But I assume this is not at the base of the problem. My iSCSI drive is partitioned like this: Device Boot Start End Sectors Size Id Type pine-a64-lts1 2048 194559 192512 94M ef EFI vfat pine-a64-lts2 * 194560 2148351 1953792 954M 83 Linux ext2 pine-a64-lts3 2148352 25585663 23437312 11.2G 83 Linux ext4 pine-a64-lts4 25585664 67106815 41521152 19.8G 83 Linux ext4 Looking at the debug output below the following questions arise: Why is ext2 not recognized as a file system? Why is the system crashing when trying to read 1024 blocks from the ext4 partition? .config contains CONFIG_FS_EXT4=y EFI: efi_bl_bind: handle 00000000b9f94560, interface 00000000b8e9f7f8 EFI: efi_bl_read: read 'efiblk#0', from block 0, 1 blocks EFI: Call: io->read_blocks( io, io->media->media_id, (u64)blknr, (efi_uintn_t)blkcnt * (efi_uintn_t)io->media->block_size, buffer) EFI: 0 returned by io->read_blocks( io, io->media->media_id, (u64)blknr, (efi_uintn_t)blkcnt * (efi_uintn_t)io->media->block_size, buffer) EFI: efi_bl_read: r = 0 EFI: efi_bl_bind: block device 'efiblk#0' created EFI: efi_bl_read: read 'efiblk#0', from block 2048, 1 blocks EFI: Call: io->read_blocks( io, io->media->media_id, (u64)blknr, (efi_uintn_t)blkcnt * (efi_uintn_t)io->media->block_size, buffer) EFI: 0 returned by io->read_blocks( io, io->media->media_id, (u64)blknr, (efi_uintn_t)blkcnt * (efi_uintn_t)io->media->block_size, buffer) EFI: efi_bl_read: r = 0 lib/efi_loader/efi_disk.c(279) efi_fs_exists: 0 returned by fs_set_blk_dev_with_part(desc, 1) EFI: efi_bl_read: read 'efiblk#0', from block 194560, 1 blocks EFI: Call: io->read_blocks( io, io->media->media_id, (u64)blknr, (efi_uintn_t)blkcnt * (efi_uintn_t)io->media->block_size, buffer) EFI: 0 returned by io->read_blocks( io, io->media->media_id, (u64)blknr, (efi_uintn_t)blkcnt * (efi_uintn_t)io->media->block_size, buffer) EFI: efi_bl_read: r = 0 EFI: efi_bl_read: read 'efiblk#0', from block 195584, 1024 blocks EFI: Call: io->read_blocks( io, io->media->media_id, (u64)blknr, (efi_uintn_t)blkcnt * (efi_uintn_t)io->media->block_size, buffer) EFI: 0 returned by io->read_blocks( io, io->media->media_id, (u64)blknr, (efi_uintn_t)blkcnt * (efi_uintn_t)io->media->block_size, buffer) EFI: efi_bl_read: r = 0 ** Unrecognized filesystem type ** lib/efi_loader/efi_disk.c(279) efi_fs_exists: -1 returned by fs_set_blk_dev_with_part(desc, 2) EFI: efi_bl_read: read 'efiblk#0', from block 2148352, 1 blocks EFI: Call: io->read_blocks( io, io->media->media_id, (u64)blknr, (efi_uintn_t)blkcnt * (efi_uintn_t)io->media->block_size, buffer) EFI: 0 returned by io->read_blocks( io, io->media->media_id, (u64)blknr, (efi_uintn_t)blkcnt * (efi_uintn_t)io->media->block_size, buffer) EFI: efi_bl_read: r = 0 EFI: efi_bl_read: read 'efiblk#0', from block 2149376, 1024 blocks EFI: Call: io->read_blocks( io, io->media->media_id, (u64)blknr, (efi_uintn_t)blkcnt * (efi_uintn_t)io->media->block_size, buffer) "Synchronous Abort" handler, esr 0x02000000 elr: ffffffff8c0a6028 lr : ffffffff8c0a6000 (reloc) elr: 0000000000000028 lr : 0000000000000000 x0 : 00000000b9f918f8 x1 : 00000000b9f31004 x2 : 00000000b9f918f8 x3 : 00000000b9f918f8 x4 : 00000000b9f918f8 x5 : 00000000b9f918f8 x6 : 00000000b9f918f8 x7 : 0000000000000000 x8 : 00000000b9f918f8 x9 : 00000000b8ea1038 x10: 0000000000000001 x11: 0000000019100bb0 x12: 0000000000000000 x13: 0000000000000001 x14: 0000000000000002 x15: 0000000000000003 x16: 00000000000000c8 x17: 00000000b9f918f8 x18: 00000000b9f31c30 x19: 00000000b9f918f8 x20: 00000000b8e9ddd0 x21: 00000000b8e99000 x22: 00000000b9f31008 x23: 00000000b9f3105a x24: 00000000b9f31068 x25: 00000000b9f31110 x26: 0000000000000000 x27: 0000000000000000 x28: 0000000000000000 x29: 00000000b9f31160 Code: ea000011 ea000000 ea000013 eafffffe (e3a00001) Looks like we jumped into nowhere land: All code ======== 0: ea000011 ands x17, x0, x0 4: ea000000 ands x0, x0, x0 8: ea000013 ands x19, x0, x0 c: eafffffe bics x30, xzr, xzr, ror #63 10:* e3a00001 .inst 0xe3a00001 ; undefined <-- trapping instruction Best regards Heinrich ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [BUG] efi_driver: crash while reading from iSCSI drive 2019-10-22 20:29 ` [U-Boot] [BUG] efi_driver: crash while reading from iSCSI drive Heinrich Schuchardt @ 2019-10-23 10:30 ` AKASHI Takahiro 2019-10-24 3:26 ` Heinrich Schuchardt 0 siblings, 1 reply; 14+ messages in thread From: AKASHI Takahiro @ 2019-10-23 10:30 UTC (permalink / raw) To: u-boot On Tue, Oct 22, 2019 at 10:29:09PM +0200, Heinrich Schuchardt wrote: > The patch > > commit 867400677cda0fac4a411f1549fe3a61bb5ed172 > efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available > > breaks booting my Pine A64 LTS board via iPXE and GRUB. But I assume > this is not at the base of the problem. > > My iSCSI drive is partitioned like this: > > Device Boot Start End Sectors Size Id Type > pine-a64-lts1 2048 194559 192512 94M ef EFI vfat > pine-a64-lts2 * 194560 2148351 1953792 954M 83 Linux ext2 > pine-a64-lts3 2148352 25585663 23437312 11.2G 83 Linux ext4 > pine-a64-lts4 25585664 67106815 41521152 19.8G 83 Linux ext4 > > Looking at the debug output below the following questions arise: > > Why is ext2 not recognized as a file system? > Why is the system crashing when trying to read 1024 blocks from the ext4 > partition? Try the workaround attached below. It seems that some fields, particularly log2blksz, in blk_dev held by ext_fs(of ext_filesystem in fs/ext4/ext4fs.c) are not initialized. I think that ext4's initialization code should be reworked. -Takahiro Akashi diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 861fcaf3747f..0792e53b32c6 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -337,6 +337,12 @@ static efi_status_t efi_disk_add_dev( diskobj->dp); if (ret != EFI_SUCCESS) return ret; + if (!part) { + char buf[10]; + + sprintf(buf, "%d:%d", dev_index, part); + fs_set_blk_dev(if_typename, buf, FS_TYPE_ANY); + } if (part >= 1 && efi_fs_exists(desc, part)) { diskobj->volume = efi_simple_file_system(desc, part, diskobj->dp); ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [U-Boot] [BUG] efi_driver: crash while reading from iSCSI drive 2019-10-23 10:30 ` AKASHI Takahiro @ 2019-10-24 3:26 ` Heinrich Schuchardt 0 siblings, 0 replies; 14+ messages in thread From: Heinrich Schuchardt @ 2019-10-24 3:26 UTC (permalink / raw) To: u-boot On 10/23/19 12:30 PM, AKASHI Takahiro wrote: > On Tue, Oct 22, 2019 at 10:29:09PM +0200, Heinrich Schuchardt wrote: >> The patch >> >> commit 867400677cda0fac4a411f1549fe3a61bb5ed172 >> efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available >> >> breaks booting my Pine A64 LTS board via iPXE and GRUB. But I assume >> this is not at the base of the problem. >> >> My iSCSI drive is partitioned like this: >> >> Device Boot Start End Sectors Size Id Type >> pine-a64-lts1 2048 194559 192512 94M ef EFI vfat >> pine-a64-lts2 * 194560 2148351 1953792 954M 83 Linux ext2 >> pine-a64-lts3 2148352 25585663 23437312 11.2G 83 Linux ext4 >> pine-a64-lts4 25585664 67106815 41521152 19.8G 83 Linux ext4 >> >> Looking at the debug output below the following questions arise: >> >> Why is ext2 not recognized as a file system? >> Why is the system crashing when trying to read 1024 blocks from the ext4 >> partition? > > Try the workaround attached below. > It seems that some fields, particularly log2blksz, in blk_dev held by > ext_fs(of ext_filesystem in fs/ext4/ext4fs.c) are not initialized. > > I think that ext4's initialization code should be reworked. Thanks for looking into this. The error is in efi_bl_bind() (lib/efi_driver/efi_block_device.c). I missed to use the block size of the block IO protocol to initialize desc->blksz and desc->log2blksz. Our FAT driver takes the sector size from the boot sector in get_fs_info() (fs/fat/fat.c) and ignores the block descriptor which will lead to errors if the logical sector size does not match the physical sector size. Best regards Heinrich ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v4 5/5] efi_loader: disk: install file system protocol to a whole disk 2019-10-07 5:59 [U-Boot] [PATCH v4 0/5] efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk AKASHI Takahiro ` (3 preceding siblings ...) 2019-10-07 5:59 ` [U-Boot] [PATCH v4 4/5] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available AKASHI Takahiro @ 2019-10-07 5:59 ` AKASHI Takahiro 2019-10-12 19:43 ` Heinrich Schuchardt 4 siblings, 1 reply; 14+ messages in thread From: AKASHI Takahiro @ 2019-10-07 5:59 UTC (permalink / raw) To: u-boot Currently, a whole disk without any partitions is not associated with EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. So even if it houses some 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 no partition table installed on it. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- lib/efi_loader/efi_disk.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 861fcaf3747f..7ee4ed26a2ea 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -337,7 +337,8 @@ static efi_status_t efi_disk_add_dev( diskobj->dp); if (ret != EFI_SUCCESS) return ret; - if (part >= 1 && efi_fs_exists(desc, part)) { + /* partitions or whole disk without partitions */ + if (efi_fs_exists(desc, part)) { diskobj->volume = efi_simple_file_system(desc, part, diskobj->dp); ret = efi_add_protocol(&diskobj->header, -- 2.21.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v4 5/5] efi_loader: disk: install file system protocol to a whole disk 2019-10-07 5:59 ` [U-Boot] [PATCH v4 5/5] efi_loader: disk: install file system protocol to a whole disk AKASHI Takahiro @ 2019-10-12 19:43 ` Heinrich Schuchardt 2019-10-15 7:34 ` AKASHI Takahiro 0 siblings, 1 reply; 14+ messages in thread From: Heinrich Schuchardt @ 2019-10-12 19:43 UTC (permalink / raw) To: u-boot On 10/7/19 7:59 AM, AKASHI Takahiro wrote: > Currently, a whole disk without any partitions is not associated > with EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. So even if it houses some > 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 no partition > table installed on it. That is what I would expect. But it is not what this patch really does. See below. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > lib/efi_loader/efi_disk.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c > index 861fcaf3747f..7ee4ed26a2ea 100644 > --- a/lib/efi_loader/efi_disk.c > +++ b/lib/efi_loader/efi_disk.c > @@ -337,7 +337,8 @@ static efi_status_t efi_disk_add_dev( > diskobj->dp); > if (ret != EFI_SUCCESS) > return ret; > - if (part >= 1 && efi_fs_exists(desc, part)) { > + /* partitions or whole disk without partitions */ > + if (efi_fs_exists(desc, part)) { Function efi_fs_exists() does not check if there is a partition table. It only checks if there is a file system. For creating a test image you can use the following commands: cat > partioning << EOF label: dos label-id: 0x6fe3a999 device: image unit: sectors image1: start= 1024, size= 524288, type=0c EOF dd if=/dev/zero of=test.img count=1 bs=1MiB /usr/sbin/sfdisk test.img < partioning dd if=test.img of=partition.tbl bs=8 count=9 skip=55 /usr/sbin/mkfs.vfat test.img 1024 dd conv=fsync,notrunc if=partition.tbl of=test.img bs=8 count=9 seek=55 sudo losetup -o 524288 --sizelimit 524288 /dev/loop1 test.img sudo mkfs.vfat /dev/loop1 sudo losetup -D /dev/loop1 sudo mount test.img /mnt sudo sh -c "echo 'file system on block device' > /mnt/description.txt" sudo umount /mnt sudo mount test.img /mnt -o offset=524288 sudo sh -c "echo 'file system on partition 1' > /mnt/description.txt" sudo umount /mnt Without your patch I get UEFI Interactive Shell v2.2 EDK II UEFI v2.80 (Das U-Boot, 0x20191000) Mapping table FS0: Alias(s):HD0a0b:;BLK1: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)/HD(1,MBR,0x6fe3a999,0x400,0x400) BLK0: Alias(s): /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0) Press ESC in 4 seconds to skip startup.nsh or any other key to continue. Shell> fs0: FS0:\> cat description.txt file system on partition 1 With your patch: UEFI Interactive Shell v2.2 EDK II UEFI v2.80 (Das U-Boot, 0x20191000) Mapping table FS0: Alias(s):F0a0:;BLK0: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0) FS1: Alias(s):HD0a0b:;BLK1: /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)/HD(1,MBR,0x6fe3a999,0x400,0x400) Press ESC in 4 seconds to skip startup.nsh or any other key to continue. Shell> fs0: FS0:\> cat description.txt file system on block device FS0:\> fs1: FS1:\> cat description.txt file system on partition 1 So though a partition table is discovered a file system is mounted on the block device. In my special case the file system on the block device really existed and was well separated from partition 1. But typically expect that there is no file system on the block device if there is a partition table. For your convenience I have uploaded the image file to https://github.com/U-Boot-EFI/test_file_system Best regards Heinrich > diskobj->volume = efi_simple_file_system(desc, part, > diskobj->dp); > ret = efi_add_protocol(&diskobj->header, > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v4 5/5] efi_loader: disk: install file system protocol to a whole disk 2019-10-12 19:43 ` Heinrich Schuchardt @ 2019-10-15 7:34 ` AKASHI Takahiro 2019-10-15 11:07 ` Heinrich Schuchardt 0 siblings, 1 reply; 14+ messages in thread From: AKASHI Takahiro @ 2019-10-15 7:34 UTC (permalink / raw) To: u-boot On Sat, Oct 12, 2019 at 09:43:33PM +0200, Heinrich Schuchardt wrote: > On 10/7/19 7:59 AM, AKASHI Takahiro wrote: > >Currently, a whole disk without any partitions is not associated > >with EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. So even if it houses some > >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 no partition > >table installed on it. > > That is what I would expect. But it is not what this patch really does. Literally, you're right, but > See below. > > > > >Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > >--- > > lib/efi_loader/efi_disk.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > >diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c > >index 861fcaf3747f..7ee4ed26a2ea 100644 > >--- a/lib/efi_loader/efi_disk.c > >+++ b/lib/efi_loader/efi_disk.c > >@@ -337,7 +337,8 @@ static efi_status_t efi_disk_add_dev( > > diskobj->dp); > > if (ret != EFI_SUCCESS) > > return ret; > >- if (part >= 1 && efi_fs_exists(desc, part)) { > >+ /* partitions or whole disk without partitions */ > >+ if (efi_fs_exists(desc, part)) { > > Function efi_fs_exists() does not check if there is a partition table. > It only checks if there is a file system. > > For creating a test image you can use the following commands: > > cat > partioning << EOF > label: dos > label-id: 0x6fe3a999 > device: image > unit: sectors > image1: start= 1024, size= 524288, type=0c > EOF > dd if=/dev/zero of=test.img count=1 bs=1MiB > /usr/sbin/sfdisk test.img < partioning > dd if=test.img of=partition.tbl bs=8 count=9 skip=55 > /usr/sbin/mkfs.vfat test.img 1024 > dd conv=fsync,notrunc if=partition.tbl of=test.img bs=8 count=9 seek=55 > sudo losetup -o 524288 --sizelimit 524288 /dev/loop1 test.img > sudo mkfs.vfat /dev/loop1 > sudo losetup -D /dev/loop1 > sudo mount test.img /mnt > sudo sh -c "echo 'file system on block device' > /mnt/description.txt" > sudo umount /mnt > sudo mount test.img /mnt -o offset=524288 > sudo sh -c "echo 'file system on partition 1' > /mnt/description.txt" > sudo umount /mnt Your example above is quite quirky, and totally unpractical. If people would create such a file system, they would expect that they could access a raw block device as well as a partition #1. However, I would like to drop this patch(#5) from my patch set because I don't find any public interface that can be used to determine if there exists a file system on a raw device. I initially thought of reverting my patch to v2, where part_get_info() was used, but it doesn't work as expected for part == 0. For example, GPT partition dirver, disk_efi.c, uses an internal function, find_valid_gpt(), but part_get_info_efi() returns an error for part == 0. For me, it is much easier to modify my pytest for UEFI secure boot than to carve out a generic interface for *all* the file systems. My patch #1 to #4 have already been reviewed by you and there are no outstanding issues. -Takahiro Akashi > Without your patch I get > > UEFI Interactive Shell v2.2 > EDK II > UEFI v2.80 (Das U-Boot, 0x20191000) > Mapping table > FS0: Alias(s):HD0a0b:;BLK1: > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)/HD(1,MBR,0x6fe3a999,0x400,0x400) > BLK0: Alias(s): > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0) > Press ESC in 4 seconds to skip startup.nsh or any other key to continue. > Shell> fs0: > FS0:\> cat description.txt > file system on partition 1 > > With your patch: > > UEFI Interactive Shell v2.2 > EDK II > UEFI v2.80 (Das U-Boot, 0x20191000) > Mapping table > FS0: Alias(s):F0a0:;BLK0: > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0) > FS1: Alias(s):HD0a0b:;BLK1: > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)/HD(1,MBR,0x6fe3a999,0x400,0x400) > Press ESC in 4 seconds to skip startup.nsh or any other key to continue. > Shell> fs0: > FS0:\> cat description.txt > file system on block device > > FS0:\> fs1: > FS1:\> cat description.txt > file system on partition 1 > > So though a partition table is discovered a file system is mounted on > the block device. In my special case the file system on the block device > really existed and was well separated from partition 1. But typically > expect that there is no file system on the block device if there is a > partition table. > > For your convenience I have uploaded the image file to > https://github.com/U-Boot-EFI/test_file_system > > Best regards > > Heinrich > > > diskobj->volume = efi_simple_file_system(desc, part, > > diskobj->dp); > > ret = efi_add_protocol(&diskobj->header, > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [U-Boot] [PATCH v4 5/5] efi_loader: disk: install file system protocol to a whole disk 2019-10-15 7:34 ` AKASHI Takahiro @ 2019-10-15 11:07 ` Heinrich Schuchardt 0 siblings, 0 replies; 14+ messages in thread From: Heinrich Schuchardt @ 2019-10-15 11:07 UTC (permalink / raw) To: u-boot On 10/15/19 9:34 AM, AKASHI Takahiro wrote: > On Sat, Oct 12, 2019 at 09:43:33PM +0200, Heinrich Schuchardt wrote: >> On 10/7/19 7:59 AM, AKASHI Takahiro wrote: >>> Currently, a whole disk without any partitions is not associated >>> with EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. So even if it houses some >>> 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 no partition >>> table installed on it. >> >> That is what I would expect. But it is not what this patch really does. > > Literally, you're right, but > >> See below. >> >>> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >>> --- >>> lib/efi_loader/efi_disk.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c >>> index 861fcaf3747f..7ee4ed26a2ea 100644 >>> --- a/lib/efi_loader/efi_disk.c >>> +++ b/lib/efi_loader/efi_disk.c >>> @@ -337,7 +337,8 @@ static efi_status_t efi_disk_add_dev( >>> diskobj->dp); >>> if (ret != EFI_SUCCESS) >>> return ret; >>> - if (part >= 1 && efi_fs_exists(desc, part)) { >>> + /* partitions or whole disk without partitions */ >>> + if (efi_fs_exists(desc, part)) { The following should work: if ((part || desc->part_type == PART_TYPE_UNKNOWN) && efi_fs_exists(desc, part)) { But unfortunately it does not because if you format the image with mkfs.vfat desc->part_type will be set to PART_TYPE_DOS. So what is missing is a fix in disk/part_dos.c. Currently it only checks for bytes 0x55 and 0xAA. One might want to check additionally if there is either a disk identifier at position 0x01B8 or one of the type bytes of the four primary partitions is set. And if neither is set return -1 from test_block_type(). Best regards Heinrich >> >> Function efi_fs_exists() does not check if there is a partition table. >> It only checks if there is a file system. >> >> For creating a test image you can use the following commands: >> >> cat > partioning << EOF >> label: dos >> label-id: 0x6fe3a999 >> device: image >> unit: sectors >> image1: start= 1024, size= 524288, type=0c >> EOF >> dd if=/dev/zero of=test.img count=1 bs=1MiB >> /usr/sbin/sfdisk test.img < partioning >> dd if=test.img of=partition.tbl bs=8 count=9 skip=55 >> /usr/sbin/mkfs.vfat test.img 1024 >> dd conv=fsync,notrunc if=partition.tbl of=test.img bs=8 count=9 seek=55 >> sudo losetup -o 524288 --sizelimit 524288 /dev/loop1 test.img >> sudo mkfs.vfat /dev/loop1 >> sudo losetup -D /dev/loop1 >> sudo mount test.img /mnt >> sudo sh -c "echo 'file system on block device' > /mnt/description.txt" >> sudo umount /mnt >> sudo mount test.img /mnt -o offset=524288 >> sudo sh -c "echo 'file system on partition 1' > /mnt/description.txt" >> sudo umount /mnt > > Your example above is quite quirky, and totally unpractical. > If people would create such a file system, they would expect > that they could access a raw block device as well as a partition #1. > > However, I would like to drop this patch(#5) from my patch set > because I don't find any public interface that can be used to > determine if there exists a file system on a raw device. > > I initially thought of reverting my patch to v2, where part_get_info() > was used, but it doesn't work as expected for part == 0. > For example, GPT partition dirver, disk_efi.c, uses an internal > function, find_valid_gpt(), but part_get_info_efi() returns an error > for part == 0. > > For me, it is much easier to modify my pytest for UEFI secure boot > than to carve out a generic interface for *all* the file systems. > > My patch #1 to #4 have already been reviewed by you and there are > no outstanding issues. > > -Takahiro Akashi > >> Without your patch I get >> >> UEFI Interactive Shell v2.2 >> EDK II >> UEFI v2.80 (Das U-Boot, 0x20191000) >> Mapping table >> FS0: Alias(s):HD0a0b:;BLK1: >> >> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)/HD(1,MBR,0x6fe3a999,0x400,0x400) >> BLK0: Alias(s): >> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0) >> Press ESC in 4 seconds to skip startup.nsh or any other key to continue. >> Shell> fs0: >> FS0:\> cat description.txt >> file system on partition 1 >> >> With your patch: >> >> UEFI Interactive Shell v2.2 >> EDK II >> UEFI v2.80 (Das U-Boot, 0x20191000) >> Mapping table >> FS0: Alias(s):F0a0:;BLK0: >> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0) >> FS1: Alias(s):HD0a0b:;BLK1: >> >> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)/HD(1,MBR,0x6fe3a999,0x400,0x400) >> Press ESC in 4 seconds to skip startup.nsh or any other key to continue. >> Shell> fs0: >> FS0:\> cat description.txt >> file system on block device >> >> FS0:\> fs1: >> FS1:\> cat description.txt >> file system on partition 1 >> >> So though a partition table is discovered a file system is mounted on >> the block device. In my special case the file system on the block device >> really existed and was well separated from partition 1. But typically >> expect that there is no file system on the block device if there is a >> partition table. >> >> For your convenience I have uploaded the image file to >> https://github.com/U-Boot-EFI/test_file_system >> >> Best regards >> >> Heinrich >> >>> diskobj->volume = efi_simple_file_system(desc, part, >>> diskobj->dp); >>> ret = efi_add_protocol(&diskobj->header, >>> > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-10-24 3:26 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-10-07 5:59 [U-Boot] [PATCH v4 0/5] efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk AKASHI Takahiro 2019-10-07 5:59 ` [U-Boot] [PATCH v4 1/5] fs: export fs_close() AKASHI Takahiro 2019-10-12 21:24 ` Heinrich Schuchardt 2019-10-07 5:59 ` [U-Boot] [PATCH v4 2/5] fs: clean up around fs_type AKASHI Takahiro 2019-10-12 21:23 ` Heinrich Schuchardt 2019-10-07 5:59 ` [U-Boot] [PATCH v4 3/5] fs: add fs_get_type() for current filesystem type AKASHI Takahiro 2019-10-07 5:59 ` [U-Boot] [PATCH v4 4/5] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available AKASHI Takahiro 2019-10-22 20:29 ` [U-Boot] [BUG] efi_driver: crash while reading from iSCSI drive Heinrich Schuchardt 2019-10-23 10:30 ` AKASHI Takahiro 2019-10-24 3:26 ` Heinrich Schuchardt 2019-10-07 5:59 ` [U-Boot] [PATCH v4 5/5] efi_loader: disk: install file system protocol to a whole disk AKASHI Takahiro 2019-10-12 19:43 ` Heinrich Schuchardt 2019-10-15 7:34 ` AKASHI Takahiro 2019-10-15 11:07 ` Heinrich Schuchardt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox