svn commit: r322941 - head/sys/boot/efi/boot1
Tomoaki AOKI
junchoon at dec.sakura.ne.jp
Sat Sep 9 06:12:29 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
> >
>
> 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
>
Read that, and have some comments. (Maybe not enough understood UEFI
2.6 boot manager spec, though)
If I understood current code correctly, third on "Current Algorithm"
looks incorrect. boot1.efi looks for ZFS, then UFS for all devices,
per-device basis.
And yes, it cannot specify which partition to boot from.
A fix was proposed by Naomichi Nonaka as bug 207940.[1]
(Not using UEFI boot manager protocol, though.)
This patch no longer applicable to head with recent changes,
but applicable for stable/11 with slight fix (attached, working for me).
Unfortunately, Naomichi is no longer working on it, as it haven't
introduced to head for a long time, and now he knows GRUB2 can
chainload loader.efi on ZFS pool.
[1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=207940
For "Proposed Algorithm", some problematic UEFI firmware could miss
non-default location. As workaround, current \EFI\BOOT\BOOTx64.efi
(within boot1.efifat) should be kept (at least, as an installer
option) to fallback.
For 2) b), some people could want largest partition number to take
precedence over smaller, especially when they create a new partition to
test new environment keeping old environment untouched.
For 2) c), the administrator should better have opportunity to
re-select another device to boot from, especially for -head users.
Sometimes head cannot boot after installworld and need convenient way
to boot from other device temporarily. (Improperly built loader.efi,
broken *.4th, ...) If the machine is at a datacenter, USB memstick
wouldn't help, but flexible boot manager could do if there's any remote
KVM switch or something alike.
*Setting BootNext and restart would be fine, if the UEFI firmware is
NOT a problematic one, and BootNext is cleared after successful
boot (just like "boot once" for MBR+UFS).
*Would be better if something like "BootEmergency" (non-existent for
now, automatically selected only when "BootFailed") can be
implemented. But maybe cannot, as it should need UEFI firmware
support and I couldn't find such a feature in UEFI spec.
> > > 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>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: boot1.c.NNonaka.diff.rev4_after_s11_r318625
Type: application/octet-stream
Size: 10984 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20170909/fbcc80b2/attachment.obj>
More information about the svn-src-head
mailing list