WIP: read idle kernel config after experiment end #310

srenblad · 2024-08-06T17:16:47+08:00

srenblad commented

2024-08-06 17:16:47 +08:00

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

read idle kernel config after experiment end ba3e38db11

mwojcik reviewed 2024-08-06 18:48:29 +08:00

src/runtime/src/comms.rs

						
				@ -806,3 +805,2 @@

				    mgmt::start(cfg);

				    let cfg: Rc<Config> = Rc::new(cfg);

mwojcik commented

2024-08-06 18:48:29 +08:00

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

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

srenblad commented

2024-08-07 10:32:03 +08:00

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.

mwojcik commented

2024-08-08 02:52:02 +08:00

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.

👍 1

srenblad marked this conversation as resolved

sb10q commented

2024-08-08 10:10:42 +08:00

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.

👍 1

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

srenblad commented

2024-09-19 10:10:33 +08:00

Superseded by #325, closing.

srenblad closed this pull request

2024-09-19 10:10:34 +08:00