flash_storage: refactor + unit tests + artiq_coreconfig.py CLI + doc

artiq/coredevice/comm_generic.py

@@ -21,6 +21,11 @@ class _H2DMsgType(Enum):
 
     RPC_REPLY = 6
 
+    FLASH_READ_REQUEST = 7
+    FLASH_WRITE_REQUEST = 8
+    FLASH_ERASE_REQUEST = 9
+    FLASH_REMOVE_REQUEST = 10
+
 
 class _D2HMsgType(Enum):
     LOG_REPLY = 1
@@ -37,6 +42,11 @@ class _D2HMsgType(Enum):
 
     RPC_REQUEST = 10
 
+    FLASH_READ_REPLY = 11
+    FLASH_WRITE_REPLY = 12
+    FLASH_OK_REPLY = 13
+    FLASH_ERROR_REPLY = 14
+
 
 class UnsupportedDevice(Exception):
     pass
@@ -122,6 +132,44 @@ class CommGeneric:
         self.write(bytes(kname, "ascii"))
         logger.debug("running kernel: %s", kname)
 
+    def flash_storage_read(self, key):
+        self._write_header(9+len(key), _H2DMsgType.FLASH_READ_REQUEST)
+        self.write(key)
+        length, ty = self._read_header()
+        if ty != _D2HMsgType.FLASH_READ_REPLY:
+            raise IOError("Incorrect reply from device: {}".format(ty))
+        value = self.read(length - 9)
+        return value
+
+    def flash_storage_write(self, key, value):
+        self._write_header(9+len(key)+1+len(value),
+                           _H2DMsgType.FLASH_WRITE_REQUEST)
+        self.write(key)
+        self.write(b"\x00")
+        self.write(value)
+        _, ty = self._read_header()
+        if ty != _D2HMsgType.FLASH_WRITE_REPLY:
+            if ty == _D2HMsgType.FLASH_ERROR_REPLY:
+                raise IOError("Invalid key: not a null-terminated string")
+            else:
+                raise IOError("Incorrect reply from device: {}".format(ty))
+        ret = self.read(1)
+        if ret != b"\x01":
+            raise IOError("Flash storage is full")
+
+    def flash_storage_erase(self):
+        self._write_header(9, _H2DMsgType.FLASH_ERASE_REQUEST)
+        _, ty = self._read_header()
+        if ty != _D2HMsgType.FLASH_OK_REPLY:
+            raise IOError("Incorrect reply from device: {}".format(ty))
+
+    def flash_storage_remove(self, key):
+        self._write_header(9+len(key), _H2DMsgType.FLASH_REMOVE_REQUEST)
+        self.write(key)
+        _, ty = self._read_header()
+        if ty != _D2HMsgType.FLASH_OK_REPLY:
+            raise IOError("Incorrect reply from device: {}".format(ty))
+
     def _receive_rpc_value(self, type_tag):
         if type_tag == "n":
             return None

artiq/frontend/artiq_coreconfig.py

@@ -0,0 +1,61 @@
+#!/usr/bin/env python3
+
+import argparse
+
+from artiq.master.worker_db import create_device
+from artiq.protocols.file_db import FlatFileDB
+
+
+def to_bytes(string):
+    return bytes(string, encoding="ascii")
+
+
+def get_argparser():
+    parser = argparse.ArgumentParser(description="ARTIQ core device config "
+                                                 "remote access")
+    parser.add_argument("-r", "--read", type=to_bytes,
+                        help="read key from core device config")
+    parser.add_argument("-w", "--write", nargs=2, action="append", default=[],
+                        metavar=("KEY", "STRING"), type=to_bytes,
+                        help="write key-value records to core device config")
+    parser.add_argument("-f", "--write-file", nargs=2, action="append",
+                        type=to_bytes, default=[], metavar=("KEY", "FILENAME"),
+                        help="write the content of a file into core device "
+                             "config")
+    parser.add_argument("-e", "--erase", action="store_true",
+                        help="erase core device config")
+    parser.add_argument("-d", "--delete", action="append", default=[],
+                        type=to_bytes,
+                        help="delete key from core device config")
+    parser.add_argument("--ddb", default="ddb.pyon",
+                        help="device database file")
+    return parser
+
+
+def main():
+    args = get_argparser().parse_args()
+    ddb = FlatFileDB(args.ddb)
+    comm = create_device(ddb.request("comm"), None)
+
+    if args.read:
+        value = comm.flash_storage_read(args.read)
+        if not value:
+            print("Key {} does not exist".format(args.read))
+        else:
+            print(value)
+    elif args.erase:
+            comm.flash_storage_erase()
+    elif args.delete:
+        for key in args.delete:
+            comm.flash_storage_remove(key)
+    else:
+        if args.write:
+            for key, value in args.write:
+                comm.flash_storage_write(key, value)
+        if args.write_file:
+            for key, filename in args.write_file:
+                with open(filename, "rb") as fi:
+                    comm.flash_storage_write(key, fi.read())
+
+if __name__ == "__main__":
+    main()

artiq/frontend/artiq_flash.sh

@@ -7,7 +7,7 @@ ARTIQ_PREFIX=$(python3 -c "import artiq; print(artiq.__path__[0])")
 # Default is kc705
 BOARD=kc705
 
-while getopts "bBrht:d:" opt
+while getopts "bBrht:d:f:" opt
 do
 	case $opt in
 		b)
