* [PATCH v4 1/1] drm: allow limiting the scatter list size. [not found] <20200907112425.15610-1-kraxel@redhat.com> @ 2020-09-07 11:24 ` Gerd Hoffmann 2020-09-07 13:53 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Gerd Hoffmann @ 2020-09-07 11:24 UTC (permalink / raw) To: dri-devel Cc: christian.koenig, Gerd Hoffmann, Alex Deucher, David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Lucas Stach, Russell King, Christian Gmeiner, Rob Clark, Sean Paul, Ben Skeggs, Sandy Huang, Heiko Stübner, Thierry Reding, Jonathan Hunter, Oleksandr Andrushchenko, open list:RADEON and AMDGPU DRM DRIVERS, open list, moderated list:DRM DRIVERS FOR VIVANTE GPU IP, open list:DRM DRIVER FOR MSM ADRENO GPU, open list:DRM DRIVER FOR MSM ADRENO GPU, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, moderated list:ARM/Rockchip SoC support, open list:ARM/Rockchip SoC support, open list:DRM DRIVERS FOR NVIDIA TEGRA, moderated list:DRM DRIVERS FOR XEN Add drm_device argument to drm_prime_pages_to_sg(), so we can call dma_max_mapping_size() to figure the segment size limit and call into __sg_alloc_table_from_pages() with the correct limit. This fixes virtio-gpu with sev. Possibly it'll fix other bugs too given that drm seems to totaly ignore segment size limits so far ... v2: place max_segment in drm driver not gem object. v3: move max_segment next to the other gem fields. v4: just use dma_max_mapping_size(). Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- include/drm/drm_prime.h | 3 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 3 ++- drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- drivers/gpu/drm/drm_prime.c | 13 ++++++++++--- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 3 ++- drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 2 +- drivers/gpu/drm/msm/msm_gem.c | 2 +- drivers/gpu/drm/msm/msm_gem_prime.c | 2 +- drivers/gpu/drm/nouveau/nouveau_prime.c | 2 +- drivers/gpu/drm/radeon/radeon_prime.c | 2 +- drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 5 +++-- drivers/gpu/drm/tegra/gem.c | 2 +- drivers/gpu/drm/vgem/vgem_drv.c | 2 +- drivers/gpu/drm/xen/xen_drm_front_gem.c | 3 ++- 14 files changed, 29 insertions(+), 17 deletions(-) diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h index 9af7422b44cf..bf141e74a1c2 100644 --- a/include/drm/drm_prime.h +++ b/include/drm/drm_prime.h @@ -88,7 +88,8 @@ void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr); int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma); int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma); -struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_pages); +struct sg_table *drm_prime_pages_to_sg(struct drm_device *dev, + struct page **pages, unsigned int nr_pages); struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj, int flags); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c index 519ce4427fce..d7050ab95946 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c @@ -302,7 +302,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct dma_buf_attachment *attach, switch (bo->tbo.mem.mem_type) { case TTM_PL_TT: - sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages, + sgt = drm_prime_pages_to_sg(obj->dev, + bo->tbo.ttm->pages, bo->tbo.num_pages); if (IS_ERR(sgt)) return sgt; diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 4b7cfbac4daa..0a952f27c184 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -656,7 +656,7 @@ struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_object *obj) WARN_ON(shmem->base.import_attach); - return drm_prime_pages_to_sg(shmem->pages, obj->size >> PAGE_SHIFT); + return drm_prime_pages_to_sg(obj->dev, shmem->pages, obj->size >> PAGE_SHIFT); } EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 1693aa7c14b5..8a6a3c99b7d8 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -802,9 +802,11 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { * * This is useful for implementing &drm_gem_object_funcs.get_sg_table. */ -struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_pages) +struct sg_table *drm_prime_pages_to_sg(struct drm_device *dev, + struct page **pages, unsigned int nr_pages) { struct sg_table *sg = NULL; + size_t max_segment = 0; int ret; sg = kmalloc(sizeof(struct sg_table), GFP_KERNEL); @@ -813,8 +815,13 @@ struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_page goto out; } - ret = sg_alloc_table_from_pages(sg, pages, nr_pages, 0, - nr_pages << PAGE_SHIFT, GFP_KERNEL); + if (dev) + max_segment = dma_max_mapping_size(dev->dev); + if (max_segment == 0 || max_segment > SCATTERLIST_MAX_SEGMENT) + max_segment = SCATTERLIST_MAX_SEGMENT; + ret = __sg_alloc_table_from_pages(sg, pages, nr_pages, 0, + nr_pages << PAGE_SHIFT, + max_segment, GFP_KERNEL); if (ret) goto out; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c index f06e19e7be04..ea19f1d27275 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c @@ -103,7 +103,8 @@ struct page **etnaviv_gem_get_pages(struct etnaviv_gem_object *etnaviv_obj) int npages = etnaviv_obj->base.size >> PAGE_SHIFT; struct sg_table *sgt; - sgt = drm_prime_pages_to_sg(etnaviv_obj->pages, npages); + sgt = drm_prime_pages_to_sg(etnaviv_obj->base.dev, + etnaviv_obj->pages, npages); if (IS_ERR(sgt)) { dev_err(dev->dev, "failed to allocate sgt: %ld\n", PTR_ERR(sgt)); diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c index 6d9e5c3c4dd5..4aa3426a9ba4 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c @@ -19,7 +19,7 @@ struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj) if (WARN_ON(!etnaviv_obj->pages)) /* should have already pinned! */ return ERR_PTR(-EINVAL); - return drm_prime_pages_to_sg(etnaviv_obj->pages, npages); + return drm_prime_pages_to_sg(obj->dev, etnaviv_obj->pages, npages); } void *etnaviv_gem_prime_vmap(struct drm_gem_object *obj) diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index b2f49152b4d4..b4553caaa196 100644 --- a/drivers/gpu/drm/msm/msm_gem.c +++ b/drivers/gpu/drm/msm/msm_gem.c @@ -126,7 +126,7 @@ static struct page **get_pages(struct drm_gem_object *obj) msm_obj->pages = p; - msm_obj->sgt = drm_prime_pages_to_sg(p, npages); + msm_obj->sgt = drm_prime_pages_to_sg(obj->dev, p, npages); if (IS_ERR(msm_obj->sgt)) { void *ptr = ERR_CAST(msm_obj->sgt); diff --git a/drivers/gpu/drm/msm/msm_gem_prime.c b/drivers/gpu/drm/msm/msm_gem_prime.c index d7c8948427fe..515ef80816a0 100644 --- a/drivers/gpu/drm/msm/msm_gem_prime.c +++ b/drivers/gpu/drm/msm/msm_gem_prime.c @@ -19,7 +19,7 @@ struct sg_table *msm_gem_prime_get_sg_table(struct drm_gem_object *obj) if (WARN_ON(!msm_obj->pages)) /* should have already pinned! */ return NULL; - return drm_prime_pages_to_sg(msm_obj->pages, npages); + return drm_prime_pages_to_sg(obj->dev, msm_obj->pages, npages); } void *msm_gem_prime_vmap(struct drm_gem_object *obj) diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c b/drivers/gpu/drm/nouveau/nouveau_prime.c index bae6a3eccee0..7766b810653f 100644 --- a/drivers/gpu/drm/nouveau/nouveau_prime.c +++ b/drivers/gpu/drm/nouveau/nouveau_prime.c @@ -32,7 +32,7 @@ struct sg_table *nouveau_gem_prime_get_sg_table(struct drm_gem_object *obj) struct nouveau_bo *nvbo = nouveau_gem_object(obj); int npages = nvbo->bo.num_pages; - return drm_prime_pages_to_sg(nvbo->bo.ttm->pages, npages); + return drm_prime_pages_to_sg(obj->dev, nvbo->bo.ttm->pages, npages); } void *nouveau_gem_prime_vmap(struct drm_gem_object *obj) diff --git a/drivers/gpu/drm/radeon/radeon_prime.c b/drivers/gpu/drm/radeon/radeon_prime.c index b906e8fbd5f3..ea4c900e7c41 100644 --- a/drivers/gpu/drm/radeon/radeon_prime.c +++ b/drivers/gpu/drm/radeon/radeon_prime.c @@ -36,7 +36,7 @@ struct sg_table *radeon_gem_prime_get_sg_table(struct drm_gem_object *obj) struct radeon_bo *bo = gem_to_radeon_bo(obj); int npages = bo->tbo.num_pages; - return drm_prime_pages_to_sg(bo->tbo.ttm->pages, npages); + return drm_prime_pages_to_sg(obj->dev, bo->tbo.ttm->pages, npages); } void *radeon_gem_prime_vmap(struct drm_gem_object *obj) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c index b9275ba7c5a5..77eeaf3439f6 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c @@ -85,7 +85,8 @@ static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj) rk_obj->num_pages = rk_obj->base.size >> PAGE_SHIFT; - rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages); + rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->base.dev, + rk_obj->pages, rk_obj->num_pages); if (IS_ERR(rk_obj->sgt)) { ret = PTR_ERR(rk_obj->sgt); goto err_put_pages; @@ -442,7 +443,7 @@ struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj) int ret; if (rk_obj->pages) - return drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages); + return drm_prime_pages_to_sg(obj->dev,rk_obj->pages, rk_obj->num_pages); sgt = kzalloc(sizeof(*sgt), GFP_KERNEL); if (!sgt) diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c index 723df142a981..47e2935b8c68 100644 --- a/drivers/gpu/drm/tegra/gem.c +++ b/drivers/gpu/drm/tegra/gem.c @@ -284,7 +284,7 @@ static int tegra_bo_get_pages(struct drm_device *drm, struct tegra_bo *bo) bo->num_pages = bo->gem.size >> PAGE_SHIFT; - bo->sgt = drm_prime_pages_to_sg(bo->pages, bo->num_pages); + bo->sgt = drm_prime_pages_to_sg(bo->gem.dev, bo->pages, bo->num_pages); if (IS_ERR(bo->sgt)) { err = PTR_ERR(bo->sgt); goto put_pages; diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c index 313339bbff90..15dd41e67de3 100644 --- a/drivers/gpu/drm/vgem/vgem_drv.c +++ b/drivers/gpu/drm/vgem/vgem_drv.c @@ -321,7 +321,7 @@ static struct sg_table *vgem_prime_get_sg_table(struct drm_gem_object *obj) { struct drm_vgem_gem_object *bo = to_vgem_bo(obj); - return drm_prime_pages_to_sg(bo->pages, bo->base.size >> PAGE_SHIFT); + return drm_prime_pages_to_sg(obj->dev, bo->pages, bo->base.size >> PAGE_SHIFT); } static struct drm_gem_object* vgem_prime_import(struct drm_device *dev, diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c index 39ff95b75357..aed7510e2710 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c @@ -179,7 +179,8 @@ struct sg_table *xen_drm_front_gem_get_sg_table(struct drm_gem_object *gem_obj) if (!xen_obj->pages) return ERR_PTR(-ENOMEM); - return drm_prime_pages_to_sg(xen_obj->pages, xen_obj->num_pages); + return drm_prime_pages_to_sg(gem_obj->dev, + xen_obj->pages, xen_obj->num_pages); } struct drm_gem_object * -- 2.27.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/1] drm: allow limiting the scatter list size. 2020-09-07 11:24 ` [PATCH v4 1/1] drm: allow limiting the scatter list size Gerd Hoffmann @ 2020-09-07 13:53 ` Daniel Vetter 2020-09-08 5:48 ` Gerd Hoffmann 0 siblings, 1 reply; 6+ messages in thread From: Daniel Vetter @ 2020-09-07 13:53 UTC (permalink / raw) To: Gerd Hoffmann Cc: dri-devel, Christian König, Alex Deucher, David Airlie, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Lucas Stach, Russell King, Christian Gmeiner, Rob Clark, Sean Paul, Ben Skeggs, Sandy Huang, Heiko Stübner, Thierry Reding, Jonathan Hunter, Oleksandr Andrushchenko, open list:RADEON and AMDGPU DRM DRIVERS, open list, moderated list:DRM DRIVERS FOR VIVANTE GPU IP, open list:DRM DRIVER FOR MSM ADRENO GPU, open list:DRM DRIVER FOR MSM ADRENO GPU, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, moderated list:ARM/Rockchip SoC support, open list:ARM/Rockchip SoC support, open list:DRM DRIVERS FOR NVIDIA TEGRA, moderated list:DRM DRIVERS FOR XEN On Mon, Sep 7, 2020 at 1:24 PM Gerd Hoffmann <kraxel@redhat.com> wrote: > > Add drm_device argument to drm_prime_pages_to_sg(), so we can > call dma_max_mapping_size() to figure the segment size limit > and call into __sg_alloc_table_from_pages() with the correct > limit. > > This fixes virtio-gpu with sev. Possibly it'll fix other bugs > too given that drm seems to totaly ignore segment size limits > so far ... > > v2: place max_segment in drm driver not gem object. > v3: move max_segment next to the other gem fields. > v4: just use dma_max_mapping_size(). > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Uh, are you sure this works in all cases for virtio? The comments I've found suggest very much not ... Or is that all very old stuff only that no one cares about anymore? -Daniel > --- > include/drm/drm_prime.h | 3 ++- > drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 3 ++- > drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- > drivers/gpu/drm/drm_prime.c | 13 ++++++++++--- > drivers/gpu/drm/etnaviv/etnaviv_gem.c | 3 ++- > drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 2 +- > drivers/gpu/drm/msm/msm_gem.c | 2 +- > drivers/gpu/drm/msm/msm_gem_prime.c | 2 +- > drivers/gpu/drm/nouveau/nouveau_prime.c | 2 +- > drivers/gpu/drm/radeon/radeon_prime.c | 2 +- > drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 5 +++-- > drivers/gpu/drm/tegra/gem.c | 2 +- > drivers/gpu/drm/vgem/vgem_drv.c | 2 +- > drivers/gpu/drm/xen/xen_drm_front_gem.c | 3 ++- > 14 files changed, 29 insertions(+), 17 deletions(-) > > diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h > index 9af7422b44cf..bf141e74a1c2 100644 > --- a/include/drm/drm_prime.h > +++ b/include/drm/drm_prime.h > @@ -88,7 +88,8 @@ void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr); > int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma); > int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma); > > -struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_pages); > +struct sg_table *drm_prime_pages_to_sg(struct drm_device *dev, > + struct page **pages, unsigned int nr_pages); > struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj, > int flags); > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c > index 519ce4427fce..d7050ab95946 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c > @@ -302,7 +302,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct dma_buf_attachment *attach, > > switch (bo->tbo.mem.mem_type) { > case TTM_PL_TT: > - sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages, > + sgt = drm_prime_pages_to_sg(obj->dev, > + bo->tbo.ttm->pages, > bo->tbo.num_pages); > if (IS_ERR(sgt)) > return sgt; > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c > index 4b7cfbac4daa..0a952f27c184 100644 > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c > @@ -656,7 +656,7 @@ struct sg_table *drm_gem_shmem_get_sg_table(struct drm_gem_object *obj) > > WARN_ON(shmem->base.import_attach); > > - return drm_prime_pages_to_sg(shmem->pages, obj->size >> PAGE_SHIFT); > + return drm_prime_pages_to_sg(obj->dev, shmem->pages, obj->size >> PAGE_SHIFT); > } > EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table); > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 1693aa7c14b5..8a6a3c99b7d8 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -802,9 +802,11 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { > * > * This is useful for implementing &drm_gem_object_funcs.get_sg_table. > */ > -struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_pages) > +struct sg_table *drm_prime_pages_to_sg(struct drm_device *dev, > + struct page **pages, unsigned int nr_pages) > { > struct sg_table *sg = NULL; > + size_t max_segment = 0; > int ret; > > sg = kmalloc(sizeof(struct sg_table), GFP_KERNEL); > @@ -813,8 +815,13 @@ struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int nr_page > goto out; > } > > - ret = sg_alloc_table_from_pages(sg, pages, nr_pages, 0, > - nr_pages << PAGE_SHIFT, GFP_KERNEL); > + if (dev) > + max_segment = dma_max_mapping_size(dev->dev); > + if (max_segment == 0 || max_segment > SCATTERLIST_MAX_SEGMENT) > + max_segment = SCATTERLIST_MAX_SEGMENT; > + ret = __sg_alloc_table_from_pages(sg, pages, nr_pages, 0, > + nr_pages << PAGE_SHIFT, > + max_segment, GFP_KERNEL); > if (ret) > goto out; > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c > index f06e19e7be04..ea19f1d27275 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c > @@ -103,7 +103,8 @@ struct page **etnaviv_gem_get_pages(struct etnaviv_gem_object *etnaviv_obj) > int npages = etnaviv_obj->base.size >> PAGE_SHIFT; > struct sg_table *sgt; > > - sgt = drm_prime_pages_to_sg(etnaviv_obj->pages, npages); > + sgt = drm_prime_pages_to_sg(etnaviv_obj->base.dev, > + etnaviv_obj->pages, npages); > if (IS_ERR(sgt)) { > dev_err(dev->dev, "failed to allocate sgt: %ld\n", > PTR_ERR(sgt)); > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c > index 6d9e5c3c4dd5..4aa3426a9ba4 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c > @@ -19,7 +19,7 @@ struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj) > if (WARN_ON(!etnaviv_obj->pages)) /* should have already pinned! */ > return ERR_PTR(-EINVAL); > > - return drm_prime_pages_to_sg(etnaviv_obj->pages, npages); > + return drm_prime_pages_to_sg(obj->dev, etnaviv_obj->pages, npages); > } > > void *etnaviv_gem_prime_vmap(struct drm_gem_object *obj) > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c > index b2f49152b4d4..b4553caaa196 100644 > --- a/drivers/gpu/drm/msm/msm_gem.c > +++ b/drivers/gpu/drm/msm/msm_gem.c > @@ -126,7 +126,7 @@ static struct page **get_pages(struct drm_gem_object *obj) > > msm_obj->pages = p; > > - msm_obj->sgt = drm_prime_pages_to_sg(p, npages); > + msm_obj->sgt = drm_prime_pages_to_sg(obj->dev, p, npages); > if (IS_ERR(msm_obj->sgt)) { > void *ptr = ERR_CAST(msm_obj->sgt); > > diff --git a/drivers/gpu/drm/msm/msm_gem_prime.c b/drivers/gpu/drm/msm/msm_gem_prime.c > index d7c8948427fe..515ef80816a0 100644 > --- a/drivers/gpu/drm/msm/msm_gem_prime.c > +++ b/drivers/gpu/drm/msm/msm_gem_prime.c > @@ -19,7 +19,7 @@ struct sg_table *msm_gem_prime_get_sg_table(struct drm_gem_object *obj) > if (WARN_ON(!msm_obj->pages)) /* should have already pinned! */ > return NULL; > > - return drm_prime_pages_to_sg(msm_obj->pages, npages); > + return drm_prime_pages_to_sg(obj->dev, msm_obj->pages, npages); > } > > void *msm_gem_prime_vmap(struct drm_gem_object *obj) > diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c b/drivers/gpu/drm/nouveau/nouveau_prime.c > index bae6a3eccee0..7766b810653f 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_prime.c > +++ b/drivers/gpu/drm/nouveau/nouveau_prime.c > @@ -32,7 +32,7 @@ struct sg_table *nouveau_gem_prime_get_sg_table(struct drm_gem_object *obj) > struct nouveau_bo *nvbo = nouveau_gem_object(obj); > int npages = nvbo->bo.num_pages; > > - return drm_prime_pages_to_sg(nvbo->bo.ttm->pages, npages); > + return drm_prime_pages_to_sg(obj->dev, nvbo->bo.ttm->pages, npages); > } > > void *nouveau_gem_prime_vmap(struct drm_gem_object *obj) > diff --git a/drivers/gpu/drm/radeon/radeon_prime.c b/drivers/gpu/drm/radeon/radeon_prime.c > index b906e8fbd5f3..ea4c900e7c41 100644 > --- a/drivers/gpu/drm/radeon/radeon_prime.c > +++ b/drivers/gpu/drm/radeon/radeon_prime.c > @@ -36,7 +36,7 @@ struct sg_table *radeon_gem_prime_get_sg_table(struct drm_gem_object *obj) > struct radeon_bo *bo = gem_to_radeon_bo(obj); > int npages = bo->tbo.num_pages; > > - return drm_prime_pages_to_sg(bo->tbo.ttm->pages, npages); > + return drm_prime_pages_to_sg(obj->dev, bo->tbo.ttm->pages, npages); > } > > void *radeon_gem_prime_vmap(struct drm_gem_object *obj) > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > index b9275ba7c5a5..77eeaf3439f6 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > @@ -85,7 +85,8 @@ static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj) > > rk_obj->num_pages = rk_obj->base.size >> PAGE_SHIFT; > > - rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages); > + rk_obj->sgt = drm_prime_pages_to_sg(rk_obj->base.dev, > + rk_obj->pages, rk_obj->num_pages); > if (IS_ERR(rk_obj->sgt)) { > ret = PTR_ERR(rk_obj->sgt); > goto err_put_pages; > @@ -442,7 +443,7 @@ struct sg_table *rockchip_gem_prime_get_sg_table(struct drm_gem_object *obj) > int ret; > > if (rk_obj->pages) > - return drm_prime_pages_to_sg(rk_obj->pages, rk_obj->num_pages); > + return drm_prime_pages_to_sg(obj->dev,rk_obj->pages, rk_obj->num_pages); > > sgt = kzalloc(sizeof(*sgt), GFP_KERNEL); > if (!sgt) > diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c > index 723df142a981..47e2935b8c68 100644 > --- a/drivers/gpu/drm/tegra/gem.c > +++ b/drivers/gpu/drm/tegra/gem.c > @@ -284,7 +284,7 @@ static int tegra_bo_get_pages(struct drm_device *drm, struct tegra_bo *bo) > > bo->num_pages = bo->gem.size >> PAGE_SHIFT; > > - bo->sgt = drm_prime_pages_to_sg(bo->pages, bo->num_pages); > + bo->sgt = drm_prime_pages_to_sg(bo->gem.dev, bo->pages, bo->num_pages); > if (IS_ERR(bo->sgt)) { > err = PTR_ERR(bo->sgt); > goto put_pages; > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c > index 313339bbff90..15dd41e67de3 100644 > --- a/drivers/gpu/drm/vgem/vgem_drv.c > +++ b/drivers/gpu/drm/vgem/vgem_drv.c > @@ -321,7 +321,7 @@ static struct sg_table *vgem_prime_get_sg_table(struct drm_gem_object *obj) > { > struct drm_vgem_gem_object *bo = to_vgem_bo(obj); > > - return drm_prime_pages_to_sg(bo->pages, bo->base.size >> PAGE_SHIFT); > + return drm_prime_pages_to_sg(obj->dev, bo->pages, bo->base.size >> PAGE_SHIFT); > } > > static struct drm_gem_object* vgem_prime_import(struct drm_device *dev, > diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c > index 39ff95b75357..aed7510e2710 100644 > --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c > +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c > @@ -179,7 +179,8 @@ struct sg_table *xen_drm_front_gem_get_sg_table(struct drm_gem_object *gem_obj) > if (!xen_obj->pages) > return ERR_PTR(-ENOMEM); > > - return drm_prime_pages_to_sg(xen_obj->pages, xen_obj->num_pages); > + return drm_prime_pages_to_sg(gem_obj->dev, > + xen_obj->pages, xen_obj->num_pages); > } > > struct drm_gem_object * > -- > 2.27.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/1] drm: allow limiting the scatter list size. 2020-09-07 13:53 ` Daniel Vetter @ 2020-09-08 5:48 ` Gerd Hoffmann 2020-09-08 8:55 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Gerd Hoffmann @ 2020-09-08 5:48 UTC (permalink / raw) To: Daniel Vetter Cc: dri-devel, Christian König, Alex Deucher, David Airlie, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Lucas Stach, Russell King, Christian Gmeiner, Rob Clark, Sean Paul, Ben Skeggs, Sandy Huang, Heiko Stübner, Thierry Reding, Jonathan Hunter, Oleksandr Andrushchenko, open list:RADEON and AMDGPU DRM DRIVERS, open list, moderated list:DRM DRIVERS FOR VIVANTE GPU IP, open list:DRM DRIVER FOR MSM ADRENO GPU, open list:DRM DRIVER FOR MSM ADRENO GPU, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, moderated list:ARM/Rockchip SoC support, open list:ARM/Rockchip SoC support, open list:DRM DRIVERS FOR NVIDIA TEGRA, moderated list:DRM DRIVERS FOR XEN On Mon, Sep 07, 2020 at 03:53:02PM +0200, Daniel Vetter wrote: > On Mon, Sep 7, 2020 at 1:24 PM Gerd Hoffmann <kraxel@redhat.com> wrote: > > > > Add drm_device argument to drm_prime_pages_to_sg(), so we can > > call dma_max_mapping_size() to figure the segment size limit > > and call into __sg_alloc_table_from_pages() with the correct > > limit. > > > > This fixes virtio-gpu with sev. Possibly it'll fix other bugs > > too given that drm seems to totaly ignore segment size limits > > so far ... > > > > v2: place max_segment in drm driver not gem object. > > v3: move max_segment next to the other gem fields. > > v4: just use dma_max_mapping_size(). > > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > Uh, are you sure this works in all cases for virtio? Sure, I've tested it ;) > The comments I've found suggest very much not ... Or is that all very > old stuff only that no one cares about anymore? I think these days it is possible to override dma_ops per device, which in turn allows virtio to deal with the quirks without the rest of the kernel knowing about these details. I also think virtio-gpu can drop the virtio_has_dma_quirk() checks, just use the dma api path unconditionally and depend on virtio core having setup dma_ops in a way that it JustWorks[tm]. I'll look into that next. take care, Gerd ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/1] drm: allow limiting the scatter list size. 2020-09-08 5:48 ` Gerd Hoffmann @ 2020-09-08 8:55 ` Daniel Vetter 2020-09-08 10:02 ` Gerd Hoffmann 0 siblings, 1 reply; 6+ messages in thread From: Daniel Vetter @ 2020-09-08 8:55 UTC (permalink / raw) To: Gerd Hoffmann Cc: Daniel Vetter, dri-devel, Christian König, Alex Deucher, David Airlie, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Lucas Stach, Russell King, Christian Gmeiner, Rob Clark, Sean Paul, Ben Skeggs, Sandy Huang, Heiko Stübner, Thierry Reding, Jonathan Hunter, Oleksandr Andrushchenko, open list:RADEON and AMDGPU DRM DRIVERS, open list, moderated list:DRM DRIVERS FOR VIVANTE GPU IP, open list:DRM DRIVER FOR MSM ADRENO GPU, open list:DRM DRIVER FOR MSM ADRENO GPU, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, moderated list:ARM/Rockchip SoC support, open list:ARM/Rockchip SoC support, open list:DRM DRIVERS FOR NVIDIA TEGRA, moderated list:DRM DRIVERS FOR XEN On Tue, Sep 08, 2020 at 07:48:58AM +0200, Gerd Hoffmann wrote: > On Mon, Sep 07, 2020 at 03:53:02PM +0200, Daniel Vetter wrote: > > On Mon, Sep 7, 2020 at 1:24 PM Gerd Hoffmann <kraxel@redhat.com> wrote: > > > > > > Add drm_device argument to drm_prime_pages_to_sg(), so we can > > > call dma_max_mapping_size() to figure the segment size limit > > > and call into __sg_alloc_table_from_pages() with the correct > > > limit. > > > > > > This fixes virtio-gpu with sev. Possibly it'll fix other bugs > > > too given that drm seems to totaly ignore segment size limits > > > so far ... > > > > > > v2: place max_segment in drm driver not gem object. > > > v3: move max_segment next to the other gem fields. > > > v4: just use dma_max_mapping_size(). > > > > > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > > > > Uh, are you sure this works in all cases for virtio? > > Sure, I've tested it ;) > > > The comments I've found suggest very much not ... Or is that all very > > old stuff only that no one cares about anymore? > > I think these days it is possible to override dma_ops per device, which > in turn allows virtio to deal with the quirks without the rest of the > kernel knowing about these details. > > I also think virtio-gpu can drop the virtio_has_dma_quirk() checks, just > use the dma api path unconditionally and depend on virtio core having > setup dma_ops in a way that it JustWorks[tm]. I'll look into that next. The comment above vring_use_dma_api() suggests that this has not yet happened, that's why I'm asking. If this has happened then I think it'd be best if you remove that todo entry and update it, as part of the overall series to add dma_max_mapping_size and remove the quirks. Otherwise this all is a bit wtf material :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/1] drm: allow limiting the scatter list size. 2020-09-08 8:55 ` Daniel Vetter @ 2020-09-08 10:02 ` Gerd Hoffmann 2020-09-08 11:37 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Gerd Hoffmann @ 2020-09-08 10:02 UTC (permalink / raw) To: dri-devel, Christian König, Alex Deucher, David Airlie, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Lucas Stach, Russell King, Christian Gmeiner, Rob Clark, Sean Paul, Ben Skeggs, Sandy Huang, Heiko Stübner, Thierry Reding, Jonathan Hunter, Oleksandr Andrushchenko, open list:RADEON and AMDGPU DRM DRIVERS, open list, moderated list:DRM DRIVERS FOR VIVANTE GPU IP, open list:DRM DRIVER FOR MSM ADRENO GPU, open list:DRM DRIVER FOR MSM ADRENO GPU, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, moderated list:ARM/Rockchip SoC support, open list:ARM/Rockchip SoC support, open list:DRM DRIVERS FOR NVIDIA TEGRA, moderated list:DRM DRIVERS FOR XEN > > > The comments I've found suggest very much not ... Or is that all very > > > old stuff only that no one cares about anymore? > > > > I think these days it is possible to override dma_ops per device, which > > in turn allows virtio to deal with the quirks without the rest of the > > kernel knowing about these details. > > > > I also think virtio-gpu can drop the virtio_has_dma_quirk() checks, just > > use the dma api path unconditionally and depend on virtio core having > > setup dma_ops in a way that it JustWorks[tm]. I'll look into that next. > > The comment above vring_use_dma_api() suggests that this has not yet > happened, that's why I'm asking. Hmm, wading through the code, seems it indeed happen yet, even though my testing didn't show any issues. Probably pure luck because devices and cpus have the same memory view on x86. Guess I need to try this on ppc64 to see it actually failing ... So dropping the virtio_has_dma_quirk() checks isn't going to fly. Using dma_max_mapping_size() should be fine though. It might use a lower limit than needed for virtio, but it should not break things. take care, Gerd ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/1] drm: allow limiting the scatter list size. 2020-09-08 10:02 ` Gerd Hoffmann @ 2020-09-08 11:37 ` Daniel Vetter 0 siblings, 0 replies; 6+ messages in thread From: Daniel Vetter @ 2020-09-08 11:37 UTC (permalink / raw) To: Gerd Hoffmann Cc: dri-devel, Christian König, Alex Deucher, David Airlie, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Lucas Stach, Russell King, Christian Gmeiner, Rob Clark, Sean Paul, Ben Skeggs, Sandy Huang, Heiko Stübner, Thierry Reding, Jonathan Hunter, Oleksandr Andrushchenko, open list:RADEON and AMDGPU DRM DRIVERS, open list, moderated list:DRM DRIVERS FOR VIVANTE GPU IP, open list:DRM DRIVER FOR MSM ADRENO GPU, open list:DRM DRIVER FOR MSM ADRENO GPU, open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS, moderated list:ARM/Rockchip SoC support, open list:ARM/Rockchip SoC support, open list:DRM DRIVERS FOR NVIDIA TEGRA, moderated list:DRM DRIVERS FOR XEN On Tue, Sep 08, 2020 at 12:02:53PM +0200, Gerd Hoffmann wrote: > > > > The comments I've found suggest very much not ... Or is that all very > > > > old stuff only that no one cares about anymore? > > > > > > I think these days it is possible to override dma_ops per device, which > > > in turn allows virtio to deal with the quirks without the rest of the > > > kernel knowing about these details. > > > > > > I also think virtio-gpu can drop the virtio_has_dma_quirk() checks, just > > > use the dma api path unconditionally and depend on virtio core having > > > setup dma_ops in a way that it JustWorks[tm]. I'll look into that next. > > > > The comment above vring_use_dma_api() suggests that this has not yet > > happened, that's why I'm asking. > > Hmm, wading through the code, seems it indeed happen yet, even though my > testing didn't show any issues. Probably pure luck because devices and > cpus have the same memory view on x86. Guess I need to try this on > ppc64 to see it actually failing ... > > So dropping the virtio_has_dma_quirk() checks isn't going to fly. > > Using dma_max_mapping_size() should be fine though. It might use a > lower limit than needed for virtio, but it should not break things. Makes sense. On this patch here: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> And I guess would be good if virtio pushes a bit more towards using the dma api abstraction fully so we can get rid of these hacks. Virtio feels like a driver that really should be using dma-api and not dig around behind it because "it' makes stuff 0.5% faster" or so, since being virtualized it's already not the king of speed anyway :-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-08 17:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20200907112425.15610-1-kraxel@redhat.com>
2020-09-07 11:24 ` [PATCH v4 1/1] drm: allow limiting the scatter list size Gerd Hoffmann
2020-09-07 13:53 ` Daniel Vetter
2020-09-08 5:48 ` Gerd Hoffmann
2020-09-08 8:55 ` Daniel Vetter
2020-09-08 10:02 ` Gerd Hoffmann
2020-09-08 11:37 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox