[PATCH] drm: Round size of mappings in drmAddMap ioctl
Currently, userspace fails to obtain the SAREA mapping on some platforms because they pass SAREA_MAX to drmAddMap without aligning it to the page size. This breaks for example on PowerPC with 64K pages. The way SAREA_MAX is defined with a bunch of ifdef's and duplicated between libdrm and the X server is gross, ultimately it should be retrieved by userspace from the kernel, but in the meantime, we have plenty of existing userspace built with bad values that need to work. The actual SAREA in the kernel is created with an aligned size by the radeon driver (I haven't tested others) so it's purely a userspace problem. This works around it by rounding the requested size in drmAddMap to the page size. There should never be any need to manipulate maps smaller than a page (MMIO regions might but there isn't much we can do about it) and our mmap() calls has enough sanity checks here if the map is actually too small to be mapped. We also only do that fir the ioctl, not the in-kernel call. Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org --- This fixes DRM with 16K and 64K pages on PowerPC embedded machines. Dunno if you want that in 2.6.30, I would personally like that but I'm not going to force your hand :-) Cheers, Ben. Index: linux-work/drivers/gpu/drm/drm_bufs.c === --- linux-work.orig/drivers/gpu/drm/drm_bufs.c 2009-05-18 10:50:04.0 +1000 +++ linux-work/drivers/gpu/drm/drm_bufs.c 2009-05-18 10:51:01.0 +1000 @@ -404,6 +404,8 @@ int drm_addmap_ioctl(struct drm_device * if (!(capable(CAP_SYS_ADMIN) || map-type == _DRM_AGP || map-type == _DRM_SHM)) return -EPERM; + /* Workaround for userspace passing smaller than page size quantities */ + map-size = PAGE_ALIGN(map-size); err = drm_addmap_core(dev, map-offset, map-size, map-type, map-flags, maplist); -- Crystal Reports - New Free Runtime and 30 Day Trial Check out the new simplified licensing option that enables unlimited royalty-free distribution of the report engine for externally facing server and web deployment. http://p.sf.net/sfu/businessobjects -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [PATCH] drm: Round size of mappings in drmAddMap ioctl
On Mon, May 18, 2009 at 03:11, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: Currently, userspace fails to obtain the SAREA mapping on some platforms because they pass SAREA_MAX to drmAddMap without aligning it to the page size. This breaks for example on PowerPC with 64K pages. The way SAREA_MAX is defined with a bunch of ifdef's and duplicated between libdrm and the X server is gross, ultimately it should be retrieved by userspace from the kernel, but in the meantime, we have plenty of existing userspace built with bad values that need to work. The actual SAREA in the kernel is created with an aligned size by the radeon driver (I haven't tested others) so it's purely a userspace problem. This works around it by rounding the requested size in drmAddMap to the page size. There should never be any need to manipulate maps smaller than a page (MMIO regions might but there isn't much we can do about it) and our mmap() calls has enough sanity checks here if the map is actually too small to be mapped. We also only do that fir the ioctl, not the in-kernel call. So, in order to fix a problem with the SAREA you align the map size for all added maps? Sounds bogus to me, especially since the range of possible page sizes is potentially unbounded. IMO the proper thing to do is to fix the drivers when they create the SAREA... Stephane -- Crystal Reports - New Free Runtime and 30 Day Trial Check out the new simplified licensing option that enables unlimited royalty-free distribution of the report engine for externally facing server and web deployment. http://p.sf.net/sfu/businessobjects -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [PATCH] drm: Round size of mappings in drmAddMap ioctl
So, in order to fix a problem with the SAREA you align the map size for all added maps? Sounds bogus to me, especially since the range of possible page sizes is potentially unbounded. Only the requested size from userspace, most drivers create the map from the kernel anyway. IMO the proper thing to do is to fix the drivers when they create the SAREA... The SAREA, in my case (radeon) is -already- created with the proper size, it's userspace making use of the bogus SAREA_MAX constant that gets it wrong and fails to map it. However, after our IRC discussion, I'll post a different patch that we agreed provides a better and safer workaround. Jesse, ignore that patch for now, new one on the cookpot. Cheers, Ben. -- Crystal Reports - New Free Runtime and 30 Day Trial Check out the new simplified licensing option that enables unlimited royalty-free distribution of the report engine for externally facing server and web deployment. http://p.sf.net/sfu/businessobjects -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
