Re: [PATCH 1/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On Fri, Apr 06, 2018 at 02:25:08PM +0300, Oleksandr Andrushchenko wrote: > On 04/03/2018 12:47 PM, Daniel Vetter wrote: > > On Thu, Mar 29, 2018 at 04:19:31PM +0300, Oleksandr Andrushchenko wrote: > > > From: Oleksandr Andrushchenko > > > +static int to_refs_grant_foreign_access(struct xen_gem_object *xen_obj) > > > +{ > > > + grant_ref_t priv_gref_head; > > > + int ret, j, cur_ref, num_pages; > > > + struct sg_page_iter sg_iter; > > > + > > > + ret = gnttab_alloc_grant_references(xen_obj->num_pages, > > > + &priv_gref_head); > > > + if (ret < 0) { > > > + DRM_ERROR("Cannot allocate grant references\n"); > > > + return ret; > > > + } > > > + > > > + j = 0; > > > + num_pages = xen_obj->num_pages; > > > + for_each_sg_page(xen_obj->sgt->sgl, &sg_iter, xen_obj->sgt->nents, 0) { > > > + struct page *page; > > > + > > > + page = sg_page_iter_page(&sg_iter); > > Quick drive-by: You can't assume that an sgt is struct page backed. > Do you mean that someone could give me sgt which never > seen sg_assign_page for its entries? Yes. > What are the other use-cases for that to happen? Sharing vram or other resources which are not backed by a struct page. See Christian König's recent work to accomplish just that for admgpu. > > And you probably want to check this at import/attach time. > The check you mean is to make sure that when I call > page = sg_page_iter_page(&sg_iter); > I have to make sure that I get a valid page? Yup. > > -Daniel > Thank you, > Oleksandr Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 1/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On 04/03/2018 12:47 PM, Daniel Vetter wrote: On Thu, Mar 29, 2018 at 04:19:31PM +0300, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko +static int to_refs_grant_foreign_access(struct xen_gem_object *xen_obj) +{ + grant_ref_t priv_gref_head; + int ret, j, cur_ref, num_pages; + struct sg_page_iter sg_iter; + + ret = gnttab_alloc_grant_references(xen_obj->num_pages, + &priv_gref_head); + if (ret < 0) { + DRM_ERROR("Cannot allocate grant references\n"); + return ret; + } + + j = 0; + num_pages = xen_obj->num_pages; + for_each_sg_page(xen_obj->sgt->sgl, &sg_iter, xen_obj->sgt->nents, 0) { + struct page *page; + + page = sg_page_iter_page(&sg_iter); Quick drive-by: You can't assume that an sgt is struct page backed. Do you mean that someone could give me sgt which never seen sg_assign_page for its entries? What are the other use-cases for that to happen? And you probably want to check this at import/attach time. The check you mean is to make sure that when I call page = sg_page_iter_page(&sg_iter); I have to make sure that I get a valid page? -Daniel Thank you, Oleksandr
Re: [PATCH 1/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On Thu, Mar 29, 2018 at 04:19:31PM +0300, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko > +static int to_refs_grant_foreign_access(struct xen_gem_object *xen_obj) > +{ > + grant_ref_t priv_gref_head; > + int ret, j, cur_ref, num_pages; > + struct sg_page_iter sg_iter; > + > + ret = gnttab_alloc_grant_references(xen_obj->num_pages, > + &priv_gref_head); > + if (ret < 0) { > + DRM_ERROR("Cannot allocate grant references\n"); > + return ret; > + } > + > + j = 0; > + num_pages = xen_obj->num_pages; > + for_each_sg_page(xen_obj->sgt->sgl, &sg_iter, xen_obj->sgt->nents, 0) { > + struct page *page; > + > + page = sg_page_iter_page(&sg_iter); Quick drive-by: You can't assume that an sgt is struct page backed. And you probably want to check this at import/attach time. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch