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

[Joel Fernandes <joelagnelf@nvidia.com>]

Hi Yury,

On 10/10/2025 1:20 PM, Yury Norov wrote:
[...]
>>>>          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
>>>             .try_set_base2(base2 >> 40)?        // skip
>>>             .write(bar, &E::ID) else { pr_err!(); return -EINVAL };
>>>
>>> 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.
>>
>> Not sure I understand what you mean, but if you are talking about fields
>> overflow, this cannot happen with the current design. The non-fallible
>> setter can only be invoked if the compiler can prove that the argument
>> does fit withing the field. Otherwise, one has to use the fallible
>> setter (as this chunk does, because `dma_start >> 40` can still spill
>> over the capacity of `base`), which performs a runtime check and returns
>> `EOVERFLOW` if the value didn't fit.
>  
> Yeah, this design addresses my major question to the bitfields series
> from Joel: setters must be fallible. I played with this approach, and
> it does exactly what I have in mind.
> 
> 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.

I don't think we could combine the setter and try setter variants on the rust
side, because the former returns Self and the latter returns Result. Also, both
the variants already have compile time asserts which may cover what you're
referring to.

The try setter variants in fact are not strictly needed, because the user can
provide a bounded integer (after performing any fallible conversions on the
caller side). Alex and me discussed adding that for a better user/caller
experience [1].

[1] https://lore.kernel.org/all/C35B5306-98C6-447B-A239-9D6A6C548A4F@nvidia.com/

Or did you mean something else?

thanks,

 - Joel