[PATCH 19/22] drm/amdgpu: Fix the warning division or modulo by zero for the variable num_xcc_per_xcp

[Lazar, Lijo]

On 5/10/2024 1:09 PM, Zhang, Jesse(Jie) wrote:
> [AMD Official Use Only - General]
> 
> Hi Lijo,
> 
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Lazar, Lijo
> Sent: Friday, May 10, 2024 3:16 PM
> To: amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 19/22] drm/amdgpu: Fix the warning division or modulo by zero for the variable num_xcc_per_xcp
> 
> 
> 
> On 5/10/2024 8:20 AM, Jesse Zhang wrote:
>> Dividing expression num_xcc_per_xcp which may be zero has undefined behavior.
>>
>> Signed-off-by: Jesse Zhang <Jesse.Zhang at amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
>> b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
>> index 414ea3f560a7..5752c6760992 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/aqua_vanjaram.c
>> @@ -522,6 +522,9 @@ static int aqua_vanjaram_switch_partition_mode(struct amdgpu_xcp_mgr *xcp_mgr,
>>               goto unlock;
>>
>>       num_xcc_per_xcp = __aqua_vanjaram_get_xcc_per_xcp(xcp_mgr, mode);
>> +     if (!num_xcc_per_xcp)
>> +             goto unlock;
>> +
> 
> This won't happen as the mode is validated before and for each valid mode a non-zero num_xcc_per_xcp is expected. To satisfy the warning-checker, before going to unlock use a proper 'ret' value also (otherwise it will look odd).
> 
> [Zhang, Jesse(Jie)]  Is that possible?
> When the initialization mode is AMDGPU_AUTO_COMPUTE_PARTITION_MODE,
> and get AMDGPU_UNKNOWN_COMPUTE_PARTITION_MODE through __aqua_vanjaram_get_auto_mode.
> 
> If so, it will skip the valid mode function (__aqua_vanjaram_is_valid_mode).
> 
Yes, that is possible. If auto detection cannot figure out a valid mode,
then it needs to return an error from there without proceeding further.
That is a better fix.

Thanks,
Lijo

> Thanks
> Jesse
> 
> 
> Thanks,
> Lijo
> 
>>       if (adev->gfx.funcs->switch_partition_mode)
>>               adev->gfx.funcs->switch_partition_mode(xcp_mgr->adev,
>>                                                      num_xcc_per_xcp);