Bug 214375 - RAIDZ pool with cheksum=skein fails to execute loader.efi from boot1.efi on UEFI system
Summary: RAIDZ pool with cheksum=skein fails to execute loader.efi from boot1.efi on U...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: amd64 Any
: --- Affects Some People
Assignee: Toomas Soome
URL:
Keywords: uefi
Depends on:
Blocks:
 
Reported: 2016-11-10 02:43 UTC by Lawrence Stewart
Modified: 2018-02-08 02:51 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Lawrence Stewart freebsd_committer freebsd_triage 2016-11-10 02:43:08 UTC
I tested with FreeBSD-CURRENT snapshot ftp://ftp.freebsd.org/pub/FreeBSD/snapshots/amd64/amd64/ISO-IMAGES/12.0/FreeBSD-12.0-CURRENT-amd64-20161021-r307747-memstick.img.xz

Installing boot1.efifat into the EFI partition and creating a single RAIDZ pool to boot from fails to boot when "-O checksum=skein" is included in the zpool creation step, but boots fine when that option is omitted i.e. default checksum algorithm is used.

Symptoms observed are that boot1 sees my pool (prints my pool name after "found the following pools" message) but hangs where it should have run loader.efi, and the only way to recover is a hard reset. Same result after every boot.

tsoome@ appears to have reproduced the problem and a likely fix is up for review at https://reviews.freebsd.org/D8487 which I will be testing shortly, but he requested that a bug report be filed.
Comment 1 Lawrence Stewart freebsd_committer freebsd_triage 2016-11-16 06:29:17 UTC
With the patch from D8487 applied on top of svn head r308477, I built a custom memstick image, wiped the harddrives and reran my build guide steps including creation of the RAIDZ zpool with "-O checksum=skein" option and can confirm the system boots, so although I did not test without this patch applied, I think it's safe to say the patch fixed things given that there have been no other relevant changes in sys/boot between the revision I tested with previously (r307747) and r308477.
Comment 2 commit-hook freebsd_committer freebsd_triage 2016-11-17 19:38:40 UTC
A commit references this bug:

Author: tsoome
Date: Thu Nov 17 19:38:31 UTC 2016
New revision: 308776
URL: https://svnweb.freebsd.org/changeset/base/308776

Log:
  loader: zfs toplevel vdev must have spa set.

  The salt based checksum mechanisms, such as skein, are storing the seed
  in spa structure, and need to access the spa to use the seed. The current
  mechanism for quick access to correct spa is via pointer provided by
  vdev structure, but unfortunately the current code does set spa only
  for the leaf vdev. This patch will fix the issue by making sure the
  loader zfs reader will set spa also for top-level vdevs.

  PR:		214375
  Reported by:	lstewart
  Reviewed by:	allanjude, imp
  Approved by:	allanjude (mentor), imp (mentor)
  MFC after:	2 weeks
  Differential Revision:	https://reviews.freebsd.org/D8487

Changes:
  head/sys/boot/zfs/zfsimpl.c
Comment 3 commit-hook freebsd_committer freebsd_triage 2016-12-01 19:07:07 UTC
A commit references this bug:

Author: tsoome
Date: Thu Dec  1 19:06:09 UTC 2016
New revision: 309368
URL: https://svnweb.freebsd.org/changeset/base/309368

Log:
  MFC r308776

  loader: zfs toplevel vdev must have spa set.

  PR:		214375
  Reported by:	lstewart
  Reviewed by:	allanjude, imp
  Approved by:	allanjude (mentor), imp (mentor)
  Differential Revision:	https://reviews.freebsd.org/D8487

Changes:
_U  stable/11/
  stable/11/sys/boot/zfs/zfsimpl.c
