Re: [PATCH] rust: io: use const generics for read/write offsets

[Joel Fernandes <joelagnelf@nvidia.com>]

On Fri, Sep 19, 2025 at 11:26:19AM +0200, Benno Lossin wrote:
> On Fri Sep 19, 2025 at 9:59 AM CEST, Joel Fernandes wrote:
> > Hello, Danilo,
> >
> >> On Sep 19, 2025, at 1:26 AM, Danilo Krummrich <dakr@kernel.org> wrote:
> >> 
> >> On Thu Sep 18, 2025 at 8:13 PM CEST, Joel Fernandes wrote:
> >>>> On Thu, Sep 18, 2025 at 03:02:11PM +0000, Alice Ryhl wrote:
> >>>> Using build_assert! to assert that offsets are in bounds is really
> >>>> fragile and likely to result in spurious and hard-to-debug build
> >>>> failures. Therefore, build_assert! should be avoided for this case.
> >>>> Thus, update the code to perform the check in const evaluation
> >>>> instead.
> >>> 
> >>> I really don't think this patch is a good idea (and nobody I spoke to
> >>> thinks so). Not only does it mess up the user's caller syntax
> >>> completely, it is also
> >> 
> >> I appreacite you raising the concern,
> >> but I rather have other people speak up
> >> themselves.
> >
> > I did not mean to speak for others, sorry it came across like that
> > (and that is certainly not what I normally do). But I discussed the
> > patch in person since we are at a conference and discussing it in
> > person, and I did not get a lot of consensus on this. That is what I
> > was trying to say. If it was a brilliant or great idea, I would have
> > hoped for at least one person to tell me that this is exactly how we
> > should do it.
> 
> I'm also not really thrilled to see lots more turbofish syntax. However,
> if we can avoid the nasty build_assert errors then in my opinion it's
> better. (yes we do have Gary's cool klint tool to handle them correctly,

Yes, thanks. Also I tried to apply this patch and it doesn't always work
because of array indexing usecase in nova, where we compute the offset based
on a runtime register index  (**/nova-core/**/macros.rs). Here idx is not a
constant:

            /// Read the array register at index `idx` from its address in `io`.
            #[inline(always)]
            pub(crate) fn read<const SIZE: usize, T>(
                io: &T,
                idx: usize,
            ) -> Self where
                T: ::core::ops::Deref<Target = ::kernel::io::Io<SIZE>>,

In **/ga102.rs, we have the following usage where ucode_idx cannot be const:

regs::NV_FUSE_OPT_FPF_SEC2_UCODE1_VERSION::read(bar, ucode_idx).data()

So I am afraid this wont work. Also even if it did work, it means now we have
to also put idx as a const generic (turbofish syntax).

thanks,

 - Joel