Re: [PATCH v7 05/10] rust: io: add IoLoc and IoWrite types

[Gary Guo]

On Sat Feb 28, 2026 at 12:33 AM GMT, Alexandre Courbot wrote:
> On Sat Feb 28, 2026 at 3:02 AM JST, Gary Guo wrote:
> <snip>
>>> +/// A pending I/O write operation, bundling a value with the [`IoLoc`] it should be written to.
>>> +///
>>> +/// Created by [`IoLoc::set`], [`IoLoc::zeroed`], [`IoLoc::default`], [`IoLoc::init`], or
>>> +/// [`IoLoc::init_default`], and consumed by [`Io::write`] or [`Io::try_write`] to perform the
>>> +/// actual write.
>>> +///
>>> +/// The value can be modified before writing using [`IoWrite::update`] or [`IoWrite::try_update`],
>>> +/// enabling a builder pattern:
>>> +///
>>> +/// ```ignore
>>> +/// io.write(REGISTER.init(|v| v.with_field(x)));
>>> +/// ```
>>
>> Thinking about this again, I think we might still want to write
>>
>>     io.write(REGISTER, value)
>
> This was the original design, but as you point out below this makes the
> very common case of writing a register value built from scratch more
> verbose than it needs. Real-world examples are significantly worse, e.g:
>
>     bar.write(
>     regs::NV_PFALCON_FALCON_DMATRFMOFFS::of::<E>()
>         .try_init(|r| r.try_with_offs(load_offsets.dst_start + pos))?,
>     );

My main dissatisfaction with this is with the function call

I wonder if we can just have

    io.write(loc, value)
    io.write_with(loc, updater)

where the latter simply is a function that does

    io.write(loc, updater(T::zeroed()))

then the example would be

    bar.write_with(
        regs::NV_PFALCON_FALCON_DMATRFMOFFS::of::<E>(),
        |r| r.try_with_offs(load_offsets.dst_start + pos)
    )

[ Note: it's possible to have write_with that works with both fallible and
non-fallible callbacks. You can define a trait like a monad (well, not fully
a monad, because this just has a map, not a return and a bind):

trait Map<T> {
    type Mapped<U>;
    
    fn map<U>(self, f: impl FnOnce(T) -> U) -> Self::Mapped<U>;
}

impl<T> Map<T> for T {
    type Mapped<U> = U;
    
    fn map<U>(self, f: impl FnOnce(T) -> U) -> Self::Mapped<U> {
        f(self)
    }
}

impl<T, E> Map<T> for Result<T, E> {
    type Mapped<U> = Result<U, E>;
    
    fn map<U>(self, f: impl FnOnce(T) -> U) -> Self::Mapped<U> {
        Ok(f(self?))
    }
}

impl Io {    
    fn write_with<R: IoLoc<T>, U: Map<T>, T: Zeroable, F: FnOnce(T) -> U>(&self, loc: R, f: F) -> U::Mapped<()> {
        f(T::zeroed).map(|x| self.write(loc, x))
    }
}

then this returns `()` if closure cannot fail, and returns `Result<()>` if it
fails).

end of note ]

BTW, I am also not very happy with the `::<E>()` syntax. I think with the I/O
projection upcoming, we might be able to get rid of relative offseting
completely by requiring user to first project into `View<'_, .., FalconBase>` and then have
some registers that can only be used on `Io<Type = FalconBase>` but nothing else.

Best,
Gary

>
> turns into
>
>     bar.write(
>         regs::NV_PFALCON_FALCON_DMATRFMOFFS::of::<E>(),
>         regs::NV_PFALCON_FALCON_DMATRFMOFFS::zeroed()
>             .try_with_offs(load_offsets.dst_start + pos)?,
>     );
>
>>
>> Granted, this does mean that we will write `REGISTER` twice in some cases:
>>
>>     io.write(REGISTER, REGISTER::default().with_field(foo));
>>
>> But, we have no redundancy for the update case:
>>
>>     io.update(REGISTER, |v| v.with_field(foo));
>
> In nova-core, `update` is less common than writing a register value
> built from scratch. That's really the only thing that holds us from
> two arguments in `write`.
>
>>
>> The reason for this thought is that conceptually, the type of a register is not
>> necessarily coupled with the register itself. This is the case currently for
>> register arrays, but also the case when we think in the long term where
>
> For these cases I was thinking we could rely on From/Into
> implementations, as it gives us the option to make the conversion
> explicit and thus less error-prone.
>
>> bitfields become decoupled from `register!`. People also might just want to
>> define a register of u32 without any bitfields at all, and in this case writing
>>
>>     io.write(REGISTER, u32_value)
>>
>> looks nicer than
>>
>>     io.write(REGISTER.set(u32_value))
>>
>> Spelling out the "offset" explictly, arguably, is also more natural for a C
>> programmer, and also simpler on the implementation side (all the helper methods
>> on `IoLoc` would go away).
>
> That's true that it would be less `(try_)init` and friends helpers
> (`IoWrite` would also go away I guess?). I'm not a huge fan of these
> because they only cover specific initial values (zeroed() and
> default()).
>
> Maybe we can macro our way out of it?
>
> It would look a little bit weird to resort to a macro just for the write
> case, but I don't think we will be able to work around this without any
> kind of compromise. FWIW, I think the single-argument `write` is a
> reasonable one that might look a bit surprising at first encounter, but
> has the benefit of reading naturally.