Tracking: Porting `embassy-rs` for `libtock-rs`

[DanutAldea]

I am currently working on porting the Embassy executor along with extending capsule API with async methods that implement the embedded-hal-async traits.

Roadmap

• Port the embassy-executor to include a arch-tock target.
• Extend the libtock-rs::share module to export an async_scope method to be used in capsule async APIs.
• Convert the userspace-level critical-section implementation in demos/st7789-slint into a crate feature.
• Implement async GPIO trait.
• Modify the embassy-time and embassy-time-driver crates to allow for the timer frequency to be determined at runtime.
• Implement async Timer using the embassy-time interface.
• Extend Tock I2C capsule command interface to include a clear separation between an in place write-read command, and a write-read command that uses a separate read-only "write" buffer and read-write "read" buffer, as well as modify the write command to use a read-only buffer.
• Modify the libtock-rs I2C controller API to use the new convention.
• Modify the libtock-c I2C controller API to use the new convention.
• Implement async I2C controller.
• Implement async Console API.
• Implement async SPI controller.
• Add demos both in libtock-rs and embassy/examples.

Encountered Issues

• The design contract of the I2c trait exported by embedded-hal-async is incompatible with the I2cMaster trait used by the HIL.

Upstreaming roadmap

This part is important, as these crates and examples will have inter-dependencies.

• Create a Pull Request to Tock with the mentioned I2C modifications.
• Publish embassy and libtock-rs public forks containing the respective modifications that reference each other.
• Merge the changes into both upstream repositories.
• Change the dependencies to reference the main repositories, instead of the forks.

These should ensure that the modifications are testable from both sides at every step.

[DanutAldea]

@jrvanwhy, sorry for the delayed response. The codebase is now public at these links:

• libtock-rs: https://github.com/OxidosAutomotive/libtock-rs/tree/embassy
• embassy: https://github.com/OxidosAutomotive/embassy/tree/tock_support
• tock: https://github.com/OxidosAutomotive/tock/tree/i2c

[jrvanwhy]

Thank you for sharing those links!

In libtock_platform::share::async_scope:

pub async fn async_scope<L: List, Output, F: AsyncFnOnce(Handle<L>) -> Output>(fcn: F) -> Output {
    let list = Default::default();
    // Safety: We do not move the L out of the `list` variable. The `list`
    // variable will be dropped at the end of the scope, immediately before the
    // L becomes invalid.
    fcn(unsafe { Handle::new(&list) }).await
}

async function scopes are not guaranteed to nest the same way as synchronous functions. It is possible for an async function's scope to outlive its parent's scope, such as by moving the async function's future around. As a result, list can outlive shared buffers, which would cause the kernel to retain access to userspace memory after that memory has been deallocated and reused (use-after-free).

I've created a demonstration of this at https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=7117fe6d448e0bed82d6d2f7b57ef0db (look at the leak_buffer function). You can execute it, and the console output will indicate that allow_rw was called but AllowRw's Drop was never called. I used a leaked box to trigger it, but it is entirely possible to trigger this issue in a #![no_std] context.

This is the same issue that afflicted #340

[DanutAldea]

Now I see the issue, and you are right. Thank you for the input, @jrvanwhy. Considering this, do you see any way forward
for this attempt. I have further discussed this with @alexandruradovici and we see a few paths this could go:

• add a disclaimer that the use of mem::forget with async code is unsound
• discuss this further and decide on an async compatible API, where the discussions in Brainstorming: what could a Future-based libtock-rs look like? #494 could be a solid starting point
• create a separate libtock-embassy project

[jrvanwhy]

• discuss this further and decide on an async compatible API, where the discussions in Brainstorming: what could a Future-based libtock-rs look like? #494 could be a solid starting point

I think this is the way to go. Specifically, I think this Pin-based API in the most promising: #494 (comment), if it is able to meet your/embassy's needs.

[DanutAldea]

Thanks for the guidance and I apologise for the conversation going silent for a few weeks.

I made some changes that utilise the Pin-based API idea mentioned in the thread and pushed the modifications to the libtock-rs fork. Could you, please, take another look at the allow mechanism to check that the design is sound?

[jrvanwhy]

I've taken an initial look at the Allow APIs in OxidosAutomotive@2ec618f. On first glance, the structure looks sound so we're good to go there. There may be some unsound methods though (I haven't done a full review -- and it's difficult to do so without comments on the unsafe functions describing their safety invariants). I also think there's work to be done to make the Pin-based APIs more powerful (e.g. to support dynamic buffer number choices) and to interoperate with the existing share() APIs (so we don't end up with 2 implementations of every driver).

My suggestion is to break off a small piece (I suggest just RW Allow) into its own PR against libtock-rs, comment it well, and send it for review. That will give us a new communication tool (give us a way to leave comments that will mostly stick around). I anticipate this code being tricky and undergoing a lot of revision, so I think breaking it into smaller chunks is a good idea. In particular, once we have a RW Allow design we're happy with, I think RO Allow will be easy (it's mostly a duplicate API).

[DanutAldea]

Thank you for the input! I will raise a proper PR in the following days and ping you there.

[DanutAldea]

I have opened a PR with the initial design refined and tested. We can continue the talks about improving it there.