From owner-svn-src-head@freebsd.org Sat Sep 2 17:26:51 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C371AE1D763 for ; Sat, 2 Sep 2017 17:26:51 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-io0-x234.google.com (mail-io0-x234.google.com [IPv6:2607:f8b0:4001:c06::234]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 86A5B6A62A for ; Sat, 2 Sep 2017 17:26:51 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-io0-x234.google.com with SMTP id b2so10775578iof.0 for ; Sat, 02 Sep 2017 10:26:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=FCxaI71pi4b6CTQ6DoviDWb6zFaV7xO7/i68XamxJ+M=; b=jfuvvUZ0RhqZx2Q3EnWGjXiPMD/qY8Tc+Hza4EkMEwnC+kL5Hj0d1tnr6l9LyNG4cM GtWAjbU+SbkJhXtQQOUZ4iI5oe0MnqVS/Uh4+nkEjY6O96DjCUZTrmndht9SUCxEHubh 9esmlGgbf3sLS2HFQEaGJkuW2NuTv/foITn4ZW6MlNlLSDcMAJkoGqn5DPOu+pa7z7Ep feqX/cUvXPEzzh2C/qaDVcpARqkkqyhbqF4CbqIERm79H3kfIhW/pDSH52yrdQ5yp/DL 5hc+PMkfsVimtL0Pbx0S/Yv5EDlaV9B8UmccolhT/iwjJX3pdbflTzxKM+2pb8EtlJbP DdpQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=FCxaI71pi4b6CTQ6DoviDWb6zFaV7xO7/i68XamxJ+M=; b=KBLd+VWG+W5wZ91RGKJA7XujEKEAczMZnkiw4mJyaZcWqIi4te0WyByr0Frbewotcn n/Txw9asnNMqyXaIuWu7CHeITV14fj4KjGNhK/KNmQX5ioKz48UcZwhbzLlBHF6YyR95 eL/1g7Y6SkJoG4R16h7dd5866k27oDyvHuVy0Uk0CbGdsMp48Yud+o6TsBiA7/hdFeID NLrnobtc+x6TjiTvGn6rA3x7+e0XTyNHEUZgRDds9Rr9m7tgyiyooWoczknyUkLBG3pu gKoPsW3Ku/HpGZ3TdeKr9CtJAw8CbNeJv58kQzQwcwHdSVasw7RDYM6r0CQy3vmt0iwb WMkw== X-Gm-Message-State: AHPjjUgtWCDsEooQyZrtLA0ealpxO7ulHy7B+RBhY6TUjzeIDCp7hGy3 oTcW9YQJ6yEOgblb1pTl+ydAutmWeZf+ X-Google-Smtp-Source: ADKCNb7Utyn8H/N6SffWxVu920EOdgsanx22oOgBu79HHACmpU98e31GvH9DwfAnq3EjOCk1SwmGiPeQ5kzxp8k5qZA= X-Received: by 10.107.39.83 with SMTP id n80mr5251589ion.182.1504373210552; Sat, 02 Sep 2017 10:26:50 -0700 (PDT) MIME-Version: 1.0 Sender: wlosh@bsdimp.com Received: by 10.79.10.71 with HTTP; Sat, 2 Sep 2017 10:26:50 -0700 (PDT) X-Originating-IP: [2603:300b:6:5100:7467:6cd9:459e:8562] In-Reply-To: References: <20170902164311.eb7e5d2fa0e40f3b4e5e6142@dec.sakura.ne.jp> From: Warner Losh Date: Sat, 2 Sep 2017 11:26:50 -0600 X-Google-Sender-Auth: 9B4ixxLYwn-k7d7YsrwapnODXNc Message-ID: Subject: Re: svn commit: r322941 - head/sys/boot/efi/boot1 To: Tomoaki AOKI Cc: "svn-src-head@freebsd.org" , Warner Losh , Steven Hartland Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.23 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 02 Sep 2017 17:26:51 -0000 On Sat, Sep 2, 2017 at 11:25 AM, Warner Losh wrote: > > > On Sat, Sep 2, 2017 at 1:43 AM, Tomoaki AOKI > 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 > > 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 >> > >