EFI: Fix ReadBlocks API reading incorrect sector for UCLASS_PARTITION devices

The requsted partition disk sector incorrectly has the parition start
sector added in twice for UCLASS_PARTITION devices. The efi_disk_rw_blocks()
routine adds the diskobj->offset to the requested lba. When the device
is a UCLASS_PARTITION, the dev_read() or dev_write() routine is called
which adds part-gpt_part_info.start. This causes I/O to the wrong sector.

Takahiro Akashi suggested removing the offset field from the efi_disk_obj
structure since disk-uclass.c handles the partition start biasing. Device
types other than UCLASS_PARTITION set the diskobj->offset field to zero
which makes the field unnecessary. This change removes the offset field
from the structure and removes all references from the code which is
isolated to the lib/efi_loader/efi_disk.c module.

This change also adds a test for the EFI ReadBlocks() API in the EFI
selftest code. There is already a test for reading a FAT file. The new
test uses ReadBlocks() to read the same "disk" block and compare it to
the data read from the file system API.

Signed-Off-by: Paul Barbieri <plb365@gmail.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 1e82f52dc07098b059bb8c87d5d8087ca1b4117e..1d700b2a6be34d6d5e2159adf0d55148c915b3a1 100644 (file)
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -35,7 +35,6 @@ const efi_guid_t efi_system_partition_guid = PARTITION_SYSTEM_GUID;
  * @dp:                device path to the block device
  * @part:      partition
  * @volume:    simple file system protocol of the partition
- * @offset:    offset into disk for simple partition
  * @dev:       associated DM device
  */
 struct efi_disk_obj {
@@ -47,7 +46,6 @@ struct efi_disk_obj {
        struct efi_device_path *dp;
        unsigned int part;
        struct efi_simple_file_system_protocol *volume;
-       lbaint_t offset;
        struct udevice *dev; /* TODO: move it to efi_object */
 };
 
@@ -117,7 +115,6 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
        diskobj = container_of(this, struct efi_disk_obj, ops);
        blksz = diskobj->media.block_size;
        blocks = buffer_size / blksz;
-       lba += diskobj->offset;
 
        EFI_PRINT("blocks=%x lba=%llx blksz=%x dir=%d\n",
                  blocks, lba, blksz, direction);
@@ -440,13 +437,11 @@ static efi_status_t efi_disk_add_dev(
 
                diskobj->dp = efi_dp_append_node(dp_parent, node);
                efi_free_pool(node);
-               diskobj->offset = part_info->start;
                diskobj->media.last_block = part_info->size - 1;
                if (part_info->bootable & PART_EFI_SYSTEM_PARTITION)
                        guid = &efi_system_partition_guid;
        } else {
                diskobj->dp = efi_dp_from_part(desc, part);
-               diskobj->offset = 0;
                diskobj->media.last_block = desc->lba - 1;
        }
        diskobj->part = part;
@@ -501,12 +496,11 @@ static efi_status_t efi_disk_add_dev(
                *disk = diskobj;
 
        EFI_PRINT("BlockIO: part %u, present %d, logical %d, removable %d"
-                 ", offset " LBAF ", last_block %llu\n",
+                 ", last_block %llu\n",
                  diskobj->part,
                  diskobj->media.media_present,
                  diskobj->media.logical_partition,
                  diskobj->media.removable_media,
-                 diskobj->offset,
                  diskobj->media.last_block);
 
        /* Store first EFI system partition */

diff --git a/lib/efi_selftest/efi_selftest_block_device.c b/lib/efi_selftest/efi_selftest_block_device.c
index 60fa655766f919546c7815ffd6e11315da669ccb..a367e8b89d17142bf97347a630be2e5e4a8cd3be 100644 (file)
--- a/lib/efi_selftest/efi_selftest_block_device.c
+++ b/lib/efi_selftest/efi_selftest_block_device.c
@@ -11,6 +11,8 @@
  * ConnectController is used to setup partitions and to install the simple
  * file protocol.
  * A known file is read from the file system and verified.
+ * The same block is read via the EFI_BLOCK_IO_PROTOCOL and compared to the file
+ * contents.
  */
 
 #include <efi_selftest.h>
@@ -312,6 +314,7 @@ static int execute(void)
        char buf[16] __aligned(ARCH_DMA_MINALIGN);
        u32 part1_size;
        u64 pos;
+       char block_io_aligned[1 << LB_BLOCK_SIZE] __aligned(1 << LB_BLOCK_SIZE);
 
        /* Connect controller to virtual disk */
        ret = boottime->connect_controller(disk_handle, NULL, NULL, 1);
@@ -449,6 +452,30 @@ static int execute(void)
                return EFI_ST_FAILURE;
        }
 
+       /*
+        * Test that read_blocks() can read same file data.
+        *
+        * In the test data, the partition starts at block 1 and the file
+        * hello.txt with the content 'Hello world!' is located at 0x5000
+        * of the disk. Here we read block 0x27 (offset 0x4e00 of the
+        * partition) and expect the string 'Hello world!' to be at the
+        * start of block.
+        */
+       ret = block_io_protocol->read_blocks(block_io_protocol,
+                                     block_io_protocol->media->media_id,
+                                     (0x5000 >> LB_BLOCK_SIZE) - 1,
+                                     block_io_protocol->media->block_size,
+                                     block_io_aligned);
+       if (ret != EFI_SUCCESS) {
+               efi_st_error("ReadBlocks failed\n");
+               return EFI_ST_FAILURE;
+       }
+
+       if (memcmp(block_io_aligned + 1, buf, 11)) {
+               efi_st_error("Unexpected block content\n");
+               return EFI_ST_FAILURE;
+       }
+
 #ifdef CONFIG_FAT_WRITE
        /* Write file */
        ret = root->open(root, &file, u"u-boot.txt", EFI_FILE_MODE_READ |