Re: RFC: staging: gasket: re-implement using UIO

From: Greg KH <gregkh@linuxfoundation.org>
To: Todd Poynor <toddpoynor@google.com>
Cc: "Ahmed S. Darwish" <darwish.07@gmail.com>,
	Rob Springer <rspringer@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	devel@linuxdriverproject.org
Subject: Re: RFC: staging: gasket: re-implement using UIO
Date: Wed, 7 Nov 2018 10:20:40 +0100	[thread overview]
Message-ID: <20181107092040.GE31015@kroah.com> (raw)
In-Reply-To: <CAAW3YpZj=U=RtKLtB3Wq-agi9Vxxd_Mn8x1qo5hXXcV7hSLV9Q@mail.gmail.com>

On Tue, Nov 06, 2018 at 04:20:49PM -0800, Todd Poynor wrote:
> On Mon, Sep 10, 2018 at 8:28 AM Ahmed S. Darwish <darwish.07@gmail.com> wrote:
> >
> > The gasket in-kernel framework, recently introduced under staging,
> > re-implements what is already long-time provided by the UIO
> > subsystem, with extra PCI BAR remapping and MSI conveniences.
> >
> > Before moving it out of staging, make sure we add the new bits to
> > the UIO framework instead, then transform its signle client, the
> > Apex driver, to a proper UIO driver (uio_driver.h).
> >
> > Link: https://lkml.kernel.org/r/20180828103817.GB1397@do-kernel
> 
> So I'm looking at this for reals now.  The BAR mapping stuff is
> straightforward with the existing framework.  Everything else could be
> done outside of UIO via the existing device interface, but figured I'd
> collect any opinions about adding the new bits to UIO.
> 
> The Apex device has 13 MSIX interrupts.  UIO does one IRQ per device.
> The PRUSS driver registers 8 instances of the UIO device with
> identical memory mappings but individual IRQs for its 8 interrupts.
> Currently gasket has userspace pass down an eventfd (via ioctl) for
> each interrupt it wants to watch.  Is there interest in modifying UIO
> to handle multiple IRQs in some perhaps similar fashion?
> 
> Speaking of ioctls, are those allowed here, or is sysfs or something
> else always required?  The aforementioned multiple IRQ stuff probably
> maps nicely to sysfs (there's a small number of them easily
> represented as attributes), while DMA buffer mappings seem more
> problematic, but maybe somebody's thought of a good way to represent
> these already.

Adding ioctls opens up a huge can of worms where each driver would
probably want to do their own mess and then we have to constantly audit
the mess all of the time.  What do you really need to do in an ioctl?

> And then we need to map buffers to our device.  We could probably
> implement this via an IOMMU driver API for our custom MMU and hook
> that up to generic IOMMU support for UIO, which sounds like something
> a lot of drivers could use.

You need to map buffers from what to what?  UIO is for pass-through
memory that your device exposes to userspace through mmap, what more do
you need that the existing api does not give you?

> There's a few other tidbits the driver does, including allocating
> coherent memory for userspace to share with the device, but that's
> probably enough for now.
> 
> If anybody wants to squash any of the above as a non-starter for UIO
> or point things in a different direction, it's appreciated.

Patches are always the best way to do review, we generally do not have
time or bandwidth to do "what do you think of my design" ideas without
real code behind it, sorry.  That way we know you really did try to do
something and you have something that starts to work for you.

thanks,

greg k-h