Skip to content

Start of Kobj support: Semaphores and Threads #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 39 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
fa00bea
zephyr::kconfig: Document module
d3zd3z Oct 11, 2024
823b48a
zephyr: Add some missing doc comments
d3zd3z Oct 11, 2024
45192f3
zephyr: Enforce documentation
d3zd3z Oct 11, 2024
ed875c2
zephyr: Add Error and Result types
d3zd3z Sep 23, 2024
e1196a0
zephyr: Make symbols from portable-atomic available
d3zd3z Sep 17, 2024
594bbc4
zephyr: Introduce static kobject support
d3zd3z Sep 18, 2024
3e362a7
zephyr: include portable atomic
d3zd3z Oct 11, 2024
67e456b
zephyr: Add sys::Sempahore
d3zd3z Oct 11, 2024
d8fbb57
zephyr: `take` in static kernel object takes args
d3zd3z Oct 11, 2024
aed43fd
zephyr: Provide critical-section for Zephyr
d3zd3z Sep 17, 2024
6598cd1
zephyr: Add alignment helper
d3zd3z Sep 23, 2024
a9ea9a1
zephyr: Add allocator support
d3zd3z Jul 18, 2024
59907d7
zephyr-sys: Export stack alignment constants
d3zd3z Sep 23, 2024
6b3e831
zephyr: Add sys::thread support
d3zd3z Oct 11, 2024
e71bd6a
samples: philosophers: Dining philosophers example
d3zd3z Oct 11, 2024
096d34b
Upgrade crate versions to match Zephyr
d3zd3z Oct 8, 2024
32b8fbf
zephyr: Add `k_uptime_get`
d3zd3z Sep 23, 2024
907c044
zephyr: Implement Debug for Semaphore
d3zd3z Oct 11, 2024
3d2477d
zephyr: StaticSemaphire needs to implement Sync
d3zd3z Oct 11, 2024
3fd043a
zephyr: Semaphore methods should not be `mut`
d3zd3z Oct 11, 2024
c88307f
zephyr: hide docs of RealStaticThreadStack
d3zd3z Oct 11, 2024
1b92ea5
zephyr: Expose some fields publicly
d3zd3z Oct 11, 2024
1971feb
zephyr: StaticThread must be Sync
d3zd3z Oct 11, 2024
97b217f
samples: hello_world: Fix dependency version
d3zd3z Oct 11, 2024
2ccb628
zephyr: Fix some warnings by being conditional
d3zd3z Oct 11, 2024
e8eebb2
samples: philosophers: Fix yaml
d3zd3z Oct 11, 2024
a34507f
tests: time: Fix dependency
d3zd3z Oct 11, 2024
91d6f33
zephyr-sys: Allow broken doc links in bindgen
d3zd3z Oct 12, 2024
177a932
zephyr: object: Improve docs about safety
d3zd3z Oct 12, 2024
1cd9299
zephyr: printk: Fix doc error
d3zd3z Oct 12, 2024
266a7a7
zephyr: sys: sync: Fix some doc links
d3zd3z Oct 12, 2024
fdd97ae
zephyr: printk: Remove dependency on alloc
d3zd3z Oct 14, 2024
d01153a
zephyr: Create export type alias
d3zd3z Oct 15, 2024
7375e44
zephyr: Use constructor for static thread stack
d3zd3z Oct 15, 2024
8bbade3
zephyr: object: Add kboj_define support for arrays of stacks
d3zd3z Oct 15, 2024
98a15fe
samples: philosophers: Convert to array of stacks
d3zd3z Oct 15, 2024
861cb83
zephyr: docgen
d3zd3z Oct 15, 2024
057e169
zephyr: Fix some broken doc comments
d3zd3z Oct 15, 2024
d11c5d2
module: Make sure all tests are run
d3zd3z Oct 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions zephyr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#![deny(missing_docs)]