@@ -19,6 +19,15 @@ do
 		r)
 			FLASH_RUNTIME=1
 			;;
+		f)
+			if [ -f $OPTARG ]
+			then
+				FILENAME=$OPTARG
+			else
+				echo "You specified a non-existing file to flash: $OPTARG"
+				exit 1
+			fi
+			;;
 		t)
 			if [ "$OPTARG" == "kc705" ]
 			then
@@ -52,6 +61,7 @@ do
 			echo "-r  Flash ARTIQ runtime"
 			echo "-h  Show this help message"
 			echo "-t  Target (kc705, pipistrello, default is: kc705)"
+			echo "-f  Flash storage image generated with artiq_mkfs"
 			echo "-d  Directory containing the binaries to be flashed"
 			exit 1
 			;;
@@ -95,6 +105,7 @@ then
 	PROXY=bscan_spi_kc705.bit
 	BIOS_ADDR=0xaf0000
 	RUNTIME_ADDR=0xb00000
+	FS_ADDR=0xb40000
 	if [ -z "$BIN_PREFIX" ]; then BIN_PREFIX=$ARTIQ_PREFIX/binaries/kc705; fi
 	search_for_proxy $PROXY
 elif [ "$BOARD" == "pipistrello" ]
@@ -105,12 +116,13 @@ then
 	PROXY=bscan_spi_lx45_csg324.bit
 	BIOS_ADDR=0x170000
 	RUNTIME_ADDR=0x180000
+	FS_ADDR=0x1c0000
 	if [ -z "$BIN_PREFIX" ]; then BIN_PREFIX=$ARTIQ_PREFIX/binaries/pipistrello; fi
 	search_for_proxy $PROXY
 fi
 
 # Check if neither of -b|-B|-r have been used
-if [ -z "$FLASH_RUNTIME" -a -z "$FLASH_BIOS" -a -z "$FLASH_BITSTREAM" ]
+if [ -z "$FLASH_RUNTIME" -a -z "$FLASH_BIOS" -a -z "$FLASH_BITSTREAM" -a -z "$FILENAME" ]
 then
 	FLASH_RUNTIME=1
 	FLASH_BIOS=1
@@ -132,6 +144,12 @@ then
 fi
 set -e
 
+if [ ! -z "$FILENAME" ]
+then
+	echo "Flashing file $FILENAME at address $FS_ADDR"
+	xc3sprog -v -c $CABLE -I$PROXY_PATH/$PROXY $FILENAME:w:$FS_ADDR:BIN
+fi
+
 if [ "${FLASH_BITSTREAM}" == "1" ]
 then
 	echo "Flashing FPGA bitstream..."

artiq/frontend/artiq_mkfs.py

@@ -20,16 +20,13 @@ def get_argparser():
 
 
 def write_record(f, key, value):
+    key_size = len(key) + 1
+    value_size = len(value)
+    record_size = key_size + value_size + 4
+    f.write(struct.pack(">l", record_size))
     f.write(key.encode())
     f.write(b"\x00")
-    key_size = len(key) + 1
-    if key_size % 4:
-        f.write(bytes(4 - (key_size % 4)))
-    f.write(struct.pack(">l", len(value)))
     f.write(value)
-    value_size = len(value)
-    if value_size % 4:
-        f.write(bytes(4 - (value_size % 4)))
 
 
 def write_end_marker(f):

artiq/master/worker_db.py

@@ -65,7 +65,7 @@ class ResultDB:
         result_dict_to_hdf5(f, self.data.read)
 
 
-def _create_device(desc, dbh):
+def create_device(desc, dbh):
     ty = desc["type"]
     if ty == "local":
         module = importlib.import_module(desc["module"])
@@ -105,7 +105,7 @@ class DBHub:
             while isinstance(desc, str):
                 # alias
                 desc = self.ddb.request(desc)
-            dev = _create_device(desc, self)
+            dev = create_device(desc, self)
             self.active_devices[name] = dev
             return dev
 

doc/manual/core_device_flash_storage.rst

@@ -0,0 +1,14 @@
+.. _core-device-flash-storage:
+
+Core device flash storage
+=========================
+
+The core device contains some flash space that can be used to store
+some configuration data.
+
+This storage area is used to store the core device MAC address, IP address and even the idle kernel.
+
+The flash storage area is one sector (64 kB) large and is organized as a list
+of key-value records.
+
+This flash storage space can be accessed by using the artiq_coreconfig.py :ref:`core-device-configuration-tool`.

doc/manual/index.rst

@@ -15,6 +15,7 @@ Contents:
     core_drivers_reference
     protocols_reference
     ndsp_reference
+    core_device_flash_storage
     utilities
     fpga_board_ports
     default_network_ports

doc/manual/installing.rst

@@ -124,6 +124,50 @@ These steps are required to generate bitstream (``.bit``) files, build the MiSoC
 
 The communication parameters are 115200 8-N-1.
 
+* Set the MAC and IP address in the :ref:`core device configuration flash storage <core-device-flash-storage>`:
+
+    * You can either set it by generating a flash storage image and then flash it: ::
+
+        $ ~/artiq-dev/artiq/frontend/artiq_mkfs.py flash_storage.img -s mac xx:xx:xx:xx:xx:xx -s ip xx.xx.xx.xx
+        $ ~/artiq-dev/artiq/frontend/artiq_flash.sh -f flash_storage.img
+
+    * Or you can set it via the runtime test mode command line
+
+        * Boot the board.
+
+        * Quickly run flterm (in ``path/to/misoc/tools``) to access the serial console.
+
+        * If you weren't quick enough to see anything in the serial console, press the reset button.
+
+        * Wait for "Press 't' to enter test mode..." to appear and hit the ``t`` key.
+
+        * Enter the following commands (which will erase the flash storage content).
+
+            ::
+
+                test> fserase
+                test> fswrite ip xx.xx.xx.xx
+                test> fswrite mac xx:xx:xx:xx:xx:xx
+
+        * Then reboot.
+
+        You should see something like this in the serial console: ::
+
+            ~/dev/misoc$ ./tools/flterm --port /dev/ttyUSB1
+            [FLTERM] Starting...
+
+            MiSoC BIOS   http://m-labs.hk
+            (c) Copyright 2007-2014 Sebastien Bourdeauducq
+            [...]
+            Press 't' to enter test mode...
+            Entering test mode.
+            test> fserase
+            test> fswrite ip 192.168.10.2
+            test> fswrite mac 11:22:33:44:55:66
+
+.. note:: The reset button of the KC705 board is the "CPU_RST" labeled button.
+.. warning:: Both those instructions will result in the flash storage being wiped out. However you can use the test mode to change the IP/MAC without erasing everything if you skip the "fserase" command.
+
 Installing the host-side software
 ---------------------------------
 

doc/manual/utilities.rst

@@ -92,3 +92,58 @@ This tool compiles key/value pairs into a binary image suitable for flashing int
 .. argparse::
    :ref: artiq.frontend.artiq_mkfs.get_argparser
    :prog: artiq_mkfs
+
+.. _core-device-configuration-tool:
+
+Core device configuration tool
+------------------------------
+
+The artiq_coreconfig tool allows to read, write and remove key-value records from the :ref:`core-device-flash-storage`.
+
+It also allows to erase the entire flash storage area.
+
+To use this tool, you need to specify a ``ddb.pyon`` DDB file which contains a ``comm`` device (an example is provided in ``artiq/examples/master/ddb.pyon``).
+This tells the tool how to connect to the core device (via serial or via TCP) and with which parameters (baudrate, serial device, IP address, TCP port).
+When not specified, the artiq_coreconfig tool will assume that there is a file named ``ddb.pyon`` in the current directory.
+
+
+To read the record whose key is ``mac``::
+
+    $ artiq_coreconfig -r mac
+
+To write the value ``test_value`` in the key ``my_key``::
+
+    $ artiq_coreconfig -w my_key test_value
+    $ artiq_coreconfig -r my_key
+    b'test_value'
+
+You can also write entire files in a record using the ``-f`` parameter::
+
+    $ echo "this_is_a_test" > my_filename
+    $ artiq_coreconfig -f my_key my_filename
+    $ artiq_coreconfig -r my_key
+    b'this_is_a_test\n'
+
+You can write several records at once::
+
+    $ artiq_coreconfig -w key1 value1 -f key2 filename -w key3 value3
+
+To remove the previously written key ``my_key``::
+
+    $ artiq_coreconfig -d my_key
+
+To erase the entire flash storage area::
+
+    $ artiq_coreconfig -e
+
+You don't need to remove a record in order to change its value, just overwrite
+it::
+
+    $ artiq_coreconfig -w my_key some_value
+    $ artiq_coreconfig -w my_key some_other_value
+    $ artiq_coreconfig -r my_key
+    b'some_other_value'
+
+.. argparse::
+   :ref: artiq.frontend.artiq_coreconfig.get_argparser
+   :prog: artiq_coreconfig

soc/runtime/flash_storage.c

@@ -3,6 +3,7 @@
  */
 
 #include <string.h>
+#include <stdio.h>
 #include <system.h>
 #include <spiflash.h>
 #include <generated/mem.h>
@@ -19,133 +20,239 @@
 #define min(a, b) (a>b?b:a)
 #define max(a, b) (a>b?a:b)
 
-#define goto_next_record(buff, addr) do { \
+struct record {
-        unsigned int key_size = strlen(&buff[addr])+1; \
+    char *key;
-        if(key_size % 4) \
+    unsigned int key_len;
-            key_size += 4 - (key_size % 4); \
+    char *value;
-        unsigned int *buflen_p = (unsigned int *)&buff[addr + key_size]; \
+    unsigned int value_len;
-        unsigned int buflen = *buflen_p; \
+    char *raw_record;
-        if(buflen % 4) \
+    unsigned int size;
-            buflen += 4 - (buflen % 4); \
-        addr += key_size + sizeof(int) + buflen; \
-    } while (0)
-
-union seek {
-    unsigned int integer;
-    char bytes[4];
 };
 
-static void write_at_offset(char *key, void *buffer, int buflen, unsigned int sector_offset);
+struct iter_state {
-static char key_exists(char *buff, char *key, char *end);
+    char *buffer;
-static char check_for_duplicates(char *buff);
+    unsigned int seek;
-static unsigned int try_to_flush_duplicates(void);
+    unsigned int buf_len;
+};
 
