[PATCH V2 0/2] rust: Introduce CpuId and fix cpumask doctest

[PATCH V2 0/2] rust: Introduce CpuId and fix cpumask doctest

[Viresh Kumar]

Hello,

Here is another attempt at fixing the cpumask doctest. This series creates a new
abstraction `CpuId`, which is used to write a cleaner cpumask example which
doesn't fail in those corner cases.

--
Viresh

V1->V2:
- Introduce CpuId.
- Use CpuId in cpufreq, opp, cpumask abstractions.
- Fix cpumask example.

Viresh Kumar (2):
  rust: cpu: Introduce CpuId abstraction
  rust: Use CpuId in place of raw CPU numbers

 drivers/cpufreq/rcpufreq_dt.rs |   4 +-
 rust/kernel/cpu.rs             | 106 ++++++++++++++++++++++++++++++++-
 rust/kernel/cpufreq.rs         |  27 ++++++---
 rust/kernel/cpumask.rs         |  51 +++++++++++-----
 4 files changed, 161 insertions(+), 27 deletions(-)


base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
-- 
2.31.1.272.g89b43f80a514

[PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Viresh Kumar]

This adds abstraction for representing a CPU identifier.

Suggested-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/kernel/cpu.rs | 102 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/rust/kernel/cpu.rs b/rust/kernel/cpu.rs
index 10c5c3b25873..0b545dbf5c83 100644
--- a/rust/kernel/cpu.rs
+++ b/rust/kernel/cpu.rs
@@ -6,6 +6,108 @@
 
 use crate::{bindings, device::Device, error::Result, prelude::ENODEV};
 
+/// Returns the maximum number of possible CPUs in the current system configuration.
+#[inline]
+pub fn nr_cpu_ids() -> u32 {
+    #[cfg(any(NR_CPUS_1, CONFIG_FORCE_NR_CPUS))]
+    {
+        bindings::NR_CPUS
+    }
+
+    #[cfg(not(any(NR_CPUS_1, CONFIG_FORCE_NR_CPUS)))]
+    // SAFETY: `nr_cpu_ids` is a valid global provided by the kernel.
+    unsafe {
+        bindings::nr_cpu_ids
+    }
+}
+
+/// The CPU ID.
+///
+/// Represents a CPU identifier as a wrapper around an `u32`.
+///
+/// # Invariants
+///
+/// The CPU ID must always lie within the range `[0, nr_cpu_ids())`.
+///
+/// ## Examples
+///
+/// ```
+/// use kernel::cpu::CpuId;
+///
+/// let cpu = 0;
+///
+/// // SAFETY: 0 is always a valid CPU number.
+/// let id = unsafe { CpuId::from_u32_unchecked(cpu) };
+///
+/// assert_eq!(id.as_u32(), cpu);
+/// assert!(CpuId::from_i32(0).is_some());
+/// assert!(CpuId::from_i32(-1).is_none());
+/// ```
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+pub struct CpuId(u32);
+
+impl CpuId {
+    /// Creates a new [`CpuId`] from the given `id` without checking bounds.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `id` is a valid CPU ID (i.e., `0 <= id < nr_cpu_ids()`).
+    #[inline]
+    pub unsafe fn from_i32_unchecked(id: i32) -> Self {
+        // INVARIANT: The function safety guarantees `id` is a valid CPU id.
+        Self(id as u32)
+    }
+
+    /// Creates a new [`CpuId`] from the given `id`, checking that it is valid.
+    pub fn from_i32(id: i32) -> Option<Self> {
+        if id < 0 || id as u32 >= nr_cpu_ids() {
+            None
+        } else {
+            // SAFETY: `id` has just been checked as a valid CPU ID.
+            Some(unsafe { Self::from_i32_unchecked(id) })
+        }
+    }
+
+    /// Creates a new [`CpuId`] from the given `id` without checking bounds.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `id` is a valid CPU ID (i.e., `0 <= id < nr_cpu_ids()`).
+    #[inline]
+    pub unsafe fn from_u32_unchecked(id: u32) -> Self {
+        // INVARIANT: The function safety guarantees `id` is a valid CPU id.
+        Self(id)
+    }
+
+    /// Creates a new [`CpuId`] from the given `id`, checking that it is valid.
+    pub fn from_u32(id: u32) -> Option<Self> {
+        if id >= nr_cpu_ids() {
+            None
+        } else {
+            // SAFETY: `id` has just been checked as a valid CPU ID.
+            Some(unsafe { Self::from_u32_unchecked(id) })
+        }
+    }
+
+    /// Returns CPU number.
+    #[inline]
+    pub fn as_u32(&self) -> u32 {
+        self.0
+    }
+}
+
+impl From<CpuId> for u32 {
+    fn from(id: CpuId) -> Self {
+        id.as_u32()
+    }
+}
+
+impl From<CpuId> for i32 {
+    fn from(id: CpuId) -> Self {
+        id.as_u32() as i32
+    }
+}
+
 /// Creates a new instance of CPU's device.
 ///
 /// # Safety
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Viresh Kumar]

