From ca93b94aeaa1c38504235c35ed103896884c0df1 Mon Sep 17 00:00:00 2001 From: whitequark Date: Wed, 16 May 2018 14:32:49 +0000 Subject: [PATCH] firmware: move config requests to management protocol. They were only in session protocol because of historical reasons. --- artiq/coredevice/comm_kernel.py | 39 -------- artiq/coredevice/comm_mgmt.py | 37 ++++++++ artiq/firmware/libproto_artiq/mgmt_proto.rs | 48 +++++++++- .../firmware/libproto_artiq/session_proto.rs | 94 +++++++------------ artiq/firmware/runtime/mgmt.rs | 37 ++++++-- artiq/firmware/runtime/session.rs | 30 ------ artiq/frontend/artiq_coremgmt.py | 23 ++--- 7 files changed, 157 insertions(+), 151 deletions(-) diff --git a/artiq/coredevice/comm_kernel.py b/artiq/coredevice/comm_kernel.py index c02f226e7..d8df2d878 100644 --- a/artiq/coredevice/comm_kernel.py +++ b/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) diff --git a/artiq/coredevice/comm_mgmt.py b/artiq/coredevice/comm_mgmt.py index f235c584a..6d8438495 100644 --- a/artiq/coredevice/comm_mgmt.py +++ b/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) diff --git a/artiq/firmware/libproto_artiq/mgmt_proto.rs b/artiq/firmware/libproto_artiq/mgmt_proto.rs index 9c44397e8..ffa2a9763 100644 --- a/artiq/firmware/libproto_artiq/mgmt_proto.rs +++ b/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 { @@ -12,6 +13,8 @@ pub enum Error { UnknownPacket(u8), #[fail(display = "unknown log level {}", _0)] UnknownLogLevel(u8), + #[fail(display = "invalid UTF-8: {}", _0)] + Utf8(Utf8Error), #[fail(display = "{}", _0)] Io(#[cause] IoError) } @@ -22,6 +25,15 @@ impl From> for Error { } } +impl From>> for Error { + fn from(value: ReadStringError>) -> Error { + match value { + ReadStringError::Utf8(err) => Error::Utf8(err), + ReadStringError::Other(err) => Error::Io(err) + } + } +} + pub fn read_magic(reader: &mut R) -> Result<(), Error> 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 }, + 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 diff --git a/artiq/firmware/libproto_artiq/session_proto.rs b/artiq/firmware/libproto_artiq/session_proto.rs index 7540ba513..54da5c5d3 100644 --- a/artiq/firmware/libproto_artiq/session_proto.rs +++ b/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 }, - FlashRemove { key: String }, - FlashErase, -} - -impl Request { - pub fn read_from(reader: &mut R) -> Result> - 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(reader: &mut R) -> Result> + 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(&self, writer: &mut W) -> Result<(), IoError> where W: Write + ?Sized @@ -218,16 +203,7 @@ impl<'a> Reply<'a> { writer.write_u8(async as u8)?; }, - Reply::FlashRead(ref bytes) => { - writer.write_u8(11)?; - writer.write_bytes(bytes)?; - }, - Reply::FlashOk => { - writer.write_u8(12)?; - }, - Reply::FlashError => { - writer.write_u8(13)?; - }, + // 11-13 were flash requests Reply::WatchdogExpired => { writer.write_u8(14)?; diff --git a/artiq/firmware/runtime/mgmt.rs b/artiq/firmware/runtime/mgmt.rs index 222108885..c03682e8a 100644 --- a/artiq/firmware/runtime/mgmt.rs +++ b/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> { Reply::LogContent(buffer.extract()).write_to(stream) })?; } - Request::ClearLog => { BufferLogger::with(|logger| -> Result<(), Error> { let mut buffer = io.until_ok(|| logger.buffer())?; @@ -34,7 +33,6 @@ fn worker(io: &Io, stream: &mut TcpStream) -> Result<(), Error> { Reply::Success.write_to(stream)?; } - Request::PullLog => { BufferLogger::with(|logger| -> Result<(), Error> { loop { @@ -64,13 +62,11 @@ fn worker(io: &Io, stream: &mut TcpStream) -> Result<(), Error> { } })?; } - 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> { 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> { 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> { warn!("hotswapping firmware"); unsafe { boot::hotswap(&firmware) } } - Request::Reboot => { Reply::RebootImminent.write_to(stream)?; stream.close()?; diff --git a/artiq/firmware/runtime/session.rs b/artiq/firmware/runtime/session.rs index cbef8d7d8..ca98fdbb6 100644 --- a/artiq/firmware/runtime/session.rs +++ b/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") diff --git a/artiq/frontend/artiq_coremgmt.py b/artiq/frontend/artiq_coremgmt.py index 6013b7359..d611b711c 100644 --- a/artiq/frontend/artiq_coremgmt.py +++ b/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", - help="delete key from core device config") - p_delete.add_argument("key", metavar="KEY", nargs=argparse.REMAINDER, + p_remove = subparsers.add_parser("remove", + help="remove key from core device config") + 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()) - if args.action == "delete": + mgmt.config_write(key, fi.read()) + 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()