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

[Boqun Feng <boqun.feng@gmail.com>]

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