On Mon, Oct 12, 2020 at 11:51:30AM +0100, Chris Wilson wrote:
> On some processors (such as arch/x86), accessing a page via a WC PAT is
> bypassed if the page is physically tagged in the CPU cache, and the
> access is serviced by the cache instead -- which leads to incoherency
> should the physical page itself be accessed using DMA. In order to
> prevent the false cache sharing of the physical pages, we need to
> explicitly flush the cache lines of those pages.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Hm I'd leave this out for now. dma-api/cache flushing, and especially on
x86 is kinda a mess. I'd just land v1 of your patch meanwhile for vgem.
-Daniel
> ---
> drivers/gpu/drm/drm_gem.c | 8 ++++++--
> drivers/gpu/drm/drm_gem_shmem_helper.c | 8 +++++++-
> drivers/gpu/drm/etnaviv/etnaviv_gem.c | 2 +-
> drivers/gpu/drm/gma500/gtt.c | 2 +-
> drivers/gpu/drm/msm/msm_gem.c | 2 +-
> drivers/gpu/drm/omapdrm/omap_gem.c | 2 +-
> drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 2 +-
> drivers/gpu/drm/tegra/gem.c | 2 +-
> drivers/gpu/drm/vgem/vgem_drv.c | 2 +-
> drivers/gpu/drm/vkms/vkms_gem.c | 2 +-
> drivers/gpu/drm/xen/xen_drm_front_gem.c | 2 +-
> include/drm/drm_gem.h | 2 +-
> 12 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 1da67d34e55d..1948855d69e6 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -40,6 +40,7 @@
> #include <linux/pagevec.h>
>
> #include <drm/drm.h>
> +#include <drm/drm_cache.h>
> #include <drm/drm_device.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_file.h>
> @@ -525,6 +526,7 @@ static void drm_gem_check_release_pagevec(struct pagevec *pvec)
> * drm_gem_get_pages - helper to allocate backing pages for a GEM object
> * from shmem
> * @obj: obj in question
> + * @clflush: whether to clear any CPU caches associated with the backing store
> *
> * This reads the page-array of the shmem-backing storage of the given gem
> * object. An array of pages is returned. If a page is not allocated or
> @@ -546,14 +548,13 @@ static void drm_gem_check_release_pagevec(struct pagevec *pvec)
> * drm_gem_object_init(), but not for those initialized with
> * drm_gem_private_object_init() only.
> */
> -struct page **drm_gem_get_pages(struct drm_gem_object *obj)
> +struct page **drm_gem_get_pages(struct drm_gem_object *obj, bool clflush)
> {
> struct address_space *mapping;
> struct page *p, **pages;
> struct pagevec pvec;
> int i, npages;
>
> -
> if (WARN_ON(!obj->filp))
> return ERR_PTR(-EINVAL);
>
> @@ -589,6 +590,9 @@ struct page **drm_gem_get_pages(struct drm_gem_object *obj)
> (page_to_pfn(p) >= 0x00100000UL));
> }
>
> + if (clflush)
> + drm_clflush_pages(pages, npages);
> +
> return pages;
>
> fail:
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index fb11df7aced5..78a2eb77802b 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -152,7 +152,13 @@ static int drm_gem_shmem_get_pages_locked(struct drm_gem_shmem_object *shmem)
> if (shmem->pages_use_count++ > 0)
> return 0;
>
> - pages = drm_gem_get_pages(obj);
> + /*
> + * On some processors (such as arch/x86), accessing a page via a WC PAT
> + * is bypassed if the page is physically tagged in the CPU cache, and
> + * the access is serviced by the cache instead -- which leads to
> + * incoherency should the physical page itself be accessed using DMA.
> + */
> + pages = drm_gem_get_pages(obj, !shmem->map_cached);
> if (IS_ERR(pages)) {
> DRM_DEBUG_KMS("Failed to get pages (%ld)\n", PTR_ERR(pages));
> shmem->pages_use_count = 0;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 67d9a2b9ea6a..d8279ea363b3 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -58,7 +58,7 @@ static void etnaviv_gem_scatterlist_unmap(struct etnaviv_gem_object *etnaviv_obj
> static int etnaviv_gem_shmem_get_pages(struct etnaviv_gem_object *etnaviv_obj)
> {
> struct drm_device *dev = etnaviv_obj->base.dev;
> - struct page **p = drm_gem_get_pages(&etnaviv_obj->base);
> + struct page **p = drm_gem_get_pages(&etnaviv_obj->base, false);
>
> if (IS_ERR(p)) {
> dev_dbg(dev->dev, "could not get pages: %ld\n", PTR_ERR(p));
> diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
> index 9278bcfad1bf..ada56aec7e68 100644
> --- a/drivers/gpu/drm/gma500/gtt.c
> +++ b/drivers/gpu/drm/gma500/gtt.c
> @@ -197,7 +197,7 @@ static int psb_gtt_attach_pages(struct gtt_range *gt)
>
> WARN_ON(gt->pages);
>
> - pages = drm_gem_get_pages(>->gem);
> + pages = drm_gem_get_pages(>->gem, false);
> if (IS_ERR(pages))
> return PTR_ERR(pages);
>
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index c79828d31822..a7a67ef4e27e 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -102,7 +102,7 @@ static struct page **get_pages(struct drm_gem_object *obj)
> int npages = obj->size >> PAGE_SHIFT;
>
> if (use_pages(obj))
> - p = drm_gem_get_pages(obj);
> + p = drm_gem_get_pages(obj, false);
> else
> p = get_pages_vram(obj, npages);
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
> index d8e09792793a..1ff272c4be33 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -236,7 +236,7 @@ static int omap_gem_attach_pages(struct drm_gem_object *obj)
> if (!(omap_obj->flags & OMAP_BO_MEM_SHMEM) || omap_obj->pages)
> return 0;
>
> - pages = drm_gem_get_pages(obj);
> + pages = drm_gem_get_pages(obj, false);
> if (IS_ERR(pages)) {
> dev_err(obj->dev->dev, "could not get pages: %ld\n", PTR_ERR(pages));
> return PTR_ERR(pages);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> index 7d5ebb10323b..003b76c3e623 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c
> @@ -80,7 +80,7 @@ static int rockchip_gem_get_pages(struct rockchip_gem_object *rk_obj)
> int ret, i;
> struct scatterlist *s;
>
> - rk_obj->pages = drm_gem_get_pages(&rk_obj->base);
> + rk_obj->pages = drm_gem_get_pages(&rk_obj->base, false);
> if (IS_ERR(rk_obj->pages))
> return PTR_ERR(rk_obj->pages);
>
> diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> index 26af8daa9a16..f075f005ecbb 100644
> --- a/drivers/gpu/drm/tegra/gem.c
> +++ b/drivers/gpu/drm/tegra/gem.c
> @@ -289,7 +289,7 @@ static int tegra_bo_get_pages(struct drm_device *drm, struct tegra_bo *bo)
> {
> int err;
>
> - bo->pages = drm_gem_get_pages(&bo->gem);
> + bo->pages = drm_gem_get_pages(&bo->gem, false);
> if (IS_ERR(bo->pages))
> return PTR_ERR(bo->pages);
>
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index fa54a6d1403d..f38dd590fa45 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -272,7 +272,7 @@ static struct page **vgem_pin_pages(struct drm_vgem_gem_object *bo)
> if (bo->pages_pin_count++ == 0) {
> struct page **pages;
>
> - pages = drm_gem_get_pages(&bo->base);
> + pages = drm_gem_get_pages(&bo->base, false);
> if (IS_ERR(pages)) {
> bo->pages_pin_count--;
> mutex_unlock(&bo->pages_lock);
> diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> index 19a0e260a4df..1d17b0ef56c7 100644
> --- a/drivers/gpu/drm/vkms/vkms_gem.c
> +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> @@ -166,7 +166,7 @@ static struct page **_get_pages(struct vkms_gem_object *vkms_obj)
> struct drm_gem_object *gem_obj = &vkms_obj->gem;
>
> if (!vkms_obj->pages) {
> - struct page **pages = drm_gem_get_pages(gem_obj);
> + struct page **pages = drm_gem_get_pages(gem_obj, false);
>
> if (IS_ERR(pages))
> return pages;
> diff --git a/drivers/gpu/drm/xen/xen_drm_front_gem.c b/drivers/gpu/drm/xen/xen_drm_front_gem.c
> index 4f34ef34ba60..520a15d75eb6 100644
> --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
> +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
> @@ -132,7 +132,7 @@ static struct xen_gem_object *gem_create(struct drm_device *dev, size_t size)
> * with the backend
> */
> xen_obj->num_pages = DIV_ROUND_UP(size, PAGE_SIZE);
> - xen_obj->pages = drm_gem_get_pages(&xen_obj->base);
> + xen_obj->pages = drm_gem_get_pages(&xen_obj->base, false);
> if (IS_ERR(xen_obj->pages)) {
> ret = PTR_ERR(xen_obj->pages);
> xen_obj->pages = NULL;
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index c38dd35da00b..118f13b1bb29 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -395,7 +395,7 @@ void drm_gem_free_mmap_offset(struct drm_gem_object *obj);
> int drm_gem_create_mmap_offset(struct drm_gem_object *obj);
> int drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size);
>
> -struct page **drm_gem_get_pages(struct drm_gem_object *obj);
> +struct page **drm_gem_get_pages(struct drm_gem_object *obj, bool clflush);
> void drm_gem_put_pages(struct drm_gem_object *obj, struct page **pages,
> bool dirty, bool accessed);
>
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
