Re: [PATCH] disk/efi: skip iPXE dummy block devices

[Michael Chang]

On Tue, Mar 14, 2017 at 06:19:04AM +0300, Andrei Borzenkov wrote:
> 14.03.2017 06:03, Michael Chang пишет:
> > On Mon, Mar 13, 2017 at 06:45:02AM +0300, Andrei Borzenkov wrote:
> >> iPXE adds Simple File System Protocol to loaded image handle, as side
> >> effect it also adds Block IO protocol (according to comments, to work
> >> around some bugs in EDK2). GRUB assumes that every device with Block IO
> >> is disk and skips network initialization entirely. But iPXE Block IO
> >> implementation is just a stub which always fails for every operation
> >> so cannot be used. Attempt to detect and skip such devices.
> >>
> >> We are using media ID which iPXE sets to "iPXE" and block IO size in
> >> hope that no real device would announce 1B block ...
> >>
> >> Closes: 50518
> >>
> >> @Vladimir, @Daniel - this is probably 2.02 material. We cannot use
> >> any device that advertises block size less than 512B anyway, so it
> >> should not cause regression for disk boot.
> >>
> >> It is my fault, I have seen it earlier but was busy with another problem
> >> so just worked around it to continue testing.
> >>
> >> ---
> >>  grub-core/disk/efi/efidisk.c | 9 +++++++++
> >>  1 file changed, 9 insertions(+)
> >>
> >> diff --git a/grub-core/disk/efi/efidisk.c b/grub-core/disk/efi/efidisk.c
> >> index 3b79f7b..47a4e99 100644
> >> --- a/grub-core/disk/efi/efidisk.c
> >> +++ b/grub-core/disk/efi/efidisk.c
> >> @@ -80,6 +80,15 @@ make_devices (void)
> >>    /* This should not happen... Why?  */
> >>    continue;
> >>  
> >> +      /* iPXE adds stub Block IO protocol to loaded image device handle. 
> >> It is
> >> +         completely non-functional and simply returns an error for every 
> >> method.
> >> +   So attempt to detect and skip it. Magic number is literal "iPXE" and
> >> +   check block size as well */
> >> +      /* FIXME: shoud we close it? We do not do it elsewhere */
> >> +      if (bio->media && bio->media->media_id == 0x69505845U &&
> > 
> > It seems that BE to LE conversion is required here as it tests for 'EXPi' 
> > on LE
> > systems. 
> > 
> 
> iPXE sets it to this literal number:

Well. The comment said magic number is literal "iPXE" so that's what I am
confused.

Thanks,
Michael

> 
> #define EFI_MEDIA_ID_MAGIC 0x69505845
> 
> /** Dummy block I/O media */
> static EFI_BLOCK_IO_MEDIA efi_block_io_media = {
>         .MediaId = EFI_MEDIA_ID_MAGIC,
>         .MediaPresent = TRUE,
>         .ReadOnly = TRUE,
>         .BlockSize = 1,
> };
> 
> 
> > Thanks,
> > Michael
> > 
> >> +    bio->media->block_size == 1)
> >> +    continue;
> >> +
> >>        d = grub_malloc (sizeof (*d));
> >>        if (! d)
> >>    {
> >> -- 
> >> tg: (bcf3c55..) bug/50518 (depends on: master)
> >>
> >> _______________________________________________
> >> Grub-devel mailing list
> >> address@hidden
> >> https://lists.gnu.org/mailman/listinfo/grub-devel
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > address@hidden
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> > 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel