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

[Steven Price <steven.price@arm.com>]

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.

I can't really think of anything better than letting user space set the
MMIO range. Having an ioctl which returned a special fd just for MMIO
would be one option (which would preserve the full 44 bit GPU VA) but
seems somewhat overkill. Hiding the mmap within an ioctl would of course
be bad as it breaks tools like Valgrind.

Oh and please do make it a range - user space submission will be adding
to the MMIO range ;)

Steve

>> +#define DRM_PANTHOR_USER_FLUSH_ID_MMIO_OFFSET	(DRM_PANTHOR_USER_MMIO_OFFSET | 0)