Support channel names in RTIO errors #209

Merged
sb10q merged 3 commits from esavkin/artiq-zynq:1697-rtiomap into master 2023-01-09 12:35:56 +08:00
7 changed files with 91 additions and 37 deletions

View File

@ -1,4 +1,4 @@
use core::fmt;
use core::{fmt, slice, str};
use core::cell::RefCell;
use alloc::{vec, vec::Vec, string::String, collections::BTreeMap, rc::Rc};
use log::{info, warn, error};
@ -31,7 +31,7 @@ use crate::rpc;
use crate::moninj;
use crate::mgmt;
use crate::analyzer;
use crate::rtio_mgt;
use crate::rtio_mgt::{self, resolve_channel_name};
#[cfg(has_drtio)]
use crate::pl;
@ -246,7 +246,16 @@ async fn handle_run_kernel(stream: Option<&TcpStream>, control: &Rc<RefCell<kern
for exception in exceptions.iter() {
let exception = exception.as_ref().unwrap();
write_i32(stream, exception.id as i32).await?;
write_exception_string(stream, exception.message).await?;
if exception.message.len() == usize::MAX { // exception with host string
esavkin marked this conversation as resolved Outdated
Outdated
Review

Obviously, all review comments from the ARTIQ PR are also applicable here.

Obviously, all review comments from the ARTIQ PR are also applicable here.
Outdated
Review

Obviously, all review comments from the ARTIQ PR are also applicable here.

Obviously, all review comments from the ARTIQ PR are also applicable here.
Outdated
Review

Obviously, all review comments from the ARTIQ PR are also applicable here.

Obviously, all review comments from the ARTIQ PR are also applicable here.
Outdated
Review

Obviously, all review comments from the ARTIQ PR are also applicable here.

Obviously, all review comments from the ARTIQ PR are also applicable here.
Outdated
Review

Obviously, all review comments from the ARTIQ PR are also applicable here.

Obviously, all review comments from the ARTIQ PR are also applicable here.
write_exception_string(stream, exception.message).await?;
} else {
let msg = str::from_utf8(unsafe { slice::from_raw_parts(exception.message.as_ptr(), exception.message.len()) })
.unwrap()
.replace("{rtio_channel_info:0}", &format!("{}:{}", exception.param[0], resolve_channel_name(exception.param[0] as u32)));
write_exception_string(stream, unsafe { CSlice::new(msg.as_ptr(), msg.len()) }).await?;
}
write_i64(stream, exception.param[0] as i64).await?;
write_i64(stream, exception.param[1] as i64).await?;
write_i64(stream, exception.param[2] as i64).await?;
@ -423,7 +432,7 @@ pub fn main(timer: GlobalTimer, cfg: Config) {
#[cfg(has_drtio_routing)]
drtio_routing::interconnect_disable_all();
rtio_mgt::startup(&aux_mutex, &drtio_routing_table, &up_destinations, timer);
rtio_mgt::startup(&aux_mutex, &drtio_routing_table, &up_destinations, timer, &cfg);
analyzer::start();
moninj::start(timer, aux_mutex, drtio_routing_table);

View File

@ -434,6 +434,7 @@ macro_rules! artiq_raise {
($name:expr, $message:expr, $param0:expr, $param1:expr, $param2:expr) => ({
use cslice::AsCSlice;
let name_id = $crate::eh_artiq::get_exception_id($name);
let message_cl = $message.clone();
let exn = $crate::eh_artiq::Exception {
id: name_id,
file: file!().as_c_slice(),
@ -441,7 +442,7 @@ macro_rules! artiq_raise {
column: column!(),
// https://github.com/rust-lang/rfcs/pull/1719
function: "(Rust function)".as_c_slice(),
message: $message.as_c_slice(),
message: message_cl.as_c_slice(),
param: [$param0, $param1, $param2]
};
#[allow(unused_unsafe)]

View File

@ -198,13 +198,13 @@ pub extern fn dma_playback(timestamp: i64, ptr: i32) {
csr::rtio_dma::error_write(1);
if error & 1 != 0 {
artiq_raise!("RTIOUnderflow",
"RTIO underflow at {0} mu, channel {1}",
timestamp as i64, channel as i64, 0);
"RTIO underflow at {1} mu, channel {rtio_channel_info:0}",
Outdated
Review

Why is it {{1}} here and {1} in artiq?

Why is it ``{{1}}`` here and ``{1}`` in ``artiq``?
Outdated
Review

This remains unanswered and unaddressed.

This remains unanswered and unaddressed.

In this version format! macro is used. In artiq - it is not

In this version `format!` macro is used. In artiq - it is not
channel as i64, timestamp as i64, 0);
}
if error & 2 != 0 {
artiq_raise!("RTIODestinationUnreachable",
"RTIO destination unreachable, output, at {0} mu, channel {1}",
timestamp as i64, channel as i64, 0);
"RTIO destination unreachable, output, at {1} mu, channel {rtio_channel_info:0}",
channel as i64, timestamp as i64, 0);
}
}
}

View File

@ -9,6 +9,7 @@
#![feature(naked_functions)]
#![feature(asm)]
#[macro_use]
Outdated
Review

Why?

Why?

format macro wasn't available

format macro wasn't available
extern crate alloc;
use log::{info, warn, error};
@ -70,16 +71,16 @@ async fn report_async_rtio_errors() {
unsafe {
let errors = pl::csr::rtio_core::async_error_read();
if errors & ASYNC_ERROR_COLLISION != 0 {
error!("RTIO collision involving channel {}",
pl::csr::rtio_core::collision_channel_read());
let channel = pl::csr::rtio_core::collision_channel_read();
error!("RTIO collision involving channel {}:{}", channel, rtio_mgt::resolve_channel_name(channel as u32));
}
if errors & ASYNC_ERROR_BUSY != 0 {
error!("RTIO busy error involving channel {}",
pl::csr::rtio_core::busy_channel_read());
let channel = pl::csr::rtio_core::busy_channel_read();
error!("RTIO busy error involving channel {}:{}", channel, rtio_mgt::resolve_channel_name(channel as u32));
}
if errors & ASYNC_ERROR_SEQUENCE_ERROR != 0 {
error!("RTIO sequence error involving channel {}",
pl::csr::rtio_core::sequence_error_channel_read());
let channel = pl::csr::rtio_core::sequence_error_channel_read();
error!("RTIO sequence error involving channel {}:{}", channel, rtio_mgt::resolve_channel_name(channel as u32));
}
SEEN_ASYNC_ERRORS = errors;
pl::csr::rtio_core::async_error_write(errors);

View File

@ -5,6 +5,7 @@ use crate::artiq_raise;
use core::sync::atomic::{fence, Ordering};
use crate::pl::csr;
use crate::rtio_mgt::resolve_channel_name;
pub const RTIO_O_STATUS_WAIT: i32 = 1;
pub const RTIO_O_STATUS_UNDERFLOW: i32 = 2;
@ -87,12 +88,12 @@ unsafe fn process_exceptional_status(channel: i32, status: i32) {
}
if status & RTIO_O_STATUS_UNDERFLOW != 0 {
artiq_raise!("RTIOUnderflow",
"RTIO underflow at {0} mu, channel {1}, slack {2} mu",
timestamp, channel as i64, timestamp - get_counter());
format!("RTIO underflow at {{1}} mu, channel {}:{}, slack {{2}} mu", channel, resolve_channel_name(channel as u32)),
channel as i64, timestamp, timestamp - get_counter());
}
if status & RTIO_O_STATUS_DESTINATION_UNREACHABLE != 0 {
artiq_raise!("RTIODestinationUnreachable",
"RTIO destination unreachable, output, at {0} mu, channel {1}",
format!("RTIO destination unreachable, output, at {{0}} mu, channel {}:{}", channel, resolve_channel_name(channel as u32)),
timestamp, channel as i64, 0);
}
}
@ -176,7 +177,7 @@ pub extern fn input_timestamp(timeout: i64, channel: i32) -> i64 {
if status & RTIO_I_STATUS_OVERFLOW != 0 {
artiq_raise!("RTIOOverflow",
"RTIO input overflow on channel {0}",
format!("RTIO input overflow on channel {}:{}", channel, resolve_channel_name(channel as u32)),
channel as i64, 0, 0);
}
if status & RTIO_I_STATUS_WAIT_EVENT != 0 {
@ -184,7 +185,7 @@ pub extern fn input_timestamp(timeout: i64, channel: i32) -> i64 {
}
if status & RTIO_I_STATUS_DESTINATION_UNREACHABLE != 0 {
artiq_raise!("RTIODestinationUnreachable",
"RTIO destination unreachable, input, on channel {0}",
format!("RTIO destination unreachable, input, on channel {}:{}", channel, resolve_channel_name(channel as u32)),
channel as i64, 0, 0);
}
@ -214,12 +215,12 @@ pub extern fn input_data(channel: i32) -> i32 {
if status & RTIO_I_STATUS_OVERFLOW != 0 {
artiq_raise!("RTIOOverflow",
"RTIO input overflow on channel {0}",
format!("RTIO input overflow on channel {}:{}", channel, resolve_channel_name(channel as u32)),
channel as i64, 0, 0);
}
if status & RTIO_I_STATUS_DESTINATION_UNREACHABLE != 0 {
artiq_raise!("RTIODestinationUnreachable",
"RTIO destination unreachable, input, on channel {0}",
format!("RTIO destination unreachable, input, on channel {}:{}", channel, resolve_channel_name(channel as u32)),
channel as i64, 0, 0);
}
@ -249,12 +250,12 @@ pub extern fn input_timestamped_data(timeout: i64, channel: i32) -> TimestampedD
if status & RTIO_I_STATUS_OVERFLOW != 0 {
artiq_raise!("RTIOOverflow",
"RTIO input overflow on channel {0}",
format!("RTIO input overflow on channel {}:{}", channel, resolve_channel_name(channel as u32)),
channel as i64, 0, 0);
}
if status & RTIO_I_STATUS_DESTINATION_UNREACHABLE != 0 {
artiq_raise!("RTIODestinationUnreachable",
"RTIO destination unreachable, input, on channel {0}",
format!("RTIO destination unreachable, input, on channel {}:{}", channel, resolve_channel_name(channel as u32)),
channel as i64, 0, 0);
}

View File

@ -4,6 +4,7 @@ use cslice::CSlice;
use crate::artiq_raise;
use crate::pl::csr;
use crate::rtio_mgt::resolve_channel_name;
pub const RTIO_O_STATUS_WAIT: u8 = 1;
pub const RTIO_O_STATUS_UNDERFLOW: u8 = 2;
@ -72,12 +73,12 @@ unsafe fn process_exceptional_status(channel: i32, status: u8) {
}
if status & RTIO_O_STATUS_UNDERFLOW != 0 {
artiq_raise!("RTIOUnderflow",
"RTIO underflow at {0} mu, channel {1}, slack {2} mu",
timestamp, channel as i64, timestamp - get_counter());
format!("RTIO underflow at {{1}} mu, channel {}:{}, slack {{2}} mu", channel, resolve_channel_name(channel as u32)),
Outdated
Review

No. It's not "at channel".

No. It's not "at channel".
channel as i64, timestamp, timestamp - get_counter());
}
if status & RTIO_O_STATUS_DESTINATION_UNREACHABLE != 0 {
artiq_raise!("RTIODestinationUnreachable",
"RTIO destination unreachable, output, at {0} mu, channel {1}",
format!("RTIO destination unreachable, output, at {{0}} mu, channel {}:{}", channel, resolve_channel_name(channel as u32)),
timestamp, channel as i64, 0);
}
}
@ -120,7 +121,7 @@ pub extern fn input_timestamp(timeout: i64, channel: i32) -> i64 {
if status & RTIO_I_STATUS_OVERFLOW != 0 {
artiq_raise!("RTIOOverflow",
"RTIO input overflow on channel {0}",
format!("RTIO input overflow on channel {}:{}", channel, resolve_channel_name(channel as u32)),
channel as i64, 0, 0);
}
if status & RTIO_I_STATUS_WAIT_EVENT != 0 {
@ -128,7 +129,7 @@ pub extern fn input_timestamp(timeout: i64, channel: i32) -> i64 {
}
if status & RTIO_I_STATUS_DESTINATION_UNREACHABLE != 0 {
artiq_raise!("RTIODestinationUnreachable",
"RTIO destination unreachable, input, on channel {0}",
format!("RTIO destination unreachable, input, on channel {}:{}", channel, resolve_channel_name(channel as u32)),
channel as i64, 0, 0);
}
@ -148,12 +149,12 @@ pub extern fn input_data(channel: i32) -> i32 {
if status & RTIO_I_STATUS_OVERFLOW != 0 {
artiq_raise!("RTIOOverflow",
"RTIO input overflow on channel {0}",
format!("RTIO input overflow on channel {}:{}", channel, resolve_channel_name(channel as u32)),
channel as i64, 0, 0);
}
if status & RTIO_I_STATUS_DESTINATION_UNREACHABLE != 0 {
artiq_raise!("RTIODestinationUnreachable",
"RTIO destination unreachable, input, on channel {0}",
format!("RTIO destination unreachable, input, on channel {}:{}", channel, resolve_channel_name(channel as u32)),
channel as i64, 0, 0);
}
@ -173,7 +174,7 @@ pub extern fn input_timestamped_data(timeout: i64, channel: i32) -> TimestampedD
if status & RTIO_I_STATUS_OVERFLOW != 0 {
artiq_raise!("RTIOOverflow",
"RTIO input overflow on channel {0}",
format!("RTIO input overflow on channel {}:{}", channel, resolve_channel_name(channel as u32)),
channel as i64, 0, 0);
}
if status & RTIO_I_STATUS_WAIT_EVENT != 0 {
@ -181,7 +182,7 @@ pub extern fn input_timestamped_data(timeout: i64, channel: i32) -> TimestampedD
}
if status & RTIO_I_STATUS_DESTINATION_UNREACHABLE != 0 {
artiq_raise!("RTIODestinationUnreachable",
"RTIO destination unreachable, input, on channel {0}",
format!("RTIO destination unreachable, input, on channel {}:{}", channel, resolve_channel_name(channel as u32)),
channel as i64, 0, 0);
}

View File

@ -1,8 +1,15 @@
use core::cell::RefCell;
use alloc::rc::Rc;
use alloc::collections::BTreeMap;
use alloc::string::String;
use libboard_zynq::timer::GlobalTimer;
use libboard_artiq::{pl::csr, drtio_routing};
use libcortex_a9::mutex::Mutex;
use libconfig::Config;
use io::{Cursor, ProtoRead};
use log::error;
static mut RTIO_DEVICE_MAP: BTreeMap<u32, String> = BTreeMap::new();
#[cfg(has_drtio)]
@ -219,15 +226,15 @@ pub mod drtio {
destination_set_up(routing_table, up_destinations, destination, false).await,
Ok(Packet::DestinationOkReply) => (),
Ok(Packet::DestinationSequenceErrorReply { channel }) =>{
error!("[DEST#{}] RTIO sequence error involving channel 0x{:04x}", destination, channel);
error!("[DEST#{}] RTIO sequence error involving channel {} 0x{:04x}", destination, resolve_channel_name(channel), channel);
Outdated
Review

Message format is inconsistent with artiq.

Message format is inconsistent with ``artiq``.
unsafe { SEEN_ASYNC_ERRORS |= ASYNC_ERROR_SEQUENCE_ERROR };
}
Ok(Packet::DestinationCollisionReply { channel }) =>{
error!("[DEST#{}] RTIO collision involving channel 0x{:04x}", destination, channel);
error!("[DEST#{}] RTIO collision involving channel {} 0x{:04x}", destination, resolve_channel_name(channel), channel);
unsafe { SEEN_ASYNC_ERRORS |= ASYNC_ERROR_COLLISION };
}
Ok(Packet::DestinationBusyReply { channel }) =>{
error!("[DEST#{}] RTIO busy error involving channel 0x{:04x}", destination, channel);
error!("[DEST#{}] RTIO busy error involving channel {} 0x{:04x}", destination, resolve_channel_name(channel), channel);
unsafe { SEEN_ASYNC_ERRORS |= ASYNC_ERROR_BUSY };
}
Ok(packet) => error!("[DEST#{}] received unexpected aux packet: {:?}", destination, packet),
@ -332,6 +339,38 @@ pub mod drtio {
}
}
fn read_device_map(cfg: &Config) -> BTreeMap<u32, String> {
let mut device_map: BTreeMap<u32, String> = BTreeMap::new();
let _ = cfg.read("device_map").and_then(|raw_bytes| {
let mut bytes_cr = Cursor::new(raw_bytes);
let size = bytes_cr.read_u32().unwrap();
for _ in 0..size {
let channel = bytes_cr.read_u32().unwrap();
let device_name = bytes_cr.read_string().unwrap();
if let Some(old_entry) = device_map.insert(channel, device_name.clone()) {
error!("read_device_map: conflicting entries for channel {}: `{}` and `{}`",
channel, old_entry, device_name);
}
}
Ok(())
} ).or_else(|err| {
error!("read_device_map: error reading `device_map` from config: {}", err);
Outdated
Review

Why do you have the "read_device_map:" prefix here and not just above?

Why do you have the "read_device_map:" prefix here and not just above?
Err(err)
});
device_map
}
fn _resolve_channel_name(channel: u32, device_map: &BTreeMap<u32, String>) -> String {
match device_map.get(&channel) {
Some(val) => val.clone(),
None => String::from("unknown")
}
}
pub fn resolve_channel_name(channel: u32) -> String {
_resolve_channel_name(channel, unsafe{&RTIO_DEVICE_MAP})
}
#[cfg(not(has_drtio))]
pub mod drtio {
use super::*;
@ -346,7 +385,9 @@ pub mod drtio {
pub fn startup(aux_mutex: &Rc<Mutex<bool>>,
routing_table: &Rc<RefCell<drtio_routing::RoutingTable>>,
up_destinations: &Rc<RefCell<[bool; drtio_routing::DEST_COUNT]>>,
timer: GlobalTimer) {
timer: GlobalTimer,
cfg: &Config) {
unsafe { RTIO_DEVICE_MAP = read_device_map(cfg); }
drtio::startup(aux_mutex, routing_table, up_destinations, timer);
unsafe {
csr::rtio_core::reset_phy_write(1);