Received-SPF: pass (google.com: domain of linux-kernel+bounces-189516-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240520172554.182094-1-dakr@redhat.com> <20240520172554.182094-11-dakr@redhat.com>
 <CANiq72kHrgOVrdw7rB9KpHvOMy244TgmEzAcL=v5O9rchs8T1g@mail.gmail.com>
 <CAPM=9txb5STBo05xiTy9+wF7=mMO=X2==BP4JVORPFAtX=nS0g@mail.gmail.com>
 <CANeycqpNeHFUu-RwSc6Ewa3r5TMhYYFDC6bO+sj3OZ398JfJ1A@mail.gmail.com> <1c8bb8044bc1943ad8d19cd6fc84a2d886004163.camel@redhat.com>
In-Reply-To: <1c8bb8044bc1943ad8d19cd6fc84a2d886004163.camel@redhat.com>
From: Wedson Almeida Filho <wedsonaf@gmail.com>
Date: Sat, 25 May 2024 16:24:18 -0300
Message-ID: <CANeycqoWVygXBO_Kzq6QaLDjp3W+66YrOi7_dK8zRVpDONJ=AA@mail.gmail.com>
Subject: Re: [RFC PATCH 10/11] rust: add basic abstractions for iomem operations
To: Philipp Stanner <pstanner@redhat.com>
Cc: Dave Airlie <airlied@gmail.com>, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>, 
	Danilo Krummrich <dakr@redhat.com>, gregkh@linuxfoundation.org, rafael@kernel.org, 
	bhelgaas@google.com, ojeda@kernel.org, alex.gaynor@gmail.com, 
	boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, 
	benno.lossin@proton.me, a.hindborg@samsung.com, aliceryhl@google.com, 
	fujita.tomonori@gmail.com, lina@asahilina.net, ajanulgu@redhat.com, 
	lyude@redhat.com, rust-for-linux@vger.kernel.org, 
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"

On Tue, 21 May 2024 at 05:03, Philipp Stanner <pstanner@redhat.com> wrote:
>
> On Tue, 2024-05-21 at 00:01 -0300, Wedson Almeida Filho wrote:
> > On Mon, 20 May 2024 at 23:07, Dave Airlie <airlied@gmail.com> wrote:
> > >
> > > >
> > > > Wedson wrote a similar abstraction in the past
> > > > (`rust/kernel/io_mem.rs` in the old `rust` branch), with a
> > > > compile-time `SIZE` -- it is probably worth taking a look.
> > > >
> > >
> > > Just on this point, we can't know in advance what size the IO BARs
> > > are
> > > at compile time.
> > >
> > > The old method just isn't useful for real devices with runtime IO
> > > BAR sizes.
> >
> > The compile-time `SIZE` in my implementation is a minimum size.
> >
> > Attempts to read/write with constants within that size (offset +
> > size)
> > were checked at compile time, that is, they would have zero
> > additional
> > runtime cost when compared to C. Reads/writes beyond the minimum
> > would
> > have to be checked at runtime.
> >
>
> We looked at this implementation
>
> Its disadvantage is that it moves the responsibility for setting that
> minimum size to the driver programmer. Andreas Hindborg is using that
> currently for rnvme [1].
>
> I believe that the driver programmer in Rust should not be responsible
> for controlling such sensitive parameters (one could far more easily
> provide invalid values), but the subsystem (e.g. PCI) should do it,
> because it knows about the exact resource lengths.

There is no responsibility being moved. The bus is still that one that
knows about the resources attached to the device.

The driver, however, can say for example: I need at least 4 registers
of 32 bits starting at offset X, which results in a minimum size of X
+ 16. If at runtime a device compatible with this driver appears and
has an io mem of at least that size, then the driver can drive it
without any additional runtime checks. I did this in the gpio driver
here: https://lwn.net/Articles/863459/

Note that in addition to not having to check offset at runtime, the
reads/writes are also infallible because all failures are caught at
compile time.

Obviously not all drivers can benefit from this, but it is
considerable simplification for the ones that can.

> The only way to set the actual, real value is through subsystem code.
> But when we (i.e., currently, the driver programmer) have to use that
> anyways, we can just use it from the very beginning and have the exact
> valid parameters.

Yes, only the bus knows. But to reiterate: if the driver declares and
checks a minimum size at attach time, it obviates the needs to check
again throughout the lifetime of the driver, which is more performant
and eliminates error paths.