svn commit: r322941 - head/sys/boot/efi/boot1

[Warner Losh]

On Sat, Sep 2, 2017 at 1:43 AM, Tomoaki AOKI <junchoon at dec.sakura.ne.jp>
wrote:

> Hi.
>
> This broke boot drive selection functionality by smh@ at r295320 on
> Feb.5, 2016. [1]
> Now, even if I forcibly select 2nd HDD via UEFI firmware, boot1.efi
> (as bootx64.efi) selects /boot/loader.efi on 1st HDD. Each boot
> partition are ZFS pools. Confirmed previous rev was OK.
>
> Attached is the boot log (with EFI_DEBUG). Pool zsysS02 should be
> selected there instead of zsysS01. As I have no serial console, there
> can be some typos. (Took video and hand-typed.)
>
> The boot order should be as below (what smh@ implemented).
>
>  1. ZFS pool on which drive boot1.efi is read from.
>  2. UFS partition on which drive boot1.efi is read from.
>  3. If both 1 and 2 are missed, try from UEFI 1st drive and later.
>
> P.S. Another possibility (not tested yet):
>      Not forcibly prefer 1st drive, but reversed selection.
>      (If boot1.efi is on 1st drive, loader.efi on 2nd drive is read.)
>
> [1]
> https://lists.freebsd.org/pipermail/svn-src-head/2016-February/082215.html


Looks like the matching function that I replaced this stuff with wasn't
quite the same. Will fix. I hadn't thought I'd broken it, honestly, since
the setup I have still worked. The intent was to keep things as they were,
which clearly didn't happen in at least your case. I'll take a look at the
logs to see if I can spot the differences between the two setups. The
intent was to keep functionality, for now, the same.

In the long term, though, this guessing and matching is going to end up
first as deprecated and then as removed. To be replaced by a boot1.efi that
follows the EFI Boot Manager protocol where exactly what to load is
contained in EFI env variables. That's what all my changes to boot1 have
been working towards.

Warner


> > Author: imp
> > Date: Sun Aug 27 03:10:16 2017
> > New Revision: 322941
> > URL: https://svnweb.freebsd.org/changeset/base/322941
> >
> > Log:
> >   Eliminate redunant device path matching.
> >
> >   Use efi_devpath_match instead of device_paths_match. They are
> >   functionally the same. Remove device_paths_match from boot1.c and call
> >   efi_devpath_match instead.
> >
> >   Sponsored by: Netflix
> >
> > Modified:
> >   head/sys/boot/efi/boot1/boot1.c
> >
> > Modified: head/sys/boot/efi/boot1/boot1.c
> > ============================================================
> ==================
> > --- head/sys/boot/efi/boot1/boot1.c   Sat Aug 26 23:13:18 2017
> (r322940)
> > +++ head/sys/boot/efi/boot1/boot1.c   Sun Aug 27 03:10:16 2017
> (r322941)
> > @@ -76,53 +76,6 @@ Free(void *buf, const char *file __unused, int line
> __ }
> >
> >  /*
> > - * nodes_match returns TRUE if the imgpath isn't NULL and the nodes
> match,
> > - * FALSE otherwise.
> > - */
> > -static BOOLEAN
> > -nodes_match(EFI_DEVICE_PATH *imgpath, EFI_DEVICE_PATH *devpath)
> > -{
> > -     size_t len;
> > -
> > -     if (imgpath == NULL || imgpath->Type != devpath->Type ||
> > -         imgpath->SubType != devpath->SubType)
> > -             return (FALSE);
> > -
> > -     len = DevicePathNodeLength(imgpath);
> > -     if (len != DevicePathNodeLength(devpath))
> > -             return (FALSE);
> > -
> > -     return (memcmp(imgpath, devpath, (size_t)len) == 0);
> > -}
> > -
> > -/*
> > - * device_paths_match returns TRUE if the imgpath isn't NULL and all
> nodes
> > - * in imgpath and devpath match up to their respective occurrences of a
> > - * media node, FALSE otherwise.
> > - */
> > -static BOOLEAN
> > -device_paths_match(EFI_DEVICE_PATH *imgpath, EFI_DEVICE_PATH *devpath)
> > -{
> > -
> > -     if (imgpath == NULL)
> > -             return (FALSE);
> > -
> > -     while (!IsDevicePathEnd(imgpath) && !IsDevicePathEnd(devpath))
> {
> > -             if (IsDevicePathType(imgpath, MEDIA_DEVICE_PATH) &&
> > -                 IsDevicePathType(devpath, MEDIA_DEVICE_PATH))
> > -                     return (TRUE);
> > -
> > -             if (!nodes_match(imgpath, devpath))
> > -                     return (FALSE);
> > -
> > -             imgpath = NextDevicePathNode(imgpath);
> > -             devpath = NextDevicePathNode(devpath);
> > -     }
> > -
> > -     return (FALSE);
> > -}
> > -
> > -/*
> >   * devpath_last returns the last non-path end node in devpath.
> >   */
> >  static EFI_DEVICE_PATH *
> > @@ -318,7 +271,7 @@ probe_handle(EFI_HANDLE h, EFI_DEVICE_PATH
> *imgpath, B
> >  if (!blkio->Media->LogicalPartition)
> >               return (EFI_UNSUPPORTED);
> >
> > -     *preferred = device_paths_match(imgpath, devpath);
> > +     *preferred = efi_devpath_match(imgpath, devpath);
> >
> >       /* Run through each module, see if it can load this partition
> */
> >  for (i = 0; i < NUM_BOOT_MODULES; i++) {
>
> --
> Tomoaki AOKI    <junchoon at dec.sakura.ne.jp>
>