On 2024-05-30 21:44, Zhang, Jesse(Jie) wrote:
[AMD Official Use Only - AMD Internal Distribution Only] Hi Felix, -----Original Message----- From: Kuehling, Felix <Felix.Kuehling@xxxxxxx> Sent: Friday, May 31, 2024 4:37 AM To: Zhang, Jesse(Jie) <Jesse.Zhang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian <Christian.Koenig@xxxxxxx>; Kim, Jonathan <Jonathan.Kim@xxxxxxx>; Huang, Tim <Tim.Huang@xxxxxxx> Subject: Re: [PATCH 8/8] drm/amdkfd: remove dead code in kfd_create_vcrat_image_gpu On 2024-05-29 23:50, Jesse Zhang wrote:Since the value of avail_size is at least VCRAT_SIZE_FOR_GPU(16384), minus struct crat_header(40UL) and struct crat_subtype_compute(40UL) it cannot be less than 0. Signed-off-by: Jesse Zhang <Jesse.Zhang@xxxxxxx> --- drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c index 71150d503dc7..ead43386a7ef 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c @@ -2213,9 +2213,6 @@ static int kfd_create_vcrat_image_gpu(void *pcrat_image, * Modify length and total_entries as subunits are added. */ avail_size -= sizeof(struct crat_header); - if (avail_size < 0) - return -ENOMEM; -Avail_size is passed in from the caller through the *size parameter. You're making an assumption about the caller here. [Zhang, Jesse(Jie)] avil_size is checked at the beginning of kfd_create_vcrat_image_gpu and it cannot be smaller than VCRAT_SIZE_FOR_GPU (16384). if (!pcrat_image || avail_size < VCRAT_SIZE_FOR_GPU) return -EINVAL;
Ok, I missed that. Makes sense. Maybe mention it in the commit description that kfd_create_vcrat_image_gpu itself checks the avail_size at the start. The patch is
Reviewed-by: Felix Kuehling <felix.kuehling@xxxxxxx>
Regards Jesse Regards, Felixmemset(crat_table, 0, sizeof(struct crat_header)); memcpy(&crat_table->signature, CRAT_SIGNATURE, @@ -2229,9 +2226,6 @@ static int kfd_create_vcrat_image_gpu(void *pcrat_image, * First fill in the sub type header and then sub type data */ avail_size -= sizeof(struct crat_subtype_computeunit); - if (avail_size < 0) - return -ENOMEM; - sub_type_hdr = (struct crat_subtype_generic *)(crat_table + 1); memset(sub_type_hdr, 0, sizeof(struct crat_subtype_computeunit));