-static char key_exists(char *buff, char *key, char *end)
+static unsigned int get_record_size(char *buff)
 {
-    unsigned int addr;
+    unsigned int record_size;
 
-    addr = 0;
+    memcpy(&record_size, buff, 4);
-    while(&buff[addr] < end && *(unsigned int*)&buff[addr] != END_MARKER) {
+    return record_size;
-        if(strcmp(&buff[addr], key) == 0)
+}
-            return 1;
+
-        goto_next_record(buff, addr);
+static void record_iter_init(struct iter_state *is, char *buffer, unsigned int buf_len)
+{
+    is->buffer = buffer;
+    is->seek = 0;
+    is->buf_len = buf_len;
+}
+
+static int record_iter_next(struct iter_state *is, struct record *record, int *fatal)
+{
+    if(is->seek >= is->buf_len)
+        return 0;
+
+    record->raw_record = &is->buffer[is->seek];
+    record->size = get_record_size(record->raw_record);
+
+    if(record->size == END_MARKER)
+        return 0;
+
+    if(is->seek > is->buf_len - sizeof(record->size) - 2) { /* 2 is the minimum key length */
+        printf("flash_storage might be corrupted: END_MARKER missing at the end of the storage sector\n");
+        if(fatal)
+            *fatal = 1;
+        return 0;
     }
+
+    if(record->size > is->buf_len - is->seek) {
+        printf("flash_storage might be corrupted: invalid record_size %d at address %08x\n", record->size, record->raw_record);
+        if(fatal)
+            *fatal = 1;
+        return 0;
+    }
+
+    record->key = record->raw_record + sizeof(record->size);
+    record->key_len = strnlen(record->key, record->size - sizeof(record->size)) + 1;
+
+    if(record->key_len == record->size - sizeof(record->size) + 1) {
+        printf("flash_storage might be corrupted: invalid key length at address %08x\n", record->raw_record);
+        if(fatal)
+            *fatal = 1;
+        return 0;
+    }
+
+    record->value = record->key + record->key_len;
+    record->value_len = record->size - record->key_len - sizeof(record->size);
+
+    is->seek += record->size;
+    return 1;
+}
+
+static unsigned int get_free_space(void)
+{
+    struct iter_state is;
+    struct record record;
+
+    record_iter_init(&is, STORAGE_ADDRESS, STORAGE_SIZE);
+    while(record_iter_next(&is, &record, NULL));
+    return STORAGE_SIZE - is.seek;
+}
+
+static int is_empty(struct record *record)
+{
+    return record->value_len == 0;
+}
+
+static int key_exists(char *buff, char *key, char *end, char accept_empty, struct record *found_record)
+{
+    struct iter_state is;
+    struct record iter_record;
+    int found = 0;
+
+    record_iter_init(&is, buff, end - buff);
+    while(record_iter_next(&is, &iter_record, NULL)) {
+        if(strcmp(iter_record.key, key) == 0) {
+            found = 1;
+            if(found_record)
+                *found_record = iter_record;
+        }
+    }
+
+    if(found && is_empty(found_record) && !accept_empty)
+        return 0;
+
+    if(found)
+        return 1;
+
     return 0;
 }
 
 static char check_for_duplicates(char *buff)
 {
-    unsigned int addr;
+    struct record record, following_record;
-    char *key_name;
+    struct iter_state is;
+    int no_error;
 
-    addr = 0;
+    record_iter_init(&is, buff, STORAGE_SIZE);
-    while(addr < STORAGE_SIZE && *(unsigned int *)&buff[addr] != END_MARKER) {
+    no_error = record_iter_next(&is, &record, NULL);
-        key_name = &buff[addr];
+    while(no_error) {
-        goto_next_record(buff, addr);
+        no_error = record_iter_next(&is, &following_record, NULL);
-        if(key_exists(&buff[addr], key_name, &buff[STORAGE_SIZE]))
+        if(no_error && key_exists(following_record.raw_record, record.key, &buff[STORAGE_SIZE], 1, NULL))
             return 1;
+        record = following_record;
     }
 
     return 0;
 }
 
-static unsigned int try_to_flush_duplicates(void)
+static char check_for_empty_records(char *buff)
 {
-    unsigned int addr, i, key_size, buflen;
+    struct iter_state is;
-    char *key_name, *last_duplicate;
+    struct record record;
+
+    record_iter_init(&is, buff, STORAGE_SIZE);
+    while(record_iter_next(&is, &record, NULL))
+        if(is_empty(&record))
+            return 1;
+
+    return 0;
+}
+
+static unsigned int try_to_flush_duplicates(char *new_key, unsigned int buf_len)
+{
+    unsigned int key_size, new_record_size, ret = 0, can_rollback = 0;
+    struct record record, previous_record;
     char sector_buff[STORAGE_SIZE];
-    union seek *seeker = (union seek *)sector_buff;
+    struct iter_state is;
 
     memcpy(sector_buff, STORAGE_ADDRESS, STORAGE_SIZE);
-    if(check_for_duplicates(sector_buff)) {
+    if(check_for_duplicates(sector_buff)
+       || key_exists(sector_buff, new_key, &sector_buff[STORAGE_SIZE], 0, NULL)
+       || check_for_empty_records(sector_buff)) {
         fs_erase();
-        for(addr = 0; addr < STORAGE_SIZE && seeker[addr >> 2].integer != END_MARKER;) {
+        record_iter_init(&is, sector_buff, STORAGE_SIZE);
-            key_name = &sector_buff[addr];
+        while(record_iter_next(&is, &record, NULL)) {
-            key_size = strlen(key_name)+1;
+            if(is_empty(&record))
-            if(key_size % 4)
+                continue;
-                key_size += 4 - (key_size % 4);
+            if(!key_exists((char *)STORAGE_ADDRESS, record.key, STORAGE_ADDRESS + STORAGE_SIZE, 1, NULL)) {
-            if(!key_exists((char *)STORAGE_ADDRESS, key_name, STORAGE_ADDRESS+STORAGE_SIZE)) {
+                struct record rec;
-                last_duplicate = key_name;
+
-                for(i = addr; i < STORAGE_SIZE;) {
+                if(!key_exists(sector_buff, record.key, &sector_buff[STORAGE_SIZE], 0, &rec))
-                    goto_next_record(sector_buff, i);
+                    continue;
-                    if(strcmp(&sector_buff[i], key_name) == 0)
+                if(strcmp(new_key, record.key) == 0) { // If we are about to write this key we don't keep the old value.
-                        last_duplicate = &sector_buff[i];
+                    previous_record = rec; // This holds the old record in case we need it back (for instance if new record is too long)
-                }
+                    can_rollback = 1;
-                buflen = *(unsigned int *)&last_duplicate[key_size];
-                fs_write(key_name, &last_duplicate[key_size+sizeof(int)], buflen);
-            }
-            goto_next_record(sector_buff, addr);
-        }
-        return 0;
                 } else
-        return 1;
+                    fs_write(record.key, rec.value, rec.value_len);
+            }
+        }
+        ret = 1;
+    }
+
+    key_size = strlen(new_key) + 1;
+    new_record_size = key_size + buf_len + sizeof(new_record_size);
+    if(can_rollback && new_record_size > get_free_space()) {
+        fs_write(new_key, previous_record.value, previous_record.value_len);
+    }
+
+    return ret;
 }
 
-static void write_at_offset(char *key, void *buffer, int buflen, unsigned int sector_offset)
+static void write_at_offset(char *key, void *buffer, int buf_len, unsigned int sector_offset)
 {
     int key_len = strlen(key) + 1;
-    int key_len_alignment = 0, buflen_alignment = 0;
+    unsigned int record_size = key_len + buf_len + sizeof(record_size);
-    unsigned char padding[3] = {0, 0, 0};
+    unsigned int flash_addr = (unsigned int)STORAGE_ADDRESS + sector_offset;
 
-    if(key_len % 4)
+    write_to_flash(flash_addr, (unsigned char *)&record_size, sizeof(record_size));
-        key_len_alignment = 4 - (key_len % 4);
+    write_to_flash(flash_addr+sizeof(record_size), (unsigned char *)key, key_len);
-
+    write_to_flash(flash_addr+sizeof(record_size)+key_len, buffer, buf_len);
-    if(buflen % 4)
-        buflen_alignment = 4 - (buflen % 4);
-
-    write_to_flash(sector_offset, (unsigned char *)key, key_len);
-    write_to_flash(sector_offset+key_len, padding, key_len_alignment);
-    write_to_flash(sector_offset+key_len+key_len_alignment, (unsigned char *)&buflen, sizeof(buflen));
-    write_to_flash(sector_offset+key_len+key_len_alignment+sizeof(buflen), buffer, buflen);
-    write_to_flash(sector_offset+key_len+key_len_alignment+sizeof(buflen)+buflen, padding, buflen_alignment);
     flush_cpu_dcache();
 }
 
 
-void fs_write(char *key, void *buffer, unsigned int buflen)
+int fs_write(char *key, void *buffer, unsigned int buf_len)
 {
-    char *addr;
+    struct record record;
-    unsigned int key_size = strlen(key)+1;
+    unsigned int key_size = strlen(key) + 1;
-    unsigned int record_size = key_size + sizeof(int) + buflen;
+    unsigned int new_record_size = key_size + sizeof(int) + buf_len;
+    int no_error, fatal = 0;
+    struct iter_state is;
 
-    for(addr = STORAGE_ADDRESS; addr < STORAGE_ADDRESS + STORAGE_SIZE - record_size; addr += 4) {
+    record_iter_init(&is, STORAGE_ADDRESS, STORAGE_SIZE);
-        if(*(unsigned int *)addr == END_MARKER) {
+    while((no_error = record_iter_next(&is, &record, &fatal)));
-            write_at_offset(key, buffer, buflen, (unsigned int)addr);
-            break;
-        }
-    }
-    if(addr >= STORAGE_ADDRESS + STORAGE_SIZE - record_size) { // Flash is full? Try to flush duplicates.
-        if(try_to_flush_duplicates())
-            return; // No duplicates found, cannot write the new key-value record: sector is full.
 
+    if(fatal)
+        goto fatal_error;
+
+    if(STORAGE_SIZE - is.seek >= new_record_size) {
+        write_at_offset(key, buffer, buf_len, is.seek);
+        return 1;
+    }
+
+    if(!try_to_flush_duplicates(key, buf_len)) // storage is full, let's try to free some space up.
+        return 0; // No duplicates found, cannot write the new key-value record: sector is full.
     // Now retrying to write, hoping enough flash was freed.
-        for(addr = STORAGE_ADDRESS; addr < STORAGE_ADDRESS + STORAGE_SIZE - record_size; addr += 4) {
+
-            if(*(unsigned int *)addr == END_MARKER) {
+    record_iter_init(&is, STORAGE_ADDRESS, STORAGE_SIZE);
-                write_at_offset(key, buffer, buflen, (unsigned int)addr);
+    while((no_error = record_iter_next(&is, &record, &fatal)));
-                break;
+
-            }
+    if(fatal)
-        }
+        goto fatal_error;
-    }
+
+    if(STORAGE_SIZE - is.seek >= new_record_size) {
+        write_at_offset(key, buffer, buf_len, is.seek);
+        return 1; // We eventually succeeded in writing the record
+    } else
+        return 0; // Storage is definitely full.
+
+fatal_error:
+    printf("fatal error: flash storage might be corrupted\n");
+    return 0;
 }
 
 void fs_erase(void)
@@ -154,33 +261,35 @@ void fs_erase(void)
     flush_cpu_dcache();
 }
 
-unsigned int fs_read(char *key, void *buffer, unsigned int buflen, unsigned int *remain)
+unsigned int fs_read(char *key, void *buffer, unsigned int buf_len, unsigned int *remain)
 {
     unsigned int read_length = 0;
-    char *addr;
+    struct iter_state is;
+    struct record record;
+    int fatal = 0;
 
-    addr = STORAGE_ADDRESS;
-    while(addr < (STORAGE_ADDRESS + STORAGE_SIZE) && (*addr != END_MARKER)) {
-        unsigned int key_len, value_len;
-        char *key_addr = addr;
-
-        key_len = strlen(addr) + 1;
-        if(key_len % 4)
-            key_len += 4 - (key_len % 4);
-        addr += key_len;
-        value_len = *(unsigned int *)addr;
-        addr += sizeof(value_len);
-        if(strcmp(key_addr, key) == 0) {
-            memcpy(buffer, addr, min(value_len, buflen));
-            read_length = min(value_len, buflen);
     if(remain)
-                *remain = max(0, (int)value_len - (int)buflen);
+        *remain = 0;
+
+    record_iter_init(&is, STORAGE_ADDRESS, STORAGE_SIZE);
+    while(record_iter_next(&is, &record, &fatal)) {
+        if(strcmp(record.key, key) == 0) {
+            memcpy(buffer, record.value, min(record.value_len, buf_len));
+            read_length = min(record.value_len, buf_len);
+            if(remain)
+                *remain = max(0, (int)(record.value_len) - (int)buf_len);
         }
-        addr += value_len;
-        if((int)addr % 4)
-            addr += 4 - ((int)addr % 4);
     }
+
+    if(fatal)
+        printf("fatal error: flash storage might be corrupted\n");
+
     return read_length;
 }
 
+void fs_remove(char *key)
+{
+    fs_write(key, NULL, 0);
+}
+
 #endif /* CSR_SPIFLASH_BASE && SPIFLASH_PAGE_SIZE */

soc/runtime/flash_storage.h

@@ -5,8 +5,9 @@
 #ifndef __FLASH_STORAGE_H
 #define __FLASH_STORAGE_H
 
+void fs_remove(char *key);
 void fs_erase(void);
-void fs_write(char *key, void *buffer, unsigned int buflen);
+int fs_write(char *key, void *buffer, unsigned int buflen);
 unsigned int fs_read(char *key, void *buffer, unsigned int buflen, unsigned int *remain);
 
 #endif /* __FLASH_STORAGE_H */

soc/runtime/session.c

@@ -12,6 +12,7 @@
 #include "kloader.h"
 #include "exceptions.h"
 #include "session.h"
+#include "flash_storage.h"
 
 #define BUFFER_IN_SIZE (1024*1024)
 #define BUFFER_OUT_SIZE (1024*1024)
@@ -86,7 +87,12 @@ enum {
     REMOTEMSG_TYPE_LOAD_OBJECT,
     REMOTEMSG_TYPE_RUN_KERNEL,
 
-    REMOTEMSG_TYPE_RPC_REPLY
+    REMOTEMSG_TYPE_RPC_REPLY,
+
+    REMOTEMSG_TYPE_FLASH_READ_REQUEST,
+    REMOTEMSG_TYPE_FLASH_WRITE_REQUEST,
+    REMOTEMSG_TYPE_FLASH_ERASE_REQUEST,
+    REMOTEMSG_TYPE_FLASH_REMOVE_REQUEST
 };
 
 /* device to host */
@@ -104,8 +110,24 @@ enum {
     REMOTEMSG_TYPE_KERNEL_EXCEPTION,
 
     REMOTEMSG_TYPE_RPC_REQUEST,
+
+    REMOTEMSG_TYPE_FLASH_READ_REPLY,
+    REMOTEMSG_TYPE_FLASH_WRITE_REPLY,
+    REMOTEMSG_TYPE_FLASH_OK_REPLY,
+    REMOTEMSG_TYPE_FLASH_ERROR_REPLY
 };
 
+static int check_flash_storage_key_len(char *key, unsigned int key_len)
+{
+    if(key_len == get_in_packet_len() - 8) {
+        log("Invalid key: not a null-terminated string");
+        buffer_out[8] = REMOTEMSG_TYPE_FLASH_ERROR_REPLY;
+        submit_output(9);
+        return 0;
+    }
+    return 1;
+}
+
 static int process_input(void)
 {
     switch(buffer_in[8]) {
@@ -204,6 +226,63 @@ static int process_input(void)
             user_kernel_state = USER_KERNEL_RUNNING;
             break;
         }
+        case REMOTEMSG_TYPE_FLASH_READ_REQUEST: {
+#if SPIFLASH_SECTOR_SIZE - 4 > BUFFER_OUT_SIZE - 9
+#error Output buffer cannot hold the flash storage data
+#elif SPIFLASH_SECTOR_SIZE - 4 > BUFFER_IN_SIZE - 9
+#error Input buffer cannot hold the flash storage data
+#endif
+            unsigned int ret, in_packet_len;
+            char *key;
+
+            in_packet_len = get_in_packet_len();
+            key = &buffer_in[9];
+            buffer_in[in_packet_len] = '\0';
+
+            buffer_out[8] = REMOTEMSG_TYPE_FLASH_READ_REPLY;
+            ret = fs_read(key, &buffer_out[9], sizeof(buffer_out) - 9, NULL);
+            submit_output(9 + ret);
+            break;
+        }
+        case REMOTEMSG_TYPE_FLASH_WRITE_REQUEST: {
+            char *key, *value;
+            unsigned int key_len, value_len, in_packet_len;
+            int ret;
+
+            in_packet_len = get_in_packet_len();
+            key = &buffer_in[9];
+            key_len = strnlen(key, in_packet_len - 9) + 1;
+            if(!check_flash_storage_key_len(key, key_len))
+                break;
+
+            value_len = in_packet_len - key_len - 9;
+            value = key + key_len;
+            ret = fs_write(key, value, value_len);
+
+            buffer_out[8] = REMOTEMSG_TYPE_FLASH_WRITE_REPLY;
+            buffer_out[9] = ret;
+            submit_output(10);
+            break;
+        }
+        case REMOTEMSG_TYPE_FLASH_ERASE_REQUEST: {
+            fs_erase();
+            buffer_out[8] = REMOTEMSG_TYPE_FLASH_OK_REPLY;
+            submit_output(9);
+            break;
+        }
+        case REMOTEMSG_TYPE_FLASH_REMOVE_REQUEST: {
+            char *key;
+            unsigned int in_packet_len;
+
+            in_packet_len = get_in_packet_len();
+            key = &buffer_in[9];
+            buffer_in[in_packet_len] = '\0';
+
+            fs_remove(key);
+            buffer_out[8] = REMOTEMSG_TYPE_FLASH_OK_REPLY;
+            submit_output(9);
+            break;
+        }
         default:
             return 0;
     }

soc/runtime/test_mode.c

@@ -259,13 +259,201 @@ static void ddstest(char *n)
 #if (defined CSR_SPIFLASH_BASE && defined SPIFLASH_PAGE_SIZE)
 static void fsread(char *key)
 {
-    char buf[256];
+    char readbuf[SPIFLASH_SECTOR_SIZE];
     int r;
 
-    r = fs_read(key, buf, sizeof(buf)-1, NULL);
+    r = fs_read(key, readbuf, sizeof(readbuf)-1, NULL);
-    buf[r] = 0;
+    readbuf[r] = 0;
-    puts(buf);
+    if(r == 0)
+        printf("key %s does not exist\n", key);
+    else
+        puts(readbuf);
 }
+
+static void fswrite(char *key, void *buffer, unsigned int length)
+{
+    if(!fs_write(key, buffer, length))
+        printf("cannot write key %s because flash storage is full\n", key);
+}
+
+static void fsfull(void)
+{
+    int i;
+    char value[4096];
+    memset(value, '@', sizeof(value));
+
+    for(i = 0; i < SPIFLASH_SECTOR_SIZE/sizeof(value); i++)
+        fs_write("plip", value, sizeof(value));
+}
+
+static void check_read(char *key, char *expected, unsigned int length, unsigned int testnum)
+{
+    char readbuf[SPIFLASH_SECTOR_SIZE];
+    unsigned int remain, readlength;
+
+    memset(readbuf, '\0', sizeof(readbuf));
+
+    readlength = fs_read(key, readbuf, sizeof(readbuf), &remain);
+    if(remain > 0)
+        printf("KO[%u] remain == %u, expected 0\n", testnum, remain);
+    if(readlength != length)
+        printf("KO[%u] read length == %u, expected %u\n", testnum, readlength, length);
+    if(remain == 0 && readlength == length)
+        printf(".");
+
+    readbuf[readlength] = 0;
+    if(memcmp(expected, readbuf, readlength) == 0)
+        printf(".\n");
+    else
+        printf("KO[%u] read %s instead of %s\n", testnum, readbuf, expected);
+}
+
+static void check_doesnt_exist(char *key, unsigned int testnum)
+{
+    char readbuf;
+    unsigned int remain, readlength;
+
+    readlength = fs_read(key, &readbuf, sizeof(readbuf), &remain);
+    if(remain > 0)
+        printf("KO[%u] remain == %u, expected 0\n", testnum, remain);
+    if(readlength > 0)
+        printf("KO[%u] readlength == %d, expected 0\n", testnum, readlength);
+    if(remain == 0 && readlength == 0)
+        printf(".\n");
+}
+
+static void check_write(unsigned int ret)
+{
+    if(!ret)
+        printf("KO");
+    else
+        printf(".");
+}
+
+static inline void test_sector_is_full(void)
+{
+    char c;
+    char value[4096];
+    char key[2] = {0, 0};
+
+    fs_erase();
+    memset(value, '@', sizeof(value));
+    for(c = 1; c <= SPIFLASH_SECTOR_SIZE/sizeof(value); c++) {
+        key[0] = c;
+        check_write(fs_write(key, value, sizeof(value) - 6));
+    }
+    check_write(!fs_write("this_should_fail", "fail", 5));
+    printf("\n");
+}
+
+static void test_one_big_record(int testnum)
+{
+    char value[SPIFLASH_SECTOR_SIZE];
+    memset(value, '@', sizeof(value));
+
+    fs_erase();
+    check_write(fs_write("a", value, sizeof(value) - 6));
+    check_read("a", value, sizeof(value) - 6, testnum);
+    check_write(fs_write("a", value, sizeof(value) - 6));
+    check_read("a", value, sizeof(value) - 6, testnum);
+    check_write(!fs_write("b", value, sizeof(value) - 6));
+    check_read("a", value, sizeof(value) - 6, testnum);
+    fs_remove("a");
+    check_doesnt_exist("a", testnum);
+    check_write(fs_write("a", value, sizeof(value) - 6));
+    check_read("a", value, sizeof(value) - 6, testnum);
+    fs_remove("a");
+    check_doesnt_exist("a", testnum);
+    value[0] = '!';
+    check_write(fs_write("b", value, sizeof(value) - 6));
+    check_read("b", value, sizeof(value) - 6, testnum);
+}
+
+static void test_flush_duplicate_rollback(int testnum)
+{
+    char value[SPIFLASH_SECTOR_SIZE];
+    memset(value, '@', sizeof(value));
+
+    fs_erase();
+    /* This makes the flash storage full with one big record */
+    check_write(fs_write("a", value, SPIFLASH_SECTOR_SIZE - 6));
+    /* This should trigger the try_to_flush_duplicate code which
+     * at first will not keep the old "a" record value because we are
+     * overwriting it. But then it should roll back to the old value
+     * because the new record is too large.
+     */
+    value[0] = '!';
+    check_write(!fs_write("a", value, sizeof(value)));
+    /* check we still have the old record value */
+    value[0] = '@';
+    check_read("a", value, SPIFLASH_SECTOR_SIZE - 6, testnum);
+}
+
+static void test_too_big_fails(int testnum)
+{
+    char value[SPIFLASH_SECTOR_SIZE];
+    memset(value, '@', sizeof(value));
+
+    fs_erase();
+    check_write(!fs_write("a", value, sizeof(value) - 6 + /* TOO BIG */ 1));
+    check_doesnt_exist("a", testnum);
+}
+
+static void fs_test(void)
+{
+    int i;
+    char writebuf[] = "abcdefghijklmnopqrst";
+    char read_check[4096];
+    int vect_length = sizeof(writebuf);
+
+    memset(read_check, '@', sizeof(read_check));
+    printf("testing...\n");
+    for(i = 0; i < vect_length; i++) {
+        printf("%u.0:", i);
+        fs_erase();
+        check_write(fs_write("a", writebuf, i));
+        check_read("a", writebuf, i, i);
+
+        printf("%u.1:", i);
+        fsfull();
+        check_read("a", writebuf, i, i);
+
+        printf("%u.2:", i);
+        check_read("plip", read_check, sizeof(read_check), i);
+
+        printf("%u.3:", i);
+        check_write(fs_write("a", "b", 2));
+        check_read("a", "b", 2, i);
+
+        printf("%u.4:", i);
+        fsfull();
+        check_read("a", "b", 2, i);
+
+        printf("%u.5:", i);
+        check_doesnt_exist("notfound", i);
+
+        printf("%u.6:", i);
+        fs_remove("a");
+        check_doesnt_exist("a", i);
+
+        printf("%u.7:", i);
+        fsfull();
+        check_doesnt_exist("a", i);
+    }
+
+    printf("%u:", vect_length);
+    test_sector_is_full();
+
+    printf("%u:", vect_length+1);
+    test_one_big_record(vect_length+1);
+
+    printf("%u:", vect_length+2);
+    test_flush_duplicate_rollback(vect_length+2);
+
+    printf("%u:", vect_length+3);
+    test_too_big_fails(vect_length+3);
+}
+
 #endif
 
 static void help(void)
@@ -288,6 +476,8 @@ static void help(void)
     puts("fserase         - erase flash storage");
     puts("fswrite <k> <v> - write to flash storage");
     puts("fsread <k>      - read flash storage");
+    puts("fsremove <k>    - remove a key-value record from flash storage");
+    puts("fstest          - run flash storage tests. WARNING: erases the storage area");
 #endif
 }
 
@@ -340,6 +530,7 @@ static char *get_token(char **str)
     return d;
 }
 
+
 static void do_command(char *c)
 {
     char *token;
@@ -365,8 +556,10 @@ static void do_command(char *c)
 
 #if (defined CSR_SPIFLASH_BASE && defined SPIFLASH_PAGE_SIZE)
     else if(strcmp(token, "fserase") == 0) fs_erase();
-    else if(strcmp(token, "fswrite") == 0) fs_write(get_token(&c), c, strlen(c));
+    else if(strcmp(token, "fswrite") == 0) fswrite(get_token(&c), c, strlen(c));
     else if(strcmp(token, "fsread") == 0) fsread(get_token(&c));
+    else if(strcmp(token, "fsremove") == 0) fs_remove(get_token(&c));
+    else if(strcmp(token, "fstest") == 0) fs_test();
 #endif
 
     else if(strcmp(token, "") != 0)