From e2fbbad90519515088c812cb5a8b00351f830bc1 Mon Sep 17 00:00:00 2001 From: Kasper Lund Date: Wed, 18 May 2022 06:11:21 +0200 Subject: [PATCH] Only relocate images on device (#736) * Only relocate images on device * Improve errors * Apply suggestions from code review Co-authored-by: Florian Loitsch Co-authored-by: Florian Loitsch --- Makefile | 2 +- src/flash_allocation.h | 7 ++-- src/primitive.h | 1 + src/resources/image.cc | 36 ++++++++++++++------ system/containers.toit | 22 ++++++++++-- system/flash/allocation.toit | 3 +- system/flash/image_writer.toit | 22 ++++++++++++ system/flash/registry.toit | 16 ++++----- tools/snapshot_to_image.toit | 62 ++++++++++++++-------------------- 9 files changed, 109 insertions(+), 62 deletions(-) diff --git a/Makefile b/Makefile index 2b4fb8c69..fe27f2ffa 100644 --- a/Makefile +++ b/Makefile @@ -208,7 +208,7 @@ build/$(ESP32_CHIP)/program.snapshot: $(ESP32_ENTRY) tools $(TOITC_BIN) -w $@ $< build/$(ESP32_CHIP)/programs.bin: build/$(ESP32_CHIP)/program.snapshot tools - $(TOITVM_BIN) tools/snapshot_to_image.toit --unique_id=$(ESP32_SYSTEM_ID) -m32 --binary --relocate=0x3f430000 $< $@ + $(TOITVM_BIN) tools/snapshot_to_image.toit --unique_id=$(ESP32_SYSTEM_ID) -m32 --binary --offset=0x0 $< $@ build/$(ESP32_CHIP)/CMakeCache.txt: check-esp32-env mkdir -p build/$(ESP32_CHIP) diff --git a/src/flash_allocation.h b/src/flash_allocation.h index 6b4fdae73..423b24ce2 100644 --- a/src/flash_allocation.h +++ b/src/flash_allocation.h @@ -21,10 +21,9 @@ namespace toit { -// Keep in sync with flash_allocation.toit. -static const uint8 QUEUE_TYPE = 1; -static const uint8 PROGRAM_TYPE = 2; -static const uint8 KEY_VALUE_STORE_TYPE = 3; +// Keep in sync with system/flash/allocation.toit. +static const uint8 PROGRAM_TYPE = 0; +static const uint8 PROGRAM_UNRELOCATED_TYPE = 1; static const int FLASH_PAGE_SIZE = 4 * KB; static const int FLASH_SEGMENT_SIZE = 16; diff --git a/src/primitive.h b/src/primitive.h index aeb22f686..252a50047 100644 --- a/src/primitive.h +++ b/src/primitive.h @@ -453,6 +453,7 @@ namespace toit { #define MODULE_IMAGE(PRIMITIVE) \ PRIMITIVE(writer_create, 2) \ PRIMITIVE(writer_write, 4) \ + PRIMITIVE(writer_write_all, 3) \ PRIMITIVE(writer_commit, 2) \ PRIMITIVE(writer_close, 1) \ diff --git a/src/resources/image.cc b/src/resources/image.cc index 63f222ab8..71443adca 100644 --- a/src/resources/image.cc +++ b/src/resources/image.cc @@ -41,14 +41,9 @@ PRIMITIVE(writer_create) { return result; } -PRIMITIVE(writer_write) { - ARGS(ImageOutputStream, output, Blob, content_bytes, int, from, int, to); - if (to < from || from < 0) INVALID_ARGUMENT; - if (to > content_bytes.length()) OUT_OF_BOUNDS; - - const word* data = reinterpret_cast(content_bytes.address() + from); - int length = (to - from) / WORD_SIZE; - int output_byte_size = (length - 1) * WORD_SIZE; // The first word is relocation bits, not part of the output. +static Object* write_image_chunk(Process* process, ImageOutputStream* output, const word* data, int length) { + // The first word is relocation bits, not part of the output. + int output_byte_size = (length - 1) * WORD_SIZE; word buffer[WORD_BIT_SIZE]; bool first = output->empty(); @@ -66,9 +61,30 @@ PRIMITIVE(writer_write) { } else { success = FlashRegistry::write_chunk(buffer, offset, output_byte_size); } + if (!success) HARDWARE_ERROR; + return null; +} - if (success) return process->program()->null_object(); - HARDWARE_ERROR; +PRIMITIVE(writer_write) { + ARGS(ImageOutputStream, output, Blob, content_bytes, int, from, int, to); + if (to < from || from < 0) INVALID_ARGUMENT; + if (to > content_bytes.length()) OUT_OF_BOUNDS; + const word* data = reinterpret_cast(content_bytes.address() + from); + int length = (to - from) / WORD_SIZE; + Object* error = write_image_chunk(process, output, data, length); + return error ? error : process->program()->null_object(); +} + +PRIMITIVE(writer_write_all) { + ARGS(ImageOutputStream, output, int, from, int, to); + while (from < to) { + const word* data = unvoid_cast(FlashRegistry::memory(from, to - from)); + int length = Utils::min((to - from) / WORD_SIZE, WORD_BIT_SIZE + 1); + Object* error = write_image_chunk(process, output, data, length); + if (error) return error; + from += length * WORD_SIZE; + } + return process->program()->null_object(); } PRIMITIVE(writer_commit) { diff --git a/system/containers.toit b/system/containers.toit index c70e83ce5..d6aae7102 100644 --- a/system/containers.toit +++ b/system/containers.toit @@ -193,9 +193,27 @@ class ContainerManager extends ContainerServiceDefinition implements SystemMessa set_system_message_handler_ SYSTEM_TERMINATED_ this set_system_message_handler_ SYSTEM_SPAWNED_ this set_system_message_handler_ SYSTEM_MIRROR_MESSAGE_ this + + unrelocated/List? := null image_registry.do: | allocation/FlashAllocation | - if allocation.type != FLASH_ALLOCATION_PROGRAM_TYPE: continue.do - add_flash_image allocation + if allocation.type == FLASH_ALLOCATION_PROGRAM_TYPE: + add_flash_image allocation + else if allocation.type == FLASH_ALLOCATION_PROGRAM_UNRELOCATED_TYPE: + if unrelocated: unrelocated.add allocation + else: unrelocated = [allocation] + + // Run through the unrelocated programs and relocate them unless we + // already did that successfully before in which case they will have + // shown up as relocated programs (added to the images map). + if unrelocated: + unrelocated.do: | allocation/FlashAllocation | + if not images_.contains allocation.id: + add_flash_image (relocate allocation image_registry) + // We always free the unrelocated programs by erasing them from flash + // if the relocation attempt is successful (doesn't throw). Maybe it + // would make sense to make sure that a rescan finds the relocated + // image before doing this? + image_registry.free allocation images -> List: return images_.values diff --git a/system/flash/allocation.toit b/system/flash/allocation.toit index 2ab8e2406..149b80d4e 100644 --- a/system/flash/allocation.toit +++ b/system/flash/allocation.toit @@ -18,7 +18,8 @@ import uuid import .region import .registry -FLASH_ALLOCATION_PROGRAM_TYPE ::= 2 +FLASH_ALLOCATION_PROGRAM_TYPE ::= 0 +FLASH_ALLOCATION_PROGRAM_UNRELOCATED_TYPE ::= 1 FLASH_ALLOCATION_HEADER_SIZE ::= 48 class FlashAllocation implements FlashRegion: diff --git a/system/flash/image_writer.toit b/system/flash/image_writer.toit index 3408c433c..a0034a4b0 100644 --- a/system/flash/image_writer.toit +++ b/system/flash/image_writer.toit @@ -13,15 +13,34 @@ // The license can be found in the file `LICENSE` in the top level // directory of this repository. +import binary import uuid import system.services show ServiceResource ServiceDefinition import .allocation +import .registry import .reservation IMAGE_WORD_SIZE ::= BYTES_PER_WORD IMAGE_CHUNK_SIZE ::= (BITS_PER_WORD + 1) * IMAGE_WORD_SIZE +relocate allocation/FlashAllocation registry/FlashRegistry -> FlashAllocation: + assert: allocation.type == FLASH_ALLOCATION_PROGRAM_UNRELOCATED_TYPE + size := binary.LITTLE_ENDIAN.uint32 allocation.metadata 0 + relocated_size ::= size - (size / IMAGE_CHUNK_SIZE) * IMAGE_WORD_SIZE + reservation ::= registry.reserve relocated_size + if not reservation: throw "No space left in flash" + image ::= image_writer_create_ reservation.offset reservation.size + try: + from ::= allocation.offset + FLASH_ALLOCATION_HEADER_SIZE + to ::= from + size + image_writer_write_all_ image from to + image_writer_commit_ image allocation.id.to_byte_array + return FlashAllocation reservation.offset + finally: + reservation.close + if image: image_writer_close_ image + class ContainerImageWriter extends ServiceResource: reservation_/FlashReservation? := ? image_/ByteArray ::= ? @@ -70,6 +89,9 @@ image_writer_create_ offset size: image_writer_write_ image part/ByteArray from/int to/int: #primitive.image.writer_write +image_writer_write_all_ image from/int to/int: + #primitive.image.writer_write_all + image_writer_commit_ image id/ByteArray: #primitive.image.writer_commit diff --git a/system/flash/registry.toit b/system/flash/registry.toit index 4336ebef1..e5c211e82 100644 --- a/system/flash/registry.toit +++ b/system/flash/registry.toit @@ -24,7 +24,7 @@ class FlashRegistry: static SCAN_HOLE_ ::= 0 static SCAN_ALLOCATION_ ::= 1 - allocations_/Map ::= {:} // Map + allocations_/Map ::= {:} // Map holes_/List? := null // List constructor.scan: @@ -56,7 +56,7 @@ class FlashRegistry: // Frees a previously allocated region in the flash. free allocation/FlashAllocation -> none: - allocations_.remove allocation.id + allocations_.remove allocation.offset // Erasing the first page removes the full header. This is enough // to fully invalidate the allocation. flash_registry_erase_ allocation.offset FLASH_REGISTRY_PAGE_SIZE @@ -81,7 +81,7 @@ class FlashRegistry: size := (info >> 10) * FLASH_REGISTRY_PAGE_SIZE type := (info >> 2) & 0xFF allocation/FlashAllocation := FlashAllocation offset size type - found[allocation.id] = allocation + found[allocation.offset] = allocation // Update the allocations map, keeping existing allocation objects. update_allocations_ found // Return the coalesced list of holes. @@ -107,16 +107,16 @@ class FlashRegistry: // Use an extra list for the allocations to be freed because we cannot update // the allocations map while iterating over it. freed := [] - allocations_.do: | id/uuid.Uuid allocation/FlashAllocation | - if found.contains id: - found.remove id + allocations_.do: | offset/int allocation/FlashAllocation | + if found.contains offset: + found.remove offset else: freed.add allocation // Remove any old freed allocations. freed.do: free it // Add the new allocations found. - found.do: | id/uuid.Uuid allocation/FlashAllocation | - allocations_[id] = allocation + found.do: | offset/int allocation/FlashAllocation | + allocations_[offset] = allocation class FlashHole_: offset/int := ? diff --git a/tools/snapshot_to_image.toit b/tools/snapshot_to_image.toit index 47d2d84cc..fdb866c0a 100644 --- a/tools/snapshot_to_image.toit +++ b/tools/snapshot_to_image.toit @@ -31,7 +31,7 @@ import services.arguments BINARY_FLAG ::= "binary" M32_FLAG ::= "machine-32-bit" M64_FLAG ::= "machine-64-bit" -RELOCATE_OPTION ::= "relocate" +OFFSET_OPTION ::= "offset" UNIQUE_ID_OPTION ::= "unique_id" abstract class RelocatedOutput: @@ -60,39 +60,29 @@ abstract class RelocatedOutput: (mask & 1) != 0 mask = mask >> 1 -class BinaryRelocatedOutput extends RelocatedOutput: +class BinaryRelocatableOutput: static HEADER_SIZE ::= 48 - size/int ::= ? - relocation_base/int ::= ? + out ::= ? + relocatable/ByteArray ::= ? + offset/int ::= ? image_uuid/uuid.Uuid ::= ? - buffer_/ByteArray := ByteArray 4 - skip_header_bytes_ := HEADER_SIZE - constructor out .size .relocation_base .image_uuid: - super out + constructor .out .relocatable .offset .image_uuid: - write_start -> none: + write -> none: write_uint32 0xDEADFACE // Marker. - write_uint32 0 // Offset in partition. + write_uint32 offset // Offset in partition. write_uuid (uuid.uuid5 "program" "$Time.now") // Program id. - out.write #[0xFF, 0xFF, 0xFF, 0xFF, 0xFF] // Metadata. + metadata := ByteArray 5: 0xFF + RelocatedOutput.ENDIAN.put_uint32 metadata 0 relocatable.size + out.write metadata // Metadata. + size := relocatable.size + HEADER_SIZE write_uint16 (size + 4095) / 4096 // Pages in flash. - out.write #[0x02] // Type = program. + out.write #[0x01] // Type = program unrelocated. write_uuid image_uuid // Image uuid. - - write_word word/int is_relocatable/bool -> none: - // Skip the words that are part of the header. They are all written - // using $write_start. - if skip_header_bytes_ > 0: - skip_header_bytes_ -= 4 - return - if is_relocatable: word += relocation_base - write_uint32 word - - write_end -> none: - // Nothing to add here. + out.write relocatable write_uint16 halfword/int: RelocatedOutput.ENDIAN.put_uint16 buffer_ 0 halfword @@ -128,7 +118,7 @@ class SourceRelocatedOutput extends RelocatedOutput: out.write "\n" print_usage: - print_ "Usage: snapshot_to_image [--$BINARY_FLAG] [--$RELOCATE_OPTION=0x...] [-m32|-m64] " + print_ "Usage: snapshot_to_image [--$BINARY_FLAG] [--$OFFSET_OPTION=0x...] [-m32|-m64] " main args: parser := arguments.ArgumentParser @@ -136,7 +126,7 @@ main args: parser.add_flag M64_FLAG --short="m64" parser.add_flag BINARY_FLAG - parser.add_option RELOCATE_OPTION + parser.add_option OFFSET_OPTION parser.add_option UNIQUE_ID_OPTION --default="00000000-0000-0000-0000-000000000000" parsed := parser.parse args @@ -154,13 +144,13 @@ main args: else: default_word_size = 4 // Use 32-bit non-binary output. - relocation_base/int? := null - relocate_option := parsed[RELOCATE_OPTION] - if relocate_option: - if not (relocate_option.starts_with "0x"): + offset/int? := null + offset_option := parsed[OFFSET_OPTION] + if offset_option: + if not (offset_option.starts_with "0x"): print_usage return - relocation_base = int.parse relocate_option[2..] --radix=16 + offset = int.parse offset_option[2..] --radix=16 word_size := null if parsed[M32_FLAG]: @@ -177,8 +167,8 @@ main args: print_ "Error: Cannot generate 64-bit non-binary output" return - if not binary_output and relocation_base: - print_ "Error: Relocation only works for 32-bit binary output" + if not binary_output and offset: + print_ "Error: Offsets only work for 32-bit binary output" return out := file.Stream.for_write output_path @@ -187,10 +177,10 @@ main args: image := build_image program word_size relocatable := image.build_relocatable if binary_output: - if relocation_base: + if offset: image_uuid := uuid.parse parsed[UNIQUE_ID_OPTION] - output := BinaryRelocatedOutput out relocatable.size relocation_base image_uuid - output.write word_size relocatable + output := BinaryRelocatableOutput out relocatable offset image_uuid + output.write else: out.write relocatable else: