efi_loader breakage

efi_loader breakage

[Mark Kettenis]

commit d97e98c887ed8fa4a339350c02f093f03cd1cf4d breaks the OpenBSD
bootloader.  Or rather, it seems that this breaks access to raw
devices through the UEFI interfaces, which is something the OpenBSD
bootloader relies on.

After this commit we end up calling dev_read() and dev_write() instead
of blk_dread() and blk_dwrite().  But dev_read() and dev_write() only
work if dev_get_blk() returns a valid struct blkdev, and that handles
only for UCLASS_PARTITION.  But when accessing raw devices we have a
UCLASS_BLK.

The easy fix would be to support UCLASS_BLK in dev_get_blk(), but that
function has an explicit comment saying:

        /*
         * We won't support UCLASS_BLK with dev_* interfaces.
         */
			  
So I'm a bit at a loss what is needed here.

Re: efi_loader breakage

[AKASHI Takahiro]

On Tue, Apr 26, 2022 at 07:07:13PM +0200, Mark Kettenis wrote:
> commit d97e98c887ed8fa4a339350c02f093f03cd1cf4d breaks the OpenBSD
> bootloader.  Or rather, it seems that this breaks access to raw
> devices through the UEFI interfaces, which is something the OpenBSD
> bootloader relies on.
> 
> After this commit we end up calling dev_read() and dev_write() instead
> of blk_dread() and blk_dwrite().  But dev_read() and dev_write() only
> work if dev_get_blk() returns a valid struct blkdev, and that handles
> only for UCLASS_PARTITION.  But when accessing raw devices we have a
> UCLASS_BLK.

Probably I forgot this use case.

> The easy fix would be to support UCLASS_BLK in dev_get_blk(), but that
> function has an explicit comment saying:

The best way to do is to modify efi_disk_rw_blocks().

-Takahiro Akashi

>         /*
>          * We won't support UCLASS_BLK with dev_* interfaces.
>          */
> 			  
> So I'm a bit at a loss what is needed here.

Re: efi_loader breakage

[AKASHI Takahiro]

On Wed, Apr 27, 2022 at 01:48:13PM +0900, AKASHI Takahiro wrote:
> On Tue, Apr 26, 2022 at 07:07:13PM +0200, Mark Kettenis wrote:
> > commit d97e98c887ed8fa4a339350c02f093f03cd1cf4d breaks the OpenBSD
> > bootloader.  Or rather, it seems that this breaks access to raw
> > devices through the UEFI interfaces, which is something the OpenBSD
> > bootloader relies on.
> > 
> > After this commit we end up calling dev_read() and dev_write() instead
> > of blk_dread() and blk_dwrite().  But dev_read() and dev_write() only
> > work if dev_get_blk() returns a valid struct blkdev, and that handles
> > only for UCLASS_PARTITION.  But when accessing raw devices we have a
> > UCLASS_BLK.
> 
> Probably I forgot this use case.
> 
> > The easy fix would be to support UCLASS_BLK in dev_get_blk(), but that
> > function has an explicit comment saying:
> 
> The best way to do is to modify efi_disk_rw_blocks().

I have already made necessary changes, but found another problem
(not relating to my patch) in device path handling.

So I will post the patch once I fix the latter issue as well.

-Takahiro Akashi


> -Takahiro Akashi
> 
> >         /*
> >          * We won't support UCLASS_BLK with dev_* interfaces.
> >          */
> > 			  
> > So I'm a bit at a loss what is needed here.

Re: efi_loader breakage

[Mark Kettenis]

> Date: Wed, 27 Apr 2022 16:26:25 +0900
> From: AKASHI Takahiro <takahiro.akashi@linaro.org>
> 
> On Wed, Apr 27, 2022 at 01:48:13PM +0900, AKASHI Takahiro wrote:
> > On Tue, Apr 26, 2022 at 07:07:13PM +0200, Mark Kettenis wrote:
> > > commit d97e98c887ed8fa4a339350c02f093f03cd1cf4d breaks the OpenBSD
> > > bootloader.  Or rather, it seems that this breaks access to raw
> > > devices through the UEFI interfaces, which is something the OpenBSD
> > > bootloader relies on.
> > > 
> > > After this commit we end up calling dev_read() and dev_write() instead
> > > of blk_dread() and blk_dwrite().  But dev_read() and dev_write() only
> > > work if dev_get_blk() returns a valid struct blkdev, and that handles
> > > only for UCLASS_PARTITION.  But when accessing raw devices we have a
> > > UCLASS_BLK.
> > 
> > Probably I forgot this use case.
> > 
> > > The easy fix would be to support UCLASS_BLK in dev_get_blk(), but that
> > > function has an explicit comment saying:
> > 
> > The best way to do is to modify efi_disk_rw_blocks().
> 
> I have already made necessary changes, but found another problem
> (not relating to my patch) in device path handling.
> 
> So I will post the patch once I fix the latter issue as well.

Thanks,

Looking forward to a patch to test.

Mark