Mapping a drm "dumb" buffer fails on 32-bit system (i686) from what
appears to be a truncated memory address that has been copied
throughout several files. The bug manifests as an -EINVAL when calling
mmap with the offset gathered from DRM_IOCTL_MODE_MAP_DUMB <--
DRM_IOCTL_MODE_ADDFB <-- DRM_IOCTL_MODE_CREATE_DUMB. I can provide
test code if needed.
The following patch will apply to 4.18 though I've only been able to
test through qemu bochs driver and nouveau. Intel driver worked
without any issues. I'm not sure if everyone is going to want to
share a constant, and the whitespace is screwed up from gmail's awful
javascript client, so let me know if I should resend this with any
specific changes. I have also attached the file with preserved
whitespace.
--- linux-4.13.8/drivers/gpu/drm/bochs/bochs.h 2017-10-18
07:38:33.000000000 +0000
+++ linux-4.13.8-modified/drivers/gpu/drm/bochs/bochs.h 2017-10-20
14:34:50.308633773 +0000
@@ -115,8 +115,6 @@
return container_of(gem, struct bochs_bo, gem);
}
-#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
-
static inline u64 bochs_bo_mmap_offset(struct bochs_bo *bo)
{
return drm_vma_node_offset_addr(&bo->bo.vma_node);
--- linux-4.13.8/drivers/gpu/drm/nouveau/nouveau_drv.h 2017-10-18
07:38:33.000000000 +0000
+++ linux-4.13.8-modified/drivers/gpu/drm/nouveau/nouveau_drv.h
2017-10-20 14:34:51.581633751 +0000
@@ -57,8 +57,6 @@
struct nouveau_channel;
struct platform_device;
-#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
-
#include "nouveau_fence.h"
#include "nouveau_bios.h"
--- linux-4.13.8/drivers/gpu/drm/ast/ast_drv.h 2017-10-18
07:38:33.000000000 +0000
+++ linux-4.13.8-modified/drivers/gpu/drm/ast/ast_drv.h 2017-10-20
14:34:50.289633773 +0000
@@ -356,8 +356,6 @@
uint32_t handle,
uint64_t *offset);
-#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
-
int ast_mm_init(struct ast_private *ast);
void ast_mm_fini(struct ast_private *ast);
--- linux-4.13.8/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
2017-10-18 07:38:33.000000000 +0000
+++ linux-4.13.8-modified/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
2017-10-20 14:34:50.644633767 +0000
@@ -21,8 +21,6 @@
#include "hibmc_drm_drv.h"
-#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
-
static inline struct hibmc_drm_private *
hibmc_bdev(struct ttm_bo_device *bd)
{
--- linux-4.13.8/drivers/gpu/drm/virtio/virtgpu_ttm.c 2017-10-18
07:38:33.000000000 +0000
+++ linux-4.13.8-modified/drivers/gpu/drm/virtio/virtgpu_ttm.c
2017-10-20 14:34:53.055633725 +0000
@@ -37,8 +37,6 @@
#include <linux/delay.h>
-#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
-
static struct
virtio_gpu_device *virtio_gpu_get_vgdev(struct ttm_bo_device *bdev)
{
--- linux-4.13.8/drivers/gpu/drm/qxl/qxl_drv.h 2017-10-18
07:38:33.000000000 +0000
+++ linux-4.13.8-modified/drivers/gpu/drm/qxl/qxl_drv.h 2017-10-20
14:34:52.072633742 +0000
@@ -88,9 +88,6 @@
} \
} while (0)
-#define DRM_FILE_OFFSET 0x100000000ULL
-#define DRM_FILE_PAGE_OFFSET (DRM_FILE_OFFSET >> PAGE_SHIFT)
-
#define QXL_INTERRUPT_MASK (\
QXL_INTERRUPT_DISPLAY |\
QXL_INTERRUPT_CURSOR |\
--- linux-4.13.8/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
2017-10-18 07:38:33.000000000 +0000
+++ linux-4.13.8-modified/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
2017-10-20 14:34:43.264633895 +0000
@@ -48,3 +48,1 @@
-#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
-
--- linux-4.13.8/drivers/gpu/drm/mgag200/mgag200_drv.h 2017-10-18
07:38:33.000000000 +0000
+++ linux-4.13.8-modified/drivers/gpu/drm/mgag200/mgag200_drv.h
2017-10-20 14:34:51.404633754 +0000
@@ -276,7 +276,6 @@
struct mga_i2c_chan *mgag200_i2c_create(struct drm_device *dev);
void mgag200_i2c_destroy(struct mga_i2c_chan *i2c);
-#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
void mgag200_ttm_placement(struct mgag200_bo *bo, int domain);
static inline int mgag200_bo_reserve(struct mgag200_bo *bo, bool no_wait)
--- linux-4.13.8/drivers/gpu/drm/radeon/radeon_ttm.c 2017-10-18
07:38:33.000000000 +0000
+++ linux-4.13.8-modified/drivers/gpu/drm/radeon/radeon_ttm.c
2017-10-20 14:34:52.588633733 +0000
@@ -45,8 +45,6 @@
#include "radeon_reg.h"
#include "radeon.h"
-#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
-
static int radeon_ttm_debugfs_init(struct radeon_device *rdev);
static void radeon_ttm_debugfs_fini(struct radeon_device *rdev);
--- linux-4.13.8/drivers/gpu/drm/cirrus/cirrus_drv.h 2017-10-18
07:38:33.000000000 +0000
+++ linux-4.13.8-modified/drivers/gpu/drm/cirrus/cirrus_drv.h
2017-10-20 14:34:50.333633772 +0000
@@ -178,7 +178,6 @@
#define to_cirrus_obj(x) container_of(x, struct cirrus_gem_object, base)
-#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
/* cirrus_mode.c */
void cirrus_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
--- linux-4.13.8/include/drm/drmP.h 2017-10-18 07:38:33.000000000 +0000
+++ linux-4.13.8-modified/include/drm/drmP.h 2017-10-20
14:35:31.300633060 +0000
@@ -503,4 +503,10 @@
/* helper for handling conditionals in various for_each macros */
#define for_each_if(condition) if (!(condition)) {} else
+#if BITS_PER_LONG == 64
+#define DRM_FILE_PAGE_OFFSET (0x100000000ULL >> PAGE_SHIFT)
+#else
+#define DRM_FILE_PAGE_OFFSET (0x10000000ULL >> PAGE_SHIFT)
+#endif
+
#endif
On Mon, 22 Oct 2018 at 07:22, Michael Tirado <[email protected]> wrote:
>
> Mapping a drm "dumb" buffer fails on 32-bit system (i686) from what
> appears to be a truncated memory address that has been copied
> throughout several files. The bug manifests as an -EINVAL when calling
> mmap with the offset gathered from DRM_IOCTL_MODE_MAP_DUMB <--
> DRM_IOCTL_MODE_ADDFB <-- DRM_IOCTL_MODE_CREATE_DUMB. I can provide
> test code if needed.
>
> The following patch will apply to 4.18 though I've only been able to
> test through qemu bochs driver and nouveau. Intel driver worked
> without any issues. I'm not sure if everyone is going to want to
> share a constant, and the whitespace is screwed up from gmail's awful
> javascript client, so let me know if I should resend this with any
> specific changes. I have also attached the file with preserved
> whitespace.
>
This shouldn't be necessary, did someone misbackport the mmap changes without:
drm: set FMODE_UNSIGNED_OFFSET for drm files
Dave.
On Mon, Oct 22, 2018 at 12:26 AM Dave Airlie <[email protected]> wrote:
>
> This shouldn't be necessary, did someone misbackport the mmap changes without:
>
> drm: set FMODE_UNSIGNED_OFFSET for drm files
>
> Dave.
The latest kernel I have had to patch was a 4.18-rc6. I'll try with a
newer 4.19 and let you know if it decides to work. If not I'll
prepare a test case for demonstration on qemu-system-i386.
On Mon, 22 Oct 2018 at 10:49, Michael Tirado <[email protected]> wrote:
>
> On Mon, Oct 22, 2018 at 12:26 AM Dave Airlie <[email protected]> wrote:
> >
> > This shouldn't be necessary, did someone misbackport the mmap changes without:
> >
> > drm: set FMODE_UNSIGNED_OFFSET for drm files
> >
> > Dave.
>
> The latest kernel I have had to patch was a 4.18-rc6. I'll try with a
> newer 4.19 and let you know if it decides to work. If not I'll
> prepare a test case for demonstration on qemu-system-i386.
If you have custom userspace software, make sure it's using
AC_SYS_LARGEFILE or whatever the equivalant is in your build system.
64-bit file offsets are important.
Dave.
On Mon, Oct 22, 2018 at 1:50 AM Dave Airlie <[email protected]> wrote:
>
> On Mon, 22 Oct 2018 at 10:49, Michael Tirado <[email protected]> wrote:
> >
> > On Mon, Oct 22, 2018 at 12:26 AM Dave Airlie <[email protected]> wrote:
> > >
> > > This shouldn't be necessary, did someone misbackport the mmap changes without:
> If you have custom userspace software, make sure it's using
> AC_SYS_LARGEFILE or whatever the equivalant is in your build system.
>
> 64-bit file offsets are important.
>
That fixed it! -D_FILE_OFFSET_BITS=64 is the pre-processor define
needed. It's a bit more than unintuitive but I'm glad I don't need
this stupid patch anymore, Thanks.
In case anyone is further interested I have attached test program
since I spent the last hour or so chopping it up anyway :S [ gcc -o
kms -D_FILE_OFFSET_BITS=64 main.c ]
That preprocessor define worked but I'm still confused about this
DRM_FILE_PAGE_OFFSET thing. Check out drivers/gpu/drm/drm_gem.c
right above drm_gem_init.
---
/*
* We make up offsets for buffer objects so we can recognize them at
* mmap time.
*/
/* pgoff in mmap is an unsigned long, so we need to make sure that
* the faked up offset will fit
*/
#if BITS_PER_LONG == 64
#define DRM_FILE_PAGE_OFFSET_START ((0xFFFFFFFFUL >> PAGE_SHIFT) + 1)
#define DRM_FILE_PAGE_OFFSET_SIZE ((0xFFFFFFFFUL >> PAGE_SHIFT) * 16)
#else
#define DRM_FILE_PAGE_OFFSET_START ((0xFFFFFFFUL >> PAGE_SHIFT) + 1)
#define DRM_FILE_PAGE_OFFSET_SIZE ((0xFFFFFFFUL >> PAGE_SHIFT) * 16)
#endif
---
Why is having a 64-bit file offsets critical, causing -EINVAL on mmap?
What problems might be associated with using (0x10000000UL >>
PAGE_SHIFT) ?
On Mon, Oct 22, 2018 at 1:50 AM Dave Airlie <[email protected]> wrote:
>
> On Mon, 22 Oct 2018 at 10:49, Michael Tirado <[email protected]> wrote:
> >
> > On Mon, Oct 22, 2018 at 12:26 AM Dave Airlie <[email protected]> wrote:
> > >
> > > This shouldn't be necessary, did someone misbackport the mmap changes without:
> > >
> > > drm: set FMODE_UNSIGNED_OFFSET for drm files
> > >
> > > Dave.
> >
> > The latest kernel I have had to patch was a 4.18-rc6. I'll try with a
> > newer 4.19 and let you know if it decides to work. If not I'll
> > prepare a test case for demonstration on qemu-system-i386.
>
> If you have custom userspace software, make sure it's using
> AC_SYS_LARGEFILE or whatever the equivalant is in your build system.
>
> 64-bit file offsets are important.
>
> Dave.
On Tue, 23 Oct 2018 at 16:13, Michael Tirado <[email protected]> wrote:
>
> That preprocessor define worked but I'm still confused about this
> DRM_FILE_PAGE_OFFSET thing. Check out drivers/gpu/drm/drm_gem.c
> right above drm_gem_init.
>
> ---
>
> /*
> * We make up offsets for buffer objects so we can recognize them at
> * mmap time.
> */
>
> /* pgoff in mmap is an unsigned long, so we need to make sure that
> * the faked up offset will fit
> */
>
> #if BITS_PER_LONG == 64
> #define DRM_FILE_PAGE_OFFSET_START ((0xFFFFFFFFUL >> PAGE_SHIFT) + 1)
> #define DRM_FILE_PAGE_OFFSET_SIZE ((0xFFFFFFFFUL >> PAGE_SHIFT) * 16)
> #else
> #define DRM_FILE_PAGE_OFFSET_START ((0xFFFFFFFUL >> PAGE_SHIFT) + 1)
> #define DRM_FILE_PAGE_OFFSET_SIZE ((0xFFFFFFFUL >> PAGE_SHIFT) * 16)
> #endif
>
>
> ---
>
> Why is having a 64-bit file offsets critical, causing -EINVAL on mmap?
> What problems might be associated with using (0x10000000UL >>
> PAGE_SHIFT) ?
a) it finds people not using the correct userspace defines. mostly
libdrm should handle this,
and possibly mesa.
b) there used to be legacy maps below that address on older drivers,
so we decided to never put stuff in the first 32-bit range that they
could clash with.
Dave.