[Tracking/Asahi] Upstream-ready Rust changes #952

asahilina · 2023-01-26T04:02:56Z

Opening this tracking PR to document the Rust changes that are ready to be merged into rust-next.

fbq · 2023-01-26T04:34:10Z

rust/kernel/time.rs

+//! C header: [`include/linux/timekeeping.h`](../../../../include/linux/timekeeping.h)
+
+use crate::bindings;
+use core::time::Duration;


hrtimer wrapper is in my TODO list, so it's nice to see this ;-)

core::time::Duration has a slight problem of being more than 64bit, we probably need to introduce our own Duration as the counterpart of ktime_t. But it's just my own opinion, time subsystem maintainers may have different ones.

I don't think it blocks this change, just want to mention it while we are at it.

rust/macros/helpers.rs

rust/kernel/print.rs

asahilina · 2023-02-05T11:22:54Z

Updated:

Rebased on latest rust-for-next
Added some commits from asahi-soc at the top (these are queued for v6.3 and can be ignored)
Imported a bunch of error module features from the rust branch.
Added the XArray abstraction (now that ForeignOwnable is in)
Added a stub Device abstraction based on the initial commit that introduced it to rust. This is enough to allow other parts of the kernel crate that interact with devices to be upstreamed, even though proper drivers will require more.
Added the Apple RTKit abstraction (this is what depends on the asahi-soc patches). Although this normally depends on ARM64 support, it can be build-tested on amd64 with CONFIG_COMPILE_TEST=y.

asahilina · 2023-02-05T12:23:23Z

Good news! With just a couple new bits, this branch is good enough to add most of the DRM abstractions my driver used as of last week, so I've gone ahead and added those. 🎉

asahilina · 2023-02-05T12:30:20Z

@wedsonaf Can you sign off on c219c9a / 6646d85? That's the only rust branch commit I've seen so far that was missing the signoff that I need...

rust/kernel/xarray.rs

fbq · 2023-02-05T18:07:42Z

rust/kernel/xarray.rs

+/// # Invariants
+///
+/// The `*mut T` is always non-NULL and owned by the referenced `XArray`
+pub struct Guard<'a, T: ForeignOwnable>(*mut T, &'a Opaque<bindings::xarray>);


Maybe:

Suggested change

pub struct Guard<'a, T: ForeignOwnable>(*mut T, &'a Opaque<bindings::xarray>);

pub struct Guard<'a, T: ForeignOwnable>((NonNull<T>, &'a XAarray<T>);

?

And you can avoid the "Invariants" section

We need an unsafe constructor for this type. Otherwise one could create a guard to say 'NonNull::dangling', and then call borrow, which would deref the invalid pointer.

IOW, an invariants sections is not sufficient: we need to ensure that the invariants actually hold. If we can't do it with the type system, we need to make the operation unsafe and have the developer promise to abide by the requirements.

@fbq Fixed for the upstream submission!

@wedsonaf But the fields are private, nobody can construct it outside of this module ^^

rust/kernel/xarray.rs

fbq · 2023-02-05T19:03:43Z

rust/kernel/xarray.rs

+    fn deref(&self) -> &Self::Target {
+        // SAFETY: See the `borrow()` method. The dereferenced `T::Borrowed` value
+        // must share the same lifetime, so we can return a reference to it.
+        // TODO: Is this really sound?


Yeah, this is very interesting. I tried a few lifetime tricks, and couldn't make it work. @bjorn3 @nbdd0121 @wedsonaf anything you guys can think of? I think the intention here is to express that the lifetime of Borrowed outlives the lifetime of &Self::Target. But I may be wrong...

This could be written as:

impl<'a, T: ForeignOwnable> Deref for Guard<'a, T> where T::Borrowed<'a>: Deref, for<'b> T::Borrowed<'b>: Into<&'b <T::Borrowed<'a> as Deref>::Target>, { type Target = <T::Borrowed<'a> as Deref>::Target; fn deref(&self) -> &Self::Target { self.borrow().into() } }

