On Thu, 17 Apr 2025 16:13:49 +0100
Steven Price <steven.pr...@arm.com> wrote:
> On 17/04/2025 15:49, Boris Brezillon wrote:
> > Currently, we pick the MMIO offset based on the size of the pgoff_t
> > type seen by the process that manipulates the FD, such that a 32-bit
> > process can always map the user MMIO ranges. But this approach doesn't
> > work well for emulators like FEX, where the emulator is a 64-bit binary
> > which might be executing 32-bit code. In that case, the kernel thinks
> > it's the 64-bit process and assumes DRM_PANTHOR_USER_MMIO_OFFSET_64BIT
> > is in use, but the UMD library expects DRM_PANTHOR_USER_MMIO_OFFSET_32BIT,
> > because it can't mmap() anything above the pgoff_t size.
> >
> > In order to solve that, we need a way to explicitly set the user MMIO
> > offset from the UMD, such that the kernel doesn't have to guess it
> > from the TIF_32BIT flag set on user thread. We keep the old behavior
> > if DRM_PANTHOR_SET_USER_MMIO_OFFSET is never called.
> >
> > Changes:
> > - Drop the lock/immutable fields and allow SET_USER_MMIO_OFFSET
> > requests to race with mmap() requests
> > - Don't do the is_user_mmio_offset test twice in panthor_mmap()
> > - Improve the uAPI docs
> >
> > Signed-off-by: Boris Brezillon <boris.brezil...@collabora.com>
>
> Much nicer, thanks!
>
> Reviewed-by: Steven Price <steven.pr...@arm.com>
>
> One note for merging - both this and Adrián's series are introducing the
> new 1.4 version. So we either need to switch one to 1.5 or combine the
> series.
I'll let Adrian series go first. I want to leave some time for others
to chime in anyway.
Thanks for the reviews/suggestions.
Boris
>
> Thanks,
> Steve
>
> > ---
> > drivers/gpu/drm/panthor/panthor_device.h | 18 ++++++++
> > drivers/gpu/drm/panthor/panthor_drv.c | 56 +++++++++++++++++-------
> > include/uapi/drm/panthor_drm.h | 38 ++++++++++++++++
> > 3 files changed, 96 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panthor/panthor_device.h
> > b/drivers/gpu/drm/panthor/panthor_device.h
> > index 4c27b6d85f46..6d8c2d5042f2 100644
> > --- a/drivers/gpu/drm/panthor/panthor_device.h
> > +++ b/drivers/gpu/drm/panthor/panthor_device.h
> > @@ -219,6 +219,24 @@ struct panthor_file {
> > /** @ptdev: Device attached to this file. */
> > struct panthor_device *ptdev;
> >
> > + /** @user_mmio: User MMIO related fields. */
> > + struct {
> > + /**
> > + * @offset: Offset used for user MMIO mappings.
> > + *
> > + * This offset should not be used to check the type of mapping
> > + * except in panthor_mmap(). After that point, MMIO mapping
> > + * offsets have been adjusted to match
> > + * DRM_PANTHOR_USER_MMIO_OFFSET and this macro should be used
> > + * instead.
> > + * Make sure this rule is followed at all times, because
> > + * userspace is in control of the offset, and can change the
> > + * value behind out back, potentially leading to erronous
Oops, typo here ^ our
> > + * branching happening in kernel space.
> > + */
> > + u64 offset;
> > + } user_mmio;
