firmware: move config requests to management protocol.

They were only in session protocol because of historical reasons.

artiq/coredevice/comm_kernel.py

@@ -28,11 +28,6 @@ class _H2DMsgType(Enum):
     RPC_REPLY = 7
     RPC_EXCEPTION = 8
 
-    FLASH_READ_REQUEST = 9
-    FLASH_WRITE_REQUEST = 10
-    FLASH_ERASE_REQUEST = 11
-    FLASH_REMOVE_REQUEST = 12
-
     HOTSWAP = 14
 
 
@@ -52,10 +47,6 @@ class _D2HMsgType(Enum):
 
     RPC_REQUEST = 10
 
-    FLASH_READ_REPLY = 11
-    FLASH_OK_REPLY = 12
-    FLASH_ERROR_REPLY = 13
-
     WATCHDOG_EXPIRED = 14
     CLOCK_FAILURE = 15
 
@@ -277,36 +268,6 @@ class CommKernel:
 
         self._read_empty(_D2HMsgType.CLOCK_SWITCH_COMPLETED)
 
-    def flash_storage_read(self, key):
-        self._write_header(_H2DMsgType.FLASH_READ_REQUEST)
-        self._write_string(key)
-
-        self._read_header()
-        self._read_expect(_D2HMsgType.FLASH_READ_REPLY)
-        return self._read_string()
-
-    def flash_storage_write(self, key, value):
-        self._write_header(_H2DMsgType.FLASH_WRITE_REQUEST)
-        self._write_string(key)
-        self._write_bytes(value)
-
-        self._read_header()
-        if self._read_type == _D2HMsgType.FLASH_ERROR_REPLY:
-            raise IOError("Flash storage is full")
-        else:
-            self._read_expect(_D2HMsgType.FLASH_OK_REPLY)
-
-    def flash_storage_erase(self):
-        self._write_empty(_H2DMsgType.FLASH_ERASE_REQUEST)
-
-        self._read_empty(_D2HMsgType.FLASH_OK_REPLY)
-
-    def flash_storage_remove(self, key):
-        self._write_header(_H2DMsgType.FLASH_REMOVE_REQUEST)
-        self._write_string(key)
-
-        self._read_empty(_D2HMsgType.FLASH_OK_REPLY)
-
     def load(self, kernel_library):
         self._write_header(_H2DMsgType.LOAD_KERNEL)
         self._write_bytes(kernel_library)

artiq/coredevice/comm_mgmt.py

@@ -16,6 +16,11 @@ class Request(Enum):
     SetLogFilter = 3
     SetUartLogFilter = 6
 
+    ConfigRead = 12
+    ConfigWrite = 13
+    ConfigRemove = 14
+    ConfigErase = 15
+
     StartProfiler = 9
     StopProfiler = 10
     GetProfile = 11
@@ -28,10 +33,13 @@ class Request(Enum):
 
 class Reply(Enum):
     Success = 1
+    Error = 6
     Unavailable = 4
 
     LogContent = 2
 
+    ConfigData = 7
+
     Profile = 5
 
     RebootImminent = 3
@@ -85,6 +93,9 @@ class CommMgmt:
         self._write_int32(len(value))
         self._write(value)
 
+    def _write_string(self, value):
+        self._write_bytes(value.encode("utf-8"))
+
     def _read(self, length):
         r = bytes()
         while len(r) < length:
@@ -147,6 +158,32 @@ class CommMgmt:
         self._write_int8(getattr(LogLevel, level).value)
         self._read_expect(Reply.Success)
 
+    def config_read(self, key):
+        self._write_header(Request.ConfigRead)
+        self._write_string(key)
+        self._read_expect(Reply.ConfigData)
+        return self._read_string()
+
+    def config_write(self, key, value):
+        self._write_header(Request.ConfigWrite)
+        self._write_string(key)
+        self._write_bytes(value)
+        ty = self._read_header()
+        if ty == Reply.Error:
+            raise IOError("Flash storage is full")
+        elif ty != Reply.Success:
+            raise IOError("Incorrect reply from device: {} (expected {})".
+                          format(ty, Reply.Success))
+
+    def config_remove(self, key):
+        self._write_header(Request.ConfigRemove)
+        self._write_string(key)
+        self._read_expect(Reply.Success)
+
+    def config_erase(self):
+        self._write_empty(Request.ConfigErase)
+        self._read_expect(Reply.Success)
+
     def start_profiler(self, interval, edges_size, hits_size):
         self._write_header(Request.StartProfiler)
         self._write_int32(interval)

artiq/firmware/libproto_artiq/mgmt_proto.rs

@@ -1,8 +1,9 @@
-use alloc::Vec;
+use core::str::Utf8Error;
+use alloc::{Vec, String};
 #[cfg(feature = "log")]
 use log;
 
-use io::{Read, ProtoRead, Write, ProtoWrite, Error as IoError};
+use io::{Read, ProtoRead, Write, ProtoWrite, Error as IoError, ReadStringError};
 
 #[derive(Fail, Debug)]
 pub enum Error<T> {
@@ -12,6 +13,8 @@ pub enum Error<T> {
     UnknownPacket(u8),
     #[fail(display = "unknown log level {}", _0)]
     UnknownLogLevel(u8),
+    #[fail(display = "invalid UTF-8: {}", _0)]
+    Utf8(Utf8Error),
     #[fail(display = "{}", _0)]
     Io(#[cause] IoError<T>)
 }
@@ -22,6 +25,15 @@ impl<T> From<IoError<T>> for Error<T> {
     }
 }
 
+impl<T> From<ReadStringError<IoError<T>>> for Error<T> {
+    fn from(value: ReadStringError<IoError<T>>) -> Error<T> {
+        match value {
+            ReadStringError::Utf8(err) => Error::Utf8(err),
+            ReadStringError::Other(err) => Error::Io(err)
+        }
+    }
+}
+
 pub fn read_magic<R>(reader: &mut R) -> Result<(), Error<R::ReadError>>
     where R: Read + ?Sized
 {
@@ -46,6 +58,11 @@ pub enum Request {
     #[cfg(feature = "log")]
     SetUartLogFilter(log::LevelFilter),
 
+    ConfigRead   { key: String },
+    ConfigWrite  { key: String, value: Vec<u8> },
+    ConfigRemove { key: String },
+    ConfigErase,
+
     StartProfiler {
         interval_us: u32,
         hits_size: u32,
@@ -62,10 +79,13 @@ pub enum Request {
 
 pub enum Reply<'a> {
     Success,
+    Error,
     Unavailable,
 
     LogContent(&'a str),
 
+    ConfigData(&'a [u8]),
+
     Profile,
 
     RebootImminent,
@@ -97,6 +117,19 @@ impl Request {
             3 => Request::SetLogFilter(read_log_level_filter(reader)?),
             #[cfg(feature = "log")]
             6 => Request::SetUartLogFilter(read_log_level_filter(reader)?),
+
+            12 => Request::ConfigRead {
+                key: reader.read_string()?
+            },
+            13 => Request::ConfigWrite {
+                key:   reader.read_string()?,
+                value: reader.read_bytes()?
+            },
+            14 => Request::ConfigRemove {
+                key: reader.read_string()?
+            },
+            15 => Request::ConfigErase,
+
             9 => Request::StartProfiler {
                 interval_us: reader.read_u32()?,
                 hits_size: reader.read_u32()?,
@@ -104,9 +137,12 @@ impl Request {
             },
             10 => Request::StopProfiler,
             11 => Request::GetProfile,
+
             4 => Request::Hotswap(reader.read_bytes()?),
             5 => Request::Reboot,
+
             8 => Request::DebugAllocator,
+
             ty => return Err(Error::UnknownPacket(ty))
         })
     }
@@ -120,6 +156,9 @@ impl<'a> Reply<'a> {
             Reply::Success => {
                 writer.write_u8(1)?;
             }
+            Reply::Error => {
+                writer.write_u8(6)?;
+            }
 
             Reply::Unavailable => {
                 writer.write_u8(4)?;
@@ -130,6 +169,11 @@ impl<'a> Reply<'a> {
                 writer.write_string(log)?;
             }
 
+            Reply::ConfigData(ref bytes) => {
+                writer.write_u8(7)?;
+                writer.write_bytes(bytes)?;
+            },
+
             Reply::Profile => {
                 writer.write_u8(5)?;
                 // profile data follows

artiq/firmware/libproto_artiq/session_proto.rs

@@ -79,51 +79,6 @@ pub enum Request {
         column:   u32,
         function: String,
     },
-
-    FlashRead   { key: String },
-    FlashWrite  { key: String, value: Vec<u8> },
-    FlashRemove { key: String },
-    FlashErase,
-}
-
-impl Request {
-    pub fn read_from<R>(reader: &mut R) -> Result<Self, Error<R::ReadError>>
-        where R: Read + ?Sized
-    {
-        read_sync(reader)?;
-        Ok(match reader.read_u8()? {
-            3  => Request::SystemInfo,
-            4  => Request::SwitchClock(reader.read_u8()?),
-            5  => Request::LoadKernel(reader.read_bytes()?),
-            6  => Request::RunKernel,
-            7  => Request::RpcReply {
-                tag: reader.read_bytes()?
-            },
-            8  => Request::RpcException {
-                name:     reader.read_string()?,
-                message:  reader.read_string()?,
-                param:    [reader.read_u64()? as i64,
-                           reader.read_u64()? as i64,
-                           reader.read_u64()? as i64],
-                file:     reader.read_string()?,
-                line:     reader.read_u32()?,
-                column:   reader.read_u32()?,
-                function: reader.read_string()?
-            },
-            9  => Request::FlashRead {
-                key: reader.read_string()?
-            },
-            10 => Request::FlashWrite {
-                key:   reader.read_string()?,
-                value: reader.read_bytes()?
-            },
-            11 => Request::FlashErase,
-            12 => Request::FlashRemove {
-                key: reader.read_string()?
-            },
-            ty  => return Err(Error::UnknownPacket(ty))
-        })
-    }
 }
 
 #[derive(Debug)]
@@ -153,14 +108,44 @@ pub enum Reply<'a> {
 
     RpcRequest { async: bool },
 
-    FlashRead(&'a [u8]),
-    FlashOk,
-    FlashError,
-
     WatchdogExpired,
     ClockFailure,
 }
 
+impl Request {
+    pub fn read_from<R>(reader: &mut R) -> Result<Self, Error<R::ReadError>>
+        where R: Read + ?Sized
+    {
+        read_sync(reader)?;
+        Ok(match reader.read_u8()? {
+            3  => Request::SystemInfo,
+            4  => Request::SwitchClock(reader.read_u8()?),
+
+            5  => Request::LoadKernel(reader.read_bytes()?),
+            6  => Request::RunKernel,
+
+            7  => Request::RpcReply {
+                tag: reader.read_bytes()?
+            },
+            8  => Request::RpcException {
+                name:     reader.read_string()?,
+                message:  reader.read_string()?,
+                param:    [reader.read_u64()? as i64,
+                           reader.read_u64()? as i64,
+                           reader.read_u64()? as i64],
+                file:     reader.read_string()?,
+                line:     reader.read_u32()?,
+                column:   reader.read_u32()?,
+                function: reader.read_string()?
+            },
+
+            // 9-12 were flash requests
+
+            ty  => return Err(Error::UnknownPacket(ty))
+        })
+    }
+}
+
 impl<'a> Reply<'a> {
     pub fn write_to<W>(&self, writer: &mut W) -> Result<(), IoError<W::WriteError>>
         where W: Write + ?Sized
@@ -218,16 +203,7 @@ impl<'a> Reply<'a> {
                 writer.write_u8(async as u8)?;
             },
 
-            Reply::FlashRead(ref bytes) => {
+            // 11-13 were flash requests
-                writer.write_u8(11)?;
-                writer.write_bytes(bytes)?;
-            },
-            Reply::FlashOk => {
-                writer.write_u8(12)?;
-            },
-            Reply::FlashError => {
-                writer.write_u8(13)?;
-            },
 
             Reply::WatchdogExpired => {
                 writer.write_u8(14)?;

artiq/firmware/runtime/mgmt.rs

@@ -1,7 +1,7 @@
 use log::{self, LevelFilter};
 
 use io::{Write, ProtoWrite, Error as IoError};
-use board_misoc::boot;
+use board_misoc::{config, boot};
 use logger_artiq::BufferLogger;
 use mgmt_proto::*;
 use sched::{Io, TcpListener, TcpStream, Error as SchedError};
@@ -25,7 +25,6 @@ fn worker(io: &Io, stream: &mut TcpStream) -> Result<(), Error<SchedError>> {
                     Reply::LogContent(buffer.extract()).write_to(stream)
                 })?;
             }
-
             Request::ClearLog => {
                 BufferLogger::with(|logger| -> Result<(), Error<SchedError>> {
                     let mut buffer = io.until_ok(|| logger.buffer())?;
@@ -34,7 +33,6 @@ fn worker(io: &Io, stream: &mut TcpStream) -> Result<(), Error<SchedError>> {
 
                 Reply::Success.write_to(stream)?;
             }
-
             Request::PullLog => {
                 BufferLogger::with(|logger| -> Result<(), Error<SchedError>> {
                     loop {
@@ -64,13 +62,11 @@ fn worker(io: &Io, stream: &mut TcpStream) -> Result<(), Error<SchedError>> {
                     }
                 })?;
             }
-
             Request::SetLogFilter(level) => {
                 info!("changing log level to {}", level);
                 log::set_max_level(level);
                 Reply::Success.write_to(stream)?;
             }
-
             Request::SetUartLogFilter(level) => {
                 info!("changing UART log level to {}", level);
                 BufferLogger::with(|logger|
@@ -78,6 +74,34 @@ fn worker(io: &Io, stream: &mut TcpStream) -> Result<(), Error<SchedError>> {
                 Reply::Success.write_to(stream)?;
             }
 
+            Request::ConfigRead { ref key } => {
+                config::read(key, |result| {
+                    match result {
+                        Ok(value) => Reply::ConfigData(&value).write_to(stream),
+                        Err(_)    => Reply::Error.write_to(stream)
+                    }
+                })?;
+            }
+            Request::ConfigWrite { ref key, ref value } => {
+                match config::write(key, value) {
+                    Ok(_)  => Reply::Success.write_to(stream),
+                    Err(_) => Reply::Error.write_to(stream)
+                }?;
+            }
+            Request::ConfigRemove { ref key } => {
+                match config::remove(key) {
+                    Ok(()) => Reply::Success.write_to(stream),
+                    Err(_) => Reply::Error.write_to(stream)
+                }?;
+
+            }
+            Request::ConfigErase => {
+                match config::erase() {
+                    Ok(()) => Reply::Success.write_to(stream),
+                    Err(_) => Reply::Error.write_to(stream)
+                }?;
+            }
+
             Request::StartProfiler { interval_us, hits_size, edges_size } => {
                 match profiler::start(interval_us as u64,
                                       hits_size as usize, edges_size as usize) {
@@ -85,12 +109,10 @@ fn worker(io: &Io, stream: &mut TcpStream) -> Result<(), Error<SchedError>> {
                     Err(()) => Reply::Unavailable.write_to(stream)?
                 }
             }
-
             Request::StopProfiler => {
                 profiler::stop();
                 Reply::Success.write_to(stream)?;
             }
-
             Request::GetProfile => {
                 profiler::pause(|profile| {
                     let profile = match profile {
@@ -130,7 +152,6 @@ fn worker(io: &Io, stream: &mut TcpStream) -> Result<(), Error<SchedError>> {
                 warn!("hotswapping firmware");
                 unsafe { boot::hotswap(&firmware) }
             }
-
             Request::Reboot => {
                 Reply::RebootImminent.write_to(stream)?;
                 stream.close()?;

artiq/firmware/runtime/session.rs

@@ -250,36 +250,6 @@ fn process_host_message(io: &Io,
             session.congress.finished_cleanly.set(true)
         }
 
-        // artiq_coreconfig
-        host::Request::FlashRead { ref key } => {
-            config::read(key, |result| {
-                match result {
-                    Ok(value) => host_write(stream, host::Reply::FlashRead(&value)),
-                    Err(_)    => host_write(stream, host::Reply::FlashError)
-                }
-            })?;
-        }
-        host::Request::FlashWrite { ref key, ref value } => {
-            match config::write(key, value) {
-                Ok(_)  => host_write(stream, host::Reply::FlashOk),
-                Err(_) => host_write(stream, host::Reply::FlashError)
-            }?;
-        }
-        host::Request::FlashRemove { ref key } => {
-            match config::remove(key) {
-                Ok(()) => host_write(stream, host::Reply::FlashOk),
-                Err(_) => host_write(stream, host::Reply::FlashError),
-            }?;
-
-        }
-        host::Request::FlashErase => {
-            match config::erase() {
-                Ok(()) => host_write(stream, host::Reply::FlashOk),
-                Err(_) => host_write(stream, host::Reply::FlashError),
-            }?;
-        }
-
-        // artiq_run/artiq_master
         host::Request::SwitchClock(clk) => {
             if session.running() {
                 unexpected!("attempted to switch RTIO clock while a kernel was running")

artiq/frontend/artiq_coremgmt.py

@@ -67,11 +67,11 @@ def get_argparser():
                          help="key and file whose content to be written to "
                               "core device config")
 
-    p_delete = subparsers.add_parser("delete",
+    p_remove = subparsers.add_parser("remove",
-                                     help="delete key from core device config")
+                                     help="remove key from core device config")
-    p_delete.add_argument("key", metavar="KEY", nargs=argparse.REMAINDER,
+    p_remove.add_argument("key", metavar="KEY", nargs=argparse.REMAINDER,
                           default=[], type=str,
-                          help="key to be deleted from core device config")
+                          help="key to be removed from core device config")
 
     subparsers.add_parser("erase", help="fully erase core device config")
 
@@ -134,11 +134,8 @@ def main():
     device_mgr = DeviceManager(DeviceDB(args.device_db))
     try:
         core_addr = DeviceDB(args.device_db).get("core")["arguments"]["host"]
-        kern = CommKernel(core_addr)
         mgmt = CommMgmt(core_addr)
 
-        kern.check_system_info()
-
         if args.tool == "log":
             if args.action == "set_level":
                 mgmt.set_log_level(args.level)
@@ -151,22 +148,22 @@ def main():
 
         if args.tool == "config":
             if args.action == "read":
-                value = kern.flash_storage_read(args.key)
+                value = mgmt.config_read(args.key)
                 if not value:
                     print("Key {} does not exist".format(args.key))
                 else:
                     print(value)
             if args.action == "write":
                 for key, value in args.string:
-                    kern.flash_storage_write(key, value.encode("utf-8"))
+                    mgmt.config_write(key, value.encode("utf-8"))
                 for key, filename in args.file:
                     with open(filename, "rb") as fi:
-                        kern.flash_storage_write(key, fi.read())
+                        mgmt.config_write(key, fi.read())
-            if args.action == "delete":
+            if args.action == "remove":
                 for key in args.key:
-                    kern.flash_storage_remove(key)
+                    mgmt.config_remove(key)
             if args.action == "erase":
-                kern.flash_storage_erase()
+                mgmt.config_erase()
 
         if args.tool == "reboot":
             mgmt.reboot()