Re: [Intel-gfx] [PATCH v13 6/7] drm/i915: Introduce GEM proxy

[Zhang, Tina]

> -Original Message-
> From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of
> Joonas Lahtinen
> Sent: Monday, August 7, 2017 4:24 PM
> To: Zhang, Tina ; intel-gfx@lists.freedesktop.org; 
> intel-
> gvt-...@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
> ville.syrj...@linux.intel.com; zhen...@linux.intel.com; Lv, Zhiyuan
> ; Wang, Zhi A ;
> alex.william...@redhat.com; kra...@redhat.com; ch...@chris-wilson.co.uk;
> dan...@ffwll.ch; kwankh...@nvidia.com; Tian, Kevin 
> Subject: Re: [Intel-gfx] [PATCH v13 6/7] drm/i915: Introduce GEM proxy
> 
> On ti, 2017-07-25 at 17:28 +0800, Tina Zhang wrote:
> > GEM proxy is a kind of GEM, whose backing physical memory is pinned
> > and produced by guest VM and is used by host as read only. With GEM
> > proxy, host is able to access guest physical memory through GEM object
> > interface. As GEM proxy is such a special kind of GEM, a new flag
> > I915_GEM_OBJECT_IS_PROXY is introduced to ban host from changing the
> > backing storage of GEM proxy.
> >
> 
> It is a good idea to add a changelog here to indicate what was changed since 
> the
> last patch revision. It'll be easier for the reviewer to spot the differences.
> 
> It's also a good idea to add Cc: line for somebody who provided previous 
> review,
> this will, too, allow spotting the patch quicker.
Indeed. Thanks.

Tina
> 
> > Signed-off-by: Tina Zhang 
> 
> 
> 
> > @@ -1730,7 +1744,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev,
> void *data,
> >      */
> >     if (!obj->base.filp) {
> >     i915_gem_object_put(obj);
> > -   return -EINVAL;
> > +   return -EPERM;
> >     }
> 
> This hunk should be separate bugfix patch, but other than that the patch looks
> OK to me.
Thanks. I will send this patch separately.

Tina
> 
> I'll let Chris comment if he feels previous comments were adequately 
> addressed.
> 
> Regards, Joonas
> --
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v13 6/7] drm/i915: Introduce GEM proxy

[Joonas Lahtinen]

On ti, 2017-07-25 at 17:28 +0800, Tina Zhang wrote:
> GEM proxy is a kind of GEM, whose backing physical memory is pinned
> and produced by guest VM and is used by host as read only. With GEM
> proxy, host is able to access guest physical memory through GEM object
> interface. As GEM proxy is such a special kind of GEM, a new flag
> I915_GEM_OBJECT_IS_PROXY is introduced to ban host from changing the
> backing storage of GEM proxy.
> 

It is a good idea to add a changelog here to indicate what was changed
since the last patch revision. It'll be easier for the reviewer to spot
the differences.

It's also a good idea to add Cc: line for somebody who provided
previous review, this will, too, allow spotting the patch quicker.

> Signed-off-by: Tina Zhang 



> @@ -1730,7 +1744,7 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data,
>    */
>   if (!obj->base.filp) {
>   i915_gem_object_put(obj);
> - return -EINVAL;
> + return -EPERM;
>   }

This hunk should be separate bugfix patch, but other than that the
patch looks OK to me.

I'll let Chris comment if he feels previous comments were adequately
addressed.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx