[PATCH] amdgpu: Use unsigned HW IP/instance/ring indices in amdgpu_queue_mgr_map

[PATCH] amdgpu: Use unsigned HW IP/instance/ring indices in amdgpu_queue_mgr_map

[Michel Dänzer]

From: Michel Dänzer <michel.daenzer@amd.com>

This matches the corresponding UAPI fields. Treating the ring index as
signed could result in accessing random unrelated memory if the MSB was
set.

Fixes: effd924d2f3b ("drm/amdgpu: untie user ring ids from kernel ring
                      ids v6")
Cc: stable@vger.kernel.org
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 86f91789de6d..f8657c37ba9d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -722,7 +722,7 @@ int amdgpu_queue_mgr_fini(struct amdgpu_device *adev,
 			  struct amdgpu_queue_mgr *mgr);
 int amdgpu_queue_mgr_map(struct amdgpu_device *adev,
 			 struct amdgpu_queue_mgr *mgr,
-			 int hw_ip, int instance, int ring,
+			 u32 hw_ip, u32 instance, u32 ring,
 			 struct amdgpu_ring **out_ring);
 
 /*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c
index 190e28cb827e..93d86619e802 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c
@@ -63,7 +63,7 @@ static int amdgpu_update_cached_map(struct amdgpu_queue_mapper *mapper,
 
 static int amdgpu_identity_map(struct amdgpu_device *adev,
 			       struct amdgpu_queue_mapper *mapper,
-			       int ring,
+			       u32 ring,
 			       struct amdgpu_ring **out_ring)
 {
 	switch (mapper->hw_ip) {
@@ -121,7 +121,7 @@ static enum amdgpu_ring_type amdgpu_hw_ip_to_ring_type(int hw_ip)
 
 static int amdgpu_lru_map(struct amdgpu_device *adev,
 			  struct amdgpu_queue_mapper *mapper,
-			  int user_ring, bool lru_pipe_order,
+			  u32 user_ring, bool lru_pipe_order,
 			  struct amdgpu_ring **out_ring)
 {
 	int r, i, j;
@@ -208,7 +208,7 @@ int amdgpu_queue_mgr_fini(struct amdgpu_device *adev,
  */
 int amdgpu_queue_mgr_map(struct amdgpu_device *adev,
 			 struct amdgpu_queue_mgr *mgr,
-			 int hw_ip, int instance, int ring,
+			 u32 hw_ip, u32 instance, u32 ring,
 			 struct amdgpu_ring **out_ring)
 {
 	int r, ip_num_rings;
-- 
2.15.0

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

RE: [PATCH] amdgpu: Use unsigned HW IP/instance/ring indices in amdgpu_queue_mgr_map

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Michel Dänzer
> Sent: Tuesday, November 21, 2017 12:30 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Andres Rodriguez
> Subject: [PATCH] amdgpu: Use unsigned HW IP/instance/ring indices in
> amdgpu_queue_mgr_map
> 
> From: Michel Dänzer <michel.daenzer@amd.com>
> 
> This matches the corresponding UAPI fields. Treating the ring index as
> signed could result in accessing random unrelated memory if the MSB was
> set.
> 
> Fixes: effd924d2f3b ("drm/amdgpu: untie user ring ids from kernel ring
>                       ids v6")
> Cc: stable@vger.kernel.org
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h           | 2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 86f91789de6d..f8657c37ba9d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -722,7 +722,7 @@ int amdgpu_queue_mgr_fini(struct amdgpu_device
> *adev,
>  			  struct amdgpu_queue_mgr *mgr);
>  int amdgpu_queue_mgr_map(struct amdgpu_device *adev,
>  			 struct amdgpu_queue_mgr *mgr,
> -			 int hw_ip, int instance, int ring,
> +			 u32 hw_ip, u32 instance, u32 ring,
>  			 struct amdgpu_ring **out_ring);
> 
>  /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c
> index 190e28cb827e..93d86619e802 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c
> @@ -63,7 +63,7 @@ static int amdgpu_update_cached_map(struct
> amdgpu_queue_mapper *mapper,
> 
>  static int amdgpu_identity_map(struct amdgpu_device *adev,
>  			       struct amdgpu_queue_mapper *mapper,
> -			       int ring,
> +			       u32 ring,
>  			       struct amdgpu_ring **out_ring)
>  {
>  	switch (mapper->hw_ip) {
> @@ -121,7 +121,7 @@ static enum amdgpu_ring_type
> amdgpu_hw_ip_to_ring_type(int hw_ip)
> 
>  static int amdgpu_lru_map(struct amdgpu_device *adev,
>  			  struct amdgpu_queue_mapper *mapper,
> -			  int user_ring, bool lru_pipe_order,
> +			  u32 user_ring, bool lru_pipe_order,
>  			  struct amdgpu_ring **out_ring)
>  {
>  	int r, i, j;
> @@ -208,7 +208,7 @@ int amdgpu_queue_mgr_fini(struct amdgpu_device
> *adev,
>   */
>  int amdgpu_queue_mgr_map(struct amdgpu_device *adev,
>  			 struct amdgpu_queue_mgr *mgr,
> -			 int hw_ip, int instance, int ring,
> +			 u32 hw_ip, u32 instance, u32 ring,
>  			 struct amdgpu_ring **out_ring)
>  {
>  	int r, ip_num_rings;
> --
> 2.15.0
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] amdgpu: Use unsigned HW IP/instance/ring indices in amdgpu_queue_mgr_map

[Christian König]

Am 21.11.2017 um 18:29 schrieb Michel Dänzer:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> This matches the corresponding UAPI fields. Treating the ring index as
> signed could result in accessing random unrelated memory if the MSB was
> set.
>
> Fixes: effd924d2f3b ("drm/amdgpu: untie user ring ids from kernel ring
>                        ids v6")
> Cc: stable@vger.kernel.org
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

Subject line is off, with that fixed Reviewed-by: Christian König 
<christian.koenig@amd.com>.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           | 2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c | 6 +++---
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 86f91789de6d..f8657c37ba9d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -722,7 +722,7 @@ int amdgpu_queue_mgr_fini(struct amdgpu_device *adev,
>   			  struct amdgpu_queue_mgr *mgr);
>   int amdgpu_queue_mgr_map(struct amdgpu_device *adev,
>   			 struct amdgpu_queue_mgr *mgr,
> -			 int hw_ip, int instance, int ring,
> +			 u32 hw_ip, u32 instance, u32 ring,
>   			 struct amdgpu_ring **out_ring);
>   
>   /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c
> index 190e28cb827e..93d86619e802 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c
> @@ -63,7 +63,7 @@ static int amdgpu_update_cached_map(struct amdgpu_queue_mapper *mapper,
>   
>   static int amdgpu_identity_map(struct amdgpu_device *adev,
>   			       struct amdgpu_queue_mapper *mapper,
> -			       int ring,
> +			       u32 ring,
>   			       struct amdgpu_ring **out_ring)
>   {
>   	switch (mapper->hw_ip) {
> @@ -121,7 +121,7 @@ static enum amdgpu_ring_type amdgpu_hw_ip_to_ring_type(int hw_ip)
>   
>   static int amdgpu_lru_map(struct amdgpu_device *adev,
>   			  struct amdgpu_queue_mapper *mapper,
> -			  int user_ring, bool lru_pipe_order,
> +			  u32 user_ring, bool lru_pipe_order,
>   			  struct amdgpu_ring **out_ring)
>   {
>   	int r, i, j;
> @@ -208,7 +208,7 @@ int amdgpu_queue_mgr_fini(struct amdgpu_device *adev,
>    */
>   int amdgpu_queue_mgr_map(struct amdgpu_device *adev,
>   			 struct amdgpu_queue_mgr *mgr,
> -			 int hw_ip, int instance, int ring,
> +			 u32 hw_ip, u32 instance, u32 ring,
>   			 struct amdgpu_ring **out_ring)
>   {
>   	int r, ip_num_rings;


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

Re: [PATCH] amdgpu: Use unsigned HW IP/instance/ring indices in amdgpu_queue_mgr_map

[Michel Dänzer]

On 2017-11-21 07:38 PM, Christian König wrote:
> Am 21.11.2017 um 18:29 schrieb Michel Dänzer:
>> From: Michel Dänzer <michel.daenzer@amd.com>
>>
>> This matches the corresponding UAPI fields. Treating the ring index as
>> signed could result in accessing random unrelated memory if the MSB was
>> set.
>>
>> Fixes: effd924d2f3b ("drm/amdgpu: untie user ring ids from kernel ring
>>                        ids v6")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> 
> Subject line is off, [...]

I assume you're referring to the prefix. Well, I'm actually trying to
set a trend there. :) Since there's no other amdgpu driver, the drm/ is
redundant. In the specific case of this patch, adding it would make the
shortlog too long, and IME it's quite often the case that those four
character cells could be put to better use than for the redundant prefix.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] amdgpu: Use unsigned HW IP/instance/ring indices in amdgpu_queue_mgr_map

[Christian König]

Am 22.11.2017 um 09:42 schrieb Michel Dänzer:
> On 2017-11-21 07:38 PM, Christian König wrote:
>> Am 21.11.2017 um 18:29 schrieb Michel Dänzer:
>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>
>>> This matches the corresponding UAPI fields. Treating the ring index as
>>> signed could result in accessing random unrelated memory if the MSB was
>>> set.
>>>
>>> Fixes: effd924d2f3b ("drm/amdgpu: untie user ring ids from kernel ring
>>>                         ids v6")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>> Subject line is off, [...]
> I assume you're referring to the prefix. Well, I'm actually trying to
> set a trend there. :) Since there's no other amdgpu driver, the drm/ is
> redundant. In the specific case of this patch, adding it would make the
> shortlog too long, and IME it's quite often the case that those four
> character cells could be put to better use than for the redundant prefix.

Yeah, making the prefix shorter is one thing I have thought about as well.

But currently we use "drm/amdgpu:" for the kernel and "amdgpu:" for 
libdrm and I actually would like to keep that.

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

Re: [PATCH] amdgpu: Use unsigned HW IP/instance/ring indices in amdgpu_queue_mgr_map

[Michel Dänzer]

On 2017-11-22 10:22 AM, Christian König wrote:
> Am 22.11.2017 um 09:42 schrieb Michel Dänzer:
>> On 2017-11-21 07:38 PM, Christian König wrote:
>>> Am 21.11.2017 um 18:29 schrieb Michel Dänzer:
>>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>>
>>>> This matches the corresponding UAPI fields. Treating the ring index as
>>>> signed could result in accessing random unrelated memory if the MSB was
>>>> set.
>>>>
>>>> Fixes: effd924d2f3b ("drm/amdgpu: untie user ring ids from kernel ring
>>>>                         ids v6")
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>>> Subject line is off, [...]
>> I assume you're referring to the prefix. Well, I'm actually trying to
>> set a trend there. :) Since there's no other amdgpu driver, the drm/ is
>> redundant. In the specific case of this patch, adding it would make the
>> shortlog too long, and IME it's quite often the case that those four
>> character cells could be put to better use than for the redundant prefix.
> 
> Yeah, making the prefix shorter is one thing I have thought about as well.
> 
> But currently we use "drm/amdgpu:" for the kernel and "amdgpu:" for
> libdrm and I actually would like to keep that.

libdrm patches should have [PATCH libdrm] .


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx