[PATCH 14/14] drm/amdgpu: set TMZ bits in PTEs for secure bo (v2)

[Koenig, Christian]

Am 11.09.19 um 13:50 schrieb Huang, Ray:
> From: Alex Deucher <alexander.deucher at amd.com>
>
> If one bo is secure (created with AMDGPU_GEM_CREATE_ENCRYPTED), the TMZ bits of
> PTEs that belongs that bo should be set. Then psp is able to protect the pages
> of this bo to avoid the access from an "untrust" domain such as CPU.
>
> v1: design and draft the skeletion of tmz bits setting on PTEs (Alex)
> v2: return failure once create secure bo on no-tmz platform  (Ray)
>
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> Reviewed-by: Huang Rui <ray.huang at amd.com>
> Signed-off-by: Huang Rui <ray.huang at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 12 +++++++++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 10 ++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  5 +++++
>   3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 22eab74..5332104 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -222,7 +222,8 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>   		      AMDGPU_GEM_CREATE_CPU_GTT_USWC |
>   		      AMDGPU_GEM_CREATE_VRAM_CLEARED |
>   		      AMDGPU_GEM_CREATE_VM_ALWAYS_VALID |
> -		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC))
> +		      AMDGPU_GEM_CREATE_EXPLICIT_SYNC |
> +		      AMDGPU_GEM_CREATE_ENCRYPTED))
>   
>   		return -EINVAL;
>   
> @@ -230,6 +231,11 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>   	if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
>   		return -EINVAL;
>   
> +	if (!adev->tmz.enabled && (flags & AMDGPU_GEM_CREATE_ENCRYPTED)) {
> +		DRM_ERROR("Cannot allocate secure buffer while tmz is disabled\n");
> +		return -EINVAL;
> +	}
> +
>   	/* create a gem object to contain this object in */
>   	if (args->in.domains & (AMDGPU_GEM_DOMAIN_GDS |
>   	    AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA)) {
> @@ -251,6 +257,10 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>   		resv = vm->root.base.bo->tbo.resv;
>   	}
>   
> +	if (flags & AMDGPU_GEM_CREATE_ENCRYPTED) {
> +		/* XXX: pad out alignment to meet TMZ requirements */
> +	}
> +
>   	r = amdgpu_gem_object_create(adev, size, args->in.alignment,
>   				     (u32)(0xffffffff & args->in.domains),
>   				     flags, ttm_bo_type_device, resv, &gobj);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 5a3c177..286e2e2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -224,6 +224,16 @@ static inline bool amdgpu_bo_explicit_sync(struct amdgpu_bo *bo)
>   	return bo->flags & AMDGPU_GEM_CREATE_EXPLICIT_SYNC;
>   }
>   
> +/**
> + * amdgpu_bo_encrypted - return whether the bo is encrypted
> + */
> +static inline bool amdgpu_bo_encrypted(struct amdgpu_bo *bo)
> +{
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> +
> +	return adev->tmz.enabled && (bo->flags & AMDGPU_GEM_CREATE_ENCRYPTED);

Checking the adev->tmz.enabled flags should be dropped here.

> +}
> +
>   bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
>   void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 3663655..8f00bb2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1434,6 +1434,8 @@ bool amdgpu_ttm_tt_is_readonly(struct ttm_tt *ttm)
>   uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_mem_reg *mem)
>   {
>   	uint64_t flags = 0;
> +	struct ttm_buffer_object *tbo = ttm_mem_reg_to_bo(mem);
> +	struct amdgpu_bo *abo = ttm_to_amdgpu_bo(tbo);

That's a clear NAK. The function is not necessarily called with 
&bo->mem, which is also the reason why this function doesn't gets the BO 
as parameter.

Christian.

>   
>   	if (mem && mem->mem_type != TTM_PL_SYSTEM)
>   		flags |= AMDGPU_PTE_VALID;
> @@ -1444,6 +1446,9 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_mem_reg *mem)
>   		if (ttm->caching_state == tt_cached)
>   			flags |= AMDGPU_PTE_SNOOPED;
>   	}
> +	if (amdgpu_bo_encrypted(abo)) {
> +		flags |= AMDGPU_PTE_TMZ;
> +	}
>   
>   	return flags;
>   }