From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Tue, 15 Oct 2019 16:34:13 +0900 Subject: [U-Boot] [PATCH v4 5/5] efi_loader: disk: install file system protocol to a whole disk In-Reply-To: References: <20191007055939.17093-1-takahiro.akashi@linaro.org> <20191007055939.17093-6-takahiro.akashi@linaro.org> Message-ID: <20191015073412.GJ18778@linaro.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > >--- > > 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, > >