Nouveau - Sep 2025 - [PATCH v3 5/5] rust: Add KUNIT tests for bitfield

On Tue, Sep 16, 2025 at 05:59:18AM -0400, Joel Fernandes wrote:
[...]
> > > In C also this is valid. If you passed a higher value than what
the
> > > bitfield can hold, the compiler will still just use the bits that
it
> > > needs and ignore the rest.
> > 
> > In C we've got FIELD_{PREP,GET,MODIFY}, implementing the checks.
> > So those who want to stay on safe side have a choice.
> 
> Ah ok. We can add these checks then for the accessors, I will do so in v4.
The C checks use BUILD_BUG_ON, in rust-for-linux we have build_assert but it
is fragile and depends on the value being a constant. Since the setter API
accepts a run-time value and not a constant, we cannot use this.

Or, we can fail at runtime, but that requires changing the set_* to try_set_*
and returning a Result instead of Self. Alternatively, we can have a debug
option that panics if the setter API is misued.

Thoughts?

Or for the moment, we can keep it simple and filter out / ignore extra bits
of the larger value passed (which is what nova-core's register macro
bitfield
implementation currently does anyway).

thanks,

 - Joel

On 9/19/25 5:39 PM, Joel Fernandes wrote:
> On Tue, Sep 16, 2025 at 05:59:18AM -0400, Joel Fernandes wrote:
> [...]
>>>> In C also this is valid. If you passed a higher value than what
the
>>>> bitfield can hold, the compiler will still just use the bits
that it
>>>> needs and ignore the rest.
>>>
>>> In C we've got FIELD_{PREP,GET,MODIFY}, implementing the
checks.
>>> So those who want to stay on safe side have a choice.
>>
>> Ah ok. We can add these checks then for the accessors, I will do so in
v4.
> 
> The C checks use BUILD_BUG_ON, in rust-for-linux we have build_assert but
it
> is fragile and depends on the value being a constant. Since the setter API
> accepts a run-time value and not a constant, we cannot use this.
> 
> Or, we can fail at runtime, but that requires changing the set_* to
try_set_*
> and returning a Result instead of Self. Alternatively, we can have a debug
> option that panics if the setter API is misued.
Please no...
> 
> Thoughts?
> 
> Or for the moment, we can keep it simple and filter out / ignore extra bits
> of the larger value passed (which is what nova-core's register macro
bitfield
> implementation currently does anyway).
> 
Yes. Assuming that I'm not completely lost here, you are proposing to
simply truncate to the size of the bitfield--no panics, no warnings. And
that's perfectly fine here IMHO.

thanks,
-- 
John Hubbard