On 09-06-25, 16:21, Viresh Kumar wrote:
> This adds abstraction for representing a CPU identifier.
> 
> Suggested-by: Boqun Feng <boqun.feng@gmail.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  rust/kernel/cpu.rs | 102 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 102 insertions(+)

Boqun,

Should I implement CpuId::current() like this ? Will fold this into
1/2, if it looks okay.

diff --git a/rust/helpers/cpu.c b/rust/helpers/cpu.c
new file mode 100644
index 000000000000..61d0387c8cf3
--- /dev/null
+++ b/rust/helpers/cpu.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/smp.h>
+
+unsigned int rust_helper_smp_processor_id(void)
+{
+       return smp_processor_id();
+}
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 0f1b5d115985..16fa9bca5949 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -13,6 +13,7 @@
 #include "build_assert.c"
 #include "build_bug.c"
 #include "clk.c"
+#include "cpu.c"
 #include "cpufreq.c"
 #include "cpumask.c"
 #include "cred.c"
diff --git a/rust/kernel/cpu.rs b/rust/kernel/cpu.rs
index da53f04da495..b093d22ccbd9 100644
--- a/rust/kernel/cpu.rs
+++ b/rust/kernel/cpu.rs
@@ -94,6 +94,12 @@ pub fn from_u32(id: u32) -> Option<Self> {
     pub fn as_u32(&self) -> u32 {
         self.0
     }
+
+    /// Returns the ID of the CPU this code is currently running on.
+    pub fn current() -> Self {
+        // SAFETY: smp_processor_id() always return valid cpu id.
+        unsafe { Self::from_u32_unchecked(bindings::smp_processor_id()) }
+    }
 }

 impl From<CpuId> for u32 {

-- 
viresh

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Boqun Feng]

On Mon, Jun 09, 2025 at 04:48:57PM +0530, Viresh Kumar wrote:
> On 09-06-25, 16:21, Viresh Kumar wrote:
> > This adds abstraction for representing a CPU identifier.
> > 
> > Suggested-by: Boqun Feng <boqun.feng@gmail.com>
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  rust/kernel/cpu.rs | 102 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 102 insertions(+)
> 
> Boqun,
> 
> Should I implement CpuId::current() like this ? Will fold this into
> 1/2, if it looks okay.
> 

Thanks!

I think you can keep this as a separate patch for the ease of review.
And I just realize that we should use raw_smp_processor_id() because
the current API only support an unstable cpuid read. For a stable cpuid
read, we need some lifetime description of the return value to make sure
it doesn't outlive the scope that guarantees the cpuid is stable. Well,
the user can still use the unstable CpuId and ensure the scope provides
the cpuid stability (see comments of smp_processor_id()), it's just
CpuId::current() doesn't need to guarantee that.

> diff --git a/rust/helpers/cpu.c b/rust/helpers/cpu.c
> new file mode 100644
> index 000000000000..61d0387c8cf3
> --- /dev/null
> +++ b/rust/helpers/cpu.c
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/smp.h>
> +
> +unsigned int rust_helper_smp_processor_id(void)
> +{
> +       return smp_processor_id();
> +}
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 0f1b5d115985..16fa9bca5949 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -13,6 +13,7 @@
>  #include "build_assert.c"
>  #include "build_bug.c"
>  #include "clk.c"
> +#include "cpu.c"
>  #include "cpufreq.c"
>  #include "cpumask.c"
>  #include "cred.c"
> diff --git a/rust/kernel/cpu.rs b/rust/kernel/cpu.rs
> index da53f04da495..b093d22ccbd9 100644
> --- a/rust/kernel/cpu.rs
> +++ b/rust/kernel/cpu.rs
> @@ -94,6 +94,12 @@ pub fn from_u32(id: u32) -> Option<Self> {
>      pub fn as_u32(&self) -> u32 {
>          self.0
>      }
> +
> +    /// Returns the ID of the CPU this code is currently running on.

We need mention that the result is an unstable one, and refer to the
raw_smp_processor_id() in include/linux/smp.h (we probably also want to
fix the typo there, i.e. s/raw_processor_id/raw_smp_processor_id, while
we are at it). Also it's worth mentioning that if the context between
CpuId::current() and use of the return value indeed guarantee cpuid
stability, the users can treat it as a stable one.

Regards,
Boqun

> +    pub fn current() -> Self {
> +        // SAFETY: smp_processor_id() always return valid cpu id.
> +        unsafe { Self::from_u32_unchecked(bindings::smp_processor_id()) }
> +    }
>  }
> 
>  impl From<CpuId> for u32 {
> 
> -- 
> viresh

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Viresh Kumar]

