M-Labs
/
artiq-zynq
10
1
Fork 19
Code Issues 13 Pull Requests Releases Wiki Activity

Support CoreMgmt over DRTIO on Zynq Devices #323

Merged
sb10q merged 27 commits from occheung/artiq-zynq:drtio-coremgmt into master 2024-11-19 18:55:03 +08:00
Conversation 1 Commits 27 Files Changed 10 +1644 -97
occheung commented 2024-08-28 13:44:53 +08:00
Owner
Copy Link

Summary

The corresponding Kasli-SoC PR for ARTIQ PR.

Main differences to the ARTIQ PR

  • No frontend
  • Kasli-SoC already supports flashing via config write

Similar issues experienced on RISC-V devices can be expected here. Protocol issues resolved on main-line ARTIQ PR.

## Summary The corresponding Kasli-SoC PR for [ARTIQ PR](https://github.com/m-labs/artiq/pull/2559). ## Main differences to the ARTIQ PR - No frontend - Kasli-SoC already supports flashing via `config write` ~~Similar issues experienced on RISC-V devices can be expected here.~~ Protocol issues resolved on main-line ARTIQ PR.
occheung added 3 commits 2024-08-28 13:44:53 +08:00
mwojcik reviewed 2024-08-28 13:55:36 +08:00
src/runtime/src/mgmt.rs Outdated
@ -232,1 +870,3 @@
pub fn start(cfg: Config) {
pub fn start(
cfg: Config,
drtio_tuple: Option<(
mwojcik commented 2024-08-28 13:55:36 +08:00
Owner
Copy Link

Slightly related: I like the tuple to keep the argument list shorter/more compartmentalized, should we adapt it in other places too?

Slightly related: I like the tuple to keep the argument list shorter/more compartmentalized, should we adapt it in other places too?
srenblad commented 2024-09-02 17:57:04 +08:00
Contributor
Copy Link

Agreed that a a lot of argument lists are needlessly long. Especially with the shared sync primitives theres a lot of repetition. Though I'm not sure that these tuples make things much shorter, without some type aliasing. Is there a case for putting these shared sync prims in a struct and passing around an Rc to that instead? (In a similar style to how the mainline repo passes around Io)

Putting the idea out there, obviously it would be a heavy refactor...

Agreed that a a lot of argument lists are needlessly long. Especially with the shared sync primitives theres a lot of repetition. Though I'm not sure that these tuples make things much shorter, without some type aliasing. Is there a case for putting these shared sync prims in a struct and passing around an `Rc` to that instead? (In a similar style to how the mainline repo passes around `Io`) Putting the idea out there, obviously it would be a heavy refactor...
occheung added 8 commits 2024-09-05 12:17:20 +08:00
occheung added 1 commit 2024-09-09 17:28:31 +08:00
sb10q reviewed 2024-09-14 11:42:12 +08:00
src/runtime/src/mgmt.rs Outdated
@ -115,0 +837,4 @@
stream: &mut TcpStream,
pull_id: Rc<RefCell<u32>>,
cfg: Rc<Config>,
_drtio_tuple: Option<(&Rc<Mutex<bool>>, &RoutingTable, GlobalTimer)>,
sb10q commented 2024-09-14 11:42:12 +08:00
Owner
Copy Link

Isn't that just an idiosyncratic object? Canonical way would be to define a struct.

Isn't that just an idiosyncratic object? Canonical way would be to define a ``struct``.
occheung force-pushed drtio-coremgmt from fbb20bc723 to 93e25169fb 2024-09-19 17:19:45 +08:00 Compare
occheung added 3 commits 2024-09-20 17:10:17 +08:00
occheung changed title from WIP: Support CoreMgmt over DRTIO on Zynq Devices to Support CoreMgmt over DRTIO on Zynq Devices 2024-09-20 17:38:58 +08:00
sb10q reviewed 2024-09-25 17:28:36 +08:00
src/runtime/src/mgmt.rs Outdated
@ -115,0 +862,4 @@
}
#[derive(Clone)]
pub struct DrtioTuple(pub Rc<Mutex<bool>>, pub Rc<RefCell<RoutingTable>>, pub GlobalTimer);
sb10q commented 2024-09-25 17:28:36 +08:00
Owner
Copy Link

I think no other software calls objects "tuples".

I think no other software calls objects "tuples".
sb10q commented 2024-11-16 13:00:32 +08:00
Owner
Copy Link

What about DrtioContext?
And name the fields, as is usually done.

What about DrtioContext? And name the fields, as is usually done.
occheung changed title from Support CoreMgmt over DRTIO on Zynq Devices to WIP: Support CoreMgmt over DRTIO on Zynq Devices 2024-10-02 15:55:20 +08:00
sb10q commented 2024-10-21 16:17:13 +08:00
Owner
Copy Link

Similar issues experienced on RISC-V devices can be expected here.

What issues?

> Similar issues experienced on RISC-V devices can be expected here. What issues?
occheung added 3 commits 2024-10-24 12:43:53 +08:00
e6df69fe04 drtio_proto: add allocate step for flashing 
This avoids reallocation while transfering binaries.
occheung added 1 commit 2024-10-24 12:59:01 +08:00
occheung force-pushed drtio-coremgmt from 3ebbb554cd to 47fc53c4bf 2024-11-18 16:09:58 +08:00 Compare
occheung changed title from WIP: Support CoreMgmt over DRTIO on Zynq Devices to Support CoreMgmt over DRTIO on Zynq Devices 2024-11-18 16:11:30 +08:00
mwojcik reviewed 2024-11-18 17:12:51 +08:00
src/runtime/src/mgmt.rs
@ -115,0 +240,4 @@
loop {
if id != *pull_id.borrow() {
// another connection attempts to pull the log...
mwojcik commented 2024-11-18 17:12:51 +08:00
Owner
Copy Link

In what scenario would that be possible?

In what scenario would that be possible?
occheung commented 2024-11-20 09:41:31 +08:00
Author
Owner
Copy Link

It might be possible if you start multiple aqctl_corelog with different binding ports specified by -p.
Though in practice, all I see is the older aqctl_corelog not emitting anything, even after exiting newer aqctl_corelogs.

It might be possible if you start multiple `aqctl_corelog` with different binding ports specified by `-p`. Though in practice, all I see is the older `aqctl_corelog` not emitting anything, even after exiting newer `aqctl_corelog`s.
sb10q commented 2024-11-20 09:42:32 +08:00
Owner
Copy Link

Though in practice, all I see is the older aqctl_corelog not emitting anything, even after exiting newer aqctl_corelogs.

Sounds buggy...

> Though in practice, all I see is the older aqctl_corelog not emitting anything, even after exiting newer aqctl_corelogs. Sounds buggy...
sb10q commented 2024-11-20 09:46:15 +08:00
Owner
Copy Link

I think we should fix this - in the field aqctl_corelog is going to be mercilessly started over and over again when something doesn't work, sent SIGKILL, having its core device connection brutally dropped e.g. by changing the host's IP address, then restarted, etc.

So this needs to be robust.

I think we should fix this - in the field aqctl_corelog is going to be mercilessly started over and over again when something doesn't work, sent SIGKILL, having its core device connection brutally dropped e.g. by changing the host's IP address, then restarted, etc. So this needs to be robust.
mwojcik reviewed 2024-11-18 17:16:13 +08:00
src/satman/src/mgmt.rs
@ -0,0 +77,4 @@
self.cfg
.read(&key)
.map(|value| {
debug!("got value");
mwojcik commented 2024-11-18 17:16:13 +08:00
Owner
Copy Link

probably can be removed

probably can be removed
sb10q merged commit 47fc53c4bf into master 2024-11-19 18:55:03 +08:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
priority:critical

   
priority:high

   
priority:low

   
priority:medium
No Label
priority:critical
priority:high
priority:low
priority:medium
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/artiq-zynq#323
No description provided.
Delete Branch "occheung/artiq-zynq:drtio-coremgmt"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?