[PATCH] efi_loader: disk: allow blk devices even without UCLASS_PARTITION
While GPT partition is mandated in UEFI specification, CONFIG_PARTITION is seen optional under the current implementation. So modify efi_disk_rw_blocks() to allow accepting UCLASS_BLK devices.
Fixes: commit d97e98c887ed ("efi_loader: disk: use udevice instead of blk_desc") Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- lib/efi_loader/efi_disk.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 8fb5b2363c45..f5b462fb164a 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -99,21 +99,22 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this, if (buffer_size & (blksz - 1)) return EFI_BAD_BUFFER_SIZE;
-#if CONFIG_IS_ENABLED(PARTITIONS) - if (direction == EFI_DISK_READ) - n = dev_read(diskobj->dev, lba, blocks, buffer); - else - n = dev_write(diskobj->dev, lba, blocks, buffer); -#else - /* dev is always a block device (UCLASS_BLK) */ - struct blk_desc *desc; + if (CONFIG_IS_ENABLED(PARTITIONS) && + device_get_uclass_id(diskobj->dev) == UCLASS_PARTITION) { + if (direction == EFI_DISK_READ) + n = dev_read(diskobj->dev, lba, blocks, buffer); + else + n = dev_write(diskobj->dev, lba, blocks, buffer); + } else { + /* dev is a block device (UCLASS_BLK) */ + struct blk_desc *desc;
- desc = dev_get_uclass_plat(diskobj->dev); - if (direction == EFI_DISK_READ) - n = blk_dread(desc, lba, blocks, buffer); - else - n = blk_dwrite(desc, lba, blocks, buffer); -#endif + desc = dev_get_uclass_plat(diskobj->dev); + if (direction == EFI_DISK_READ) + n = blk_dread(desc, lba, blocks, buffer); + else + n = blk_dwrite(desc, lba, blocks, buffer); + }
/* We don't do interrupts, so check for timers cooperatively */ efi_timer_check();
On 4/28/22 06:49, AKASHI Takahiro wrote:
While GPT partition is mandated in UEFI specification, CONFIG_PARTITION is seen optional under the current implementation. So modify efi_disk_rw_blocks() to allow accepting UCLASS_BLK devices.
Fixes: commit d97e98c887ed ("efi_loader: disk: use udevice instead of blk_desc") Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_disk.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 8fb5b2363c45..f5b462fb164a 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -99,21 +99,22 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this, if (buffer_size & (blksz - 1)) return EFI_BAD_BUFFER_SIZE;
-#if CONFIG_IS_ENABLED(PARTITIONS)
- if (direction == EFI_DISK_READ)
n = dev_read(diskobj->dev, lba, blocks, buffer);- else
n = dev_write(diskobj->dev, lba, blocks, buffer);-#else
- /* dev is always a block device (UCLASS_BLK) */
- struct blk_desc *desc;
- if (CONFIG_IS_ENABLED(PARTITIONS) &&
device_get_uclass_id(diskobj->dev) == UCLASS_PARTITION) {if (direction == EFI_DISK_READ)n = dev_read(diskobj->dev, lba, blocks, buffer);elsen = dev_write(diskobj->dev, lba, blocks, buffer);
Thanks for the patch. It solves the problem with the block IO protocol.
Why should dev_read() and dev_write only work for UCLASS_PARTITION? Can't we make it work for any block device? I think the treatment of different types of block devices should be moved to disk/disk-uclass.c.
I will pull this patch as a fast fix. But this should not be the final design.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
- } else {
/* dev is a block device (UCLASS_BLK) */struct blk_desc *desc;
- desc = dev_get_uclass_plat(diskobj->dev);
- if (direction == EFI_DISK_READ)
n = blk_dread(desc, lba, blocks, buffer);- else
n = blk_dwrite(desc, lba, blocks, buffer);-#endif
desc = dev_get_uclass_plat(diskobj->dev);if (direction == EFI_DISK_READ)n = blk_dread(desc, lba, blocks, buffer);elsen = blk_dwrite(desc, lba, blocks, buffer);}
/* We don't do interrupts, so check for timers cooperatively */ efi_timer_check();
Simon,
On Thu, Apr 28, 2022 at 12:43:03PM +0200, Heinrich Schuchardt wrote:
On 4/28/22 06:49, AKASHI Takahiro wrote:
While GPT partition is mandated in UEFI specification, CONFIG_PARTITION is seen optional under the current implementation. So modify efi_disk_rw_blocks() to allow accepting UCLASS_BLK devices.
Fixes: commit d97e98c887ed ("efi_loader: disk: use udevice instead of blk_desc") Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_disk.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 8fb5b2363c45..f5b462fb164a 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -99,21 +99,22 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this, if (buffer_size & (blksz - 1)) return EFI_BAD_BUFFER_SIZE;
-#if CONFIG_IS_ENABLED(PARTITIONS)
- if (direction == EFI_DISK_READ)
n = dev_read(diskobj->dev, lba, blocks, buffer);- else
n = dev_write(diskobj->dev, lba, blocks, buffer);-#else
- /* dev is always a block device (UCLASS_BLK) */
- struct blk_desc *desc;
- if (CONFIG_IS_ENABLED(PARTITIONS) &&
device_get_uclass_id(diskobj->dev) == UCLASS_PARTITION) {if (direction == EFI_DISK_READ)n = dev_read(diskobj->dev, lba, blocks, buffer);elsen = dev_write(diskobj->dev, lba, blocks, buffer);Thanks for the patch. It solves the problem with the block IO protocol.
Why should dev_read() and dev_write only work for UCLASS_PARTITION?
I don't remember well, but I have expected that even a whole disk be represented as a partition (0).
Can't we make it work for any block device? I think the treatment of different types of block devices should be moved to disk/disk-uclass.c.
@Simon, what are your thoughts?
-Takahiro Akashi
I will pull this patch as a fast fix. But this should not be the final design.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
- } else {
/* dev is a block device (UCLASS_BLK) */struct blk_desc *desc;
- desc = dev_get_uclass_plat(diskobj->dev);
- if (direction == EFI_DISK_READ)
n = blk_dread(desc, lba, blocks, buffer);- else
n = blk_dwrite(desc, lba, blocks, buffer);-#endif
desc = dev_get_uclass_plat(diskobj->dev);if (direction == EFI_DISK_READ)n = blk_dread(desc, lba, blocks, buffer);elsen = blk_dwrite(desc, lba, blocks, buffer);}
/* We don't do interrupts, so check for timers cooperatively */ efi_timer_check();
Hi Simon,
On Mon, May 09, 2022 at 02:33:10PM +0900, AKASHI Takahiro wrote:
Simon,
On Thu, Apr 28, 2022 at 12:43:03PM +0200, Heinrich Schuchardt wrote:
On 4/28/22 06:49, AKASHI Takahiro wrote:
While GPT partition is mandated in UEFI specification, CONFIG_PARTITION is seen optional under the current implementation. So modify efi_disk_rw_blocks() to allow accepting UCLASS_BLK devices.
Fixes: commit d97e98c887ed ("efi_loader: disk: use udevice instead of blk_desc") Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_disk.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 8fb5b2363c45..f5b462fb164a 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -99,21 +99,22 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this, if (buffer_size & (blksz - 1)) return EFI_BAD_BUFFER_SIZE;
-#if CONFIG_IS_ENABLED(PARTITIONS)
- if (direction == EFI_DISK_READ)
n = dev_read(diskobj->dev, lba, blocks, buffer);- else
n = dev_write(diskobj->dev, lba, blocks, buffer);-#else
- /* dev is always a block device (UCLASS_BLK) */
- struct blk_desc *desc;
- if (CONFIG_IS_ENABLED(PARTITIONS) &&
device_get_uclass_id(diskobj->dev) == UCLASS_PARTITION) {if (direction == EFI_DISK_READ)n = dev_read(diskobj->dev, lba, blocks, buffer);elsen = dev_write(diskobj->dev, lba, blocks, buffer);Thanks for the patch. It solves the problem with the block IO protocol.
Why should dev_read() and dev_write only work for UCLASS_PARTITION?
I don't remember well, but I have expected that even a whole disk be represented as a partition (0).
Can't we make it work for any block device? I think the treatment of different types of block devices should be moved to disk/disk-uclass.c.
@Simon, what are your thoughts?
Gentle ping. Any comment here?
-Takahiro Akashi
-Takahiro Akashi
I will pull this patch as a fast fix. But this should not be the final design.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
- } else {
/* dev is a block device (UCLASS_BLK) */struct blk_desc *desc;
- desc = dev_get_uclass_plat(diskobj->dev);
- if (direction == EFI_DISK_READ)
n = blk_dread(desc, lba, blocks, buffer);- else
n = blk_dwrite(desc, lba, blocks, buffer);-#endif
desc = dev_get_uclass_plat(diskobj->dev);if (direction == EFI_DISK_READ)n = blk_dread(desc, lba, blocks, buffer);elsen = blk_dwrite(desc, lba, blocks, buffer);}
/* We don't do interrupts, so check for timers cooperatively */ efi_timer_check();
Hi Simon,
On Thu, May 19, 2022 at 02:11:02PM +0900, AKASHI Takahiro wrote:
Hi Simon,
On Mon, May 09, 2022 at 02:33:10PM +0900, AKASHI Takahiro wrote:
Simon,
On Thu, Apr 28, 2022 at 12:43:03PM +0200, Heinrich Schuchardt wrote:
On 4/28/22 06:49, AKASHI Takahiro wrote:
While GPT partition is mandated in UEFI specification, CONFIG_PARTITION is seen optional under the current implementation. So modify efi_disk_rw_blocks() to allow accepting UCLASS_BLK devices.
Fixes: commit d97e98c887ed ("efi_loader: disk: use udevice instead of blk_desc") Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_disk.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 8fb5b2363c45..f5b462fb164a 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -99,21 +99,22 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this, if (buffer_size & (blksz - 1)) return EFI_BAD_BUFFER_SIZE;
-#if CONFIG_IS_ENABLED(PARTITIONS)
- if (direction == EFI_DISK_READ)
n = dev_read(diskobj->dev, lba, blocks, buffer);- else
n = dev_write(diskobj->dev, lba, blocks, buffer);-#else
- /* dev is always a block device (UCLASS_BLK) */
- struct blk_desc *desc;
- if (CONFIG_IS_ENABLED(PARTITIONS) &&
device_get_uclass_id(diskobj->dev) == UCLASS_PARTITION) {if (direction == EFI_DISK_READ)n = dev_read(diskobj->dev, lba, blocks, buffer);elsen = dev_write(diskobj->dev, lba, blocks, buffer);Thanks for the patch. It solves the problem with the block IO protocol.
Why should dev_read() and dev_write only work for UCLASS_PARTITION?
I don't remember well, but I have expected that even a whole disk be represented as a partition (0).
Can't we make it work for any block device? I think the treatment of different types of block devices should be moved to disk/disk-uclass.c.
@Simon, what are your thoughts?
Gentle ping. Any comment here?
Ping again. I hope that you have some opinion here as the maintainer of DM.
-Takahiro Akashi
-Takahiro Akashi
-Takahiro Akashi
I will pull this patch as a fast fix. But this should not be the final design.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
- } else {
/* dev is a block device (UCLASS_BLK) */struct blk_desc *desc;
- desc = dev_get_uclass_plat(diskobj->dev);
- if (direction == EFI_DISK_READ)
n = blk_dread(desc, lba, blocks, buffer);- else
n = blk_dwrite(desc, lba, blocks, buffer);-#endif
desc = dev_get_uclass_plat(diskobj->dev);if (direction == EFI_DISK_READ)n = blk_dread(desc, lba, blocks, buffer);elsen = blk_dwrite(desc, lba, blocks, buffer);}
/* We don't do interrupts, so check for timers cooperatively */ efi_timer_check();
From: AKASHI Takahiro takahiro.akashi@linaro.org Date: Thu, 28 Apr 2022 13:49:16 +0900
While GPT partition is mandated in UEFI specification, CONFIG_PARTITION is seen optional under the current implementation. So modify efi_disk_rw_blocks() to allow accepting UCLASS_BLK devices.
Fixes: commit d97e98c887ed ("efi_loader: disk: use udevice instead of blk_desc") Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_disk.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)
confirmed that this does indeed fix the regression
Tested-by: Mark Kettenis kettenis@openbsd.org
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index 8fb5b2363c45..f5b462fb164a 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -99,21 +99,22 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this, if (buffer_size & (blksz - 1)) return EFI_BAD_BUFFER_SIZE;
-#if CONFIG_IS_ENABLED(PARTITIONS)
- if (direction == EFI_DISK_READ)
n = dev_read(diskobj->dev, lba, blocks, buffer);- else
n = dev_write(diskobj->dev, lba, blocks, buffer);-#else
- /* dev is always a block device (UCLASS_BLK) */
- struct blk_desc *desc;
- if (CONFIG_IS_ENABLED(PARTITIONS) &&
device_get_uclass_id(diskobj->dev) == UCLASS_PARTITION) {if (direction == EFI_DISK_READ)n = dev_read(diskobj->dev, lba, blocks, buffer);elsen = dev_write(diskobj->dev, lba, blocks, buffer);- } else {
/* dev is a block device (UCLASS_BLK) */struct blk_desc *desc;
- desc = dev_get_uclass_plat(diskobj->dev);
- if (direction == EFI_DISK_READ)
n = blk_dread(desc, lba, blocks, buffer);- else
n = blk_dwrite(desc, lba, blocks, buffer);-#endif
desc = dev_get_uclass_plat(diskobj->dev);if (direction == EFI_DISK_READ)n = blk_dread(desc, lba, blocks, buffer);elsen = blk_dwrite(desc, lba, blocks, buffer);}
/* We don't do interrupts, so check for timers cooperatively */ efi_timer_check();
-- 2.33.0
participants (3)
-
AKASHI Takahiro -
Heinrich Schuchardt -
Mark Kettenis