WIP: read idle kernel config after experiment end #310

Draft
srenblad wants to merge 1 commits from srenblad/artiq-zynq:consistent_idle_kernel into master
Contributor

I expect that this PR will be superseded by implementing idle kernel reading on flash, hence the WIP label.

Read idle kernel from flash after experiment ends

Currently, the idle kernel is only read once on startup. This PR allows updating idle kernels by flashing -> running an experiment. The new idle kernel will start as expected on experiment finish. This is consistent with the current idle kernel behavior on RISC-V devices.

Testing

[x] Tested on Kasli-SOC v1.1 with demo firmware, new idle kernel starts as expected

_I expect that this PR will be superseded by implementing idle kernel reading on flash, hence the WIP label._ ## Read idle kernel from flash after experiment ends Currently, the idle kernel is only read once on startup. This PR allows updating idle kernels by flashing -> running an experiment. The new idle kernel will start as expected on experiment finish. This is consistent with the current idle kernel behavior on RISC-V devices. ## Testing [x] Tested on Kasli-SOC v1.1 with demo firmware, new idle kernel starts as expected
srenblad added 1 commit 2024-08-06 17:16:47 +08:00
mwojcik reviewed 2024-08-06 18:48:29 +08:00
@ -806,3 +805,2 @@
mgmt::start(cfg);
let cfg: Rc<Config> = Rc::new(cfg);
Owner

Should Rc be passed to main() rather than created here?

Should Rc<Config> be passed to main() rather than created here?
Author
Contributor

This would also necessitate changing the signatures of rtio_mgt::startup and ksupport::setup_device_map (or borrow() into the calls). Additionally, should this be the case for soft_panic_main? Still, I am happy to change it, if you have a preference for it.

This would also necessitate changing the signatures of `rtio_mgt::startup` and `ksupport::setup_device_map` (or `borrow()` into the calls). Additionally, should this be the case for `soft_panic_main`? Still, I am happy to change it, if you have a preference for it.
Owner

On second thought, this is unnecessary here, and this would be fine. I was thinking of RISC-V firmware that does this in that way. Here it wouldn't do much except add a bunch of unnecessary borrows.

On second thought, this is unnecessary here, and this would be fine. I was thinking of RISC-V firmware that does this in that way. Here it wouldn't do much except add a bunch of unnecessary borrows.
srenblad marked this conversation as resolved
Owner

This PR allows updating idle kernels by flashing -> running an experiment. The new idle kernel will start as expected on experiment finish.

That's not what I would expect. I would expect it to either start on boot (simplest code probably) or right after it is flashed. Running an experiment to trigger it sounds like an obscure workaround.

This is consistent with the current idle kernel behavior on RISC-V devices.

Consistent but looks broken.

> This PR allows updating idle kernels by flashing -> running an experiment. The new idle kernel will start as expected on experiment finish. That's not what I would expect. I would expect it to either start on boot (simplest code probably) or right after it is flashed. Running an experiment to trigger it sounds like an obscure workaround. > This is consistent with the current idle kernel behavior on RISC-V devices. Consistent but looks broken.
srenblad changed title from read idle kernel config after experiment end to WIP: read idle kernel config after experiment end 2024-08-26 10:04:52 +08:00
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b srenblad-consistent_idle_kernel master
git pull consistent_idle_kernel

Step 2:

Merge the changes and update on Gitea.
git checkout master
git merge --no-ff srenblad-consistent_idle_kernel
git push origin master
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 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#310
No description provided.