Date: Sat, 9 Sep 2017 15:12:17 +0900 From: Tomoaki AOKI <junchoon@dec.sakura.ne.jp> To: svn-src-head@freebsd.org Cc: Warner Losh <imp@bsdimp.com> Subject: Re: svn commit: r322941 - head/sys/boot/efi/boot1 Message-ID: <20170909151217.9082b61a9ef00b86bf90ff8e@dec.sakura.ne.jp> In-Reply-To: <CANCZdfoK=2%2BFa=jEjGOUeipxdFZTnQ8hxxThQDsQAY2KJWEOtA@mail.gmail.com> References: <20170902164311.eb7e5d2fa0e40f3b4e5e6142@dec.sakura.ne.jp> <CANCZdfoDk6fyNL=jJAxn%2BL=WZ84HwkpJDtM1PDhPPJ6HWmeYPw@mail.gmail.com> <CANCZdfoK=2%2BFa=jEjGOUeipxdFZTnQ8hxxThQDsQAY2KJWEOtA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On Sat, 2 Sep 2017 11:26:50 -0600 Warner Losh <imp@bsdimp.com> wrote: > On Sat, Sep 2, 2017 at 11:25 AM, Warner Losh <imp@bsdimp.com> wrote: > > > > > > > On Sat, Sep 2, 2017 at 1:43 AM, Tomoaki AOKI <junchoon@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@dec.sakura.ne.jp> > >> > > > > -- Tomoaki AOKI <junchoon@dec.sakura.ne.jp> [-- Attachment #2 --] Index: sys/boot/efi/boot1/boot1.c =================================================================== --- sys/boot/efi/boot1/boot1.c (revision 295683) +++ sys/boot/efi/boot1/boot1.c (working copy) @@ -29,6 +29,7 @@ #include <efi.h> #include <eficonsctl.h> +#include <efilib.h> #include "boot_module.h" #include "paths.h" @@ -47,11 +48,18 @@ /* The initial number of handles used to query EFI for partitions. */ #define NUM_HANDLES_INIT 24 +void efi_cons_putchar(int); +int efi_cons_getchar(void); +int efi_cons_poll(void); +int getchar(void); +char *humanize_number_kmgt(UINT64); EFI_STATUS efi_main(EFI_HANDLE Ximage, EFI_SYSTEM_TABLE* Xsystab); EFI_SYSTEM_TABLE *systab; EFI_BOOT_SERVICES *bs; static EFI_HANDLE *image; +static SIMPLE_TEXT_OUTPUT_INTERFACE *conout; +static SIMPLE_INPUT_INTERFACE *conin; static EFI_GUID BlockIoProtocolGUID = BLOCK_IO_PROTOCOL; static EFI_GUID DevicePathGUID = DEVICE_PATH_PROTOCOL; @@ -59,6 +67,35 @@ static EFI_GUID LoadedImageGUID = LOADED_IMAGE_PROTOCOL; static EFI_GUID ConsoleControlGUID = EFI_CONSOLE_CONTROL_PROTOCOL_GUID; + +#define DEFAULT_FGCOLOR EFI_LIGHTGRAY +#define DEFAULT_BGCOLOR EFI_BLACK + +char *humanize_number_kmgt(UINT64 size) { + static char buf2[6]; + UINT64 size0; + + if (size < 1024) + sprintf(buf2, "%luB", size); + else if (size0 = (size*10+512)/1024, size0 < 100) + sprintf(buf2, "%lu.%luK", size0 / 10, size0 % 10); + else if (size0 = (size+512)/1024, size0 < 1024) + sprintf(buf2, "%luK", size0); + else if (size0 = (size*10/1024+512)/1024, size0 < 100) + sprintf(buf2, "%lu.%luM", size0 / 10, size0 % 10); + else if (size0 = (size/1024+512)/1024, size0 < 1024) + sprintf(buf2, "%luM", size0); + else if (size0 = (size*10/1024/1024+512)/1024, size0 < 100) + sprintf(buf2, "%lu.%luG", size0 / 10, size0 % 10); + else if (size0 = (size/1024/1024+512)/1024, size0 < 1024) + sprintf(buf2, "%luG", size0); + else if (size0 = (size*10/1024/1024/1024+512)/1024, size0 < 100) + sprintf(buf2, "%lu.%luT", size0 / 10, size0 % 10); + else + sprintf(buf2, "%luT", (size/1024/1024/1024+512)/1024); + return(buf2); +} + /* * Provide Malloc / Free backed by EFIs AllocatePool / FreePool which ensures * memory is correctly aligned avoiding EFI_INVALID_PARAMETER returns from @@ -151,7 +188,6 @@ static int devpath_node_str(char *buf, size_t size, EFI_DEVICE_PATH *devpath) { - switch (devpath->Type) { case MESSAGING_DEVICE_PATH: switch (devpath->SubType) { @@ -251,8 +287,9 @@ HARDDRIVE_DEVICE_PATH *hd; hd = (HARDDRIVE_DEVICE_PATH *)(void *)devpath; - return snprintf(buf, size, "hd(%x)", - hd->PartitionNumber); + return snprintf(buf, size, "hd(p%d) (%s)", + hd->PartitionNumber, + humanize_number_kmgt(hd->PartitionSize * 512)); } default: return snprintf(buf, size, "media(0x%02x)", @@ -350,6 +387,186 @@ return (EFI_NOT_FOUND); } +/* quick dirty work. */ +#define NUM_DEV_LIST 35 + +typedef struct { + const boot_module_t *modp; + dev_info_t *devinfop; +} moddev_t; + +static moddev_t dev_list[NUM_DEV_LIST]; + +static int +list_devices(void) +{ + UINTN i, j; + dev_info_t *dev; + const boot_module_t *mod; + + j = 0; + for (i = 0; i < NUM_BOOT_MODULES; i++) { + if (boot_modules[i] == NULL) + continue; + mod = boot_modules[i]; + for (dev = mod->devices(); dev != NULL; dev = dev->next) { + dev_list[j].devinfop = dev; + dev_list[j].modp = mod; + j++; + if (j >= NUM_DEV_LIST) + break; + } + } + + return (j); +} + +#define SELECT_TIMEOUT 10 + +static char +idx2char(int i) +{ + return (i<10 ? '0'+i : 'a'+i-10); +} + +static int +char2idx(char c) +{ + return ((c >= '0' && c <= '9') ? c - '0' : + (c >= 'A' && c <= 'Z') ? c - 'A' + 10 : + (c >= 'a' && c <= 'z') ? c - 'a' + 10 : + -1 ); +} + +static void +move_to_tol() +{ + int x,y; + + x = conout->Mode->CursorColumn; + y = conout->Mode->CursorRow; + conout->SetCursorPosition(conout, 0, y); +} + +static EFI_STATUS +select_bootdev(int ndevs, const boot_module_t **modp, dev_info_t **devinfop, + void **bufp, size_t *bufsize) +{ + int i; + int c, n; + int time_left; + EFI_STATUS status; + EFI_EVENT timer; + EFI_EVENT events[2]; + UINTN idx; + dev_info_t *dev; + const boot_module_t *mod; + + if ((ndevs <= 0) || (ndevs >= NUM_DEV_LIST)) { + return (EFI_NOT_FOUND); + } else if (ndevs == 1) { + /* Only one condidate. */ + *modp = mod = dev_list[0].modp; + *devinfop = dev = dev_list[0].devinfop; + return (mod->load(PATH_LOADER_EFI, dev, + bufp, bufsize)); + } + /* Two or more candidate exist. */ + status = bs->CreateEvent(EVT_TIMER, 0, 0, NULL, &timer); + if (status != EFI_SUCCESS) { + printf("Can't allocate timer event.\n"); + return (status); + } + printf(" 0: AutoSelected Partition (Default): 0\n"); + for (i = 0; i < ndevs; i++) { + if (dev_list[i].devinfop->preferred == TRUE) + c = '*'; + else + c = ' '; + printf(" %c%c: %s: %s: %c\n", c, idx2char(i+1), + dev_list[i].modp->name, + devpath_str(dev_list[i].devinfop->devpath), + idx2char(i+1)); + } + /* One alpha-num selection only. Is this big enough ?? */ + bs->SetTimer(timer, TimerPeriodic, 10000000); + events[0] = timer; + events[1] = conin->WaitForKey; + time_left = SELECT_TIMEOUT; + + while (1) { + if (time_left > 0) { + printf("Select from 0 to %c. Timeout in %2d seconds," + " [Space] to pause : ", + idx2char(ndevs), time_left); + } + status = bs->WaitForEvent(2, events, &idx); + if (status != EFI_SUCCESS) { + bs->CloseEvent(timer); + return (status); + } + if (idx == 0) { + time_left--; + if (time_left <=0) { + printf("\nTimeout. " + "Partition is AutoSelected.\n"); + n = 0; + break; + } else { + move_to_tol(); + } + } + if (idx == 1) { + c = getchar(); + if ((c == '\n') || (c == '\r')) { + putchar('\n'); + n = 0; + break; + } else if ((time_left > 0) && (c == ' ')) { + bs->SetTimer(timer, TimerCancel, 0); + time_left = -1; + printf("\nTimer stopeed.\n Please Key in: "); + } + n = char2idx(c); + if ((n >= 0) && (n <= ndevs)) { + bs->SetTimer(timer, TimerCancel, 0); + time_left = -1; + printf("\n %c is selected.\n", c); + if (n == 0) { /* AutoSelect */ + break; + } else { + mod = dev_list[n-1].modp; + dev = dev_list[n-1].devinfop; + status = mod->load(PATH_LOADER_EFI, + dev, bufp, bufsize); + if (status == EFI_SUCCESS) { + break; + } + printf("Failed to load '%s'\n", + PATH_LOADER_EFI); + printf("Please select again: "); + } + } else { + /* Invalid charecter. */ + move_to_tol(); + } + } + }; + bs->CloseEvent(timer); + if (n == 0) { /* AutoSelect */ + status = load_loader(modp, devinfop, bufp, bufsize, TRUE); + if (status != EFI_SUCCESS) { + status = load_loader(modp, devinfop, bufp, bufsize, + FALSE); + } + } else { + *modp = dev_list[n-1].modp; + *devinfop = dev_list[n-1].devinfop; + status = EFI_SUCCESS; + } + return (status); +} + /* * try_boot only returns if it fails to load the loader. If it succeeds * it simply boots, otherwise it returns the status of last EFI call. @@ -358,7 +575,7 @@ try_boot() { size_t bufsize, loadersize, cmdsize; - void *buf, *loaderbuf; + void *buf = NULL, *loaderbuf = NULL; char *cmd; dev_info_t *dev; const boot_module_t *mod; @@ -365,15 +582,13 @@ EFI_HANDLE loaderhandle; EFI_LOADED_IMAGE *loaded_image; EFI_STATUS status; + int ndevs; - status = load_loader(&mod, &dev, &loaderbuf, &loadersize, TRUE); + ndevs = list_devices(); + status = select_bootdev(ndevs, &mod, &dev, &loaderbuf, &loadersize); if (status != EFI_SUCCESS) { - status = load_loader(&mod, &dev, &loaderbuf, &loadersize, - FALSE); - if (status != EFI_SUCCESS) { - printf("Failed to load '%s'\n", PATH_LOADER_EFI); - return (status); - } + printf("Failed to load '%s'\n", PATH_LOADER_EFI); + return (EFI_NOT_FOUND); } /* @@ -423,6 +638,9 @@ loaded_image->LoadOptionsSize = cmdsize; loaded_image->LoadOptions = cmd; + printf("Boot from: %s in 1 second\n", devpath_str(dev->devpath)); + bs->Stall(1000000); + DPRINTF("Starting '%s' in 5 seconds...", PATH_LOADER_EFI); DSTALL(1000000); DPRINTF("."); @@ -559,21 +777,13 @@ DSTALL(500000); } -EFI_STATUS -efi_main(EFI_HANDLE Ximage, EFI_SYSTEM_TABLE *Xsystab) +static EFI_STATUS +init_console() { - EFI_HANDLE *handles; - EFI_LOADED_IMAGE *img; - EFI_DEVICE_PATH *imgpath; EFI_STATUS status; EFI_CONSOLE_CONTROL_PROTOCOL *ConsoleControl = NULL; - SIMPLE_TEXT_OUTPUT_INTERFACE *conout = NULL; - UINTN i, max_dim, best_mode, cols, rows, hsize, nhandles; - /* Basic initialization*/ - systab = Xsystab; - image = Ximage; - bs = Xsystab->BootServices; + UINTN i, max_dim, best_mode, cols, rows; /* Set up the console, so printf works. */ status = bs->LocateProtocol(&ConsoleControlGUID, NULL, @@ -584,8 +794,11 @@ /* * Reset the console and find the best text mode. */ + conin = systab->ConIn; conout = systab->ConOut; + conin->Reset(conin, TRUE); conout->Reset(conout, TRUE); + max_dim = best_mode = 0; for (i = 0; ; i++) { status = conout->QueryMode(conout, i, &cols, &rows); @@ -598,9 +811,31 @@ } if (max_dim > 0) conout->SetMode(conout, best_mode); + + conout->SetAttribute(conout, EFI_TEXT_ATTR(DEFAULT_FGCOLOR, + DEFAULT_BGCOLOR)); conout->EnableCursor(conout, TRUE); conout->ClearScreen(conout); + return (status); +} + +EFI_STATUS +efi_main(EFI_HANDLE Ximage, EFI_SYSTEM_TABLE *Xsystab) +{ + EFI_HANDLE *handles; + EFI_LOADED_IMAGE *img; + EFI_DEVICE_PATH *imgpath; + EFI_STATUS status; + UINTN i, hsize, nhandles; + + /* Basic initialization*/ + systab = Xsystab; + image = Ximage; + bs = Xsystab->BootServices; + + init_console(); + printf("\n>> FreeBSD EFI boot block\n"); printf(" Loader path: %s\n\n", PATH_LOADER_EFI); printf(" Initializing modules:"); @@ -715,14 +950,57 @@ void putchar(int c) { + efi_cons_putchar(c); +} + +void +efi_cons_putchar(int c) +{ CHAR16 buf[2]; if (c == '\n') { buf[0] = '\r'; buf[1] = 0; - systab->ConOut->OutputString(systab->ConOut, buf); + conout->OutputString(conout, buf); } buf[0] = c; buf[1] = 0; - systab->ConOut->OutputString(systab->ConOut, buf); + conout->OutputString(conout, buf); } + +int +getchar(void) +{ + return efi_cons_getchar(); +} + + +int +efi_cons_getchar() +{ + EFI_INPUT_KEY key; + EFI_STATUS status; + UINTN junk; + + /* Try to read a key stroke. We wait for one if none is pending. */ + status = conin->ReadKeyStroke(conin, &key); + if (status == EFI_NOT_READY) { + bs->WaitForEvent(1, &conin->WaitForKey, &junk); + status = conin->ReadKeyStroke(conin, &key); + } + switch (key.ScanCode) { + case 0x17: /* ESC */ + return (0x1b); /* esc */ + } + + /* this can return */ + return (key.UnicodeChar); +} + +int +efi_cons_poll() +{ + /* This can clear the signaled state. */ + return (bs->CheckEvent(conin->WaitForKey) == EFI_SUCCESS); +} +
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20170909151217.9082b61a9ef00b86bf90ff8e>