On Fri Oct 10, 2025 at 1:40 AM JST, Yury Norov wrote:> Hi Alexandre,
>
> On Thu, Oct 09, 2025 at 09:37:10PM +0900, Alexandre Courbot wrote:
>> Use BoundedInt with the register!() macro and adapt the nova-core code
>> accordingly. This makes it impossible to trim values when setting a
>> register field, because either the value of the field has been inferred
>> at compile-time to fit within the bounds of the field, or the user has
>> been forced to check at runtime that it does indeed fit.
>
> In C23 we've got _BitInt(), which works like:
>
> unsigned _BitInt(2) a = 5; // compile-time error
>
> Can you consider a similar name and syntax in rust?
I like the shorter `BitInt`! For the syntax, we will have to conform to
what is idiomatic Rust. And I don't think we can make something similar
to `= 5` work here - that would require overloading the `=` operator,
which cannot be done AFAICT. A constructor is a requirement.
>
>> The use of BoundedInt actually simplifies register fields definitions,
>> as they don't need an intermediate storage type (the "as
..." part of
>> fields definitions). Instead, the internal storage type for each field
>> is now the bounded integer of its width in bits, which can optionally
be
>> converted to another type that implements `From`` or `TryFrom`` for
that
>> bounded integer type.
>>
>> This means that something like
>>
>> register!(NV_PDISP_VGA_WORKSPACE_BASE @ 0x00625f04 {
>> 3:3 status_valid as bool,
>> 31:8 addr as u32,
>> });
>>
>> Now becomes
>>
>> register!(NV_PDISP_VGA_WORKSPACE_BASE @ 0x00625f04 {
>> 3:3 status_valid => bool,
>> 31:8 addr,
>> });
>
> That looks nicer, really. But now that you don't make user to provide
> a representation type, how would one distinguish signed and unsigned
> fields? Assuming that BoundedInt is intended to become a generic type,
> people may want to use it as a storage for counters and other
> non-bitfield type of things. Maybe:
>
> register!(NV_PDISP_VGA_WORKSPACE_BASE @ 0x00625f04 {
> s 3:0 cnt,
> 7:4 flags, // implies unsigned - ?
> u 31:8 addr,
> });
The expectation would be to use the `=>` syntax to convert the field to
a signed type (similarly to how `status_valid` is turned into a `bool`
in my example).
>
>> (here `status_valid` is infallibly converted to a bool for convenience
>> and to remain compatible with the previous semantics)
>>
>> 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.
Imagine you have this enum:
enum GpioState {
Low = 0,
High = 1,
Floating = 2,
}
and this field:
2:0 gpio_state ?=> GpioState,
When you set it, you must pass an instance of `GpioState` as argument,
which means that the value will always be valid. However, when you try
to access the field, you have no guarantee at all that the value of the
field won't be `3` - the IO space might be inaccessible, or the register
value be forged arbitrarily. Thus the getter needs to return a
`Result<GpioState>`.
>
>> 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 at
gmail.com/
>> [2]
https://lore.kernel.org/rust-for-linux/20251003154748.1687160-1-joelagnelf at
nvidia.com/
>>
>> Signed-off-by: Alexandre Courbot <acourbot at 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
> .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.
>
> If your syntax above allows to handle errors in .try_set() path this way
> or another, I think the rest is manageable.
>
> As a side note: it's a huge pain in C to grep for functions that
> defined by using a macro. Here you do a similar thing. One can't
> easily grep the 'try_set_base' implementation, and would have to
> make a not so pleasant detour to the low-level internals. Maybe
> switch it to:
>
> regs::NV_PFALCON_FALCON_DMATRFBASE1::default()
> .try_set(base, dma_start >> 40)?
> .write(bar, &E::ID);
`base` here is passed by value, what type would it be? I don't think it
is easily doable without jumping through many hoops.
Using LSP with Rust actually makes it very easy to jump to either the
definition of the register, or of the `try_set` block in the macro -
I've done this many times. LSP is pretty much a requirement to code
efficiently in Rust, so I think it is reasonable to rely on it here.