Only relocate images on device (#736)

* Only relocate images on device

* Improve errors

* Apply suggestions from code review

Co-authored-by: Florian Loitsch <[email protected]>

Co-authored-by: Florian Loitsch <[email protected]>

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)

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;

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)                 \
 

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<const word*>(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<const word*>(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<const word*>(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) {

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

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:

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
 

system/flash/registry.toit

@@ -24,7 +24,7 @@ class FlashRegistry:
   static SCAN_HOLE_ ::= 0
   static SCAN_ALLOCATION_ ::= 1
 
-  allocations_/Map ::= {:}  // Map<uuid.Uuid, FlashAllocation>
+  allocations_/Map ::= {:}  // Map<int, FlashAllocation>
   holes_/List? := null      // List<FlashHole_>
 
   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 := ?

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,15 +118,15 @@ class SourceRelocatedOutput extends RelocatedOutput:
     out.write "\n"
 
 print_usage:
-  print_ "Usage: snapshot_to_image [--$BINARY_FLAG] [--$RELOCATE_OPTION=0x...] [-m32|-m64] <snapshot> <output>"
+  print_ "Usage: snapshot_to_image [--$BINARY_FLAG] [--$OFFSET_OPTION=0x...] [-m32|-m64] <snapshot> <output>"
 
 main args:
   parser := arguments.ArgumentParser
   parser.add_flag M32_FLAG --short="m32"
   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: