Paul Mackerras writes:
> 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.
That's fine. This can be done, it will just make the patch bigger, which
should be less of a concern than future confusion.
>
> 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.
>
Well, both approaches are fine.
The disadvantage of the second apporach however is that we have no
control over the token value. Therefore we cannot take the gentle
approach to make the attempt to leave values that fit into 32bit
unchanged. This would definitely change the kernel ABI and may
have side effects for some exisiting drivers - especially binary
only drivers.
Egbert.
-------------------------------------------------------
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
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
