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@xxxxxxxxxx> 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