[PATCH v2] efi_driver: fix duplicate efiblk#0 issue

[Simon Glass]

Hi Heinrich,

On Tue, 4 Jul 2023 at 00:43, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 03.07.23 15:30, Simon Glass wrote:
> > Hi Masahisa,
> >
> > On Mon, 3 Jul 2023 at 07:09, Masahisa Kojima <masahisa.kojima at linaro.org> wrote:
> >>
> >> The devnum value of the blk_desc structure starts from 0,
> >> current efi_bl_create_block_device() function creates
> >> two "efiblk#0" devices for the cases that blk_find_max_devnum()
> >> returns -ENODEV and blk_find_max_devnum() returns 0(one device
> >> found in this case).
> >>
> >> This commit uses blk_next_free_devnum() instead of blk_find_max_devnum().
> >>
>
> Fixes: 05ef48a2484b ("efi_driver: EFI block driver")
>
> >> Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> >> ---
> >> Changes in v2:
> >> - uses blk_next_free_devnum() instead of blk_find_max_devnum()
> >>
> >>   lib/efi_driver/efi_block_device.c | 6 ++----
> >>   1 file changed, 2 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c
> >> index add00eeebb..e3abd90275 100644
> >> --- a/lib/efi_driver/efi_block_device.c
> >> +++ b/lib/efi_driver/efi_block_device.c
> >> @@ -124,10 +124,8 @@ efi_bl_create_block_device(efi_handle_t handle, void *interface)
> >>          struct efi_block_io *io = interface;
> >>          struct efi_blk_plat *plat;
> >>
> >> -       devnum = blk_find_max_devnum(UCLASS_EFI_LOADER);
>
> Simon, this line was last changed by your patch
> e33a5c6be55e ("blk: Switch over to using uclass IDs")
>
> >> -       if (devnum == -ENODEV)
> >> -               devnum = 0;
> >> -       else if (devnum < 0)
> >> +       devnum = blk_next_free_devnum(UCLASS_EFI_LOADER);
> >
> > This really should be an internal function but I see it was exported
> > as part of the virtio work.
> >
> > How come the EFI and DM block devices are getting out of sync?
>
> They never were in sync:
>
> The bug dates back to Jan 2018:
> 05ef48a2484b ("efi_driver: EFI block driver")

OK I see. Well it looks like the original code was a bit off, as per
my comment. It should be fixed.

>
> Best regards
>
> Heinrich
>
> >
> > Anyway this function is munging around in the internals of the device
> > and should be fixed before it causes more problems.
> >
> > For now, I suggest following what most other drivers so which is to
> > call blk_create_devicef() passing a devnum of -1.
> >
> >> +       if (devnum < 0)
> >>                  return EFI_OUT_OF_RESOURCES;
> >>
> >>          name = calloc(1, 18); /* strlen("efiblk#2147483648") + 1 */
> >> --
> >> 2.34.1

Regards,
Simon