Paul Mackerras wrote:
Keith Whitwell writes:
In general I'd prefer that the values passed to clients (and back again)
to be abstract tokens rather than actual addresses or offsets into some
unspecified address space.
Which should mean that 32 bits is more than ample to contain them...
The problem that we have is that currently the kernel DRM uses the
map->offset field for two different things: (a) as a token for
userspace to use to identify a particular map and (b) to store
map-type-specific information such as the physical address for
_DRM_REGISTERS and _DRM_FRAME_BUFFER maps, or the address within the
GART aperture for _DRM_AGP maps, or the kernel virtual address within
a scatter/gather mapping for _DRM_SCATTER_GATHER maps. For _DRM_SHM
maps, map->offset is set to the kernel virtual address of the memory
allocated for the map, but that is only used as a token, not for
internal use inside the DRM.
In fact it is only by luck that we don't get collisions between the
physical addresses used for _DRM_REGISTERS and _DRM_FRAME_BUFFER maps
and the kernel virtual addresses used for _DRM_SHM maps at the moment.
My patch took the approach of only creating abstract tokens for
_DRM_SHM maps, which meant that I didn't need to disentangle (a) and
(b). If we are going to create abstract tokens for all maps, we need
to do something different.
One alternative is to add another field to the kernel's (i.e. the
DRM's) version of the drm_map_t, in which to store the userspace
token. This is the approach taken by Egbert's patch. The current
version of Egbert's patch has the kernel version of the drm_map_t
called the same but different from the userspace drm_map_t. I would
prefer that the kernel structure be called something different if its
contents are different, but that will admittedly make the patch pretty
intrusive.
Another alternative is to have a separate data structure to translate
userspace tokens to drm_map_t pointers. The "idr" structure in the
kernel (include/linux/idr.h) is commonly used for this sort of thing.
With this approach, we would allocate map handles as small multiples
of PAGE_SIZE, and use idr_find(&dev->map_idr, handle >> PAGE_SHIFT) to
map from userspace tokens to drm_map_t *'s. I believe we don't ever
need to go from drm_map_t * to the userspace token. I'll try to code
this up over the weekend so that we can see how intrusive it is likely
to be. The advantage is of course that it means we don't have to have
different drm_map_t's in the kernel and userspace.
Sounds good Paul. I think it's worthwhile to at least investigate this
as it'd be a cleaner result all round at the end.
Keith
-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click
--
_______________________________________________
Dri-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel
