Re: [PATCH 2/2] drm/amdgpu: handle domain mask checking

Christian König
2018-04-17T12:07:54Z

Am 17.04.2018 um 12:38 schrieb Chunming Zhou:
> if domain is illegal, we should return error.

This check is already in amdgpu_gem_create_ioctl():
>         if (args->in.domains & ~(AMDGPU_GEM_DOMAIN_CPU |
>                                  AMDGPU_GEM_DOMAIN_GTT |
>                                  AMDGPU_GEM_DOMAIN_VRAM |
>                                  AMDGPU_GEM_DOMAIN_GDS |
>                                  AMDGPU_GEM_DOMAIN_GWS |
>                                  AMDGPU_GEM_DOMAIN_OA))
>                 return -EINVAL;

We should just check it at one place I think.

Either remove it in amdgpu_bo_do_create() or in amdgpu_gem_create_ioctl().

Regards,
Christian.

>
> Change-Id: I65a738f5ac4fc34be76de867afb0db1d4bd27c24
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 15 +++++++--------
> include/uapi/drm/amdgpu_drm.h | 6 ++++++
> 2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 9258f0694922..ddf1b3580d37 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -378,17 +378,16 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
> bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
> if (bo == NULL)
> return -ENOMEM;
> + preferred_domains = bp->preferred_domain ? bp->preferred_domain :
> + bp->domain;
> + if (preferred_domains & ~AMDGPU_GEM_DOMAIN_MASK) {
> + kfree(bo);
> + return -EINVAL;
> + }
> drm_gem_private_object_init(adev->ddev, &bo->gem_base, size);
> INIT_LIST_HEAD(&bo->shadow_list);
> INIT_LIST_HEAD(&bo->va);
> - preferred_domains = bp->preferred_domain ? bp->preferred_domain :
> - bp->domain;
> - bo->preferred_domains = preferred_domains & (AMDGPU_GEM_DOMAIN_VRAM |
> - AMDGPU_GEM_DOMAIN_GTT |
> - AMDGPU_GEM_DOMAIN_CPU |
> - AMDGPU_GEM_DOMAIN_GDS |
> - AMDGPU_GEM_DOMAIN_GWS |
> - AMDGPU_GEM_DOMAIN_OA);
> + bo->preferred_domains = preferred_domains;
> bo->allowed_domains = bo->preferred_domains;
> if (bp->type != ttm_bo_type_kernel &&
> bo->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index 0087799962cf..d023d3c4ee51 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -81,6 +81,12 @@ extern "C" {
> #define AMDGPU_GEM_DOMAIN_GDS 0x8
> #define AMDGPU_GEM_DOMAIN_GWS 0x10
> #define AMDGPU_GEM_DOMAIN_OA 0x20
> +#define AMDGPU_GEM_DOMAIN_MASK (AMDGPU_GEM_DOMAIN_CPU | \
> + AMDGPU_GEM_DOMAIN_GTT | \
> + AMDGPU_GEM_DOMAIN_VRAM | \
> + AMDGPU_GEM_DOMAIN_GDS | \
> + AMDGPU_GEM_DOMAIN_GWS | \
> + AMDGPU_GEM_DOMAIN_OA)
>
> /* Flag that CPU access will be required for the case of VRAM domain */
> #define AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED (1 << 0)