* Re: [PATCH 2/2] rust: add hw_random module
2026-05-29 15:50 ` [PATCH 2/2] rust: add hw_random module Manos Pitsidianakis
@ 2026-05-29 19:48 ` Onur Özkan
2026-05-29 19:56 ` Onur Özkan
2026-05-29 20:01 ` Miguel Ojeda
2 siblings, 0 replies; 8+ messages in thread
From: Onur Özkan @ 2026-05-29 19:48 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Herbert Xu, rust-for-linux, linux-kernel,
linux-crypto, manos.pitsidianakis
On Fri, 29 May 2026 18:50:27 +0300
Manos Pitsidianakis <manos@pitsidianak.is> wrote:
> Add abstraction for the hardware random number generator core subsystem.
>
> The registration is guarded by an atomic boolean, because we cannot yet
> use IRQ disabling spinlocks in Rust. Once they are supported, we should
> switch to that, because it's theoretically possible to construct a data
> race. In practice I do not think it's possible, since registration
> happens once in driver probe and unregistration happens on driver
> teardown; there shouldn't be multiple threads doing their own thing in
> both cases.
hw_random/core.c seem to use Mutex, why are you not using Mutex<bool> instead of
flipping atomics which doesn't really serialize register & unregister properly?
>
> Signed-off-by: Manos Pitsidianakis <manos@pitsidianak.is>
> ---
> MAINTAINERS | 8 ++
> rust/kernel/hw_random.rs | 320 +++++++++++++++++++++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 2 +
> 3 files changed, 330 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4f60b323c796fc0968fd67d1c7afee6802990572..a3b372ccbd07c4ae2c735ba31f2acf40472b384a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11304,6 +11304,14 @@ F: Documentation/devicetree/bindings/rng/
> F: drivers/char/hw_random/
> F: include/linux/hw_random.h
>
> +HARDWARE RANDOM NUMBER GENERATOR CORE [RUST]
> +M: Manos Pitsidianakis <manos@pitsidianak.is>
> +M: Herbert Xu <herbert@gondor.apana.org.au>
> +L: linux-crypto@vger.kernel.org
> +L: rust-for-linux@vger.kernel.org
> +S: Maintained
> +F: rust/kernel/hw_random.rs
> +
> HARDWARE SPINLOCK CORE
> M: Bjorn Andersson <andersson@kernel.org>
> R: Baolin Wang <baolin.wang7@gmail.com>
> diff --git a/rust/kernel/hw_random.rs b/rust/kernel/hw_random.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..29fc180b4a3b4157a45c8fdb2d94bf1d9d781a3c
> --- /dev/null
> +++ b/rust/kernel/hw_random.rs
> @@ -0,0 +1,320 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Author: Manos Pitsidianakis <manos@pitsidianak.is>
> +
> +//! Hardware Random Number Generators
> +//!
> +//! This module provides an abstraction for implementing a hardware random number generator and
> +//! using it with the kernel's `hw_random` system.
> +//!
> +//! # Example
> +//!
> +//! ```no_run
> +//!# fn no_run() {
> +//!# use kernel::hw_random::*;
> +//!# use kernel::str::CString;
> +//!# use kernel::prelude::*;
> +//! #[pin_data]
> +//! struct ExampleHwRng {}
> +//!
> +//! #[vtable]
> +//! impl HwRngImpl for ExampleHwRng {
> +//! fn read(&self, data: &mut Buffer<'_>, can_wait: bool) -> Result<()> {
> +//! // write zeroes - in your driver, this should write actual data from your hardware.
> +//! data.write(&[0_u8; 8]);
> +//! Ok(())
> +//! }
> +//! }
> +//!
> +//! let name = CString::try_from(c"example_hwrng").unwrap();
> +//! let my_rng = KBox::pin_init(
> +//! HwRng::new(
> +//! name,
> +//! 0,
> +//! try_pin_init!(ExampleHwRng {})
> +//! ),
> +//! GFP_KERNEL
> +//! ).unwrap();
> +//! // Register `my_rng`: after this succeeds, the kernel may call our `HwRngImpl` method at any
> +//! // time.
> +//! my_rng.register().unwrap();
> +//!
> +//! // ...
> +//!
> +//! my_rng.unregister();
> +//!# }
> +//!```
> +
> +use crate::{
> + error::{
> + from_result, //
> + to_result, //
> + VTABLE_DEFAULT_ERROR, //
> + },
> + prelude::*, //
> + str::{
> + CString, //
> + },
> + types::{
> + Opaque, //
> + },
> +};
> +
> +use core::{
> + ffi::{
> + c_int, //
> + c_ushort, //
> + c_void, //
You don't need to put "//" to every line. You can simply do:
a,
b,
c,
//
> + },
> + mem::{
> + MaybeUninit, //
> + },
> + ptr::{
> + slice_from_raw_parts, //
> + slice_from_raw_parts_mut, //
> + },
> + sync::atomic::{
> + AtomicBool, //
> + Ordering, //
> + },
> +};
> +
> +use pin_init::pin_init_from_closure;
> +
> +/// A buffer to write random bytes in using [`Buffer::write`] that tracks how many bytes were
> +/// written.
> +///
> +/// See also [`HwRngImpl::read`].
> +pub struct Buffer<'a> {
> + inner: &'a mut [MaybeUninit<u8>],
> + written: usize,
> +}
> +
> +impl Buffer<'_> {
> + /// Returns `true` if the buffer has been filled.
Either the doc or function name is wrong. Looking at the function logic, this
should be called "is_full"?
> + #[inline]
> + pub const fn is_empty(&self) -> bool {
> + self.written == self.inner.len()
> + }
> +
> + /// Returns the number of bytes that can be written.
> + #[inline]
> + pub const fn len(&self) -> usize {
> + self.inner.len() - self.written
> + }
> +
> + /// Writes bytes from `buf` into buffer and returns the amount of bytes written.
> + #[inline]
> + pub fn write(&mut self, buf: &[u8]) -> usize {
> + let to_copy = self.len().min(buf.len());
> + let ptr = buf.as_ptr();
> + // SAFETY: u8 and MaybeUninit<u8> have the same layout
> + let buf = unsafe { &*slice_from_raw_parts(ptr.cast::<MaybeUninit<u8>>(), to_copy) };
> + self.inner[self.written..][..to_copy].copy_from_slice(buf);
> + self.written += to_copy;
> + to_copy
> + }
> +}
> +
> +/// An adapter type for the registration of hardware random number generators drivers.
> +///
> +/// [`struct hwrng`]: srctree/include/linux/hw_random.h
> +#[pin_data(PinnedDrop)]
> +pub struct HwRng<T: HwRngImpl + 'static> {
> + #[pin]
> + registration: Opaque<bindings::hwrng>,
> + registered: AtomicBool,
> + #[pin]
> + name: CString,
> + #[pin]
> + inner: T,
> +}
> +
> +impl<T: HwRngImpl + 'static> core::ops::Deref for HwRng<T> {
> + type Target = T;
> +
> + #[inline]
> + fn deref(&self) -> &Self::Target {
> + &self.inner
> + }
> +}
> +
> +// SAFETY: HwRng contains a `*const u8` reference but it is opaque for us in Rust.
> +unsafe impl<T: HwRngImpl + 'static> Send for HwRng<T> {}
> +
> +// SAFETY: `HwRng` has no interior mutability from Rust, and C manages it with the rng_mutex lock.
> +unsafe impl<T: HwRngImpl + 'static> Sync for HwRng<T> {}
> +
> +#[pinned_drop]
> +impl<T: HwRngImpl> PinnedDrop for HwRng<T> {
> + fn drop(self: Pin<&mut Self>) {
> + self.unregister();
> + }
> +}
> +
> +#[vtable]
> +/// Trait for the implementation of hardware RNGs.
Doc-comment should come first.
> +pub trait HwRngImpl: Send + Sync {
> + #[inline]
> + /// Initialization callback, can be optionally implemented.
> + fn init(&self) -> Result {
> + build_error!(VTABLE_DEFAULT_ERROR)
> + }
> +
> + #[inline]
> + /// Cleanup callback, can be optionally implemented.
> + fn cleanup(&self) {
> + build_error!(VTABLE_DEFAULT_ERROR)
> + }
> +
> + /// Places random bytes in `data`.
> + fn read(&self, data: &mut Buffer<'_>, can_wait: bool) -> Result<()>;
> +}
> +
> +impl<T: HwRngImpl + 'static> HwRng<T> {
> + /// Create a new [`HwRng`] without registering it.
> + pub fn new(
> + name: CString,
> + quality: c_ushort,
> + inner: impl PinInit<T, Error>,
> + ) -> impl PinInit<Self, Error> {
> + // We use pin_init_from_closure because we need to store the `slot` address as `priv` field
> + // of `hwrng` struct.
> +
> + // SAFETY:
> + // - when the closure returns `Ok(())`, then it has successfully initialized all fields,
> + // - when it returns `Err(e)`, it does not need to perform any cleanup.
> + unsafe {
> + pin_init_from_closure(move |slot: *mut Self| {
> + inner.__pinned_init(&raw mut (*slot).inner)?;
> +
> + let registration = (&raw mut (*slot).registration).cast::<bindings::hwrng>();
> + registration.write(bindings::hwrng {
> + name: name.as_char_ptr(),
> + read: Some(Self::read_callback),
> + init: if <T as HwRngImpl>::HAS_INIT {
> + Some(Self::init_callback)
> + } else {
> + None
> + },
> + cleanup: if <T as HwRngImpl>::HAS_CLEANUP {
> + Some(Self::cleanup_callback)
> + } else {
> + None
> + },
> + quality,
> + priv_: slot as usize,
> + ..Default::default()
> + });
> +
> + let name_ptr = &raw mut (*slot).name;
> + name_ptr.write(name);
> +
> + let registered = &raw mut (*slot).registered;
> + registered.write(AtomicBool::new(false));
> +
> + // All fields of `HwRng` have been initialized
> + Ok(())
> + })
> + }
> + }
> +
> + /// Register `self` with the `hwrng` subsystem.
> + ///
> + /// After this function successfully returns, the `hwrng` subsystem can start calling the
> + /// [`HwRngImpl`] methods at any time.
> + ///
> + /// [`hwrng_register`]: srctree/include/linux/hw_random.h
> + #[inline]
> + #[doc(alias = "hwrng_register")]
> + pub fn register(&self) -> Result {
> + if self
> + .registered
> + .compare_exchange(false, true, Ordering::SeqCst, Ordering::Acquire)
> + .is_ok()
> + {
> + // SAFETY: `registration` is properly initialized.
> + if let Err(err) = to_result(unsafe {
> + bindings::hwrng_register(self.registration.get().cast::<bindings::hwrng>())
> + }) {
> + self.registered.store(false, Ordering::Release);
> + return Err(err);
> + }
> + }
> + Ok(())
> + }
> +
> + /// Unregister `self` from `hwrng` subsystem.
> + ///
> + /// [`hwrng_unregister`]: srctree/include/linux/hw_random.h
> + #[inline]
> + #[doc(alias = "hwrng_unregister")]
> + pub fn unregister(&self) {
> + if self
> + .registered
> + .compare_exchange(true, false, Ordering::SeqCst, Ordering::Acquire)
> + .is_ok()
> + {
> + // SAFETY: Since `registration` is properly initialized and registered, destroying is
> + // safe.
> + unsafe {
> + bindings::hwrng_unregister(self.registration.get().cast::<bindings::hwrng>())
> + };
> + }
> + }
> +}
> +
> +impl<T: HwRngImpl + 'static> HwRng<T> {
> + extern "C" fn init_callback(ptr: *mut bindings::hwrng) -> c_int {
> + // SAFETY: we set `priv_` as the value of `*mut Self` when initializing.
> + let priv_ = unsafe { (*ptr).priv_ };
> + let this_ptr = priv_ as *mut Self;
> +
> + // SAFETY: we set `inner` to point to a valid `T` when initializing.
> + let inner: &T = unsafe { &(*this_ptr).inner };
> + from_result(|| {
> + inner.init()?;
> + Ok(0)
> + })
> + }
> +
> + extern "C" fn cleanup_callback(ptr: *mut bindings::hwrng) {
> + // SAFETY: we set `priv_` as the value of `*mut Self` when initializing.
> + let priv_ = unsafe { (*ptr).priv_ };
> + let this_ptr = priv_ as *mut Self;
> +
> + // SAFETY: we set `inner` to point to a valid `T` when initializing.
> + let inner: &T = unsafe { &(*this_ptr).inner };
> + inner.cleanup();
> + }
> +
> + extern "C" fn read_callback(
> + ptr: *mut bindings::hwrng,
> + data: *mut c_void,
> + max: usize,
> + wait: bool,
> + ) -> c_int {
> + if data.is_null() || max == 0 {
> + return 0;
> + }
> +
> + // SAFETY: we set `priv_` as the value of `*mut Self` when initializing.
> + let priv_ = unsafe { (*ptr).priv_ };
> + let this_ptr = priv_ as *mut Self;
> +
> + let buf_ptr = slice_from_raw_parts_mut(data.cast::<MaybeUninit<u8>>(), max);
> + // SAFETY: By the hw_random API contract, data points to a bytes buffer `max` bytes long.
> + let buf_ref = unsafe { &mut *buf_ptr };
> +
> + let mut buffer = Buffer {
> + inner: buf_ref,
> + written: 0,
> + };
> +
> + // SAFETY: we set `inner` to point to a valid `T` when initializing.
> + let inner: &T = unsafe { &(*this_ptr).inner };
> + from_result(|| {
> + inner.read(&mut buffer, wait)?;
> + Ok(buffer.written.try_into().unwrap_or(c_int::MAX))
> + })
> + }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index ea08641919c26faba97cf5dd9b67b0df55fcd698..096b6d9d57d20612864289e87a359331058fb01c 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -74,6 +74,8 @@
> pub mod fs;
> #[cfg(CONFIG_GPU_BUDDY = "y")]
> pub mod gpu;
> +#[cfg(CONFIG_HW_RANDOM = "y")]
> +pub mod hw_random;
> #[cfg(CONFIG_I2C = "y")]
> pub mod i2c;
> pub mod id_pool;
>
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/2] rust: add hw_random module
2026-05-29 15:50 ` [PATCH 2/2] rust: add hw_random module Manos Pitsidianakis
2026-05-29 19:48 ` Onur Özkan
@ 2026-05-29 19:56 ` Onur Özkan
2026-05-29 20:01 ` Miguel Ojeda
2 siblings, 0 replies; 8+ messages in thread
From: Onur Özkan @ 2026-05-29 19:56 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Herbert Xu, rust-for-linux, linux-kernel,
linux-crypto, manos.pitsidianakis
On Fri, 29 May 2026 18:50:27 +0300
Manos Pitsidianakis <manos@pitsidianak.is> wrote:
> Add abstraction for the hardware random number generator core subsystem.
>
> The registration is guarded by an atomic boolean, because we cannot yet
> use IRQ disabling spinlocks in Rust. Once they are supported, we should
> switch to that, because it's theoretically possible to construct a data
> race. In practice I do not think it's possible, since registration
> happens once in driver probe and unregistration happens on driver
> teardown; there shouldn't be multiple threads doing their own thing in
> both cases.
>
> Signed-off-by: Manos Pitsidianakis <manos@pitsidianak.is>
> ---
> MAINTAINERS | 8 ++
> rust/kernel/hw_random.rs | 320 +++++++++++++++++++++++++++++++++++++++++++++++
> rust/kernel/lib.rs | 2 +
> 3 files changed, 330 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4f60b323c796fc0968fd67d1c7afee6802990572..a3b372ccbd07c4ae2c735ba31f2acf40472b384a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11304,6 +11304,14 @@ F: Documentation/devicetree/bindings/rng/
> F: drivers/char/hw_random/
> F: include/linux/hw_random.h
>
> +HARDWARE RANDOM NUMBER GENERATOR CORE [RUST]
> +M: Manos Pitsidianakis <manos@pitsidianak.is>
> +M: Herbert Xu <herbert@gondor.apana.org.au>
> +L: linux-crypto@vger.kernel.org
> +L: rust-for-linux@vger.kernel.org
> +S: Maintained
> +F: rust/kernel/hw_random.rs
> +
> HARDWARE SPINLOCK CORE
> M: Bjorn Andersson <andersson@kernel.org>
> R: Baolin Wang <baolin.wang7@gmail.com>
> diff --git a/rust/kernel/hw_random.rs b/rust/kernel/hw_random.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..29fc180b4a3b4157a45c8fdb2d94bf1d9d781a3c
> --- /dev/null
> +++ b/rust/kernel/hw_random.rs
> @@ -0,0 +1,320 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Author: Manos Pitsidianakis <manos@pitsidianak.is>
> +
> +//! Hardware Random Number Generators
> +//!
> +//! This module provides an abstraction for implementing a hardware random number generator and
> +//! using it with the kernel's `hw_random` system.
> +//!
> +//! # Example
> +//!
> +//! ```no_run
> +//!# fn no_run() {
> +//!# use kernel::hw_random::*;
> +//!# use kernel::str::CString;
> +//!# use kernel::prelude::*;
> +//! #[pin_data]
> +//! struct ExampleHwRng {}
> +//!
> +//! #[vtable]
> +//! impl HwRngImpl for ExampleHwRng {
> +//! fn read(&self, data: &mut Buffer<'_>, can_wait: bool) -> Result<()> {
> +//! // write zeroes - in your driver, this should write actual data from your hardware.
> +//! data.write(&[0_u8; 8]);
> +//! Ok(())
> +//! }
> +//! }
> +//!
> +//! let name = CString::try_from(c"example_hwrng").unwrap();
> +//! let my_rng = KBox::pin_init(
> +//! HwRng::new(
> +//! name,
> +//! 0,
> +//! try_pin_init!(ExampleHwRng {})
> +//! ),
> +//! GFP_KERNEL
> +//! ).unwrap();
> +//! // Register `my_rng`: after this succeeds, the kernel may call our `HwRngImpl` method at any
> +//! // time.
> +//! my_rng.register().unwrap();
> +//!
> +//! // ...
> +//!
> +//! my_rng.unregister();
> +//!# }
> +//!```
> +
> +use crate::{
> + error::{
> + from_result, //
> + to_result, //
> + VTABLE_DEFAULT_ERROR, //
> + },
> + prelude::*, //
> + str::{
> + CString, //
> + },
> + types::{
> + Opaque, //
> + },
> +};
> +
> +use core::{
> + ffi::{
> + c_int, //
> + c_ushort, //
> + c_void, //
> + },
> + mem::{
> + MaybeUninit, //
> + },
> + ptr::{
> + slice_from_raw_parts, //
> + slice_from_raw_parts_mut, //
> + },
> + sync::atomic::{
> + AtomicBool, //
> + Ordering, //
> + },
> +};
> +
> +use pin_init::pin_init_from_closure;
> +
> +/// A buffer to write random bytes in using [`Buffer::write`] that tracks how many bytes were
> +/// written.
> +///
> +/// See also [`HwRngImpl::read`].
> +pub struct Buffer<'a> {
> + inner: &'a mut [MaybeUninit<u8>],
> + written: usize,
> +}
> +
> +impl Buffer<'_> {
> + /// Returns `true` if the buffer has been filled.
> + #[inline]
> + pub const fn is_empty(&self) -> bool {
> + self.written == self.inner.len()
> + }
> +
> + /// Returns the number of bytes that can be written.
> + #[inline]
> + pub const fn len(&self) -> usize {
This name is quite confusing for what it actually does. `len()` is a very common
API across many types and readers would usually expect it to return the total
length of the buffer. A more explicit name like `remaining_len()` or
`writable_len()` would make it much clearer.
> + self.inner.len() - self.written
> + }
> +
> + /// Writes bytes from `buf` into buffer and returns the amount of bytes written.
> + #[inline]
> + pub fn write(&mut self, buf: &[u8]) -> usize {
> + let to_copy = self.len().min(buf.len());
> + let ptr = buf.as_ptr();
> + // SAFETY: u8 and MaybeUninit<u8> have the same layout
> + let buf = unsafe { &*slice_from_raw_parts(ptr.cast::<MaybeUninit<u8>>(), to_copy) };
> + self.inner[self.written..][..to_copy].copy_from_slice(buf);
> + self.written += to_copy;
> + to_copy
> + }
> +}
> +
> +/// An adapter type for the registration of hardware random number generators drivers.
> +///
> +/// [`struct hwrng`]: srctree/include/linux/hw_random.h
> +#[pin_data(PinnedDrop)]
> +pub struct HwRng<T: HwRngImpl + 'static> {
> + #[pin]
> + registration: Opaque<bindings::hwrng>,
> + registered: AtomicBool,
> + #[pin]
> + name: CString,
> + #[pin]
> + inner: T,
> +}
> +
> +impl<T: HwRngImpl + 'static> core::ops::Deref for HwRng<T> {
> + type Target = T;
> +
> + #[inline]
> + fn deref(&self) -> &Self::Target {
> + &self.inner
> + }
> +}
> +
> +// SAFETY: HwRng contains a `*const u8` reference but it is opaque for us in Rust.
> +unsafe impl<T: HwRngImpl + 'static> Send for HwRng<T> {}
> +
> +// SAFETY: `HwRng` has no interior mutability from Rust, and C manages it with the rng_mutex lock.
> +unsafe impl<T: HwRngImpl + 'static> Sync for HwRng<T> {}
> +
> +#[pinned_drop]
> +impl<T: HwRngImpl> PinnedDrop for HwRng<T> {
> + fn drop(self: Pin<&mut Self>) {
> + self.unregister();
> + }
> +}
> +
> +#[vtable]
> +/// Trait for the implementation of hardware RNGs.
> +pub trait HwRngImpl: Send + Sync {
> + #[inline]
> + /// Initialization callback, can be optionally implemented.
> + fn init(&self) -> Result {
> + build_error!(VTABLE_DEFAULT_ERROR)
> + }
> +
> + #[inline]
> + /// Cleanup callback, can be optionally implemented.
> + fn cleanup(&self) {
> + build_error!(VTABLE_DEFAULT_ERROR)
> + }
> +
> + /// Places random bytes in `data`.
> + fn read(&self, data: &mut Buffer<'_>, can_wait: bool) -> Result<()>;
> +}
> +
> +impl<T: HwRngImpl + 'static> HwRng<T> {
> + /// Create a new [`HwRng`] without registering it.
> + pub fn new(
> + name: CString,
> + quality: c_ushort,
> + inner: impl PinInit<T, Error>,
> + ) -> impl PinInit<Self, Error> {
> + // We use pin_init_from_closure because we need to store the `slot` address as `priv` field
> + // of `hwrng` struct.
> +
> + // SAFETY:
> + // - when the closure returns `Ok(())`, then it has successfully initialized all fields,
> + // - when it returns `Err(e)`, it does not need to perform any cleanup.
> + unsafe {
> + pin_init_from_closure(move |slot: *mut Self| {
> + inner.__pinned_init(&raw mut (*slot).inner)?;
> +
> + let registration = (&raw mut (*slot).registration).cast::<bindings::hwrng>();
> + registration.write(bindings::hwrng {
> + name: name.as_char_ptr(),
> + read: Some(Self::read_callback),
> + init: if <T as HwRngImpl>::HAS_INIT {
> + Some(Self::init_callback)
> + } else {
> + None
> + },
> + cleanup: if <T as HwRngImpl>::HAS_CLEANUP {
> + Some(Self::cleanup_callback)
> + } else {
> + None
> + },
> + quality,
> + priv_: slot as usize,
> + ..Default::default()
> + });
> +
> + let name_ptr = &raw mut (*slot).name;
> + name_ptr.write(name);
> +
> + let registered = &raw mut (*slot).registered;
> + registered.write(AtomicBool::new(false));
> +
> + // All fields of `HwRng` have been initialized
> + Ok(())
> + })
> + }
> + }
> +
> + /// Register `self` with the `hwrng` subsystem.
> + ///
> + /// After this function successfully returns, the `hwrng` subsystem can start calling the
> + /// [`HwRngImpl`] methods at any time.
> + ///
> + /// [`hwrng_register`]: srctree/include/linux/hw_random.h
> + #[inline]
> + #[doc(alias = "hwrng_register")]
> + pub fn register(&self) -> Result {
> + if self
> + .registered
> + .compare_exchange(false, true, Ordering::SeqCst, Ordering::Acquire)
> + .is_ok()
> + {
> + // SAFETY: `registration` is properly initialized.
> + if let Err(err) = to_result(unsafe {
> + bindings::hwrng_register(self.registration.get().cast::<bindings::hwrng>())
> + }) {
> + self.registered.store(false, Ordering::Release);
> + return Err(err);
> + }
> + }
> + Ok(())
> + }
> +
> + /// Unregister `self` from `hwrng` subsystem.
> + ///
> + /// [`hwrng_unregister`]: srctree/include/linux/hw_random.h
> + #[inline]
> + #[doc(alias = "hwrng_unregister")]
> + pub fn unregister(&self) {
> + if self
> + .registered
> + .compare_exchange(true, false, Ordering::SeqCst, Ordering::Acquire)
> + .is_ok()
> + {
> + // SAFETY: Since `registration` is properly initialized and registered, destroying is
> + // safe.
> + unsafe {
> + bindings::hwrng_unregister(self.registration.get().cast::<bindings::hwrng>())
> + };
> + }
> + }
> +}
> +
> +impl<T: HwRngImpl + 'static> HwRng<T> {
> + extern "C" fn init_callback(ptr: *mut bindings::hwrng) -> c_int {
> + // SAFETY: we set `priv_` as the value of `*mut Self` when initializing.
> + let priv_ = unsafe { (*ptr).priv_ };
> + let this_ptr = priv_ as *mut Self;
> +
> + // SAFETY: we set `inner` to point to a valid `T` when initializing.
> + let inner: &T = unsafe { &(*this_ptr).inner };
> + from_result(|| {
> + inner.init()?;
> + Ok(0)
> + })
> + }
> +
> + extern "C" fn cleanup_callback(ptr: *mut bindings::hwrng) {
> + // SAFETY: we set `priv_` as the value of `*mut Self` when initializing.
> + let priv_ = unsafe { (*ptr).priv_ };
> + let this_ptr = priv_ as *mut Self;
> +
> + // SAFETY: we set `inner` to point to a valid `T` when initializing.
> + let inner: &T = unsafe { &(*this_ptr).inner };
> + inner.cleanup();
> + }
> +
> + extern "C" fn read_callback(
> + ptr: *mut bindings::hwrng,
> + data: *mut c_void,
> + max: usize,
> + wait: bool,
> + ) -> c_int {
> + if data.is_null() || max == 0 {
> + return 0;
> + }
> +
> + // SAFETY: we set `priv_` as the value of `*mut Self` when initializing.
> + let priv_ = unsafe { (*ptr).priv_ };
> + let this_ptr = priv_ as *mut Self;
> +
> + let buf_ptr = slice_from_raw_parts_mut(data.cast::<MaybeUninit<u8>>(), max);
> + // SAFETY: By the hw_random API contract, data points to a bytes buffer `max` bytes long.
> + let buf_ref = unsafe { &mut *buf_ptr };
> +
> + let mut buffer = Buffer {
> + inner: buf_ref,
> + written: 0,
> + };
> +
> + // SAFETY: we set `inner` to point to a valid `T` when initializing.
> + let inner: &T = unsafe { &(*this_ptr).inner };
> + from_result(|| {
> + inner.read(&mut buffer, wait)?;
> + Ok(buffer.written.try_into().unwrap_or(c_int::MAX))
> + })
> + }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index ea08641919c26faba97cf5dd9b67b0df55fcd698..096b6d9d57d20612864289e87a359331058fb01c 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -74,6 +74,8 @@
> pub mod fs;
> #[cfg(CONFIG_GPU_BUDDY = "y")]
> pub mod gpu;
> +#[cfg(CONFIG_HW_RANDOM = "y")]
> +pub mod hw_random;
> #[cfg(CONFIG_I2C = "y")]
> pub mod i2c;
> pub mod id_pool;
>
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/2] rust: add hw_random module
2026-05-29 15:50 ` [PATCH 2/2] rust: add hw_random module Manos Pitsidianakis
2026-05-29 19:48 ` Onur Özkan
2026-05-29 19:56 ` Onur Özkan
@ 2026-05-29 20:01 ` Miguel Ojeda
2 siblings, 0 replies; 8+ messages in thread
From: Miguel Ojeda @ 2026-05-29 20:01 UTC (permalink / raw)
To: Manos Pitsidianakis
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Herbert Xu, rust-for-linux, linux-kernel,
linux-crypto, manos.pitsidianakis
Hi Manos,
Some quick doc nits I noticed while managing our list...
On Fri, May 29, 2026 at 5:50 PM Manos Pitsidianakis
<manos@pitsidianak.is> wrote:
>
> +//! # Example
"Examples" is the section name we use.
> +//!# fn no_run() {
> +//!# use kernel::hw_random::*;
> +//!# use kernel::str::CString;
> +//!# use kernel::prelude::*;
Missing indentation.
> +//! fn read(&self, data: &mut Buffer<'_>, can_wait: bool) -> Result<()> {
-> Result
> +//! // write zeroes - in your driver, this should write actual data from your hardware.
"Write"
> +//! let name = CString::try_from(c"example_hwrng").unwrap();
Could you avoid `unwrap()`s, perhaps using `?` etc.? It is not
critical, but good practice to try to show how "real code" would be
written.
> + from_result, //
> + to_result, //
> + VTABLE_DEFAULT_ERROR, //
Please only use the slashes in the last one:
> + },
...but add one here, since this level doesn't have it.
> + /// Returns `true` if the buffer has been filled.
[`true`]
> + // SAFETY: u8 and MaybeUninit<u8> have the same layout
Please use Markdown (but no intra-doc links needed) in comments too, e.g.
`u8` ... `MaybeUninit<u8>`
There are other instances below.
> +/// [`struct hwrng`]: srctree/include/linux/hw_random.h
Is this reference used?
> +#[vtable]
> +/// Trait for the implementation of hardware RNGs.
Attributes after documentation.
Other instances below too.
> + /// [`hwrng_unregister`]: srctree/include/linux/hw_random.h
> + #[inline]
> + #[doc(alias = "hwrng_unregister")]
Is this one for the alias? How does it behave when rendered?
> + pub fn unregister(&self) {
> + if self
> + .registered
> + .compare_exchange(true, false, Ordering::SeqCst, Ordering::Acquire)
> + .is_ok()
We are starting to add `// ORDERING: ...` comments for things like
this, so it would be nice to have them here already.
> + // SAFETY: we set `priv_` as the value of `*mut Self` when initializing.
Please start comments capitalized. Other instances elsewhere too.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 8+ messages in thread