[PATCH 2/2] drm/ttm: stop dangerous caching attribute change
Alex Deucher
alexdeucher at gmail.com
Tue Sep 22 15:15:07 UTC 2020
On Fri, Sep 18, 2020 at 11:01 AM Christian König
<ckoenig.leichtzumerken at gmail.com> wrote:
>
> When we swapout/in a BO we try to change the caching
> attributes of the pages before/after doing the copy.
>
> On x86 this is done by calling set_pages_uc(),
> set_memory_wc() or set_pages_wb() for not highmem pages
> to update the linear mapping of the page.
>
> On all other platforms we do exactly nothing.
>
> Now on x86 this is unnecessary because copy_highpage() will
> either create a temporary mapping of the page which is wb
> anyway and destroyed immediately again or use the linear
> mapping with the correct caching attributes.
>
> So stop this nonsense and just keep the caching as it is and
> return an error when a driver tries to change the caching of
> an already populated TT object.
>
> This is much more defensive since changing caching
> attributes is platform and driver specific and usually
> doesn't work after the page was initially allocated.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
Series is:
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> ---
> drivers/gpu/drm/ttm/ttm_bo.c | 5 +--
> drivers/gpu/drm/ttm/ttm_tt.c | 71 ++------------------------------
> include/drm/ttm/ttm_set_memory.h | 22 ----------
> 3 files changed, 5 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 1441bc86ac1c..1fa8d87c13ce 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1555,14 +1555,13 @@ int ttm_bo_swapout(struct ttm_bo_global *glob, struct ttm_operation_ctx *ctx)
> * Move to system cached
> */
>
> - if (bo->mem.mem_type != TTM_PL_SYSTEM ||
> - bo->ttm->caching_state != tt_cached) {
> + if (bo->mem.mem_type != TTM_PL_SYSTEM) {
> struct ttm_operation_ctx ctx = { false, false };
> struct ttm_resource evict_mem;
>
> evict_mem = bo->mem;
> evict_mem.mm_node = NULL;
> - evict_mem.placement = TTM_PL_FLAG_CACHED;
> + evict_mem.placement = TTM_PL_MASK_CACHING;
> evict_mem.mem_type = TTM_PL_SYSTEM;
>
> ret = ttm_bo_handle_move_mem(bo, &evict_mem, true, &ctx);
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index 014fb3656407..7c278216a188 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -38,7 +38,6 @@
> #include <drm/drm_cache.h>
> #include <drm/ttm/ttm_bo_driver.h>
> #include <drm/ttm/ttm_page_alloc.h>
> -#include <drm/ttm/ttm_set_memory.h>
>
> /**
> * Allocates a ttm structure for the given BO.
> @@ -115,81 +114,19 @@ static int ttm_sg_tt_alloc_page_directory(struct ttm_dma_tt *ttm)
> return 0;
> }
>
> -static int ttm_tt_set_page_caching(struct page *p,
> - enum ttm_caching_state c_old,
> - enum ttm_caching_state c_new)
> -{
> - int ret = 0;
> -
> - if (PageHighMem(p))
> - return 0;
> -
> - if (c_old != tt_cached) {
> - /* p isn't in the default caching state, set it to
> - * writeback first to free its current memtype. */
> -
> - ret = ttm_set_pages_wb(p, 1);
> - if (ret)
> - return ret;
> - }
> -
> - if (c_new == tt_wc)
> - ret = ttm_set_pages_wc(p, 1);
> - else if (c_new == tt_uncached)
> - ret = ttm_set_pages_uc(p, 1);
> -
> - return ret;
> -}
> -
> -/*
> - * Change caching policy for the linear kernel map
> - * for range of pages in a ttm.
> - */
> -
> static int ttm_tt_set_caching(struct ttm_tt *ttm,
> enum ttm_caching_state c_state)
> {
> - int i, j;
> - struct page *cur_page;
> - int ret;
> -
> if (ttm->caching_state == c_state)
> return 0;
>
> - if (!ttm_tt_is_populated(ttm)) {
> - /* Change caching but don't populate */
> - ttm->caching_state = c_state;
> - return 0;
> - }
> -
> - if (ttm->caching_state == tt_cached)
> - drm_clflush_pages(ttm->pages, ttm->num_pages);
> -
> - for (i = 0; i < ttm->num_pages; ++i) {
> - cur_page = ttm->pages[i];
> - if (likely(cur_page != NULL)) {
> - ret = ttm_tt_set_page_caching(cur_page,
> - ttm->caching_state,
> - c_state);
> - if (unlikely(ret != 0))
> - goto out_err;
> - }
> - }
> + /* Can't change the caching state after TT is populated */
> + if (WARN_ON_ONCE(ttm_tt_is_populated(ttm)))
> + return -EINVAL;
>
> ttm->caching_state = c_state;
>
> return 0;
> -
> -out_err:
> - for (j = 0; j < i; ++j) {
> - cur_page = ttm->pages[j];
> - if (likely(cur_page != NULL)) {
> - (void)ttm_tt_set_page_caching(cur_page, c_state,
> - ttm->caching_state);
> - }
> - }
> -
> - return ret;
> }
>
> int ttm_tt_set_placement_caching(struct ttm_tt *ttm, uint32_t placement)
> @@ -353,8 +290,6 @@ int ttm_tt_swapout(struct ttm_bo_device *bdev, struct ttm_tt *ttm)
> gfp_t gfp_mask;
> int i, ret;
>
> - BUG_ON(ttm->caching_state != tt_cached);
> -
> swap_storage = shmem_file_setup("ttm swap",
> ttm->num_pages << PAGE_SHIFT,
> 0);
> diff --git a/include/drm/ttm/ttm_set_memory.h b/include/drm/ttm/ttm_set_memory.h
> index 3966655b72f1..2343c18a6133 100644
> --- a/include/drm/ttm/ttm_set_memory.h
> +++ b/include/drm/ttm/ttm_set_memory.h
> @@ -57,18 +57,6 @@ static inline int ttm_set_pages_wb(struct page *page, int numpages)
> return set_pages_wb(page, numpages);
> }
>
> -static inline int ttm_set_pages_wc(struct page *page, int numpages)
> -{
> - unsigned long addr = (unsigned long)page_address(page);
> -
> - return set_memory_wc(addr, numpages);
> -}
> -
> -static inline int ttm_set_pages_uc(struct page *page, int numpages)
> -{
> - return set_pages_uc(page, numpages);
> -}
> -
> #else /* for CONFIG_X86 */
>
> static inline int ttm_set_pages_array_wb(struct page **pages, int addrinarray)
> @@ -91,16 +79,6 @@ static inline int ttm_set_pages_wb(struct page *page, int numpages)
> return 0;
> }
>
> -static inline int ttm_set_pages_wc(struct page *page, int numpages)
> -{
> - return 0;
> -}
> -
> -static inline int ttm_set_pages_uc(struct page *page, int numpages)
> -{
> - return 0;
> -}
> -
> #endif /* for CONFIG_X86 */
>
> #endif
> --
> 2.17.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
More information about the dri-devel
mailing list