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

[Joel Fernandes <joelagnelf@nvidia.com>]

On 10/9/2025 12:40 PM, Yury Norov wrote:
[..]
>> The field setter/getters are also simplified. If a field has no target
>> type, then its setter expects any type that implements `Into` to the
>> field's bounded integer type. Due to the many `From` implementations for
>> primitive types, this means that most calls can be left unchanged. If
>> the caller passes a value that is potentially larger than the field's
>> capacity, it must use the `try_` variant of the setter, which returns an
>> error if the value cannot be converted at runtime.
>>
>> For fields that use `=>` to convert to another type, both setter and
>> getter are always infallible.
>>
>> For fields that use `?=>` to fallibly convert to another type, only the
>> getter needs to be fallible as the setter always provide valid values by
>> design.
> 
> Can you share a couple examples? Not sure I understand this part,
> especially how setters may not be fallible, and getters may fail.

Because a getter has to convert the raw bitfield value from memory into an enum,
there is no guarantee that the memory in concern does not overflow the enum, say
if register bit meanings change, or something. ?=> was before fallible in both
directions hence the "?". Now with Alex's patch it is fallible only in one
direction.

>> Outside of the register macro, the biggest changes occur in `falcon.rs`,
>> which defines many enums for fields - their conversion implementations
>> need to be changed from the original primitive type of the field to the
>> new corresponding bounded int type. Hopefully the TryFrom/Into derive
>> macros [1] can take care of implementing these, but it will need to be
>> adapted to support bounded integers... :/
>>
>> But overall, I am rather happy at how simple it was to convert the whole
>> of nova-core to this.
>>
>> Note: This RFC uses nova-core's register!() macro for practical
>> purposes, but the hope is to move this patch on top of the bitfield
>> macro after it is split out [2].
>>
>> [1] https://lore.kernel.org/rust-for-linux/cover.1755235180.git.y.j3ms.n@gmail.com/
>> [2] https://lore.kernel.org/rust-for-linux/20251003154748.1687160-1-joelagnelf@nvidia.com/
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
> 
> ...
> 
>>          regs::NV_PFALCON_FALCON_DMATRFBASE1::default()
>> -            .set_base((dma_start >> 40) as u16)
>> +            .try_set_base(dma_start >> 40)?
>>              .write(bar, &E::ID);
> 
> Does it mean that something like the following syntax is possible?
> 
>         regs::NV_PFALCON_FALCON_DMATRFBASE1::default()
>             .try_set_base1(base1 >> 40)?        // fail here

Danilo already replied here, but this code is fine as we early return due to the
try operator (?).

>             .try_set_base2(base2 >> 40)?        // skip
>             .write(bar, &E::ID) else { pr_err!(); return -EINVAL };

Here I am guessing (based on your concern from an earlier thread) but is your
concern was that -EINVAL will get written to the register accidentally? That
wont happen, the above code is fine. Or do you mean something else?

> 
> This is my main concern: Rust is advertised a as runtime-safe language
> (at lease safer than C), but current design isn't safe against one of
> the most common errors: type overflow.

I'd be a bit careful when using "isn't safe" in the context of Rust. Rust's
notion of "safety" is about preventing undefined behavior (UB), not preventing
every possible runtime mistake. In rust, integer overflows for example are not
undefined so overflows are not "unsafe" (it might be a logical bug but that's
not what unsafety is about). In fact an overflow might be exactly what some
algorithm needs (wrap counters in RCU are example of harmless overflow). Another
example is a deadlock is not undefined behavior in Rust, it is defined, even if
bad :).

Thanks.