[PATCH 1/5] drm/amdgpu: allow variable BO struct creation

[PATCH 1/5] drm/amdgpu: allow variable BO struct creation

[Nirmoy Das]

Allow allocating BO structures with different structure size
than struct amdgpu_bo.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 0bd22ed1dacf..745393472564 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -551,6 +551,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
 
 	acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
 				       sizeof(struct amdgpu_bo));
+	if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
+		bp->bo_ptr_size = sizeof(struct amdgpu_bo);
 
 	bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
 	if (bo == NULL)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 8cd96c9330dd..848dc0a017dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -40,6 +40,7 @@
 struct amdgpu_bo_param {
 	unsigned long			size;
 	int				byte_align;
+	u32				bo_ptr_size;
 	u32				domain;
 	u32				preferred_domain;
 	u64				flags;
-- 
2.30.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/5] drm/amdgpu: introduce struct amdgpu_bo_user

[Nirmoy Das]

Implement a new struct amdgpu_bo_user as subclass of
struct amdgpu_bo and a function to created amdgpu_bo_user
bo with a flag to identify the owner.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 28 ++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 12 ++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 745393472564..355d01ebce51 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -699,6 +699,34 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 	return r;
 }
 
+/**
+ * amdgpu_bo_create_user - create an &amdgpu_bo_user buffer object
+ * @adev: amdgpu device object
+ * @bp: parameters to be used for the buffer object
+ * @ubo_ptr: pointer to the buffer object pointer
+ *
+ * Create a BO to be used by user application;
+ *
+ * Returns:
+ * 0 for success or a negative error code on failure.
+ */
+
+int amdgpu_bo_create_user(struct amdgpu_device *adev,
+		     struct amdgpu_bo_param *bp,
+		     struct amdgpu_bo_user **ubo_ptr)
+{
+	struct amdgpu_bo *bo_ptr;
+	int r;
+
+	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
+	bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
+	r = amdgpu_bo_do_create(adev, bp, &bo_ptr);
+	if (r)
+		return r;
+
+	*ubo_ptr = (struct amdgpu_bo_user *)bo_ptr;
+	return r;
+}
 /**
  * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
  * @bo: pointer to the buffer object
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 848dc0a017dd..cbb881afe6da 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -114,6 +114,15 @@ struct amdgpu_bo {
 	struct kgd_mem                  *kfd_bo;
 };
 
+struct amdgpu_bo_user {
+	struct amdgpu_bo		bo;
+	u64				tiling_flags;
+	u64				metadata_flags;
+	void				*metadata;
+	u32				metadata_size;
+
+};
+
 static inline struct amdgpu_bo *ttm_to_amdgpu_bo(struct ttm_buffer_object *tbo)
 {
 	return container_of(tbo, struct amdgpu_bo, tbo);
@@ -257,6 +266,9 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
 int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
 			       uint64_t offset, uint64_t size, uint32_t domain,
 			       struct amdgpu_bo **bo_ptr, void **cpu_addr);
+int amdgpu_bo_create_user(struct amdgpu_device *adev,
+		     struct amdgpu_bo_param *bp,
+		     struct amdgpu_bo_user **ubo_ptr);
 void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr,
 			   void **cpu_addr);
 int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr);
-- 
2.30.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/5] drm/amdgpu: use amdgpu_bo_create_user() for gem object

[Nirmoy Das]

GEM objects encapsulate amdgpu_bo for userspace applications.
Now that we have a new amdgpu_bo_user subclass for that purpose,
let's use that instead.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 8e9b8a6e6ef0..9d2b55eb31c2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -54,6 +54,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
 			     struct drm_gem_object **obj)
 {
 	struct amdgpu_bo *bo;
+	struct amdgpu_bo_user *ubo;
 	struct amdgpu_bo_param bp;
 	int r;
 
@@ -68,7 +69,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
 retry:
 	bp.flags = flags;
 	bp.domain = initial_domain;
-	r = amdgpu_bo_create(adev, &bp, &bo);
+	r = amdgpu_bo_create_user(adev, &bp, &ubo);
 	if (r) {
 		if (r != -ERESTARTSYS) {
 			if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
@@ -85,6 +86,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
 		}
 		return r;
 	}
+	bo = &ubo->bo;
 	*obj = &bo->tbo.base;
 
 	return 0;
-- 
2.30.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/5] drm/amdgpu: use tiling_flags of struct amdgpu_bo_user

[Nirmoy Das]

This flag is only needed for BOs created by amdgpu_gem_object_create(),
so we can remove tiling_flags from the base class.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 19 +++++++++++++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 -
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 355d01ebce51..2e5cf46251af 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1174,12 +1174,19 @@ int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
 int amdgpu_bo_set_tiling_flags(struct amdgpu_bo *bo, u64 tiling_flags)
 {
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+	struct amdgpu_bo_user *ubo;
+
+	if (bo->tbo.type != ttm_bo_type_device) {
+		DRM_ERROR("can not set tiling_flags for a non-amdgpu_bo_user type BO\n");
+		return -EINVAL;
+	}
 
 	if (adev->family <= AMDGPU_FAMILY_CZ &&
 	    AMDGPU_TILING_GET(tiling_flags, TILE_SPLIT) > 6)
 		return -EINVAL;
 
-	bo->tiling_flags = tiling_flags;
+	ubo = container_of((bo), struct amdgpu_bo_user, bo);
+	ubo->tiling_flags = tiling_flags;
 	return 0;
 }
 
@@ -1193,10 +1200,18 @@ int amdgpu_bo_set_tiling_flags(struct amdgpu_bo *bo, u64 tiling_flags)
  */
 void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, u64 *tiling_flags)
 {
+	struct amdgpu_bo_user *ubo;
+
+	if (bo->tbo.type != ttm_bo_type_device) {
+		DRM_ERROR("can not get tiling_flags for a non-amdgpu_bo_user type BO\n");
+		return;
+	}
+
 	dma_resv_assert_held(bo->tbo.base.resv);
+	ubo = container_of((bo), struct amdgpu_bo_user, bo);
 
 	if (tiling_flags)
-		*tiling_flags = bo->tiling_flags;
+		*tiling_flags = ubo->tiling_flags;
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index cbb881afe6da..6cc38b71f7ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -91,7 +91,6 @@ struct amdgpu_bo {
 	struct ttm_bo_kmap_obj		kmap;
 	u64				flags;
 	unsigned			pin_count;
-	u64				tiling_flags;
 	u64				metadata_flags;
 	void				*metadata;
 	u32				metadata_size;
-- 
2.30.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 5/5] drm/amdgpu: use metadata members of struct amdgpu_bo_user

[Nirmoy Das]

These memebers are only needed for BOs created by amdgpu_gem_object_create(),
so we can remove these from the base class.

CC:felix.kuehling@amd.com
Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |  2 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 48 +++++++++++++++-------
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 --
 3 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index c5343a5eecbe..f8c8cbd8ab59 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -494,8 +494,6 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, int dma_buf_fd,
 		*dma_buf_kgd = (struct kgd_dev *)adev;
 	if (bo_size)
 		*bo_size = amdgpu_bo_size(bo);
-	if (metadata_size)
-		*metadata_size = bo->metadata_size;
 	if (metadata_buffer)
 		r = amdgpu_bo_get_metadata(bo, metadata_buffer, buffer_size,
 					   metadata_size, &metadata_flags);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 2e5cf46251af..81b2cff42481 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -77,6 +77,7 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
 {
 	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
 	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
+	struct amdgpu_bo_user *ubo;
 
 	if (bo->pin_count > 0)
 		amdgpu_bo_subtract_pin_size(bo);
@@ -94,7 +95,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
 	}
 	amdgpu_bo_unref(&bo->parent);
 
-	kfree(bo->metadata);
+	if (bo->tbo.type == ttm_bo_type_device) {
+		ubo = container_of((bo), struct amdgpu_bo_user, bo);
+		kfree(ubo->metadata);
+	}
+
 	kfree(bo);
 }
 
