> -----Original Message----- > From: Chris Wilson [mailto:chris@xxxxxxxxxxxxxxxxxx] > Sent: Tuesday, October 17, 2017 10:37 PM > To: Zhang, Tina <tina.zhang@xxxxxxxxx>; zhenyuw@xxxxxxxxxxxxxxx; Lv, Zhiyuan > <zhiyuan.lv@xxxxxxxxx>; Wang, Zhi A <zhi.a.wang@xxxxxxxxx>; Tian, Kevin > <kevin.tian@xxxxxxxxx>; daniel@xxxxxxxx > Cc: Zhang, Tina <tina.zhang@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; intel- > gvt-dev@xxxxxxxxxxxxxxxxxxxxx; Daniel Vetter <daniel.vetter@xxxxxxxx> > Subject: Re: [PATCH v1 1/2] drm/i915: Introduce GEM proxy > > Quoting Tina Zhang (2017-10-16 09:57:33) > > 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. > > > > V1: > > - the patch is separated from the "Dma-buf support for Gvt-g" > > patch-set. (Joonas) > > > > Here are the histories of this patch in "Dma-buf support for Gvt-g" > > patch-set: > > > > v14: > > - return -ENXIO when gem proxy object is banned by ioctl. > > (Chris) (Daniel) > > > > v13: > > - add comments to GEM proxy. (Chris) > > - don't ban GEM proxy in i915_gem_sw_finish_ioctl. (Chris) > > - check GEM proxy bar after finishing i915_gem_object_wait. (Chris) > > - remove GEM proxy bar in i915_gem_madvise_ioctl. > > > > v6: > > - add gem proxy barrier in the following ioctls. (Chris) > > i915_gem_set_caching_ioctl > > i915_gem_set_domain_ioctl > > i915_gem_sw_finish_ioctl > > i915_gem_set_tiling_ioctl > > i915_gem_madvise_ioctl > > gem_busy: sees the local object, not the proxy activity; that's ok > get_caching: returns the constant value > pread: works by forcing GTT access > pwrite: works by forcing GTT access > mmap: disallowed by !filp, errno fixup in next patch > mmap_gtt: works > set-domain: fixed > sw-finish: noted as impossible (please add checks) I thought we were agreed on this: https://lists.freedesktop.org/archives/intel-gvt-dev/2017-July/001523.html So, in this version I just add the comments without checking > set-tiling: fixed > get_tiling: returns constant value > madvise: works, since it's a discard of the local page array overlay-put-image? No. The original idea is that the GEM proxy could support madvise as it only invokes unpin/pin. And supporting madivse might benefit userspace. So, do you think we'd better ban GEM proxy in madvise? > not supported on any relevant platforms! > gem_wait: works on local bo (as expected) > > execbuf: works (or should at least work fine on a proxy object as either a batch > or one of the auxiliaries) > > Hmm, use as a batch + cmdparser is broken; should result in ENODEV to > userspace. An admittedly odd result, but not an oops. GEM proxy isn't designed to be used as a batch buffer, only as an auxiliary. So I think here you mean we need to ban GEM proxy in i915_gem_execbuffer/i915_gem_execbuffer2, right? > > display: pin-to-display and fencing will work, I think, and flush_for_display is > irrelevant as no direct cpu access. > > dmabuf: works partially, but what doesn't work is optional. Userspace mmap is > barred, most inter-driver interaction is barred. On paper a nuisance should it > want to be used, but we should not oops. Hmm, no, we need to make the > GEM_BUG_ON(!struct_page(obj)) in > i915_gem_object_pin_map() a real return. So, as this can benefit all kinds of gem object w/o backing storage, can we split the following part into another patch? Thanks. BR, Tina > > Please add > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > b/drivers/gpu/drm/i915/i915_gem.c index d699ea3ab80b..6a7ca9a502ba > 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2650,7 +2650,8 @@ void *i915_gem_object_pin_map(struct > drm_i915_gem_object *obj, > void *ptr; > int ret; > > - GEM_BUG_ON(!i915_gem_object_has_struct_page(obj)); > + if (unlikely(!i915_gem_object_has_struct_page(obj))) > + return ERR_PTR(-ENODEV); > > ret = mutex_lock_interruptible(&obj->mm.lock); > if (ret) > > and my > Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
