svn commit: r322941 - head/sys/boot/efi/boot1
Tomoaki AOKI
junchoon at dec.sakura.ne.jp
Sun Sep 3 05:38:43 UTC 2017
On Sat, 2 Sep 2017 11:26:50 -0600
Warner Losh <imp at bsdimp.com> wrote:
> On Sat, Sep 2, 2017 at 11:25 AM, Warner Losh <imp at bsdimp.com> wrote:
>
> >
> >
> > 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-Februa
> >> ry/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
> >
Confirmed fixed (on both 1st [stable/11] and 2nd [head] HDD) on r323131.
Thanks for your quick response!
But one thing to mention. There could be problematic UEFI firmware
and functions like this could be kept on required for workaround.
(Like LenovoFix on gpart / geom.)
> P.S. This is the doc I put together for discussion. I'm keeping it updated
> as each stage is implemented.
>
> https://docs.google.com/document/d/1aK9IqF-60JPEbUeSAUAkYjF2W_8EnmczFs6RqCT90Jg/edit
>
> Lemme know if you have any comments.
>
> Warner
Thanks for the info.
Not yet, but I'll read it later and let you know if some comments looks
needed.
Thanks again!
>
> > > 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>
> >>
> >
> >
--
青木 知明 [Tomoaki AOKI] <junchoon at dec.sakura.ne.jp>
More information about the svn-src-head
mailing list