@@ -1230,13 +1235,20 @@ void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, u64 *tiling_flags)
 int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void *metadata,
 			    uint32_t metadata_size, uint64_t flags)
 {
+	struct amdgpu_bo_user *ubo;
 	void *buffer;
 
+	if (bo->tbo.type != ttm_bo_type_device) {
+		DRM_ERROR("can not set metadata for a non-amdgpu_bo_user type BO\n");
+		return -EINVAL;
+	}
+
+	ubo = container_of((bo), struct amdgpu_bo_user, bo);
 	if (!metadata_size) {
-		if (bo->metadata_size) {
-			kfree(bo->metadata);
-			bo->metadata = NULL;
-			bo->metadata_size = 0;
+		if (ubo->metadata_size) {
+			kfree(ubo->metadata);
+			ubo->metadata = NULL;
+			ubo->metadata_size = 0;
 		}
 		return 0;
 	}
@@ -1248,10 +1260,10 @@ int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void *metadata,
 	if (buffer == NULL)
 		return -ENOMEM;
 
-	kfree(bo->metadata);
-	bo->metadata_flags = flags;
-	bo->metadata = buffer;
-	bo->metadata_size = metadata_size;
+	kfree(ubo->metadata);
+	ubo->metadata_flags = flags;
+	ubo->metadata = buffer;
+	ubo->metadata_size = metadata_size;
 
 	return 0;
 }
@@ -1275,21 +1287,29 @@ int amdgpu_bo_get_metadata(struct amdgpu_bo *bo, void *buffer,
 			   size_t buffer_size, uint32_t *metadata_size,
 			   uint64_t *flags)
 {
+	struct amdgpu_bo_user *ubo;
+
 	if (!buffer && !metadata_size)
 		return -EINVAL;
 
+	if (bo->tbo.type != ttm_bo_type_device) {
+		DRM_ERROR("can not get metadata for a non-amdgpu_bo_user type BO\n");
+		return -EINVAL;
+	}
+
+	ubo = container_of((bo), struct amdgpu_bo_user, bo);
 	if (buffer) {
-		if (buffer_size < bo->metadata_size)
+		if (buffer_size < ubo->metadata_size)
 			return -EINVAL;
 
-		if (bo->metadata_size)
-			memcpy(buffer, bo->metadata, bo->metadata_size);
+		if (ubo->metadata_size)
+			memcpy(buffer, ubo->metadata, ubo->metadata_size);
 	}
 
 	if (metadata_size)
-		*metadata_size = bo->metadata_size;
+		*metadata_size = ubo->metadata_size;
 	if (flags)
-		*flags = bo->metadata_flags;
+		*flags = ubo->metadata_flags;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 6cc38b71f7ca..466081fa2083 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -91,9 +91,6 @@ struct amdgpu_bo {
 	struct ttm_bo_kmap_obj		kmap;
 	u64				flags;
 	unsigned			pin_count;
-	u64				metadata_flags;
-	void				*metadata;
-	u32				metadata_size;
 	unsigned			prime_shared_count;
 	/* per VM structure for page tables and with virtual addresses */
 	struct amdgpu_vm_bo_base	*vm_bo;
-- 
2.30.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/amdgpu: allow variable BO struct creation

[Christian König]

Am 05.03.21 um 15:35 schrieb Nirmoy Das:
> Allow allocating BO structures with different structure size
> than struct amdgpu_bo.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 +
>   2 files changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 0bd22ed1dacf..745393472564 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -551,6 +551,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
>   
>   	acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
>   				       sizeof(struct amdgpu_bo));
> +	if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
> +		bp->bo_ptr_size = sizeof(struct amdgpu_bo);

You should probably rather fix up all callers and make sure that the 
parameter structure is correctly filled in.

Christian.

>   
>   	bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
>   	if (bo == NULL)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 8cd96c9330dd..848dc0a017dd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -40,6 +40,7 @@
>   struct amdgpu_bo_param {
>   	unsigned long			size;
>   	int				byte_align;
> +	u32				bo_ptr_size;
>   	u32				domain;
>   	u32				preferred_domain;
>   	u64				flags;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/amdgpu: allow variable BO struct creation

[Nirmoy]

On 3/5/21 3:37 PM, Christian König wrote:
>
>
> Am 05.03.21 um 15:35 schrieb Nirmoy Das:
>> Allow allocating BO structures with different structure size
>> than struct amdgpu_bo.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 ++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 +
>>   2 files changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 0bd22ed1dacf..745393472564 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -551,6 +551,8 @@ static int amdgpu_bo_do_create(struct 
>> amdgpu_device *adev,
>>         acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
>>                          sizeof(struct amdgpu_bo));
>> +    if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
>> +        bp->bo_ptr_size = sizeof(struct amdgpu_bo);
>
> You should probably rather fix up all callers and make sure that the 
> parameter structure is correctly filled in.


Ok will resend with that change.


Thanks,

Nirmoy


>
> Christian.
>
>>         bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
>>       if (bo == NULL)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> index 8cd96c9330dd..848dc0a017dd 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> @@ -40,6 +40,7 @@
>>   struct amdgpu_bo_param {
>>       unsigned long            size;
>>       int                byte_align;
>> +    u32                bo_ptr_size;
>>       u32                domain;
>>       u32                preferred_domain;
>>       u64                flags;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: use amdgpu_bo_create_user() for gem object

[Christian König]

We might need to use this for the KFD as well.

Christian.

Am 05.03.21 um 15:35 schrieb Nirmoy Das:
> GEM objects encapsulate amdgpu_bo for userspace applications.
> Now that we have a new amdgpu_bo_user subclass for that purpose,
> let's use that instead.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 8e9b8a6e6ef0..9d2b55eb31c2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -54,6 +54,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
>   			     struct drm_gem_object **obj)
>   {
>   	struct amdgpu_bo *bo;
> +	struct amdgpu_bo_user *ubo;
>   	struct amdgpu_bo_param bp;
>   	int r;
>   
> @@ -68,7 +69,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
>   retry:
>   	bp.flags = flags;
>   	bp.domain = initial_domain;
> -	r = amdgpu_bo_create(adev, &bp, &bo);
> +	r = amdgpu_bo_create_user(adev, &bp, &ubo);
>   	if (r) {
>   		if (r != -ERESTARTSYS) {
>   			if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
> @@ -85,6 +86,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
>   		}
>   		return r;
>   	}
> +	bo = &ubo->bo;
>   	*obj = &bo->tbo.base;
>   
>   	return 0;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/5] drm/amdgpu: use tiling_flags of struct amdgpu_bo_user

[Christian König]

Am 05.03.21 um 15:35 schrieb Nirmoy Das:
> This flag is only needed for BOs created by amdgpu_gem_object_create(),
> so we can remove tiling_flags from the base class.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 19 +++++++++++++++++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 -
>   2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 355d01ebce51..2e5cf46251af 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1174,12 +1174,19 @@ int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
>   int amdgpu_bo_set_tiling_flags(struct amdgpu_bo *bo, u64 tiling_flags)
>   {
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> +	struct amdgpu_bo_user *ubo;
> +
> +	if (bo->tbo.type != ttm_bo_type_device) {
> +		DRM_ERROR("can not set tiling_flags for a non-amdgpu_bo_user type BO\n");
> +		return -EINVAL;
> +	}

I would just replace this with a WARN_ON or even BUG_ON since we should 
never ever make kernel BOs accessible to users space.

But what could be is that this is used with an imported SG table BO, so 
you need to make this check bo->tbo.type == ttm_bo_type_kernel.

Christian.

>   
>   	if (adev->family <= AMDGPU_FAMILY_CZ &&
>   	    AMDGPU_TILING_GET(tiling_flags, TILE_SPLIT) > 6)
>   		return -EINVAL;
>   
> -	bo->tiling_flags = tiling_flags;
> +	ubo = container_of((bo), struct amdgpu_bo_user, bo);
> +	ubo->tiling_flags = tiling_flags;
>   	return 0;
>   }
>   
> @@ -1193,10 +1200,18 @@ int amdgpu_bo_set_tiling_flags(struct amdgpu_bo *bo, u64 tiling_flags)
>    */
>   void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, u64 *tiling_flags)
>   {
> +	struct amdgpu_bo_user *ubo;
> +
> +	if (bo->tbo.type != ttm_bo_type_device) {
> +		DRM_ERROR("can not get tiling_flags for a non-amdgpu_bo_user type BO\n");
> +		return;
> +	}
> +
>   	dma_resv_assert_held(bo->tbo.base.resv);
> +	ubo = container_of((bo), struct amdgpu_bo_user, bo);
>   
>   	if (tiling_flags)
> -		*tiling_flags = bo->tiling_flags;
> +		*tiling_flags = ubo->tiling_flags;
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index cbb881afe6da..6cc38b71f7ca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -91,7 +91,6 @@ struct amdgpu_bo {
>   	struct ttm_bo_kmap_obj		kmap;
>   	u64				flags;
>   	unsigned			pin_count;
> -	u64				tiling_flags;
>   	u64				metadata_flags;
>   	void				*metadata;
>   	u32				metadata_size;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm/amdgpu: use metadata members of struct amdgpu_bo_user

[Christian König]

Am 05.03.21 um 15:35 schrieb Nirmoy Das:
> These memebers are only needed for BOs created by amdgpu_gem_object_create(),
> so we can remove these from the base class.

This patch must be squashed together with the previous one or we would 
have a broken driver in between.

Christian.

>
> CC:felix.kuehling@amd.com
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |  2 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 48 +++++++++++++++-------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  3 --
>   3 files changed, 34 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> index c5343a5eecbe..f8c8cbd8ab59 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> @@ -494,8 +494,6 @@ int amdgpu_amdkfd_get_dmabuf_info(struct kgd_dev *kgd, int dma_buf_fd,
>   		*dma_buf_kgd = (struct kgd_dev *)adev;
>   	if (bo_size)
>   		*bo_size = amdgpu_bo_size(bo);
> -	if (metadata_size)
> -		*metadata_size = bo->metadata_size;
>   	if (metadata_buffer)
>   		r = amdgpu_bo_get_metadata(bo, metadata_buffer, buffer_size,
>   					   metadata_size, &metadata_flags);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 2e5cf46251af..81b2cff42481 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -77,6 +77,7 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>   {
>   	struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev);
>   	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
> +	struct amdgpu_bo_user *ubo;
>   
>   	if (bo->pin_count > 0)
>   		amdgpu_bo_subtract_pin_size(bo);
> @@ -94,7 +95,11 @@ static void amdgpu_bo_destroy(struct ttm_buffer_object *tbo)
>   	}
>   	amdgpu_bo_unref(&bo->parent);
>   
> -	kfree(bo->metadata);
> +	if (bo->tbo.type == ttm_bo_type_device) {
> +		ubo = container_of((bo), struct amdgpu_bo_user, bo);
> +		kfree(ubo->metadata);
> +	}
> +
>   	kfree(bo);
>   }
>   
> @@ -1230,13 +1235,20 @@ void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, u64 *tiling_flags)
>   int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void *metadata,
>   			    uint32_t metadata_size, uint64_t flags)
>   {
> +	struct amdgpu_bo_user *ubo;
>   	void *buffer;
>   
> +	if (bo->tbo.type != ttm_bo_type_device) {
> +		DRM_ERROR("can not set metadata for a non-amdgpu_bo_user type BO\n");
> +		return -EINVAL;
> +	}
> +
> +	ubo = container_of((bo), struct amdgpu_bo_user, bo);
>   	if (!metadata_size) {
> -		if (bo->metadata_size) {
> -			kfree(bo->metadata);
> -			bo->metadata = NULL;
> -			bo->metadata_size = 0;
> +		if (ubo->metadata_size) {
> +			kfree(ubo->metadata);
> +			ubo->metadata = NULL;
> +			ubo->metadata_size = 0;
>   		}
>   		return 0;
>   	}
> @@ -1248,10 +1260,10 @@ int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void *metadata,
>   	if (buffer == NULL)
>   		return -ENOMEM;
>   
> -	kfree(bo->metadata);
> -	bo->metadata_flags = flags;
> -	bo->metadata = buffer;
> -	bo->metadata_size = metadata_size;
> +	kfree(ubo->metadata);
> +	ubo->metadata_flags = flags;
> +	ubo->metadata = buffer;
> +	ubo->metadata_size = metadata_size;
>   
>   	return 0;
>   }
> @@ -1275,21 +1287,29 @@ int amdgpu_bo_get_metadata(struct amdgpu_bo *bo, void *buffer,
>   			   size_t buffer_size, uint32_t *metadata_size,
>   			   uint64_t *flags)
>   {
> +	struct amdgpu_bo_user *ubo;
> +
>   	if (!buffer && !metadata_size)
>   		return -EINVAL;
>   
> +	if (bo->tbo.type != ttm_bo_type_device) {
> +		DRM_ERROR("can not get metadata for a non-amdgpu_bo_user type BO\n");
> +		return -EINVAL;
> +	}
> +
> +	ubo = container_of((bo), struct amdgpu_bo_user, bo);
>   	if (buffer) {
> -		if (buffer_size < bo->metadata_size)
> +		if (buffer_size < ubo->metadata_size)
>   			return -EINVAL;
>   
> -		if (bo->metadata_size)
> -			memcpy(buffer, bo->metadata, bo->metadata_size);
> +		if (ubo->metadata_size)
> +			memcpy(buffer, ubo->metadata, ubo->metadata_size);
>   	}
>   
>   	if (metadata_size)
> -		*metadata_size = bo->metadata_size;
> +		*metadata_size = ubo->metadata_size;
>   	if (flags)
> -		*flags = bo->metadata_flags;
> +		*flags = ubo->metadata_flags;
>   
>   	return 0;
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 6cc38b71f7ca..466081fa2083 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -91,9 +91,6 @@ struct amdgpu_bo {
>   	struct ttm_bo_kmap_obj		kmap;
>   	u64				flags;
>   	unsigned			pin_count;
> -	u64				metadata_flags;
> -	void				*metadata;
> -	u32				metadata_size;
>   	unsigned			prime_shared_count;
>   	/* per VM structure for page tables and with virtual addresses */
>   	struct amdgpu_vm_bo_base	*vm_bo;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/5] drm/amdgpu: introduce struct amdgpu_bo_user

[Christian König]

Am 05.03.21 um 15:35 schrieb Nirmoy Das:
> Implement a new struct amdgpu_bo_user as subclass of
> struct amdgpu_bo and a function to created amdgpu_bo_user
> bo with a flag to identify the owner.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 28 ++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 12 ++++++++++
>   2 files changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 745393472564..355d01ebce51 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -699,6 +699,34 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   	return r;
>   }
>   
> +/**
> + * amdgpu_bo_create_user - create an &amdgpu_bo_user buffer object
> + * @adev: amdgpu device object
> + * @bp: parameters to be used for the buffer object
> + * @ubo_ptr: pointer to the buffer object pointer
> + *
> + * Create a BO to be used by user application;
> + *
> + * Returns:
> + * 0 for success or a negative error code on failure.
> + */
> +
> +int amdgpu_bo_create_user(struct amdgpu_device *adev,
> +		     struct amdgpu_bo_param *bp,
> +		     struct amdgpu_bo_user **ubo_ptr)
> +{
> +	struct amdgpu_bo *bo_ptr;
> +	int r;
> +
> +	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
> +	bp->bo_ptr_size = sizeof(struct amdgpu_bo_user);
> +	r = amdgpu_bo_do_create(adev, bp, &bo_ptr);
> +	if (r)
> +		return r;
> +
> +	*ubo_ptr = (struct amdgpu_bo_user *)bo_ptr;

Maybe use container_of here instead. Even better would be a helper to 
upcast a amdgpu_bo pointer into a amdgpu_bo_user pointer.

Christian.

> +	return r;
> +}
>   /**
>    * amdgpu_bo_validate - validate an &amdgpu_bo buffer object
>    * @bo: pointer to the buffer object
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 848dc0a017dd..cbb881afe6da 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -114,6 +114,15 @@ struct amdgpu_bo {
>   	struct kgd_mem                  *kfd_bo;
>   };
>   
> +struct amdgpu_bo_user {
> +	struct amdgpu_bo		bo;
> +	u64				tiling_flags;
> +	u64				metadata_flags;
> +	void				*metadata;
> +	u32				metadata_size;
> +
> +};
> +
>   static inline struct amdgpu_bo *ttm_to_amdgpu_bo(struct ttm_buffer_object *tbo)
>   {
>   	return container_of(tbo, struct amdgpu_bo, tbo);
> @@ -257,6 +266,9 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev,
>   int amdgpu_bo_create_kernel_at(struct amdgpu_device *adev,
>   			       uint64_t offset, uint64_t size, uint32_t domain,
>   			       struct amdgpu_bo **bo_ptr, void **cpu_addr);
> +int amdgpu_bo_create_user(struct amdgpu_device *adev,
> +		     struct amdgpu_bo_param *bp,
> +		     struct amdgpu_bo_user **ubo_ptr);
>   void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr,
>   			   void **cpu_addr);
>   int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr);

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/amdgpu: allow variable BO struct creation

[Christian König]

Am 05.03.21 um 15:48 schrieb Nirmoy:
>
> On 3/5/21 3:37 PM, Christian König wrote:
>>
>>
>> Am 05.03.21 um 15:35 schrieb Nirmoy Das:
>>> Allow allocating BO structures with different structure size
>>> than struct amdgpu_bo.
>>>
>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 ++
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 +
>>>   2 files changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> index 0bd22ed1dacf..745393472564 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> @@ -551,6 +551,8 @@ static int amdgpu_bo_do_create(struct 
>>> amdgpu_device *adev,
>>>         acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
>>>                          sizeof(struct amdgpu_bo));
>>> +    if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
>>> +        bp->bo_ptr_size = sizeof(struct amdgpu_bo);
>>
>> You should probably rather fix up all callers and make sure that the 
>> parameter structure is correctly filled in.
>
>
> Ok will resend with that change.

You are to fast, let me at least comment on all patches :)

Christian.

>
>
> Thanks,
>
> Nirmoy
>
>
>>
>> Christian.
>>
>>>         bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
>>>       if (bo == NULL)
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> index 8cd96c9330dd..848dc0a017dd 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> @@ -40,6 +40,7 @@
>>>   struct amdgpu_bo_param {
>>>       unsigned long            size;
>>>       int                byte_align;
>>> +    u32                bo_ptr_size;
>>>       u32                domain;
>>>       u32                preferred_domain;
>>>       u64                flags;
>>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/amdgpu: allow variable BO struct creation

[Nirmoy]

On 3/5/21 4:18 PM, Christian König wrote:
>
>
> Am 05.03.21 um 15:48 schrieb Nirmoy:
>>
>> On 3/5/21 3:37 PM, Christian König wrote:
>>>
>>>
>>> Am 05.03.21 um 15:35 schrieb Nirmoy Das:
>>>> Allow allocating BO structures with different structure size
>>>> than struct amdgpu_bo.
>>>>
>>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 ++
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 +
>>>>   2 files changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> index 0bd22ed1dacf..745393472564 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>>> @@ -551,6 +551,8 @@ static int amdgpu_bo_do_create(struct 
>>>> amdgpu_device *adev,
>>>>         acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
>>>>                          sizeof(struct amdgpu_bo));
>>>> +    if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
>>>> +        bp->bo_ptr_size = sizeof(struct amdgpu_bo);
>>>
>>> You should probably rather fix up all callers and make sure that the 
>>> parameter structure is correctly filled in.
>>
>>
>> Ok will resend with that change.
>
> You are to fast, let me at least comment on all patches :)


Sorry about that :)


Nirmoy

>
> Christian.
>
>>
>>
>> Thanks,
>>
>> Nirmoy
>>
>>
>>>
>>> Christian.
>>>
>>>>         bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
>>>>       if (bo == NULL)
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>> index 8cd96c9330dd..848dc0a017dd 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>>> @@ -40,6 +40,7 @@
>>>>   struct amdgpu_bo_param {
>>>>       unsigned long            size;
>>>>       int                byte_align;
>>>> +    u32                bo_ptr_size;
>>>>       u32                domain;
>>>>       u32                preferred_domain;
>>>>       u64                flags;
>>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: use amdgpu_bo_create_user() for gem object

[Nirmoy]

On 3/5/21 4:11 PM, Christian König wrote:
> We might need to use this for the KFD as well.

Do you mean for amdgpu_amdkfd_alloc_gws() ?


Regards,

Nirmoy



>
> Christian.
>
> Am 05.03.21 um 15:35 schrieb Nirmoy Das:
>> GEM objects encapsulate amdgpu_bo for userspace applications.
>> Now that we have a new amdgpu_bo_user subclass for that purpose,
>> let's use that instead.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index 8e9b8a6e6ef0..9d2b55eb31c2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -54,6 +54,7 @@ int amdgpu_gem_object_create(struct amdgpu_device 
>> *adev, unsigned long size,
>>                    struct drm_gem_object **obj)
>>   {
>>       struct amdgpu_bo *bo;
>> +    struct amdgpu_bo_user *ubo;
>>       struct amdgpu_bo_param bp;
>>       int r;
>>   @@ -68,7 +69,7 @@ int amdgpu_gem_object_create(struct amdgpu_device 
>> *adev, unsigned long size,
>>   retry:
>>       bp.flags = flags;
>>       bp.domain = initial_domain;
>> -    r = amdgpu_bo_create(adev, &bp, &bo);
>> +    r = amdgpu_bo_create_user(adev, &bp, &ubo);
>>       if (r) {
>>           if (r != -ERESTARTSYS) {
>>               if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
>> @@ -85,6 +86,7 @@ int amdgpu_gem_object_create(struct amdgpu_device 
>> *adev, unsigned long size,
>>           }
>>           return r;
>>       }
>> +    bo = &ubo->bo;
>>       *obj = &bo->tbo.base;
>>         return 0;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: use amdgpu_bo_create_user() for gem object

[Christian König]

Am 08.03.21 um 14:56 schrieb Nirmoy:
>
> On 3/5/21 4:11 PM, Christian König wrote:
>> We might need to use this for the KFD as well.
>
> Do you mean for amdgpu_amdkfd_alloc_gws() ?

For example, yes. Basically all places where KFD allocated an BO with 
the TTM type device/user.

Regards,
Christian.

>
>
> Regards,
>
> Nirmoy
>
>
>
>>
>> Christian.
>>
>> Am 05.03.21 um 15:35 schrieb Nirmoy Das:
>>> GEM objects encapsulate amdgpu_bo for userspace applications.
>>> Now that we have a new amdgpu_bo_user subclass for that purpose,
>>> let's use that instead.
>>>
>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> index 8e9b8a6e6ef0..9d2b55eb31c2 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> @@ -54,6 +54,7 @@ int amdgpu_gem_object_create(struct amdgpu_device 
>>> *adev, unsigned long size,
>>>                    struct drm_gem_object **obj)
>>>   {
>>>       struct amdgpu_bo *bo;
>>> +    struct amdgpu_bo_user *ubo;
>>>       struct amdgpu_bo_param bp;
>>>       int r;
>>>   @@ -68,7 +69,7 @@ int amdgpu_gem_object_create(struct 
>>> amdgpu_device *adev, unsigned long size,
>>>   retry:
>>>       bp.flags = flags;
>>>       bp.domain = initial_domain;
>>> -    r = amdgpu_bo_create(adev, &bp, &bo);
>>> +    r = amdgpu_bo_create_user(adev, &bp, &ubo);
>>>       if (r) {
>>>           if (r != -ERESTARTSYS) {
>>>               if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
>>> @@ -85,6 +86,7 @@ int amdgpu_gem_object_create(struct amdgpu_device 
>>> *adev, unsigned long size,
>>>           }
>>>           return r;
>>>       }
>>> +    bo = &ubo->bo;
>>>       *obj = &bo->tbo.base;
>>>         return 0;
>>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdgpu: use amdgpu_bo_create_user() for gem object

[Nirmoy]

On 3/8/21 2:58 PM, Christian König wrote:
>
>
> Am 08.03.21 um 14:56 schrieb Nirmoy:
>>
>> On 3/5/21 4:11 PM, Christian König wrote:
>>> We might need to use this for the KFD as well.
>>
>> Do you mean for amdgpu_amdkfd_alloc_gws() ?
>
> For example, yes. Basically all places where KFD allocated an BO with 
> the TTM type device/user.


Thanks Christian!


>
> Regards,
> Christian.
>
>>
>>
>> Regards,
>>
>> Nirmoy
>>
>>
>>
>>>
>>> Christian.
>>>
>>> Am 05.03.21 um 15:35 schrieb Nirmoy Das:
>>>> GEM objects encapsulate amdgpu_bo for userspace applications.
>>>> Now that we have a new amdgpu_bo_user subclass for that purpose,
>>>> let's use that instead.
>>>>
>>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 4 +++-
>>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>> index 8e9b8a6e6ef0..9d2b55eb31c2 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>>> @@ -54,6 +54,7 @@ int amdgpu_gem_object_create(struct amdgpu_device 
>>>> *adev, unsigned long size,
>>>>                    struct drm_gem_object **obj)
>>>>   {
>>>>       struct amdgpu_bo *bo;
>>>> +    struct amdgpu_bo_user *ubo;
>>>>       struct amdgpu_bo_param bp;
>>>>       int r;
>>>>   @@ -68,7 +69,7 @@ int amdgpu_gem_object_create(struct 
>>>> amdgpu_device *adev, unsigned long size,
>>>>   retry:
>>>>       bp.flags = flags;
>>>>       bp.domain = initial_domain;
>>>> -    r = amdgpu_bo_create(adev, &bp, &bo);
>>>> +    r = amdgpu_bo_create_user(adev, &bp, &ubo);
>>>>       if (r) {
>>>>           if (r != -ERESTARTSYS) {
>>>>               if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
>>>> @@ -85,6 +86,7 @@ int amdgpu_gem_object_create(struct amdgpu_device 
>>>> *adev, unsigned long size,
>>>>           }
>>>>           return r;
>>>>       }
>>>> +    bo = &ubo->bo;
>>>>       *obj = &bo->tbo.base;
>>>>         return 0;
>>>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/amdgpu: allow variable BO struct creation

[Christian König]

Am 08.03.21 um 16:37 schrieb Nirmoy Das:
> Allow allocating BO structures with different structure size
> than struct amdgpu_bo.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 +++++++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 +
>   2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index ac1bb5089260..d32379cbad89 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -543,9 +543,10 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
>   	if (!amdgpu_bo_validate_size(adev, size, bp->domain))
>   		return -ENOMEM;
>   
> -	*bo_ptr = NULL;
> +	BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo));
>   
> -	bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
> +	*bo_ptr = NULL;
> +	bo = kzalloc(bp->bo_ptr_size, GFP_KERNEL);
>   	if (bo == NULL)
>   		return -ENOMEM;
>   	drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
> @@ -635,6 +636,7 @@ static int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
>   		AMDGPU_GEM_CREATE_SHADOW;
>   	bp.type = ttm_bo_type_kernel;
>   	bp.resv = bo->tbo.base.resv;
> +	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
>   
>   	r = amdgpu_bo_do_create(adev, &bp, &bo->shadow);
>   	if (!r) {
> @@ -669,6 +671,9 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   	int r;
>   
>   	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
> +	if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
> +		bp->bo_ptr_size = sizeof(struct amdgpu_bo);
> +

It's not strictly necessary, but I would prefer if you change all 
callers of amdgpu_bo_create() to correctly do this instead of the check 
here.

Christian.

>   	r = amdgpu_bo_do_create(adev, bp, bo_ptr);
>   	if (r)
>   		return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 54ceb065e546..8e2b556f0b7b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -40,6 +40,7 @@
>   struct amdgpu_bo_param {
>   	unsigned long			size;
>   	int				byte_align;
> +	u32				bo_ptr_size;
>   	u32				domain;
>   	u32				preferred_domain;
>   	u64				flags;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/5] drm/amdgpu: allow variable BO struct creation

[Nirmoy Das]

Allow allocating BO structures with different structure size
than struct amdgpu_bo.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 9 +++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index ac1bb5089260..d32379cbad89 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -543,9 +543,10 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
 	if (!amdgpu_bo_validate_size(adev, size, bp->domain))
 		return -ENOMEM;
 
-	*bo_ptr = NULL;
+	BUG_ON(bp->bo_ptr_size < sizeof(struct amdgpu_bo));
 
-	bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
+	*bo_ptr = NULL;
+	bo = kzalloc(bp->bo_ptr_size, GFP_KERNEL);
 	if (bo == NULL)
 		return -ENOMEM;
 	drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
@@ -635,6 +636,7 @@ static int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
 		AMDGPU_GEM_CREATE_SHADOW;
 	bp.type = ttm_bo_type_kernel;
 	bp.resv = bo->tbo.base.resv;
+	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
 
 	r = amdgpu_bo_do_create(adev, &bp, &bo->shadow);
 	if (!r) {
@@ -669,6 +671,9 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 	int r;
 
 	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
+	if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
+		bp->bo_ptr_size = sizeof(struct amdgpu_bo);
+
 	r = amdgpu_bo_do_create(adev, bp, bo_ptr);
 	if (r)
 		return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 54ceb065e546..8e2b556f0b7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -40,6 +40,7 @@
 struct amdgpu_bo_param {
 	unsigned long			size;
 	int				byte_align;
+	u32				bo_ptr_size;
 	u32				domain;
 	u32				preferred_domain;
 	u64				flags;
-- 
2.30.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/amdgpu: allow variable BO struct creation

[Christian König]

Am 05.03.21 um 16:06 schrieb Nirmoy Das:
> Allow allocating BO structures with different structure size
> than struct amdgpu_bo.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 10 +++++++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
>   2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 0bd22ed1dacf..1ff8c3570c82 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -551,8 +551,12 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
>   
>   	acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
>   				       sizeof(struct amdgpu_bo));
> +	if (bp->bo_ptr_size < sizeof(struct amdgpu_bo)) {
> +		DRM_ERROR("can't create BO with size < struct amdgpu_bo\n");
> +		return -EINVAL;
> +	}

Please make this a BUG_ON instead. When this is wrong we corrupt kernel 
memory.

Christian.

>   
> -	bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
> +	bo = kzalloc(bp->bo_ptr_size, GFP_KERNEL);
>   	if (bo == NULL)
>   		return -ENOMEM;
>   	drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
> @@ -642,6 +646,7 @@ static int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
>   		AMDGPU_GEM_CREATE_SHADOW;
>   	bp.type = ttm_bo_type_kernel;
>   	bp.resv = bo->tbo.base.resv;
> +	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
>   
>   	r = amdgpu_bo_do_create(adev, &bp, &bo->shadow);
>   	if (!r) {
> @@ -676,6 +681,9 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   	int r;
>   
>   	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
> +	if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
> +		bp->bo_ptr_size = sizeof(struct amdgpu_bo);
> +
>   	r = amdgpu_bo_do_create(adev, bp, bo_ptr);
>   	if (r)
>   		return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 8cd96c9330dd..848dc0a017dd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -40,6 +40,7 @@
>   struct amdgpu_bo_param {
>   	unsigned long			size;
>   	int				byte_align;
> +	u32				bo_ptr_size;
>   	u32				domain;
>   	u32				preferred_domain;
>   	u64				flags;

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/5] drm/amdgpu: allow variable BO struct creation

[Nirmoy Das]

Allow allocating BO structures with different structure size
than struct amdgpu_bo.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 10 +++++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 0bd22ed1dacf..1ff8c3570c82 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -551,8 +551,12 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
 
 	acc_size = ttm_bo_dma_acc_size(&adev->mman.bdev, size,
 				       sizeof(struct amdgpu_bo));
+	if (bp->bo_ptr_size < sizeof(struct amdgpu_bo)) {
+		DRM_ERROR("can't create BO with size < struct amdgpu_bo\n");
+		return -EINVAL;
+	}
 
-	bo = kzalloc(sizeof(struct amdgpu_bo), GFP_KERNEL);
+	bo = kzalloc(bp->bo_ptr_size, GFP_KERNEL);
 	if (bo == NULL)
 		return -ENOMEM;
 	drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size);
@@ -642,6 +646,7 @@ static int amdgpu_bo_create_shadow(struct amdgpu_device *adev,
 		AMDGPU_GEM_CREATE_SHADOW;
 	bp.type = ttm_bo_type_kernel;
 	bp.resv = bo->tbo.base.resv;
+	bp.bo_ptr_size = sizeof(struct amdgpu_bo);
 
 	r = amdgpu_bo_do_create(adev, &bp, &bo->shadow);
 	if (!r) {
@@ -676,6 +681,9 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
 	int r;
 
 	bp->flags = bp->flags & ~AMDGPU_GEM_CREATE_SHADOW;
+	if (bp->bo_ptr_size < sizeof(struct amdgpu_bo))
+		bp->bo_ptr_size = sizeof(struct amdgpu_bo);
+
 	r = amdgpu_bo_do_create(adev, bp, bo_ptr);
 	if (r)
 		return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 8cd96c9330dd..848dc0a017dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -40,6 +40,7 @@
 struct amdgpu_bo_param {
 	unsigned long			size;
 	int				byte_align;
+	u32				bo_ptr_size;
 	u32				domain;
 	u32				preferred_domain;
 	u64				flags;
-- 
2.30.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx