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 <d...@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