then we just need to ensure ArcBorrow<'a, T>: Into<'a T> for this to be usable for Arc.

Current code, as is, isn't really sound because T::Borrowed<'self>: Deref can do something funky inside and return a pointer to itself, rather than the thing it points to. What we really want here is a way to convert T::Borrowed<'self> to &'self Target, so it seems to me that just adding an explicit Into is the best way.

Yeah, that was my feeling when writing this too (that it's safe given existing T::Borrowed implementations that follow certain rules but not sound in the general case). I'll add an Into impl to ArcBorrow for the next revision and see how that goes, thanks! @nbdd0121 do you want to be added as Co-developed-by?

I couldn't figure this out... Into conflicts with the blanket From impl.

Since this is just a convenience, I think we can leave it for a future patch, so I've dropped this for now ^^;;

rust/kernel/xarray.rs

bjorn3 · 2023-02-05T19:37:18Z

rust/kernel/xarray.rs

+where
+    T::Borrowed<'static>: Deref,
+{
+    type Target = <T::Borrowed<'static> as Deref>::Target;


I think this will need to be <T::Borrowed<'a> as Deref>::Target as T is only guaranteed to live for 'a and not for 'static. Otherwise depending on the ForeignOwnable impl I think it may be possible to extract an 'static reference out of it.

I think so, but if we want use self.borrow() instead of T::borrow() in the deref implementation, how can we make it work?

rust/kernel/xarray.rs

wedsonaf · 2023-02-11T17:41:34Z

@wedsonaf Can you sign off on c219c9a / 6646d85? That's the only rust branch commit I've seen so far that was missing the signoff that I need...

Ah, this predates our practice of signing off commits. Anyway:

Signed-off-by: Wedson Almeida Filho [email protected]

wedsonaf

Leaving some comments on xarray abstraction

rust/kernel/xarray.rs

wedsonaf · 2023-02-11T18:09:09Z

rust/kernel/xarray.rs

+/// # Invariants
+///
+/// The `*mut T` is always non-NULL and owned by the referenced `XArray`
+pub struct Guard<'a, T: ForeignOwnable>(*mut T, &'a Opaque<bindings::xarray>);


We need an unsafe constructor for this type. Otherwise one could create a guard to say 'NonNull::dangling', and then call borrow, which would deref the invalid pointer.

IOW, an invariants sections is not sufficient: we need to ensure that the invariants actually hold. If we can't do it with the type system, we need to make the operation unsafe and have the developer promise to abide by the requirements.

rust/kernel/xarray.rs

The rust_fmt_argument function is called from printk() to handle the %pA format specifier. Since it's called from C, we should mark it extern "C" to make sure it's ABI compatible. Cc: [email protected] Fixes: 247b365 ("rust: add `kernel` crate") Signed-off-by: David Gow <[email protected]> Link: Rust-for-Linux#967 Reviewed-by: Björn Roy Baron <[email protected]> Reviewed-by: Gary Guo <[email protected]> [Applied `rustfmt`] Signed-off-by: Miguel Ojeda <[email protected]>

This allows downstream consumers to keep track of private data for shmem mappings. In particular, the Rust abstraction will use this to safely drop data associated with a mapping when it is unmapped. Signed-off-by: Asahi Lina <[email protected]> Reviewed-by: Sven Peter <[email protected]> Reviewed-by: Eric Curtin <[email protected]> Signed-off-by: Hector Martin <[email protected]>

While we normally encourage devm usage by drivers, some consumers (and in particular the upcoming Rust abstractions) might want to manually manage memory. Export the raw functions to make this possible. Signed-off-by: Asahi Lina <[email protected]> Reviewed-by: Sven Peter <[email protected]> Reviewed-by: Eric Curtin <[email protected]> Signed-off-by: Hector Martin <[email protected]>

Other functions touching shmem->sgt take the pages lock, so do that here too. drm_gem_shmem_get_pages() & co take the same lock, so move to the _locked() variants to avoid recursive locking. Signed-off-by: Asahi Lina <[email protected]>

This commit provides the build flags for Rust for AArch64. The core Rust support already in the kernel does the rest. The Rust samples have been tested with this commit. [jcunliffe: Arm specific parts taken from Miguel's upstream tree] Signed-off-by: Miguel Ojeda <[email protected]> Co-developed-by: Jamie Cunliffe <[email protected]> Signed-off-by: Jamie Cunliffe <[email protected]> Reviewed-by: Vincenzo Palazzo <[email protected]> Signed-off-by: Vincenzo Palazzo <[email protected]> Reviewed-by: Gary Guo <[email protected]>

Enable the PAC ret and BTI options in the Rust build flags to match the options that are used when building C. Signed-off-by: Jamie Cunliffe <[email protected]> Reviewed-by: Vincenzo Palazzo <[email protected]>

Disable the neon and fp target features to avoid fp & simd registers. The use of fp-armv8 will cause a warning from rustc about an unknown feature that is specified. The target feature is still passed through to LLVM, this behaviour is documented as part of the warning. This will be fixed in a future version of the rustc toolchain. Signed-off-by: Jamie Cunliffe <[email protected]> Reviewed-by: Vincenzo Palazzo <[email protected]>

This allows printing the inner data of `Arc` and its friends if the inner data implements `Display` or `Debug`. It's useful for logging and debugging purpose. Signed-off-by: Boqun Feng <[email protected]> Reviwed-by: Vincenzo Palazzo <[email protected]> Reviewed-by: Gary Guo <[email protected]> Reviewed-by: Vincenzo Palazzo <[email protected]> Reviewed-by: Andreas Hindborg <[email protected]> Reviewed-by: Björn Roy Baron <[email protected]>

This both demonstrates the usage of different print format in Rust and serves as a selftest for the `Display` and `Debug` implementation of `Arc` and its friends. Signed-off-by: Boqun Feng <[email protected]> Reviewed-by: Björn Roy Baron <[email protected]> Reviewed-by: Finn Behrens <[email protected]> Reviewed-by: Vincenzo Palazzo <[email protected]> Reviewed-by: Gary Guo <[email protected]> Reviewed-by: Andreas Hindborg <[email protected]>

This module is intended to contain functions related to kernel timekeeping and time. Initially, this just wraps ktime_get() and ktime_get_boottime() and returns them as core::time::Duration instances. Signed-off-by: Asahi Lina <[email protected]>

This makes it mirror the way expect_ident() works, and means we can more easily push the result back into the token stream. Signed-off-by: Asahi Lina <[email protected]>

The XArray is an abstract data type which behaves like a very large array of pointers. Add a Rust abstraction for this data type. The initial implementation uses explicit locking on get operations and returns a guard which blocks mutation, ensuring that the referenced object remains alive. To avoid excessive serialization, users are expected to use an inner type that can be efficiently cloned (such as Arc<T>), and eagerly clone and drop the guard to unblock other users after a lookup. Future variants may support using RCU instead to avoid mutex locking. This abstraction also introduces a reservation mechanism, which can be used by alloc-capable XArrays to reserve a free slot without immediately filling it, and then do so at a later time. If the reservation is dropped without being filled, the slot is freed again for other users, which eliminates the need for explicit cleanup code. Signed-off-by: Asahi Lina <[email protected]> --- Hi everyone! This abstraction is part of the set of dependencies for the drm/asahi Apple M1/M2 GPU driver. The branch at [1] contains the full series of patches rebased on upstream leading to the complete driver, for reference on how it is intended to be used. Thank you everyone who helped review this on GitHub [2]! I hope I didn't forget any CCs... Note that I dropped the convenience `Deref` impl for `Guard`, since I couldn't figure out how to do it safely. Suggestions welcome, or we can leave it for a future improvement ^^ [1] https://github.com/AsahiLinux/linux/tree/gpu/rebase-20230224 [2] Rust-for-Linux/linux#952 Changes in v3: - Updated to the error v2/v3 series API. - Renamed `err` to `ret` for consistency with the other instance. - Link to v2: https://lore.kernel.org/r/[email protected] Changes in v2: - Added Pin requirement for all XArray operations, to close a soundness hole due to the lock in the XArray (locks are not safe to move while locked). Creation does not require pinning in place, since the lock cannot be acquired at that point. - Added safety note to Drop impl about why we don't need to do the lock unlock dance to ensure soundness in case of a dropped lock guard. - Downstream drm/asahi driver was also rebased on this version to prove it works (previously it was still on a pre-v1 version). - This still depends on the Error series (v1). v2 of that will need a trivial rename of Error::from_kernel_errno -> Error::from_errno. If this version of XArray ends up looking good, I'll send a trivial v4 of XArray with the rename, after sending the v2 of the Error series. - Link to v1: https://lore.kernel.org/r/[email protected]

The XArray is an abstract data type which behaves like a very large array of pointers. Add a Rust abstraction for this data type. The initial implementation uses explicit locking on get operations and returns a guard which blocks mutation, ensuring that the referenced object remains alive. To avoid excessive serialization, users are expected to use an inner type that can be efficiently cloned (such as Arc<T>), and eagerly clone and drop the guard to unblock other users after a lookup. Future variants may support using RCU instead to avoid mutex locking. This abstraction also introduces a reservation mechanism, which can be used by alloc-capable XArrays to reserve a free slot without immediately filling it, and then do so at a later time. If the reservation is dropped without being filled, the slot is freed again for other users, which eliminates the need for explicit cleanup code. Signed-off-by: Asahi Lina <[email protected]> --- Hi everyone! This abstraction is part of the set of dependencies for the drm/asahi Apple M1/M2 GPU driver. The branch at [1] contains the full series of patches rebased on upstream leading to the complete driver, for reference on how it is intended to be used. Thank you everyone who helped review this on GitHub [2]! I hope I didn't forget any CCs... Note that I dropped the convenience `Deref` impl for `Guard`, since I couldn't figure out how to do it safely. Suggestions welcome, or we can leave it for a future improvement ^^ [1] https://github.com/AsahiLinux/linux/tree/gpu/rebase-20230224 [2] Rust-for-Linux#952 Changes in v3: - Updated to the error v2/v3 series API. - Renamed `err` to `ret` for consistency with the other instance. - Link to v2: https://lore.kernel.org/r/[email protected] Changes in v2: - Added Pin requirement for all XArray operations, to close a soundness hole due to the lock in the XArray (locks are not safe to move while locked). Creation does not require pinning in place, since the lock cannot be acquired at that point. - Added safety note to Drop impl about why we don't need to do the lock unlock dance to ensure soundness in case of a dropped lock guard. - Downstream drm/asahi driver was also rebased on this version to prove it works (previously it was still on a pre-v1 version). - This still depends on the Error series (v1). v2 of that will need a trivial rename of Error::from_kernel_errno -> Error::from_errno. If this version of XArray ends up looking good, I'll send a trivial v4 of XArray with the rename, after sending the v2 of the Error series. - Link to v1: https://lore.kernel.org/r/[email protected]

The XArray is an abstract data type which behaves like a very large array of pointers. Add a Rust abstraction for this data type. The initial implementation uses explicit locking on get operations and returns a guard which blocks mutation, ensuring that the referenced object remains alive. To avoid excessive serialization, users are expected to use an inner type that can be efficiently cloned (such as Arc<T>), and eagerly clone and drop the guard to unblock other users after a lookup. Future variants may support using RCU instead to avoid mutex locking. This abstraction also introduces a reservation mechanism, which can be used by alloc-capable XArrays to reserve a free slot without immediately filling it, and then do so at a later time. If the reservation is dropped without being filled, the slot is freed again for other users, which eliminates the need for explicit cleanup code. Signed-off-by: Asahi Lina <[email protected]> --- Hi everyone! This abstraction is part of the set of dependencies for the drm/asahi Apple M1/M2 GPU driver. The branch at [1] contains the full series of patches rebased on upstream leading to the complete driver, for reference on how it is intended to be used. Thank you everyone who helped review this on GitHub [2]! I hope I didn't forget any CCs... Note that I dropped the convenience `Deref` impl for `Guard`, since I couldn't figure out how to do it safely. Suggestions welcome, or we can leave it for a future improvement ^^ [1] https://github.com/AsahiLinux/linux/tree/gpu/rebase-20230224 [2] Rust-for-Linux/linux#952 Changes in v3: - Updated to the error v2/v3 series API. - Renamed `err` to `ret` for consistency with the other instance. - Link to v2: https://lore.kernel.org/r/[email protected] Changes in v2: - Added Pin requirement for all XArray operations, to close a soundness hole due to the lock in the XArray (locks are not safe to move while locked). Creation does not require pinning in place, since the lock cannot be acquired at that point. - Added safety note to Drop impl about why we don't need to do the lock unlock dance to ensure soundness in case of a dropped lock guard. - Downstream drm/asahi driver was also rebased on this version to prove it works (previously it was still on a pre-v1 version). - This still depends on the Error series (v1). v2 of that will need a trivial rename of Error::from_kernel_errno -> Error::from_errno. If this version of XArray ends up looking good, I'll send a trivial v4 of XArray with the rename, after sending the v2 of the Error series. - Link to v1: https://lore.kernel.org/r/[email protected]

The XArray is an abstract data type which behaves like a very large array of pointers. Add a Rust abstraction for this data type. The initial implementation uses explicit locking on get operations and returns a guard which blocks mutation, ensuring that the referenced object remains alive. To avoid excessive serialization, users are expected to use an inner type that can be efficiently cloned (such as Arc<T>), and eagerly clone and drop the guard to unblock other users after a lookup. Future variants may support using RCU instead to avoid mutex locking. This abstraction also introduces a reservation mechanism, which can be used by alloc-capable XArrays to reserve a free slot without immediately filling it, and then do so at a later time. If the reservation is dropped without being filled, the slot is freed again for other users, which eliminates the need for explicit cleanup code. Signed-off-by: Asahi Lina <[email protected]> --- Hi everyone! This abstraction is part of the set of dependencies for the drm/asahi Apple M1/M2 GPU driver. The branch at [1] contains the full series of patches rebased on upstream leading to the complete driver, for reference on how it is intended to be used. Thank you everyone who helped review this on GitHub [2]! I hope I didn't forget any CCs... Note that I dropped the convenience `Deref` impl for `Guard`, since I couldn't figure out how to do it safely. Suggestions welcome, or we can leave it for a future improvement ^^ [1] https://github.com/AsahiLinux/linux/tree/gpu/rebase-20230224 [2] Rust-for-Linux#952 Changes in v3: - Updated to the error v2/v3 series API. - Renamed `err` to `ret` for consistency with the other instance. - Link to v2: https://lore.kernel.org/r/[email protected] Changes in v2: - Added Pin requirement for all XArray operations, to close a soundness hole due to the lock in the XArray (locks are not safe to move while locked). Creation does not require pinning in place, since the lock cannot be acquired at that point. - Added safety note to Drop impl about why we don't need to do the lock unlock dance to ensure soundness in case of a dropped lock guard. - Downstream drm/asahi driver was also rebased on this version to prove it works (previously it was still on a pre-v1 version). - This still depends on the Error series (v1). v2 of that will need a trivial rename of Error::from_kernel_errno -> Error::from_errno. If this version of XArray ends up looking good, I'll send a trivial v4 of XArray with the rename, after sending the v2 of the Error series. - Link to v1: https://lore.kernel.org/r/[email protected]

asahilina force-pushed the gpu/rust-for-next branch from ea31663 to a7b131f Compare January 26, 2023 04:03

fbq reviewed Jan 26, 2023

View reviewed changes

bjorn3 reviewed Jan 26, 2023

View reviewed changes

rust/macros/helpers.rs Show resolved Hide resolved

bjorn3 reviewed Jan 26, 2023

View reviewed changes

rust/kernel/print.rs Outdated Show resolved Hide resolved

ojeda force-pushed the rust-next branch from bb64468 to 8fb2742 Compare February 2, 2023 22:58

asahilina force-pushed the gpu/rust-for-next branch from a7b131f to 165cafd Compare February 5, 2023 11:17

YakoYakoYokuYoku suggested changes Feb 5, 2023

View reviewed changes

rust/kernel/xarray.rs Show resolved Hide resolved

asahilina force-pushed the gpu/rust-for-next branch from 66a57fa to 0f49432 Compare February 5, 2023 15:12

fbq reviewed Feb 5, 2023

View reviewed changes

bjorn3 reviewed Feb 5, 2023

View reviewed changes

nbdd0121 reviewed Feb 5, 2023

View reviewed changes

rust/kernel/xarray.rs Show resolved Hide resolved

ojeda force-pushed the rust-next branch 2 times, most recently from 892313f to 7ea01d3 Compare February 7, 2023 10:25

asahilina force-pushed the gpu/rust-for-next branch from 0f49432 to b6bbaa8 Compare February 11, 2023 08:16

wedsonaf reviewed Feb 12, 2023

View reviewed changes

sulix and others added 11 commits February 17, 2023 00:50

arm64: rust: Enable PAC support for Rust.

135e133

Enable the PAC ret and BTI options in the Rust build flags to match the options that are used when building C. Signed-off-by: Jamie Cunliffe <[email protected]> Reviewed-by: Vincenzo Palazzo <[email protected]>

rust: macros: Make expect_punct() return the Punct directly

dcc390a

This makes it mirror the way expect_ident() works, and means we can more easily push the result back into the token stream. Signed-off-by: Asahi Lina <[email protected]>

ojeda force-pushed the rust-next branch from dd21715 to edc5e6e Compare May 12, 2025 11:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Tracking/Asahi] Upstream-ready Rust changes #952

[Tracking/Asahi] Upstream-ready Rust changes #952

asahilina commented Jan 26, 2023

fbq Jan 26, 2023

asahilina commented Feb 5, 2023

asahilina commented Feb 5, 2023

asahilina commented Feb 5, 2023

fbq Feb 5, 2023

wedsonaf Feb 11, 2023

asahilina Feb 24, 2023

fbq Feb 5, 2023

nbdd0121 Feb 5, 2023 •

edited

Loading

nbdd0121 Feb 5, 2023

asahilina Feb 6, 2023

asahilina Feb 24, 2023

bjorn3 Feb 5, 2023

fbq Feb 5, 2023

bjorn3 Feb 5, 2023

wedsonaf commented Feb 11, 2023

wedsonaf left a comment

wedsonaf Feb 11, 2023

	pub struct Guard<'a, T: ForeignOwnable>(*mut T, &'a Opaque<bindings::xarray>);
	pub struct Guard<'a, T: ForeignOwnable>((NonNull<T>, &'a XAarray<T>);

[Tracking/Asahi] Upstream-ready Rust changes #952

Are you sure you want to change the base?

[Tracking/Asahi] Upstream-ready Rust changes #952

Conversation

asahilina commented Jan 26, 2023

Choose a reason for hiding this comment

asahilina commented Feb 5, 2023

asahilina commented Feb 5, 2023

asahilina commented Feb 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nbdd0121 Feb 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wedsonaf commented Feb 11, 2023

wedsonaf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nbdd0121 Feb 5, 2023 •

edited

Loading