Re: [PATCH v2 02/15] drm/panthor: Add uAPI

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Steven Price <steven.price@arm.com>
Cc: "Nicolas Boichat" <drinkcat@chromium.org>,
	"Daniel Stone" <daniels@collabora.com>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Liviu Dudau" <Liviu.Dudau@arm.com>,
	dri-devel@lists.freedesktop.org,
	"Clément Péron" <peron.clem@gmail.com>,
	"Marty E . Plummer" <hanetzer@startmail.com>,
	"Robin Murphy" <robin.murphy@arm.com>,
	"Faith Ekstrand" <faith.ekstrand@collabora.com>
Subject: Re: [PATCH v2 02/15] drm/panthor: Add uAPI
Date: Mon, 4 Sep 2023 11:26:09 +0200	[thread overview]
Message-ID: <20230904112609.2e0466a5@collabora.com> (raw)
In-Reply-To: <fe1bd580-2c3e-555a-0c1f-34a9ac4bdf9a@arm.com>

On Mon, 4 Sep 2023 08:42:08 +0100
Steven Price <steven.price@arm.com> wrote:

> On 01/09/2023 17:10, Boris Brezillon wrote:
> > On Wed,  9 Aug 2023 18:53:15 +0200
> > Boris Brezillon <boris.brezillon@collabora.com> wrote:
> >   
> >> +/**
> >> + * DOC: MMIO regions exposed to userspace.
> >> + *
> >> + * .. c:macro:: DRM_PANTHOR_USER_MMIO_OFFSET
> >> + *
> >> + * File offset for all MMIO regions being exposed to userspace. Don't use
> >> + * this value directly, use DRM_PANTHOR_USER_<name>_OFFSET values instead.
> >> + *
> >> + * .. c:macro:: DRM_PANTHOR_USER_FLUSH_ID_MMIO_OFFSET
> >> + *
> >> + * File offset for the LATEST_FLUSH_ID register. The Userspace driver controls
> >> + * GPU cache flushling through CS instructions, but the flush reduction
> >> + * mechanism requires a flush_id. This flush_id could be queried with an
> >> + * ioctl, but Arm provides a well-isolated register page containing only this
> >> + * read-only register, so let's expose this page through a static mmap offset
> >> + * and allow direct mapping of this MMIO region so we can avoid the
> >> + * user <-> kernel round-trip.
> >> + */
> >> +#define DRM_PANTHOR_USER_MMIO_OFFSET		(0x1ull << 56)  
> > 
> > I'm playing with a 32-bit kernel/userspace, and this is problematic,
> > because vm_pgoff is limited to 32-bit there, meaning we can only map up
> > to (1ull << (PAGE_SHIFT + 32)) - 1. Should we add a DEV_QUERY to let
> > userspace set the mmio range?  
> 
> Hmm, I was rather hoping we could ignore 32 bit these days ;) But while
> I can't see why anyone would be running a 32 bit kernel, I guess 32 bit
> user space is likely to still be needed.

Uh, I just hit a new problem with 32-bit kernels: the io-pgtable
interface (io_pgtable_ops) passes device VAs as unsigned longs, meaning
the GPU VA space is limited to 4G on a 32-bit build :-(. Robin, any
chance you could advise me on what to do here?

1. assume this limitation is here for a good reason, and limit the GPU
VA space to 32-bits on 32-bit kernels

or

2. update the interface to make iova an u64