> Date: Thu, 3 Feb 2022 13:58:21 +0000
> From: Visa Hankala <[email protected]>
>
> On Wed, Feb 02, 2022 at 01:23:56PM -0700, Theo de Raadt wrote:
> > Miod Vallat <[email protected]> wrote:
> >
> > > > Index: usr.sbin/installboot/armv7_installboot.c
> > > > ===================================================================
> > > > RCS file: src/usr.sbin/installboot/armv7_installboot.c,v
> > > > retrieving revision 1.11
> > > > diff -u -p -r1.11 armv7_installboot.c
> > > > --- usr.sbin/installboot/armv7_installboot.c 20 Jul 2021 14:51:56
> > > > -0000 1.11
> > > > +++ usr.sbin/installboot/armv7_installboot.c 2 Feb 2022 14:11:08
> > > > -0000
> > > > @@ -55,6 +55,19 @@
> > > >
> > > > #include "installboot.h"
> > > >
> > > > +#if defined(__aarch64__)
> > > > +#define BOOTEFI_SRC "BOOTAA64.EFI"
> > > > +#define BOOTEFI_DST "bootaa64.efi"
> > > > +#elif defined(__arm__)
> > > > +#define BOOTEFI_SRC "BOOTARM.EFI"
> > > > +#define BOOTEFI_DST "bootarm.efi"
> > > > +#elif defined(__riscv)
> > > > +#define BOOTEFI_SRC "BOOTRISCV64.EFI"
> > > > +#define BOOTEFI_DST "bootriscv64.efi"
> > > > +#else
> > > > +#error "unhandled architecture"
> > > > +#endif
> > >
> > > Wouldn't these defines better set at the Makefile level, since there is
> > > already logic to pick different files depending upon the architecture?
I'd prefer to keep the names in the .c file, especially if the result
is that they're scatter though the Makefile instead of in a single
block.
> > Also, if these are written in lowercase to the filesystem, does that
> > create a problem?
>
> The installer and installboot have so far used lowercase filenames.
> However, the FAT filesystem should be about case-insensitive. OpenBSD
> and U-Boot show FAT filenames in the form that was used when the files
> were created, but can find names with non-matching case.
I think I used lower case to make the path look more consistent. But
it is purely cosmetic. EFI implementations will definitely recognize
the all-uppercase names as this is how they're listed in the UEFI
standard.
> The unification and Makefile-level setting of names could be done
> as follows:
>
> Index: distrib/special/installboot/Makefile
> ===================================================================
> RCS file: src/distrib/special/installboot/Makefile,v
> retrieving revision 1.16
> diff -u -p -r1.16 Makefile
> --- distrib/special/installboot/Makefile 3 Feb 2022 10:27:33 -0000
> 1.16
> +++ distrib/special/installboot/Makefile 3 Feb 2022 13:33:59 -0000
> @@ -15,7 +15,11 @@ CFLAGS += -DSOFTRAID
> SRCS += i386_installboot.c
> SRCS += i386_nlist.c
> SRCS += i386_softraid.c
> -.elif ${MACHINE} == "armv7" || ${MACHINE} == "arm64" || ${MACHINE} ==
> "riscv64"
> +.elif ${MACHINE} == "arm64"
> +CFLAGS += -DBOOTEFI=\"BOOTAA64.EFI\"
> +SRCS += efi_installboot.c
> +.elif ${MACHINE} == "armv7"
> +CFLAGS += -DBOOTEFI=\"BOOTARM.EFI\"
> SRCS += efi_installboot.c
> .elif ${MACHINE} == "hppa"
> CFLAGS += -DBOOTSTRAP
> @@ -31,6 +35,9 @@ SRCS += macppc_installboot.c
> SRCS += powerpc64_installboot.c
> .elif ${MACHINE} == "octeon"
> SRCS += octeon_installboot.c
> +.elif ${MACHINE} == "riscv64"
> +CFLAGS += -DBOOTEFI=\"BOOTRISCV64.EFI\"
> +SRCS += efi_installboot.c
> .elif ${MACHINE} == "sparc64"
> CFLAGS += -DSOFTRAID
> SRCS += sparc64_installboot.c
> Index: usr.sbin/installboot/Makefile
> ===================================================================
> RCS file: src/usr.sbin/installboot/Makefile,v
> retrieving revision 1.24
> diff -u -p -r1.24 Makefile
> --- usr.sbin/installboot/Makefile 3 Feb 2022 10:21:13 -0000 1.24
> +++ usr.sbin/installboot/Makefile 3 Feb 2022 13:33:59 -0000
> @@ -15,7 +15,11 @@ CFLAGS += -DSOFTRAID
> SRCS += i386_installboot.c
> SRCS += i386_nlist.c
> SRCS += i386_softraid.c
> -.elif ${MACHINE} == "armv7" || ${MACHINE} == "arm64" || ${MACHINE} ==
> "riscv64"
> +.elif ${MACHINE} == "arm64"
> +CFLAGS += -DBOOTEFI=\"BOOTAA64.EFI\"
> +SRCS += efi_installboot.c
> +.elif ${MACHINE} == "armv7"
> +CFLAGS += -DBOOTEFI=\"BOOTARM.EFI\"
> SRCS += efi_installboot.c
> .elif ${MACHINE} == "hppa"
> CFLAGS += -DBOOTSTRAP
> @@ -31,6 +35,9 @@ SRCS += macppc_installboot.c
> SRCS += powerpc64_installboot.c
> .elif ${MACHINE} == "octeon"
> SRCS += octeon_installboot.c
> +.elif ${MACHINE} == "riscv64"
> +CFLAGS += -DBOOTEFI=\"BOOTRISCV64.EFI\"
> +SRCS += efi_installboot.c
> .elif ${MACHINE} == "sparc64"
> CFLAGS += -DSOFTRAID
> SRCS += sparc64_installboot.c
> Index: usr.sbin/installboot/efi_installboot.c
> ===================================================================
> RCS file: src/usr.sbin/installboot/efi_installboot.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 efi_installboot.c
> --- usr.sbin/installboot/efi_installboot.c 3 Feb 2022 10:25:14 -0000
> 1.2
> +++ usr.sbin/installboot/efi_installboot.c 3 Feb 2022 13:33:59 -0000
> @@ -55,19 +55,6 @@
>
> #include "installboot.h"
>
> -#if defined(__aarch64__)
> -#define BOOTEFI_SRC "BOOTAA64.EFI"
> -#define BOOTEFI_DST "bootaa64.efi"
> -#elif defined(__arm__)
> -#define BOOTEFI_SRC "BOOTARM.EFI"
> -#define BOOTEFI_DST "bootarm.efi"
> -#elif defined(__riscv)
> -#define BOOTEFI_SRC "BOOTRISCV64.EFI"
> -#define BOOTEFI_DST "bootriscv64.efi"
> -#else
> -#error "unhandled architecture"
> -#endif
> -
> static int create_filesystem(struct disklabel *, char);
> static void write_filesystem(struct disklabel *, char);
> static int findgptefisys(int, struct disklabel *);
> @@ -267,12 +254,12 @@ write_filesystem(struct disklabel *dl, c
>
> /* Copy EFI bootblocks to /efi/boot/. */
> pathlen = strlen(dst);
> - if (strlcat(dst, "/" BOOTEFI_DST, sizeof(dst)) >= sizeof(dst)) {
> + if (strlcat(dst, "/" BOOTEFI, sizeof(dst)) >= sizeof(dst)) {
> rslt = -1;
> - warn("unable to build /%s path", BOOTEFI_DST);
> + warn("unable to build /%s path", BOOTEFI);
> goto umount;
> }
> - src = fileprefix(root, "/usr/mdec/" BOOTEFI_SRC);
> + src = fileprefix(root, "/usr/mdec/" BOOTEFI);
> if (src == NULL) {
> rslt = -1;
> goto umount;
> @@ -298,7 +285,7 @@ write_filesystem(struct disklabel *dl, c
> fprintf(stderr, "%s %s\n",
> (nowrite ? "would write" : "writing"), dst);
> if (!nowrite) {
> - rslt = fileprintf(dst, "%s\n", BOOTEFI_DST);
> + rslt = fileprintf(dst, "%s\n", BOOTEFI);
> if (rslt == -1)
> goto umount;
> }
>