On 09-06-25, 07:04, Boqun Feng wrote:
> I think you can keep this as a separate patch for the ease of review.
> And I just realize that we should use raw_smp_processor_id() because
> the current API only support an unstable cpuid read. For a stable cpuid
> read, we need some lifetime description of the return value to make sure
> it doesn't outlive the scope that guarantees the cpuid is stable. Well,
> the user can still use the unstable CpuId and ensure the scope provides
> the cpuid stability (see comments of smp_processor_id()), it's just
> CpuId::current() doesn't need to guarantee that.
 
> We need mention that the result is an unstable one, and refer to the
> raw_smp_processor_id() in include/linux/smp.h (we probably also want to
> fix the typo there, i.e. s/raw_processor_id/raw_smp_processor_id, while
> we are at it). Also it's worth mentioning that if the context between
> CpuId::current() and use of the return value indeed guarantee cpuid
> stability, the users can treat it as a stable one.

Thanks. How about this:

Subject: [PATCH] rust: cpu: Add CpuId::current() to retrieve current CPU ID

Introduce `CpuId::current()`, a constructor that wraps the C function
`raw_smp_processor_id()` to retrieve the current CPU identifier without
guaranteeing stability.

This function should be used only when the caller can ensure that
the CPU ID won't change unexpectedly due to preemption or migration.

Suggested-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 rust/helpers/cpu.c     |  8 ++++++++
 rust/helpers/helpers.c |  1 +
 rust/kernel/cpu.rs     | 10 ++++++++++
 3 files changed, 19 insertions(+)
 create mode 100644 rust/helpers/cpu.c

diff --git a/rust/helpers/cpu.c b/rust/helpers/cpu.c
new file mode 100644
index 000000000000..824e0adb19d4
--- /dev/null
+++ b/rust/helpers/cpu.c
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/smp.h>
+
+unsigned int rust_helper_raw_smp_processor_id(void)
+{
+	return raw_smp_processor_id();
+}
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 0f1b5d115985..16fa9bca5949 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -13,6 +13,7 @@
 #include "build_assert.c"
 #include "build_bug.c"
 #include "clk.c"
+#include "cpu.c"
 #include "cpufreq.c"
 #include "cpumask.c"
 #include "cred.c"
diff --git a/rust/kernel/cpu.rs b/rust/kernel/cpu.rs
index fbc47d2814df..fd2240fe6e2d 100644
--- a/rust/kernel/cpu.rs
+++ b/rust/kernel/cpu.rs
@@ -99,6 +99,16 @@ pub fn from_u32(id: u32) -> Option<Self> {
     pub fn as_u32(&self) -> u32 {
         self.0
     }
+
+    /// Returns the ID of the CPU the code is currently running on.
+    ///
+    /// The returned value is considered unstable because it may change
+    /// unexpectedly due to preemption or CPU migration. It should only be
+    /// used when the context ensures that the task remains on the same CPU.
+    pub fn current() -> Self {
+        // SAFETY: raw_smp_processor_id() always returns a valid CPU ID.
+        unsafe { Self::from_u32_unchecked(bindings::raw_smp_processor_id()) }
+    }
 }
 
 impl From<CpuId> for u32 {

-- 
viresh

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Miguel Ojeda]

On Mon, Jun 9, 2025 at 12:51 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> +/// Represents a CPU identifier as a wrapper around an `u32`.

[`u32`]

> +/// # Invariants
> +///
> +/// The CPU ID must always lie within the range `[0, nr_cpu_ids())`.

