Re: efiboot/arm64: fix "mach dtb" return code to avoid bogus boot
Am Sat, Mar 27, 2021 at 12:09:25AM +0100 schrieb Klemens Nanni:
> On Fri, Mar 26, 2021 at 11:28:37PM +0100, Patrick Wildt wrote:
> > > fdt = (void *)addr;
> > > - return (0);
> > > + return (1);
> >
> > Wait, you've been saying that return code 1 makes it boot. So now you
> > changed it so that "mach dtb" kicks of booting the kernel? That does
> > not seem tight to me. This should stay 0, right?
> Absolutely, my bad.
>
> I've tested all scenarios with and without this fixed diff below to
> double check.
>
> OK?
>
Yes.
>
> Index: efiboot.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/stand/efiboot/efiboot.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 efiboot.c
> --- efiboot.c 9 Mar 2021 21:11:24 - 1.31
> +++ efiboot.c 26 Mar 2021 23:05:46 -
> @@ -980,24 +980,26 @@ Xdtb_efi(void)
>
> #define O_RDONLY 0
>
> - if (cmd.argc != 2)
> - return (1);
> + if (cmd.argc != 2) {
> + printf("dtb file\n");
> + return (0);
> + }
>
> snprintf(path, sizeof(path), "%s:%s", cmd.bootdev, cmd.argv[1]);
>
> fd = open(path, O_RDONLY);
> if (fd < 0 || fstat(fd, ) == -1) {
> printf("cannot open %s\n", path);
> - return (1);
> + return (0);
> }
> if (efi_memprobe_find(EFI_SIZE_TO_PAGES(sb.st_size),
> 0x1000, ) != EFI_SUCCESS) {
> printf("cannot allocate memory for %s\n", path);
> - return (1);
> + return (0);
> }
> if (read(fd, (void *)addr, sb.st_size) != sb.st_size) {
> printf("cannot read from %s\n", path);
> - return (1);
> + return (0);
> }
>
> fdt = (void *)addr;
Re: efiboot/arm64: fix "mach dtb" return code to avoid bogus boot
On Fri, Mar 26, 2021 at 11:28:37PM +0100, Patrick Wildt wrote:
> > fdt = (void *)addr;
> > - return (0);
> > + return (1);
>
> Wait, you've been saying that return code 1 makes it boot. So now you
> changed it so that "mach dtb" kicks of booting the kernel? That does
> not seem tight to me. This should stay 0, right?
Absolutely, my bad.
I've tested all scenarios with and without this fixed diff below to
double check.
OK?
Index: efiboot.c
===
RCS file: /cvs/src/sys/arch/arm64/stand/efiboot/efiboot.c,v
retrieving revision 1.31
diff -u -p -r1.31 efiboot.c
--- efiboot.c 9 Mar 2021 21:11:24 - 1.31
+++ efiboot.c 26 Mar 2021 23:05:46 -
@@ -980,24 +980,26 @@ Xdtb_efi(void)
#define O_RDONLY 0
- if (cmd.argc != 2)
- return (1);
+ if (cmd.argc != 2) {
+ printf("dtb file\n");
+ return (0);
+ }
snprintf(path, sizeof(path), "%s:%s", cmd.bootdev, cmd.argv[1]);
fd = open(path, O_RDONLY);
if (fd < 0 || fstat(fd, ) == -1) {
printf("cannot open %s\n", path);
- return (1);
+ return (0);
}
if (efi_memprobe_find(EFI_SIZE_TO_PAGES(sb.st_size),
0x1000, ) != EFI_SUCCESS) {
printf("cannot allocate memory for %s\n", path);
- return (1);
+ return (0);
}
if (read(fd, (void *)addr, sb.st_size) != sb.st_size) {
printf("cannot read from %s\n", path);
- return (1);
+ return (0);
}
fdt = (void *)addr;
Re: efiboot/arm64: fix "mach dtb" return code to avoid bogus boot
Am Wed, Mar 24, 2021 at 07:20:29PM +0100 schrieb Klemens Nanni:
> Bootloader command functions must return zero in case of failure,
> returning 1 tells the bootloader to boot the file.
>
> arm64's `machine dtb' command has it the wrong way so using it triggers
> a boot that doesn't make any sense:
>
> >> OpenBSD/arm64 BOOTAA64 1.4
> boot> mach dtb
> booting sd0a:/etc/boot.conf: open sd0a:/etc/boot.conf: No such file or
> directory
> failed(2). will try /bsd
> boot> mach dtb /foo
> cannot open sd0a:/foo
> NOTE: random seed is being reused.
> booting sd0a:/etc/boot.conf: open sd0a:/etc/boot.conf: No such file or
> directory
> failed(2). will try /bsd
>
> With this diff:
>
> >> OpenBSD/arm64 BOOTAA64 1.4
> boot> mach dtb
> dtb file
> boot> mach dtb /foo
> cannot open sd0a:/foo
>
> While here, tell users how to use that command (like other commands
> such as `hexdump' do).
>
> Feedback? OK?
>
> Index: arch/arm64/stand/efiboot/efiboot.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/stand/efiboot/efiboot.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 efiboot.c
> --- arch/arm64/stand/efiboot/efiboot.c9 Mar 2021 21:11:24 -
> 1.31
> +++ arch/arm64/stand/efiboot/efiboot.c24 Mar 2021 17:59:52 -
> @@ -980,28 +980,30 @@ Xdtb_efi(void)
>
> #define O_RDONLY 0
>
> - if (cmd.argc != 2)
> - return (1);
> + if (cmd.argc != 2) {
> + printf("dtb file\n");
> + return (0);
> + }
>
> snprintf(path, sizeof(path), "%s:%s", cmd.bootdev, cmd.argv[1]);
>
> fd = open(path, O_RDONLY);
> if (fd < 0 || fstat(fd, ) == -1) {
> printf("cannot open %s\n", path);
> - return (1);
> + return (0);
> }
> if (efi_memprobe_find(EFI_SIZE_TO_PAGES(sb.st_size),
> 0x1000, ) != EFI_SUCCESS) {
> printf("cannot allocate memory for %s\n", path);
> - return (1);
> + return (0);
> }
> if (read(fd, (void *)addr, sb.st_size) != sb.st_size) {
> printf("cannot read from %s\n", path);
> - return (1);
> + return (0);
> }
>
> fdt = (void *)addr;
> - return (0);
> + return (1);
Wait, you've been saying that return code 1 makes it boot. So now you
changed it so that "mach dtb" kicks of booting the kernel? That does
not seem tight to me. This should stay 0, right?
> }
>
> int
>
