From dc9d354ed22b4c6230c6bfe885e8a3d2519b1285 Mon Sep 17 00:00:00 2001 From: Aidan MacDonald Date: Thu, 22 Dec 2022 20:31:06 +0000 Subject: multiboot: Add v1 boot protocol v1 passes the drive and partition number of the boot volume instead of using the volume number. The volume number isn't reliable because the same filesystem might get a different volume number once the firmware is loaded, which will cause the firmware to use the wrong root volume and fail to locate the correct .rockbox directory. Using drive and partition numbers avoids this issue because drive numbering is fixed and determined by the target. Change-Id: I7e68b892d9424a1f686197a6122e139b438e5f7e --- apps/debug_menu.c | 11 ++++++++- firmware/common/bootdata.c | 15 +++++++++++++ firmware/common/multiboot.c | 15 ++++++++++++- firmware/export/bootdata.h | 6 +++-- firmware/include/dircache_redirect.h | 43 ++++++++++++++++++++++++++++++------ firmware/rolo.c | 21 +++++++++++++++++- 6 files changed, 99 insertions(+), 12 deletions(-) diff --git a/apps/debug_menu.c b/apps/debug_menu.c index 5e2451e41b..36927dd890 100644 --- a/apps/debug_menu.c +++ b/apps/debug_menu.c @@ -2546,7 +2546,16 @@ static bool dbg_boot_data(void) { simplelist_addline("Boot data valid"); simplelist_addline("Version: %d", (int)boot_data.version); - simplelist_addline("Boot volume: %d", (int)boot_data.boot_volume); + + if (boot_data.version == 0) + { + simplelist_addline("Boot volume: %d", (int)boot_data._boot_volume); + } + else if (boot_data.version == 1) + { + simplelist_addline("Boot drive: %d", (int)boot_data.boot_drive); + simplelist_addline("Boot partition: %d", (int)boot_data.boot_partition); + } } simplelist_addline("Bootdata RAW:"); diff --git a/firmware/common/bootdata.c b/firmware/common/bootdata.c index fa74c5fe91..224a48d0c1 100644 --- a/firmware/common/bootdata.c +++ b/firmware/common/bootdata.c @@ -42,6 +42,20 @@ static bool verify_boot_data_v0(void) return true; } +static bool verify_boot_data_v1(void) INIT_ATTR; +static bool verify_boot_data_v1(void) +{ + /* validate protocol version */ + if (boot_data.version != 1) + return false; + + /* validate length */ + if (boot_data.length != 4) + return false; + + return true; +} + struct verify_bd_entry { int version; @@ -50,6 +64,7 @@ struct verify_bd_entry static const struct verify_bd_entry verify_bd[] INITDATA_ATTR = { { 0, verify_boot_data_v0 }, + { 1, verify_boot_data_v1 }, }; void verify_boot_data(void) diff --git a/firmware/common/multiboot.c b/firmware/common/multiboot.c index c292aa1c30..8d6573d2dd 100644 --- a/firmware/common/multiboot.c +++ b/firmware/common/multiboot.c @@ -23,6 +23,7 @@ #include "crc32.h" #include "loader_strerror.h" #include "file.h" +#include "disk.h" #include #include @@ -30,10 +31,20 @@ static void write_bootdata_v0(struct boot_data_t *data, unsigned int boot_volume { memset(data->payload, data->length, 0); - data->boot_volume = boot_volume; + data->_boot_volume = boot_volume; data->version = 0; } +static void write_bootdata_v1(struct boot_data_t *data, unsigned int boot_volume) +{ + memset(data->payload, data->length, 0); + + data->_boot_volume = 0xff; + data->version = 1; + data->boot_drive = volume_drive(boot_volume); + data->boot_partition = volume_partition(boot_volume); +} + /* Write bootdata into location in FIRMWARE marked by magic header * Assumes buffer is already loaded with the firmware image * We just need to find the location and write data into the @@ -68,6 +79,8 @@ bool write_bootdata(unsigned char* buf, int len, unsigned int boot_volume) /* Write boot data according to the selected protocol */ if (proto_version == 0) write_bootdata_v0(data, boot_volume); + else if (proto_version == 1) + write_bootdata_v1(data, boot_volume); else break; diff --git a/firmware/export/bootdata.h b/firmware/export/bootdata.h index 319f442511..2a6f96d22e 100644 --- a/firmware/export/bootdata.h +++ b/firmware/export/bootdata.h @@ -37,7 +37,7 @@ #define BOOT_DATA_MAGIC0 ('r' | 'b' << 8 | 'm' << 16 | 'a' << 24) #define BOOT_DATA_MAGIC1 ('g' | 'i' << 8 | 'c' << 16 | '!' << 24) -#define BOOT_DATA_VERSION 0 +#define BOOT_DATA_VERSION 1 /* maximum size of payload */ #define BOOT_DATA_PAYLOAD_SIZE 4 @@ -59,8 +59,10 @@ struct boot_data_t { struct { - uint8_t boot_volume; + uint8_t _boot_volume; /* IGNORED */ uint8_t version; + uint8_t boot_drive; + uint8_t boot_partition; }; uint8_t payload[BOOT_DATA_PAYLOAD_SIZE]; }; diff --git a/firmware/include/dircache_redirect.h b/firmware/include/dircache_redirect.h index d4d72978c0..32e441c5b3 100644 --- a/firmware/include/dircache_redirect.h +++ b/firmware/include/dircache_redirect.h @@ -136,6 +136,41 @@ static inline void fileop_onsync_internal(struct filestr_base *stream) #endif } +#if defined(HAVE_MULTIBOOT) && !defined(SIMULATOR) && !defined(BOOTLOADER) +static inline bool multiboot_is_boot_volume(int volume) +{ + if (boot_data.version == 0) + { + /* + * Version 0 bootloaders just pass the volume number, but that's + * dynamically assigned and sometimes differs by the time we get + * into the firmware. So we can't rely on the volume passed by + * the bootloader. + */ +#if CONFIG_CPU == X1000 + /* The affected X1000 players only have one drive to begin with */ + return volume_drive(volume) == 0; +#else + /* FIXME: Anything else that can get here is a Sansa. */ + return volume_drive(volume) == boot_data.boot_volume || + volume == boot_data.boot_volume; +#endif + } + + if (boot_data.version == 1) + { + /* + * Since version 1 the bootloader passes drive and partition + * number which unambiguously identifies the boot volume. + */ + return volume_drive(volume) == boot_data.boot_drive && + volume_partition(volume) == boot_data.boot_partition; + } + + return false; +} +#endif + static inline void volume_onmount_internal(IF_MV_NONVOID(int volume)) { #if defined(HAVE_MULTIBOOT) && !defined(SIMULATOR) && !defined(BOOTLOADER) @@ -148,13 +183,7 @@ static inline void volume_onmount_internal(IF_MV_NONVOID(int volume)) /* we need to mount the drive before we can access it */ root_mount_path(path, 0); /* root could be different folder don't hide */ -/*BUGFIX bootloader is less selective about which drives it will mount -- revisit */ -#if defined(HAVE_MULTIDRIVE) && (NUM_VOLUMES_PER_DRIVE == 1) - if (volume_drive(volume) == boot_data.boot_volume - || volume == boot_data.boot_volume) -#else - if (volume == boot_data.boot_volume) /* boot volume contained in uint8_t payload */ -#endif + if (multiboot_is_boot_volume(IF_MV_VOL(volume))) { int rtlen = get_redirect_dir(rtpath, sizeof(rtpath), volume, "", ""); while (rtlen > 0 && rtpath[--rtlen] == PATH_SEPCH) diff --git a/firmware/rolo.c b/firmware/rolo.c index 1b37b6f771..f9b0cc9e61 100644 --- a/firmware/rolo.c +++ b/firmware/rolo.c @@ -253,7 +253,26 @@ int rolo_load(const char* filename) /* write the bootdata as if rolo were the bootloader * FIXME: this won't work for root redirect... */ if (!strcmp(filename, BOOTDIR "/" BOOTFILE) && boot_data_valid) - write_bootdata(filebuf, filebuf_size, boot_data.boot_volume); /* rb-loader.c */ + { + int volume = 0; + + if (boot_data.version == 0) + volume = boot_data._boot_volume; + else if (boot_data.version == 1) + { + for (int i = 0; i < NUM_VOLUMES; ++i) + { + if (volume_drive(i) == boot_data.boot_drive && + volume_partition(i) == boot_data.boot_partition) + { + volume = i; + break; + } + } + } + + write_bootdata(filebuf, filebuf_size, volume); + } #endif if (err <= 0) -- cgit v1.2.3