I think we can simplify to "ID lies", i.e. "must always" is not needed
(but we use it elsewhere form time to time, though we may end up
cleaning those too).

> +/// ## Examples

Single `#`.

> +    pub unsafe fn from_i32_unchecked(id: i32) -> Self {

Why do we need the `i32` versions?

Is it just for `bios_limit_callback`? If so, I would just convert there.

From a quick look at the C side, it seems that could be an `u32` -- I
am not suggesting to change the C side now since we don't want to
complicate the fix, but perhaps something to consider in the future,
assuming there is no reason to have a signed integer there (e.g. an
unsigned integer is used in the policy struct).

Relatedly, why isn't that callback's type `c_int` on the Rust side?

I also opened a "good first issue" for a docs bit:
https://github.com/Rust-for-Linux/linux/issues/1169. I can open one
for the C FFI types if you think it should be changed.

Finally, can we add a `debug_assert!()` on the `_unchecked()` variant,
since it is something we can easily check?

Thanks!

Cheers,
Miguel

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Viresh Kumar]

Thanks Miguel for the feedback.

On 09-06-25, 14:01, Miguel Ojeda wrote:
> On Mon, Jun 9, 2025 at 12:51 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > +    pub unsafe fn from_i32_unchecked(id: i32) -> Self {
> 
> Why do we need the `i32` versions?
> 
> Is it just for `bios_limit_callback`? If so, I would just convert there.

It looked like that there are few interfaces in the kernel which
accept an `i32` CPU ID. One example is cpumask layer, where
`cpumask_set_cpu()` expects an `unsigned int` but
`cpumask_{clear|test}_cpu()` expects an `int`.

Though the only user of the `i32` version for now is
`bios_limit_callback`, which I can handle without it too.

What do you suggest ?

> Relatedly, why isn't that callback's type `c_int` on the Rust side?

Sorry, not sure I understood that. Are you talking about type of the
`cpu` argument ?

    extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> kernel::ffi::c_int

> Finally, can we add a `debug_assert!()` on the `_unchecked()` variant,
> since it is something we can easily check?

Changes so far:

diff --git a/rust/kernel/cpu.rs b/rust/kernel/cpu.rs
index 68d7d86df27c..8d48946090e3 100644
--- a/rust/kernel/cpu.rs
+++ b/rust/kernel/cpu.rs
@@ -23,11 +23,11 @@ pub fn nr_cpu_ids() -> u32 {

 /// The CPU ID.
 ///
-/// Represents a CPU identifier as a wrapper around an `u32`.
+/// Represents a CPU identifier as a wrapper around an [`u32`].
 ///
 /// # Invariants
 ///
-/// The CPU ID must always lie within the range `[0, nr_cpu_ids())`.
+/// The CPU ID lies within the range `[0, nr_cpu_ids())`.
 ///
 /// # Examples
 ///
@@ -54,6 +54,9 @@ impl CpuId {
     /// The caller must ensure that `id` is a valid CPU ID (i.e., `0 <= id < nr_cpu_ids()`).
     #[inline]
     pub unsafe fn from_i32_unchecked(id: i32) -> Self {
+        debug_assert!(id >= 0);
+        debug_assert!((id as u32) < nr_cpu_ids());
+
         // INVARIANT: The function safety guarantees `id` is a valid CPU id.
         Self(id as u32)
     }
@@ -63,8 +66,8 @@ pub fn from_i32(id: i32) -> Option<Self> {
         if id < 0 || id as u32 >= nr_cpu_ids() {
             None
         } else {
-            // SAFETY: `id` has just been checked as a valid CPU ID.
-            Some(unsafe { Self::from_i32_unchecked(id) })
+            // INVARIANT: `id` has just been checked as a valid CPU ID.
+            Some(Self(id as u32))
         }
     }

@@ -75,6 +78,8 @@ pub fn from_i32(id: i32) -> Option<Self> {
     /// The caller must ensure that `id` is a valid CPU ID (i.e., `0 <= id < nr_cpu_ids()`).
     #[inline]
     pub unsafe fn from_u32_unchecked(id: u32) -> Self {
+        debug_assert!(id < nr_cpu_ids());
+
         // INVARIANT: The function safety guarantees `id` is a valid CPU id.
         Self(id)
     }
@@ -84,8 +89,8 @@ pub fn from_u32(id: u32) -> Option<Self> {
         if id >= nr_cpu_ids() {
             None
         } else {
-            // SAFETY: `id` has just been checked as a valid CPU ID.
-            Some(unsafe { Self::from_u32_unchecked(id) })
+            // INVARIANT: `id` has just been checked as a valid CPU ID.
+            Some(Self(id))
         }
     }


-- 
viresh

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Miguel Ojeda]

On Tue, Jun 10, 2025 at 8:07 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> What do you suggest ?

I think you are right, we should keep both directions of conversion.

(By the way, as far as I understand there is no way we could wrap
going to `i32`, but it may not hurt to add a `debug_assert!` in the
`u32` constructors.)

> Sorry, not sure I understood that. Are you talking about type of the
> `cpu` argument ?

Yeah.

Thanks!

Cheers,
Miguel

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Viresh Kumar]

On 10-06-25, 11:43, Miguel Ojeda wrote:
> (By the way, as far as I understand there is no way we could wrap
> going to `i32`, but it may not hurt to add a `debug_assert!` in the
> `u32` constructors.)

Something like this ?

@@ -79,6 +79,7 @@ pub fn from_i32(id: i32) -> Option<Self> {
     #[inline]
     pub unsafe fn from_u32_unchecked(id: u32) -> Self {
         debug_assert!(id < nr_cpu_ids());
+        debug_assert!(id <= i32::MAX as u32);

         // INVARIANT: The function safety guarantees `id` is a valid CPU id.
         Self(id)

-- 
viresh

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Miguel Ojeda]

On Tue, Jun 10, 2025 at 12:18 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Something like this ?

Yeah, something like that was what I was thinking (maybe with a
comment if it is not obvious why we have that one). Thanks!

Cheers,
Miguel

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Viresh Kumar]

On 09-06-25, 14:01, Miguel Ojeda wrote:
> I also opened a "good first issue" for a docs bit:
> https://github.com/Rust-for-Linux/linux/issues/1169.

Just wanted to make sure before I post this, you were talking about
these, right ?

@@ -1057,7 +1057,10 @@ pub fn new_foreign_owned(dev: &Device<Bound>) -> Result {
 impl<T: Driver> Registration<T> {
     /// Driver's `init` callback.
     ///
-    /// SAFETY: Called from C. Inputs must be valid pointers.
+    /// # Safety
+    ///
+    /// - This function may only be called from the cpufreq C infrastructure.
+    /// - The pointer arguments must be valid pointers.
     extern "C" fn init_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int {
         from_result(|| {
             // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
@@ -1072,7 +1075,10 @@ extern "C" fn init_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::

     /// Driver's `exit` callback.
     ///
-    /// SAFETY: Called from C. Inputs must be valid pointers.
+    /// # Safety
+    ///
+    /// - This function may only be called from the cpufreq C infrastructure.
+    /// - The pointer arguments must be valid pointers.
     extern "C" fn exit_callback(ptr: *mut bindings::cpufreq_policy) {
         // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the
         // lifetime of `policy`.

-- 
viresh

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Miguel Ojeda]

On Tue, Jun 10, 2025 at 12:00 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Just wanted to make sure before I post this, you were talking about
> these, right ?

Yeah.

The "good first issues" are intended for newcomers to the kernel, i.e.
to try to help others get involved into the kernel.

But if you already did it, then please post it of course. (And maybe
you just don't want "good first issues" for files, of course).

Thanks!

Cheers,
Miguel

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Miguel Ojeda]

On Tue, Jun 10, 2025 at 12:14 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> But if you already did it, then please post it of course. (And maybe
> you just don't want "good first issues" for files, of course).

I meant "for your files", I meant, sorry.

i.e. it takes some work to guide people and help them get their patch
setup right etc., so it can be often quite more work than just doing
it oneself. So it is understandable if a maintainer doesn't want
those.

Cheers,
Miguel

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Viresh Kumar]

On 10-06-25, 12:21, Miguel Ojeda wrote:
> I meant "for your files", I meant, sorry.
> 
> i.e. it takes some work to guide people and help them get their patch
> setup right etc., so it can be often quite more work than just doing
> it oneself. So it is understandable if a maintainer doesn't want
> those.

Its perfectly fine and I am available to guide them and review patches
:)

-- 
viresh

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Viresh Kumar]

On 09-06-25, 14:01, Miguel Ojeda wrote:
> I can open one for the C FFI types if you think it should be
> changed.

Yes please (And I won't try to solve it this time :) ). Thanks.

-- 
viresh

Re: [PATCH V2 1/2] rust: cpu: Introduce CpuId abstraction

[Miguel Ojeda]

On Tue, Jun 10, 2025 at 12:19 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Yes please (And I won't try to solve it this time :) ). Thanks.

Done!

    https://github.com/Rust-for-Linux/linux/issues/1170

I wasn't sure if it should be considered a fix (or if you would
consider it a fix), so I used the Suggested-by+Link template instead.

Cheers,
Miguel

[PATCH V2 2/2] rust: Use CpuId in place of raw CPU numbers

[Viresh Kumar]

Use the newly defined `CpuId` abstraction instead of raw CPU numbers.

This also fixes a doctest failure for configurations where `nr_cpu_ids <
4`.

The C `cpumask_{set|clear}_cpu()` APIs emit a warning when given an
invalid CPU number — but only if `CONFIG_DEBUG_PER_CPU_MAPS=y` is set.

Meanwhile, `cpumask_weight()` only considers CPUs up to `nr_cpu_ids`,
which can cause inconsistencies: a CPU number greater than `nr_cpu_ids`
may be set in the mask, yet the weight calculation won't reflect it.

This leads to doctest failures when `nr_cpu_ids < 4`, as the test tries
to set CPUs 2 and 3:

  rust_doctest_kernel_cpumask_rs_0.location: rust/kernel/cpumask.rs:180
  rust_doctest_kernel_cpumask_rs_0: ASSERTION FAILED at rust/kernel/cpumask.rs:190

Fixes: 8961b8cb3099 ("rust: cpumask: Add initial abstractions")
Reported-by: Miguel Ojeda <ojeda@kernel.org>
Closes: https://lore.kernel.org/rust-for-linux/CANiq72k3ozKkLMinTLQwvkyg9K=BeRxs1oYZSKhJHY-veEyZdg@mail.gmail.com/
Reported-by: Andreas Hindborg <a.hindborg@kernel.org>
Closes: https://lore.kernel.org/all/87qzzy3ric.fsf@kernel.org/
Suggested-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/rcpufreq_dt.rs |  4 +--
 rust/kernel/cpu.rs             |  4 +--
 rust/kernel/cpufreq.rs         | 27 ++++++++++++------
 rust/kernel/cpumask.rs         | 51 ++++++++++++++++++++++++----------
 4 files changed, 59 insertions(+), 27 deletions(-)

diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
index 94ed81644fe1..43c87d0259b6 100644
--- a/drivers/cpufreq/rcpufreq_dt.rs
+++ b/drivers/cpufreq/rcpufreq_dt.rs
@@ -26,9 +26,9 @@ fn find_supply_name_exact(dev: &Device, name: &str) -> Option<CString> {
 }
 
 /// Finds supply name for the CPU from DT.
-fn find_supply_names(dev: &Device, cpu: u32) -> Option<KVec<CString>> {
+fn find_supply_names(dev: &Device, cpu: cpu::CpuId) -> Option<KVec<CString>> {
     // Try "cpu0" for older DTs, fallback to "cpu".
-    let name = (cpu == 0)
+    let name = (cpu.as_u32() == 0)
         .then(|| find_supply_name_exact(dev, "cpu0"))
         .flatten()
         .or_else(|| find_supply_name_exact(dev, "cpu"))?;
diff --git a/rust/kernel/cpu.rs b/rust/kernel/cpu.rs
index 0b545dbf5c83..da53f04da495 100644
--- a/rust/kernel/cpu.rs
+++ b/rust/kernel/cpu.rs
@@ -119,9 +119,9 @@ fn from(id: CpuId) -> Self {
 /// Callers must ensure that the CPU device is not used after it has been unregistered.
 /// This can be achieved, for example, by registering a CPU hotplug notifier and removing
 /// any references to the CPU device within the notifier's callback.
-pub unsafe fn from_cpu(cpu: u32) -> Result<&'static Device> {
+pub unsafe fn from_cpu(cpu: CpuId) -> Result<&'static Device> {
     // SAFETY: It is safe to call `get_cpu_device()` for any CPU.
-    let ptr = unsafe { bindings::get_cpu_device(cpu) };
+    let ptr = unsafe { bindings::get_cpu_device(cpu.into()) };
     if ptr.is_null() {
         return Err(ENODEV);
     }
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index b0a9c6182aec..1cb9c6c8cd4b 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -10,6 +10,7 @@
 
 use crate::{
     clk::Hertz,
+    cpu::CpuId,
     cpumask,
     device::{Bound, Device},
     devres::Devres,
@@ -465,8 +466,9 @@ fn as_mut_ref(&mut self) -> &mut bindings::cpufreq_policy {
 
     /// Returns the primary CPU for the [`Policy`].
     #[inline]
-    pub fn cpu(&self) -> u32 {
-        self.as_ref().cpu
+    pub fn cpu(&self) -> CpuId {
+        // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.
+        unsafe { CpuId::from_u32_unchecked(self.as_ref().cpu) }
     }
 
     /// Returns the minimum frequency for the [`Policy`].
@@ -525,7 +527,7 @@ pub fn generic_suspend(&mut self) -> Result {
     #[inline]
     pub fn generic_get(&self) -> Result<u32> {
         // SAFETY: By the type invariant, the pointer stored in `self` is valid.
-        Ok(unsafe { bindings::cpufreq_generic_get(self.cpu()) })
+        Ok(unsafe { bindings::cpufreq_generic_get(self.cpu().into()) })
     }
 
     /// Provides a wrapper to the register with energy model using the OPP core.
@@ -678,9 +680,9 @@ fn clear_data<T: ForeignOwnable>(&mut self) -> Option<T> {
 struct PolicyCpu<'a>(&'a mut Policy);
 
 impl<'a> PolicyCpu<'a> {
-    fn from_cpu(cpu: u32) -> Result<Self> {
+    fn from_cpu(cpu: CpuId) -> Result<Self> {
         // SAFETY: It is safe to call `cpufreq_cpu_get` for any valid CPU.
-        let ptr = from_err_ptr(unsafe { bindings::cpufreq_cpu_get(cpu) })?;
+        let ptr = from_err_ptr(unsafe { bindings::cpufreq_cpu_get(cpu.into()) })?;
 
         Ok(Self(
             // SAFETY: The `ptr` is guaranteed to be valid and remains valid for the lifetime of
@@ -1218,7 +1220,10 @@ extern "C" fn adjust_perf_callback(
         target_perf: usize,
         capacity: usize,
     ) {
-        if let Ok(mut policy) = PolicyCpu::from_cpu(cpu) {
+        // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.
+        let cpu_id = unsafe { CpuId::from_u32_unchecked(cpu) };
+
+        if let Ok(mut policy) = PolicyCpu::from_cpu(cpu_id) {
             T::adjust_perf(&mut policy, min_perf, target_perf, capacity);
         }
     }
@@ -1263,7 +1268,10 @@ extern "C" fn target_intermediate_callback(
 
     /// Driver's `get` callback.
     extern "C" fn get_callback(cpu: u32) -> kernel::ffi::c_uint {
-        PolicyCpu::from_cpu(cpu).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f))
+        // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.
+        let cpu_id = unsafe { CpuId::from_u32_unchecked(cpu) };
+
+        PolicyCpu::from_cpu(cpu_id).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f))
     }
 
     /// Driver's `update_limit` callback.
@@ -1278,8 +1286,11 @@ extern "C" fn update_limits_callback(ptr: *mut bindings::cpufreq_policy) {
     ///
     /// SAFETY: Called from C. Inputs must be valid pointers.
     extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> kernel::ffi::c_int {
+        // SAFETY: The C API guarantees that `cpu` refers to a valid CPU number.
+        let cpu_id = unsafe { CpuId::from_i32_unchecked(cpu) };
+
         from_result(|| {
-            let mut policy = PolicyCpu::from_cpu(cpu as u32)?;
+            let mut policy = PolicyCpu::from_cpu(cpu_id)?;
 
             // SAFETY: `limit` is guaranteed by the C code to be valid.
             T::bios_limit(&mut policy, &mut (unsafe { *limit })).map(|()| 0)
diff --git a/rust/kernel/cpumask.rs b/rust/kernel/cpumask.rs
index c90bfac9346a..11ddd43edcb5 100644
--- a/rust/kernel/cpumask.rs
+++ b/rust/kernel/cpumask.rs
@@ -6,6 +6,7 @@
 
 use crate::{
     alloc::{AllocError, Flags},
+    cpu::CpuId,
     prelude::*,
     types::Opaque,
 };
@@ -35,9 +36,10 @@
 ///
 /// ```
 /// use kernel::bindings;
+/// use kernel::cpu::CpuId;
 /// use kernel::cpumask::Cpumask;
 ///
-/// fn set_clear_cpu(ptr: *mut bindings::cpumask, set_cpu: u32, clear_cpu: i32) {
+/// fn set_clear_cpu(ptr: *mut bindings::cpumask, set_cpu: CpuId, clear_cpu: CpuId) {
 ///     // SAFETY: The `ptr` is valid for writing and remains valid for the lifetime of the
 ///     // returned reference.
 ///     let mask = unsafe { Cpumask::as_mut_ref(ptr) };
@@ -90,9 +92,9 @@ pub fn as_raw(&self) -> *mut bindings::cpumask {
     /// This mismatches kernel naming convention and corresponds to the C
     /// function `__cpumask_set_cpu()`.
     #[inline]
-    pub fn set(&mut self, cpu: u32) {
+    pub fn set(&mut self, cpu: CpuId) {
         // SAFETY: By the type invariant, `self.as_raw` is a valid argument to `__cpumask_set_cpu`.
-        unsafe { bindings::__cpumask_set_cpu(cpu, self.as_raw()) };
+        unsafe { bindings::__cpumask_set_cpu(cpu.into(), self.as_raw()) };
     }
 
     /// Clear `cpu` in the cpumask.
@@ -101,19 +103,19 @@ pub fn set(&mut self, cpu: u32) {
     /// This mismatches kernel naming convention and corresponds to the C
     /// function `__cpumask_clear_cpu()`.
     #[inline]
-    pub fn clear(&mut self, cpu: i32) {
+    pub fn clear(&mut self, cpu: CpuId) {
         // SAFETY: By the type invariant, `self.as_raw` is a valid argument to
         // `__cpumask_clear_cpu`.
-        unsafe { bindings::__cpumask_clear_cpu(cpu, self.as_raw()) };
+        unsafe { bindings::__cpumask_clear_cpu(cpu.into(), self.as_raw()) };
     }
 
     /// Test `cpu` in the cpumask.
     ///
     /// Equivalent to the kernel's `cpumask_test_cpu` API.
     #[inline]
-    pub fn test(&self, cpu: i32) -> bool {
+    pub fn test(&self, cpu: CpuId) -> bool {
         // SAFETY: By the type invariant, `self.as_raw` is a valid argument to `cpumask_test_cpu`.
-        unsafe { bindings::cpumask_test_cpu(cpu, self.as_raw()) }
+        unsafe { bindings::cpumask_test_cpu(cpu.into(), self.as_raw()) }
     }
 
     /// Set all CPUs in the cpumask.
@@ -178,21 +180,40 @@ pub fn copy(&self, dstp: &mut Self) {
 /// The following example demonstrates how to create and update a [`CpumaskVar`].
 ///
 /// ```
+/// use kernel::cpu::CpuId;
 /// use kernel::cpumask::CpumaskVar;
 ///
 /// let mut mask = CpumaskVar::new_zero(GFP_KERNEL).unwrap();
 ///
 /// assert!(mask.empty());
-/// mask.set(2);
-/// assert!(mask.test(2));
-/// mask.set(3);
-/// assert!(mask.test(3));
-/// assert_eq!(mask.weight(), 2);
+/// let mut count = 0;
+///
+/// let cpu2 = CpuId::from_u32(2);
+/// if let Some(cpu) = cpu2 {
+///     mask.set(cpu);
+///     assert!(mask.test(cpu));
+///     count += 1;
+/// }
+///
+/// let cpu3 = CpuId::from_u32(3);
+/// if let Some(cpu) = cpu3 {
+///     mask.set(cpu);
+///     assert!(mask.test(cpu));
+///     count += 1;
+/// }
+///
+/// assert_eq!(mask.weight(), count);
 ///
 /// let mask2 = CpumaskVar::try_clone(&mask).unwrap();
-/// assert!(mask2.test(2));
-/// assert!(mask2.test(3));
-/// assert_eq!(mask2.weight(), 2);
+///
+/// if let Some(cpu) = cpu2 {
+///     assert!(mask2.test(cpu));
+/// }
+///
+/// if let Some(cpu) = cpu3 {
+///     assert!(mask2.test(cpu));
+/// }
+/// assert_eq!(mask2.weight(), count);
 /// ```
 pub struct CpumaskVar {
     #[cfg(CONFIG_CPUMASK_OFFSTACK)]
-- 
2.31.1.272.g89b43f80a514