svn commit: r346501 - in stable/12/stand: i386/common i386/gptboot i386/loader i386/zfsboot libsa/zfs

[Ian Lepore]

Author: ian
Date: Sun Apr 21 22:13:07 2019
New Revision: 346501
URL: https://svnweb.freebsd.org/changeset/base/346501

Log:
  MFC r341420, r341473, r341651
  
  r341420:
  Eliminate duplicated code and struct member definitions in the handoff
  of args data between gptboot/zfsboot and loader(8).
  
  Despite what seems like a lot of changes here, there are no actual
  changes in behavior, or in the data layout in the structures involved.
  This is just eliminating identical code pasted into multiple locations.
  
  In detail, the changes are...
  
  - Move struct zfs_boot_args definition from libsa/zfs/libzfs.h to
i386/common/bootargs.h because it is specific to x86 booting and the
handoff between zfsboot and loader, and has no relation to the zfs
library code in general.
  
  - The geli_boot_args and zfs_boot_args structs both contain an identical
set of member variables containing geli information.  Extract this out
to a new geli_boot_data struct, and embed it in the arg-passing structs.
  
  - Provide new routines geli_import_boot_data() and geli_export_boot_data()
that can be shared between gptboot, zfsboot, and loader instead of
pasting identical code into several different .c files.
  
  - Remove some checks for a NULL pointer that can never be true because the
pointer being tested was set using pointer math (kargs + 1) and that can
never result in NULL in this code.
  
  r341473:
  Fix args cross-threading between gptboot(8) and loader(8) with zfs support.
  
  When loader(8) is built with zfs support enabled, it assumes that any extarg
  data present is a zfs_boot_args struct, but if the first-stage loader was
  gptboot(8) the extarg data is actually a geli_boot_args struct.  Luckily,
  zfsboot(8) and gptzfsboot(8) have always passed KARGS_FLAGS_ZFS along with
  KARGS_FLAGS_EXTARG, so we can use KARGS_FLAGS_ZFS to decide whether the
  extarg data is a zfs_boot_args struct.
  
  To avoid similar problems in the future, gptboot(8) now passes a new
  KARGS_FLAGS_GELI to indicate that extarg data is geli_boot_args.  In
  loader(8), if the neither KARGS_FLAGS_ZFS nor KARGS_FLAGS_GELI is set but
  extarg data is present (which will be the case for gptboot compiled before
  this change), we now check for the known size of the geli_boot_args struct
  passed by the older versions of gptboot as a way of confirming what type of
  extarg data is present.
  
  In a semi-related tidying up, since loader's main() has already decided
  what type of extarg data is present and set the global 'zargs' var
  accordingly, don't repeat the check in extract_currdev, just check whether
  zargs is NULL or not.
  
  r341651:
  Don't reference zfs-specific variables if LOADER_ZFS_SUPPORT is undefined
  because the variables will be undefined too.

Modified:
  stable/12/stand/i386/common/bootargs.h
  stable/12/stand/i386/gptboot/gptboot.c
  stable/12/stand/i386/loader/main.c
  stable/12/stand/i386/zfsboot/zfsboot.c
  stable/12/stand/libsa/zfs/libzfs.h
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/stand/i386/common/bootargs.h
==
--- stable/12/stand/i386/common/bootargs.h  Sun Apr 21 20:55:33 2019
(r346500)
+++ stable/12/stand/i386/common/bootargs.h  Sun Apr 21 22:13:07 2019
(r346501)
@@ -18,10 +18,11 @@
 #ifndef _BOOT_I386_ARGS_H_
 #define_BOOT_I386_ARGS_H_
 
-#defineKARGS_FLAGS_CD  0x1
-#defineKARGS_FLAGS_PXE 0x2
-#defineKARGS_FLAGS_ZFS 0x4
-#defineKARGS_FLAGS_EXTARG  0x8 /* variably sized extended 
argument */
+#defineKARGS_FLAGS_CD  0x0001  /* .bootdev is a bios CD dev */
+#defineKARGS_FLAGS_PXE 0x0002  /* .pxeinfo is valid */
+#defineKARGS_FLAGS_ZFS 0x0004  /* .zfspool is valid, EXTARG is 
zfs_boot_args */
+#defineKARGS_FLAGS_EXTARG  0x0008  /* variably sized extended 
argument */
+#defineKARGS_FLAGS_GELI0x0010  /* EXTARG is geli_boot_args */
 
 #defineBOOTARGS_SIZE   24  /* sizeof(struct bootargs) */
 #defineBA_BOOTFLAGS8   /* offsetof(struct bootargs, bootflags) 
*/
@@ -84,11 +85,15 @@ struct bootargs
 
 #ifdef LOADER_GELI_SUPPORT
 #include 
+#include "geliboot.h"
 #endif
 
