Re: [PATCH RFC v2 3/3] gpu: nova-core: use BoundedInt

["Danilo Krummrich" <dakr@kernel.org>]

On Fri Oct 10, 2025 at 7:20 PM CEST, Yury Norov wrote:
> On Fri, Oct 10, 2025 at 06:19:17PM +0900, Alexandre Courbot wrote:
>> On Fri Oct 10, 2025 at 1:40 AM JST, Yury Norov wrote:
>     register!(NV_PDISP_VGA_WORKSPACE_BASE @ 0x00625f04 {
>         3:0     cnt => i8,
>         7:4     flags, // implied unsigned
>         31:8    addr,  // implied unsigned
>     });
>
> That answers my question. Can you please highlight this use case
> in commit message?
>
> And just to wrap up:
>
>  - all fields by default are unsigned integers;
>  - one may use '=>' to switch to signed integers, enums or booleans;
>  - all other types are not allowed.

We do allow other types.

In Rust we have the From [1] and TryFrom [2] traits (analogously Into and
TryInto).

This way, if some type T implements, for instance, From<u8> it means that we
can always derive an instance of T from any u8.

Similarly, if T implements TryFrom<u8> we can always derive a Result<T>, which
either contains a valid instance of T or some error that we can handle instead.

Hence, the following is valid is well:

	register!(NV_PFALCON_FALCON_HWCFG1 @ PFalconBase[0x0000012c] {
	    3:0     core_rev ?=> FalconCoreRev, "Core revision";
	    5:4     security_model ?=> FalconSecurityModel, "Security model";
	    7:6     core_rev_subversion as ?=> FalconCoreRevSubversion, "Core revision subversion";
	});

The '?=>' notation means TryFrom, hence the generated accessor method - e.g.
security_model() - returns a Result<FalconCoreRevSubversion>.

In this exaple all three types are actually enums, but it would be the exact
same for structs.

In fact, enums in Rust are very different than enums in C anyways and can be
complex types, such as:

	enum Message {
	    Quit,
	    Move { x: i32, y: i32 },
	    Write(String),
	    ChangeColor(i32, i32, i32),
	}

[1] https://rust.docs.kernel.org/core/convert/trait.From.html
[2] https://rust.docs.kernel.org/core/convert/trait.TryFrom.html

> I still have a question regarding compile-time flavor of the setter.
> In C we've got a builtin_constant_p, and use it like:
>         
>    static inline int set_base(unsigned int base)
>    {
>         BUILD_BUG_ON_ZERO(const_true(base > MAX_BASE));
>
>         // Eliminated for compile-time 'base'
>         if (base > MAX_BASE)
>                 return -EOVERFLOW;
>
>         __set_base(base);
>
>         return 0;
>    }
>
> Can we do the same trick in rust? Would be nice to have a single
> setter for both compile and runtime cases.

Technically, we could, but it would be very inefficient: Even if the function /
method is called in an infallible way it would still return a Result<T> instead
of just T, which the caller would need to handle.

Analogously, for a setter the function / method would still return a Result,
which must be handled.

Ignoring a returned Result causes a compiler warning:

	warning: unused `core::result::Result` that must be used
	  --> drivers/gpu/nova-core/driver.rs:64:9
	   |
	64 |         foo();
	   |         ^^^^^
	   |
	   = note: this `Result` may be an `Err` variant, which should be handled
	   = note: `#[warn(unused_must_use)]` on by default
	help: use `let _ = ...` to ignore the resulting value
	   |
	64 |         let _ = foo();
	   |         +++++++
	
	warning: 1 warning emitted

This is intentional, users should not be able to ignore error conditions
willy-nilly:

It is a potential source for bugs if errors are ignored. For instance, a caller
might not check the returned Result initially because a compile time check is
expected. However, when changed later on to derive the value at runtime it is
easy to forget handling the Result instead.