Comment 4 Conrad Meyer freebsd_committer freebsd_triage 2017-01-13 05:40:45 UTC
Can this bug be closed, Toomas?
Comment 5 Toomas Soome freebsd_committer freebsd_triage 2017-01-13 06:34:19 UTC
(In reply to Conrad Meyer from comment #4)

Yes.
Comment 6 commit-hook freebsd_committer freebsd_triage 2018-02-08 02:51:38 UTC
A commit references this bug:

Author: kevans
Date: Thu Feb  8 02:50:50 UTC 2018
New revision: 329011
URL: https://svnweb.freebsd.org/changeset/base/329011

Log:
  MFC r307322,r307323,r307324,r307326,r307327,r307338,r307879,r307908,r307911,
  r307942,r307950,r307951,r307954,r307955,r308125,r308195,r308476,r308534,
  r308535,r308776,r308843,r310236,r310726: Loader fixes, 2016q4

  r307322: Remove /boot/boot.conf, deprecated for 16 years

  r307323: Remove fetching of pInterp.

  r307324: Create a new linker set, Xficl_compile_set which contains a list of
  functions to call to register new forth words.

  r307326: In UEFI mode expose the SMBIOS anchor base address via kenv

  r307327: Update i386 build of loader.efi (but leave it disabled) so that we
  at least build it now.

  r307338: Create a pcibios-version environment FORTH word.

  r307879: Preliminary support for EFI in boot loader.

  r307908: Fix the build on both arm64 and when WITHOUT_FORTH is defined.

  r307911: Add better comment...

  r307942: Really make WITHOUT_FORTH (MK_FORTH==no) work.

  r307950: Add it (Makefile.ficl) to the right place

  r307951: Fix two backwards tests.

  r307954: Back out the move to the loader script from -N.

  r307955: LIBSTAND goes last, so put it last here too.

  r308125: In loader.efi, instead of exiting directly, try to fallback.

  r308195: efinet_dev_print should honor verbose option.

  r308476: boot/forth spelling issue in forth word

  r308534: The file_loadraw function grew an argument, update install function

  r308535: Add support for LOADER_RC setting in the pkgfs manifes

  r308776: loader: zfs toplevel vdev must have spa set.

  r308843: loader: smbios version check is not correct

  r310236: Renumber license clauses to avoid skipping #3

  r310726: cdboot: add explict suffix to ambiguous or instruction

  PR:		214375

Changes:
_U  stable/11/
  stable/11/sys/boot/Makefile.ficl
  stable/11/sys/boot/common/Makefile.inc
  stable/11/sys/boot/common/bootstrap.h
  stable/11/sys/boot/common/install.c
  stable/11/sys/boot/common/interp.c
  stable/11/sys/boot/common/interp_forth.c
  stable/11/sys/boot/common/loader.8
  stable/11/sys/boot/common/newvers.sh
  stable/11/sys/boot/common/pnp.c
  stable/11/sys/boot/efi/libefi/Makefile
  stable/11/sys/boot/efi/libefi/efinet.c
  stable/11/sys/boot/efi/libefi/env.c
  stable/11/sys/boot/efi/loader/Makefile
  stable/11/sys/boot/efi/loader/arch/amd64/ldscript.amd64
  stable/11/sys/boot/efi/loader/arch/arm/ldscript.arm
  stable/11/sys/boot/efi/loader/arch/arm64/ldscript.arm64
  stable/11/sys/boot/efi/loader/arch/i386/efimd.c
  stable/11/sys/boot/efi/loader/arch/i386/elf32_freebsd.c
  stable/11/sys/boot/efi/loader/arch/i386/exec.c
  stable/11/sys/boot/efi/loader/arch/i386/ldscript.i386
  stable/11/sys/boot/efi/loader/main.c
  stable/11/sys/boot/ficl/Makefile
  stable/11/sys/boot/ficl/efi.c
  stable/11/sys/boot/ficl/ficl.h
  stable/11/sys/boot/ficl/i386/sysdep.c
  stable/11/sys/boot/ficl/loader.c
  stable/11/sys/boot/ficl32/Makefile
  stable/11/sys/boot/forth/Makefile.inc
  stable/11/sys/boot/forth/efi.4th
  stable/11/sys/boot/forth/loader.4th
  stable/11/sys/boot/forth/support.4th
  stable/11/sys/boot/i386/Makefile.inc
  stable/11/sys/boot/i386/cdboot/cdboot.S
  stable/11/sys/boot/i386/gptboot/Makefile
  stable/11/sys/boot/i386/gptzfsboot/Makefile
  stable/11/sys/boot/i386/libi386/Makefile
  stable/11/sys/boot/i386/libi386/biospci.c
  stable/11/sys/boot/i386/libi386/libi386.h
  stable/11/sys/boot/i386/libi386/smbios.c
  stable/11/sys/boot/i386/loader/Makefile
  stable/11/sys/boot/mips/beri/loader/loader.ldscript
  stable/11/sys/boot/pc98/libpc98/Makefile
  stable/11/sys/boot/zfs/zfsimpl.c
  stable/11/targets/pseudo/userland/misc/Makefile.depend