-struct geli_boot_args
+/*
+ * geli_boot_data is embedded in geli_boot_args (passed from gptboot to loader)
+ * and in zfs_boot_args (passed from zfsboot and gptzfsboot to loader).
+ */
+struct geli_boot_data
 {
-uint32_t   size;
 union {
 chargelipw[256];
 struct {
@@ -104,6 +109,49 @@ struct geli_boot_args
 #endif
 };
 };
+};
+
+#ifdef LOADER_GELI_SUPPORT
+
+static inline void
+export_geli_boot_data(struct geli_boot_data *gbdata)
+{
+
+   gbdata->notapw = '\0';
+   gbdata->keybuf_sentinel = KEYBUF_SENTINEL;
+   gbdata->keybuf = malloc(sizeof(struct k

svn commit: r346501 - in stable/12/stand: i386/common i386/gptboot i386/loader i386/zfsboot libsa/zfs

[Ian Lepore]

Author: ian
Date: Sun Apr 21 22:13:07 2019
New Revision: 346501
URL: https://svnweb.freebsd.org/changeset/base/346501

Log:
  MFC r341420, r341473, r341651
  
  r341420:
  Eliminate duplicated code and struct member definitions in the handoff
  of args data between gptboot/zfsboot and loader(8).
  
  Despite what seems like a lot of changes here, there are no actual
  changes in behavior, or in the data layout in the structures involved.
  This is just eliminating identical code pasted into multiple locations.
  
  In detail, the changes are...
  
  - Move struct zfs_boot_args definition from libsa/zfs/libzfs.h to
i386/common/bootargs.h because it is specific to x86 booting and the
handoff between zfsboot and loader, and has no relation to the zfs
library code in general.
  
  - The geli_boot_args and zfs_boot_args structs both contain an identical
set of member variables containing geli information.  Extract this out
to a new geli_boot_data struct, and embed it in the arg-passing structs.
  
  - Provide new routines geli_import_boot_data() and geli_export_boot_data()
that can be shared between gptboot, zfsboot, and loader instead of
pasting identical code into several different .c files.
  
  - Remove some checks for a NULL pointer that can never be true because the
pointer being tested was set using pointer math (kargs + 1) and that can
never result in NULL in this code.
  
  r341473:
  Fix args cross-threading between gptboot(8) and loader(8) with zfs support.
  
  When loader(8) is built with zfs support enabled, it assumes that any extarg
  data present is a zfs_boot_args struct, but if the first-stage loader was
  gptboot(8) the extarg data is actually a geli_boot_args struct.  Luckily,
  zfsboot(8) and gptzfsboot(8) have always passed KARGS_FLAGS_ZFS along with
  KARGS_FLAGS_EXTARG, so we can use KARGS_FLAGS_ZFS to decide whether the
  extarg data is a zfs_boot_args struct.
  
  To avoid similar problems in the future, gptboot(8) now passes a new
  KARGS_FLAGS_GELI to indicate that extarg data is geli_boot_args.  In
  loader(8), if the neither KARGS_FLAGS_ZFS nor KARGS_FLAGS_GELI is set but
  extarg data is present (which will be the case for gptboot compiled before
  this change), we now check for the known size of the geli_boot_args struct
  passed by the older versions of gptboot as a way of confirming what type of
  extarg data is present.
  
  In a semi-related tidying up, since loader's main() has already decided
  what type of extarg data is present and set the global 'zargs' var
  accordingly, don't repeat the check in extract_currdev, just check whether
  zargs is NULL or not.
  
  r341651:
  Don't reference zfs-specific variables if LOADER_ZFS_SUPPORT is undefined
  because the variables will be undefined too.

Modified:
  stable/12/stand/i386/common/bootargs.h
  stable/12/stand/i386/gptboot/gptboot.c
  stable/12/stand/i386/loader/main.c
  stable/12/stand/i386/zfsboot/zfsboot.c
  stable/12/stand/libsa/zfs/libzfs.h
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/stand/i386/common/bootargs.h
==
--- stable/12/stand/i386/common/bootargs.h  Sun Apr 21 20:55:33 2019
(r346500)
+++ stable/12/stand/i386/common/bootargs.h  Sun Apr 21 22:13:07 2019
(r346501)
@@ -18,10 +18,11 @@
 #ifndef _BOOT_I386_ARGS_H_
 #define_BOOT_I386_ARGS_H_
 
-#defineKARGS_FLAGS_CD  0x1
-#defineKARGS_FLAGS_PXE 0x2
-#defineKARGS_FLAGS_ZFS 0x4
-#defineKARGS_FLAGS_EXTARG  0x8 /* variably sized extended 
argument */
+#defineKARGS_FLAGS_CD  0x0001  /* .bootdev is a bios CD dev */
+#defineKARGS_FLAGS_PXE 0x0002  /* .pxeinfo is valid */
+#defineKARGS_FLAGS_ZFS 0x0004  /* .zfspool is valid, EXTARG is 
zfs_boot_args */
+#defineKARGS_FLAGS_EXTARG  0x0008  /* variably sized extended 
argument */
+#defineKARGS_FLAGS_GELI0x0010  /* EXTARG is geli_boot_args */
 
 #defineBOOTARGS_SIZE   24  /* sizeof(struct bootargs) */
 #defineBA_BOOTFLAGS8   /* offsetof(struct bootargs, bootflags) 
*/
@@ -84,11 +85,15 @@ struct bootargs
 
 #ifdef LOADER_GELI_SUPPORT
 #include 
+#include "geliboot.h"
 #endif
 
-struct geli_boot_args
+/*
+ * geli_boot_data is embedded in geli_boot_args (passed from gptboot to loader)
+ * and in zfs_boot_args (passed from zfsboot and gptzfsboot to loader).
+ */
+struct geli_boot_data
 {
-uint32_t   size;
 union {
 chargelipw[256];
 struct {
@@ -104,6 +109,49 @@ struct geli_boot_args
 #endif
 };
 };
+};
+
+#ifdef LOADER_GELI_SUPPORT
+
+static inline void
+export_geli_boot_data(struct geli_boot_data *gbdata)
+{
+
+   gbdata->notapw = '\0';
+   gbdata->keybuf_sentinel = KEYBUF_SENTINEL;
+   gbdata->keybuf = malloc(sizeof(struct k