Skip to content

Commit

Permalink
resolve some TODOs
Browse files Browse the repository at this point in the history
  • Loading branch information
@lzrd
lzrd committed Feb 9, 2025
1 parent a794699 commit 165bbb4
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 49 deletions.
1 change: 0 additions & 1 deletion app/oxide-rot-1/app.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ start = true
name = "lpc55-update-server"
priority = 3
max-sizes = {flash = 26080, ram = 17000, usbsram = 4096}
# TODO: Size this appropriately
stacksize = 8192
start = true
sections = {bootstate = "usbsram"}
Expand Down
13 changes: 11 additions & 2 deletions drv/lpc55-swd/build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,20 @@ where

// Do a sanity check on the size of the blob based on what we've seen.
//
// Note that clippy ignores the profile (or equivalent) when building the
// Notes:
//
// Clippy ignores the profile (or equivalent) when building the
// endoscope blob which results in an enormous binary. Since we're not
// going to actually build the swd task when running clippy, the large
// size can be ignored.
// TODO: Track down an existing cargo or rust bug or create a new one.
//
// The artifact-specific profiles do not allow `lto` to be
// specified. That would save at least 500 bytes of executable.
// In Cargo.toml, see [profile.*.package.endoscope]
//
// TODO: Add to or create relevant cargo bug(s):
// e.g. https://github.com/rust-lang/cargo/issues/11680
//
#[cfg(not(clippy))]
if bin_size > 6 * 1024 {
bail!("bin_size of {bin_size} is over 6KiB. Was it built with the wrong profile?");
Expand Down
79 changes: 33 additions & 46 deletions drv/lpc55-swd/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ use idol_runtime::{
use lpc55_pac as device;
use ringbuf::*;
use userlib::{
hl, set_timer_relative, sys_get_timer, sys_irq_control, sys_set_timer,
task_slot, RecvMessage, TaskId, UnwrapLite,
sys_irq_control_clear_pending,
hl, set_timer_relative, sys_get_timer, sys_irq_control,
sys_irq_control_clear_pending, sys_set_timer, task_slot, RecvMessage,
TaskId, UnwrapLite,
};
use zerocopy::AsBytes;

Expand Down Expand Up @@ -602,7 +602,6 @@ impl idl::InOrderSpCtrlImpl for ServerImpl {
/// Remote debugging support.
/// Yet another way to reset the SP. This one is known to finish before
/// any other code in this task runs.
/// TODO: XXX: put this behind a feature
#[cfg(feature = "enable_ext_sp_reset")]
fn db_reset_sp(
&mut self,
Expand All @@ -614,8 +613,7 @@ impl idl::InOrderSpCtrlImpl for ServerImpl {
// Don't clean up interrupt state, we want to trigger a measurement.
let _ = self.swd_setup();
let _ = self.dp_write_bitflags::<Demcr>(Demcr::from_bits_retain(0));
let _ =
self.dp_write_bitflags::<Dhcsr>(Dhcsr::end_debug());
let _ = self.dp_write_bitflags::<Dhcsr>(Dhcsr::end_debug());
self.swd_finish();
self.sp_reset_leave(false);
self.init = false;
Expand All @@ -630,7 +628,6 @@ impl idl::InOrderSpCtrlImpl for ServerImpl {
) -> Result<(), RequestError<core::convert::Infallible>> {
return Err(ClientError::AccessViolation.fail());
}

}

impl NotificationHandler for ServerImpl {
Expand Down Expand Up @@ -706,12 +703,14 @@ impl NotificationHandler for ServerImpl {
// handled successfully.
//}

// TODO: Get rid of spurious interrupts cause by do_handle_sp_reset()
// toggling SP_RESET.

const SLOT: PintSlot = ROT_TO_SP_RESET_L_IN_PINT_SLOT;
let _ = gpio.pint_op(SLOT, PintOp::Clear, PintCondition::Status);
sys_irq_control(notifications::SP_RESET_IRQ_MASK, true);
// Get rid of spurious interrupts cause by do_handle_sp_reset()
// toggling SP_RESET.
sys_irq_control_clear_pending(
notifications::SP_RESET_IRQ_MASK,
true,
);
ringbuf_entry!(Trace::EndOfNotificationHandler);
}

Expand Down Expand Up @@ -1115,7 +1114,6 @@ impl ServerImpl {
return Err(Ack::Fault);
}
let mut limit = 37;
// TODO: Can be more efficient using transactions for write & verify?
for slice in data.chunks(4) {
let word =
u32::from_le_bytes(slice[0..=3].try_into().unwrap_lite());
Expand Down Expand Up @@ -1187,7 +1185,8 @@ impl ServerImpl {
}

fn dp_write_bitflags<T>(&mut self, val: T) -> Result<(), Ack>
where T: bitflags::Flags<Bits = u32> + DpAddressable,
where
T: bitflags::Flags<Bits = u32> + DpAddressable,
{
self.write_single_target_addr(T::addr(), val.bits())
}
Expand Down Expand Up @@ -1330,10 +1329,6 @@ impl ServerImpl {
PintOp::Clear,
PintCondition::Falling,
);
// We've cleared the underlying interrupt cause but
// need to also clear the pending interrupt that we
// just generated.
sys_irq_control_clear_pending(notifications::SP_RESET_IRQ_MASK, true);
}
}

Expand Down Expand Up @@ -1395,9 +1390,7 @@ impl ServerImpl {
});

// Resume execution by turning off DHCSR_C_HALT
if let Err(e) =
self.dp_write_bitflags::<Dhcsr>(Dhcsr::resume())
{
if let Err(e) = self.dp_write_bitflags::<Dhcsr>(Dhcsr::resume()) {
ringbuf_entry!(Trace::AckErr(e));
return Err(());
}
Expand Down Expand Up @@ -1596,10 +1589,6 @@ impl ServerImpl {
// The dump client is trying to be non-intrusive and will try to resume
// the SP after finishing its work, the last call being `resume` or, if
// that fails, `setup` followed by `resume`.
// TODO: Client state or a session ID could be maintained.
// The TaskId of the current client (includeing task swd's TaskID) can be
// tested and an error returned if there is not a match.
// Calls to setup (or do_setup) would change the stored client TaskId.
if !self.init {
ringbuf_entry!(Trace::PinSetupDefaults);
// This should be redundant since setup_pins needs to be
Expand Down Expand Up @@ -1663,10 +1652,7 @@ impl ServerImpl {
}

fn do_resume(&mut self) -> Result<(), SpCtrlError> {
if self
.dp_write_bitflags::<Dhcsr>(Dhcsr::resume())
.is_ok()
{
if self.dp_write_bitflags::<Dhcsr>(Dhcsr::resume()).is_ok() {
ringbuf_entry!(Trace::Resumed);
Ok(())
} else {
Expand Down Expand Up @@ -1701,10 +1687,10 @@ impl ServerImpl {
.pint_op(SLOT, PintOp::Detected, PintCondition::Falling)
.map_or(false, |v| v.unwrap_lite())
{
// This is a "spurious" intrerrupt that can probably be eliminated.
// Cases where we assert SP_RESET then clean-up the PINT condition will
// still have a pending notification.
// TODO: clean that up.
// Use of sys_irq_control_clear_pending(...) should avoid
// appearance of a "spurious" intrerrupt.
// Otherwise, cases where we assert SP_RESET then clean-up the PINT
// condition will have a pending notification.
ringbuf_entry!(Trace::SpResetNotAsserted);
return true; // no work required.
}
Expand Down Expand Up @@ -1754,21 +1740,20 @@ impl ServerImpl {
self.sp_reset_enter();
*undo |= Undo::RESET;

// TODO: What is the required assertion time for SP RESET if any?
// It is likely less than 1ms.
// Asserting SP_RESET for >1ms here works.
hl::sleep_for(1);

if self
.dp_write_bitflags::<Dhcsr>(Dhcsr::resume())
.is_err()
{
if self.dp_write_bitflags::<Dhcsr>(Dhcsr::resume()).is_err() {
ringbuf_entry!(Trace::DemcrWriteError);
*undo |= Undo::DEBUGEN;
return Err(());
}
*undo |= Undo::DEBUGEN;

if self.dp_write_bitflags::<Demcr>(Demcr::VC_CORERESET).is_err() {
if self
.dp_write_bitflags::<Demcr>(Demcr::VC_CORERESET)
.is_err()
{
ringbuf_entry!(Trace::DemcrWriteError);
*undo |= Undo::VC_CORERESET;
return Err(());
Expand All @@ -1793,7 +1778,10 @@ impl ServerImpl {
}

// We don't want to catch the next reset.
if self.dp_write_bitflags::<Demcr>(Demcr::from_bits_retain(0)).is_err() {
if self
.dp_write_bitflags::<Demcr>(Demcr::from_bits_retain(0))
.is_err()
{
ringbuf_entry!(Trace::DemcrWriteError);
return Err(());
}
Expand Down Expand Up @@ -1841,7 +1829,10 @@ impl ServerImpl {
ringbuf_entry!(Trace::IncompleteUndo(need_undo));
error = true;

if self.dp_write_bitflags::<Demcr>(Demcr::from_bits_retain(0)).is_ok() {
if self
.dp_write_bitflags::<Demcr>(Demcr::from_bits_retain(0))
.is_ok()
{
need_undo &= !Undo::VC_CORERESET;
ringbuf_entry!(Trace::VcCoreReset(false));
} else {
Expand All @@ -1867,10 +1858,7 @@ impl ServerImpl {

// Unless `prep` failed, this will always be needed.
if need_undo & Undo::DEBUGEN == Undo::DEBUGEN {
if self
.dp_write_bitflags::<Dhcsr>(Dhcsr::end_debug())
.is_ok()
{
if self.dp_write_bitflags::<Dhcsr>(Dhcsr::end_debug()).is_ok() {
need_undo &= !Undo::DEBUGEN;
} else {
ringbuf_entry!(Trace::DhcsrWriteError);
Expand Down Expand Up @@ -1983,7 +1971,6 @@ fn main() -> ! {

let mut incoming = [0; idl::INCOMING_SIZE];


// TODO: If this is a power-on situation and SP and RoT are booting
// at nearly the same time, can that be detected? That may be a
// case where it is ok for the RoT to reset the SP and measure it.
Expand Down

0 comments on commit 165bbb4

Please sign in to comment.