[igt-dev] [PATCH i-g-t v3 1/4] lib/i915/gem_mman: add mmap_offset support

[Chris Wilson]

Quoting Zbigniew Kempczyński (2019-11-21 07:25:20)
> On Wed, Nov 20, 2019 at 07:41:45PM +0000, Chris Wilson wrote:
> > > +bool __gem_mmap_offset__has_wc(int fd)
> > > +{
> > > +       int has_wc = 0;
> > > +
> > > +       if (gem_has_mmap_offset(fd)) {
> > 
> > Don't need the double ioctl... Hmm, oh wait. That's what you get for not
> > checking arg.pad. Gah.
> 
> You suggest we can skip gem_has_mmap_offset() call by trial by fire 
> I915_GEM_MMAP_OFFSET ioctl()?

That's what I would have done, but alas the inherited ioctl does not
allow it and we need to use the extra GETPARAM to verify MMAP_OFFSET
exists.

> > > +/**
> > > + * __gem_mmap_offset:
> > > + * @fd: open i915 drm file descriptor
> > > + * @handle: gem buffer object handle
> > > + * @offset: offset in the gem buffer of the mmap arena
> > > + * @size: size of the mmap arena
> > > + * @prot: memory protection bits as used by mmap()
> > > + * @flags: flags used to determine caching
> > > + *
> > > + * Similar to __gem_mmap but use MMAP_OFFSET IOCTL.
> > > + *
> > > + * Returns: A pointer to the created memory mapping, NULL on failure.
> > > + */
> > > +void *__gem_mmap_offset(int fd, uint32_t handle, uint64_t offset, uint64_t size,
> > > +                       unsigned int prot, uint64_t flags)
> > > +{
> > > +       struct local_i915_gem_mmap_offset arg;
> > > +       void *ptr;
> > > +
> > > +       memset(&arg, 0, sizeof(arg));
> > > +       arg.handle = handle;
> > > +       arg.offset = offset;
> > 
> > That's an interesting extension :-p
> > 
> > Yeah, being able to mmap a slice of the object is one of the extensions
> > I had in mind.
> 
> Doesn't it work as I expect? Isn't memory range verified in do_mmap()
> and after syscall our implementation does the real job then?

Nope. The mmap_offset only returns an offset valid for the whole buffer.
Technically you could mmap() a slice of that, but that's rejected in the
fault handler.

The arg.offset is an outparam only.
-Chris