pub mod error;
pub mod object;
pub mod sync;
pub mod sys;
pub mod time;
Expand Down
148 changes: 148 additions & 0 deletions zephyr/src/object.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
//! # Zephyr Kernel Objects
//!
//! Zephyr has a concept of a 'kernel object' that is handled a bit magically. In kernel mode
//! threads, these are just pointers to the data structures that Zephyr uses to manage that item.
//! In userspace, they are still pointers, but those data structures aren't accessible to the
//! thread. When making syscalls, the kernel validates that the objects are both valid kernel
//! objects and that the are supposed to be accessible to this thread.
//!
//! In many Zephyr apps, the kernel objects in the app are defined as static, using special macros.
//! These macros make sure that the objects get registered so that they are accessible to userspace
//! (at least after that access is granted).
//!
//! There are also kernel objects that are synthesized as part of the build. Most notably, there
//! are ones generated by the device tree.
//!
//! There are some funny rules about references and mutable references to memory that is
//! inaccessible. Basically, it is never valid to have a reference to something that isn't
//! accessible. However, we can have `*mut ktype` or `*const ktype`. In Rust, having multiple
//! `*mut` pointers live is undefined behavior. Zephyr makes extensive use of shared mutable
//! pointers (or sometimes immutable). We will not dereference these in Rust code, but merely pass
//! them to APIs in Zephyr that require them.
//!
//! Most kernel objects use mutable pointers, and it makes sense to require the wrapper structure
//! to be mutable to these accesses. There a few cases, mostly generated ones that live in
//! read-only memory, notably device instances, that need const pointers. These will be
//! represented by a separate wrapper.
//!
//! # Initialization tracking
//!
//! The Kconfig `CONFIG_RUST_CHECK_KOBJ_INIT` enabled extra checking in Rust-based kernel objects.
//! This will result in a panic if the objects are used before the underlying object has been
//! initialized. The initialization must happen through the `StaticKernelObject::init_help`
//! method.
//!
//! TODO: Document how the wrappers work once we figure out how to implement them.

use core::{cell::UnsafeCell, mem};

use crate::sync::atomic::{AtomicUsize, Ordering};

/// A kernel object represented statically in Rust code.
///
/// These should not be declared directly by the user, as they generally need linker decorations to
/// be properly registered in Zephyr as kernel objects. The object has the underlying Zephyr type
/// T, and the wrapper type W.
///
/// TODO: Handling const-defined alignment for these.
pub struct StaticKernelObject<T> {
#[allow(dead_code)]
/// The underlying zephyr kernel object.
value: UnsafeCell<T>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may not have grasped the full context yet, but to me this mostly seems to be used as a MaybeUninit? 🤔 Could that be used instead here? It reads like it will do the same job, but provide some more explicit meaning.

Also: Nothing prevents somebody from moving such an object, right? Are those C counterparts fine with being moved? I would assume synchronization primitives keep all kind of self referential pointers internally. So I am a bit surprised that this can safely be wrapped without some sort of Pinning / modeling things are references to ensure things cannot be moved around.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UnsafeCell is absolutely critical to this working. It is treated as magical by the Rust compiler and basically tells it that even though the container isn't mut, this T inside of it might change. Since the C code is likely to change these, we need to declare this like this.

/// Initialization status of this object. Most objects will start uninitialized and be
/// initialized manually.
init: AtomicUsize,
}

/// Each can be wrapped appropriately. The wrapped type is the instance that holds the raw pointer.
pub trait Wrapped {
/// The wrapped type. This is what `take()` on the StaticKernelObject will return after
/// initialization.
type T;

/// Initialize this kernel object, and return the wrapped pointer.
fn get_wrapped(&self) -> Self::T;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit confused about the "pointer" documentation here while the trait really takes any type.

Overall, I am a bit confused about this being abstracted with pointers. Couldn't this also be abstracted with references on the Rust side?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They don't follow the semantics of Rust references, they are never dereferenced from Rust, and are really just part of the C api. Also, this has been remove anyway (but it is still a wrapped pointer).

}

/// A state indicating an uninitialized kernel object.
///
/// This must be zero, as kernel objects will
/// be represetned as zero-initialized memory.
pub const KOBJ_UNINITIALIZED: usize = 0;

/// A state indicating a kernel object that is being initialized.
pub const KOBJ_INITING: usize = 1;

/// A state indicating a kernel object that has completed initialization. This also means that the
/// take has been called. And shouldn't be allowed additional times.
pub const KOBJ_INITIALIZED: usize = 2;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I typically see people keeping the struct definition + impl's together, so I would consider moving this block before the struct definition?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look through it, it gets messy when there are multiple structs and traits that interact with each other.


impl<T> StaticKernelObject<T>
where
StaticKernelObject<T>: Wrapped,
{
/// Construct an empty of these objects, with the zephyr data zero-filled. This is safe in the
/// sense that Zephyr we track the initialization, and they start in the uninitialized state.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the attention to good docs.

Not sure if these suggestions are the same as your original intent.

Suggested change
/// Construct an empty of these objects, with the zephyr data zero-filled. This is safe in the
/// sense that Zephyr we track the initialization, and they start in the uninitialized state.
/// Construct an empty instance of these objects, with the zephyr data zero-filled. This is safe in the
/// sense that in Zephyr we track the initialization, and they start in the uninitialized state.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should be addressed, namely by rewriting the whole doc section.

pub const fn new() -> StaticKernelObject<T> {
StaticKernelObject {
value: unsafe { mem::zeroed() },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may lack context here, but I like how most projects put // SAFETY comments before unsafe blocks. That makes reviewing a lot easier IMHO. Here one could probably add a comment about how later code ensures that this is fully initialized at some point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I think this whole method should probably just be unsafe.

init: AtomicUsize::new(KOBJ_UNINITIALIZED),
}
}

/// Get the instance of the kernel object.
///
/// Will return a single wrapped instance of this object. This will invoke the initialization,
/// and return `Some<Wrapped>` for the wrapped containment type.
pub fn take(&self) -> Option<<Self as Wrapped>::T> {
if let Err(_) = self.init.compare_exchange(
KOBJ_UNINITIALIZED,
KOBJ_INITING,
Ordering::AcqRel,
Ordering::Acquire)
{
return None;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you already have a Result<> type, you can also do: .ok()? on it [1]

[1] https://doc.rust-lang.org/std/result/enum.Result.html#method.ok

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look at it.

let result = self.get_wrapped();
self.init.store(KOBJ_INITIALIZED, Ordering::Release);
Some(result)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an example that uses this (maybe my grep skills fail me :P)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look in samples. It is used by any user code using statically declared kernel objects.

The reason to have these two types is that most kernel objects in Zephyr need to be declared statically. But static globals aren't very useful in Rust (without unsafe), so we have this init_once method that is called on the global static to return a single instance that wraps the pointer to the static, and can be used more freely in Rust.

}
}

/// Declare a static kernel object. This helps declaring static values of Zephyr objects.
///
/// This can typically be used as:
/// ```
/// kobj_define! {
/// static A_MUTEX: StaticMutex;
/// static MUTEX_ARRAY: [StaticMutex; 4];
/// }
/// ```
#[macro_export]
macro_rules! kobj_define {
($v:vis static $name:ident: $type:tt; $($rest:tt)*) => {
$crate::_kobj_rule!($v, $name, $type);
$crate::kobj_define!($($rest)*);
};
($v:vis static $name:ident: $type:tt<$size:ident>; $($rest:tt)*) => {
$crate::_kobj_rule!($v, $name, $type<$size>);
$crate::kobj_define!($($rest)*);
};
($v:vis static $name:ident: $type:tt<$size:literal>; $($rest:tt)*) => {
$crate::_kobj_rule!($v, $name, $type<$size>);
$crate::kobj_define!($($rest)*);
};
($v:vis static $name:ident: $type:tt<{$size:expr}>; $($rest:tt)*) => {
$crate::_kobj_rule!($v, $name, $type<{$size}>);
$crate::kobj_define!($($rest)*);
};
() => {};
}

#[doc(hidden)]
#[macro_export]
macro_rules! _kobj_rule {
() => {
compile_errro!("TODO: Add kobj rules");
};
}