<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> [AMD Official Use Only - General] <div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);" class="elementToProof ContentPasted0"> Reviewed-by: Mukul Joshi <mukul.joshi@amd.com> </div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Alex Deucher <alexdeucher@gmail.com> Sent: Wednesday, May 17, 2023 12:28 PM To: Joshi, Mukul <Mukul.Joshi@amd.com> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; Chen, Guchun <Guchun.Chen@amd.com> Subject: Re: [PATCH] drm/amdgpu/gmc9: fix 64 bit division in partition code <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. On Wed, May 17, 2023 at 12:26 PM Joshi, Mukul <Mukul.Joshi@amd.com> wrote: > > [AMD Official Use Only - General] > > > Hi Alex, > > ________________________________ > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Alex Deucher <alexander.deucher@amd.com> > Sent: Wednesday, May 17, 2023 10:31 AM > To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Chen, Guchun <Guchun.Chen@amd.com> > Subject: [PATCH] drm/amdgpu/gmc9: fix 64 bit division in partition code > > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > Rework logic or use do_div() to avoid problems on 32 bit. > > v2: add a missing case for XCP macro > > Acked-by: Guchun Chen <guchun.chen@amd.com> (v1) > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 14 ++++++++++++++ > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 9 ++++----- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 5 ++++- > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 11 ++++++----- > 4 files changed, 28 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > index 739eb7c0d133..fed545cdd1de 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > @@ -794,3 +794,17 @@ void amdgpu_amdkfd_unlock_kfd(struct amdgpu_device *adev) > { > kgd2kfd_unlock_kfd(); > } > + > + > +u64 amdgpu_amdkfd_xcp_memory_size(struct amdgpu_device *adev, int xcp_id) > +{ > + u64 tmp; > + > + if (adev->gmc.num_mem_partitions && xcp_id >= 0) { > + tmp = adev->gmc.mem_partitions[KFD_XCP_MEM_ID(adev, xcp_id)].size; > + do_div(tmp, adev->xcp_mgr->num_xcp_per_mem_partition); > + return tmp; > + } else { > + return adev->gmc.real_vram_size; > + } > +} > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > index be43d71ba7ef..94cc456761e5 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > @@ -333,15 +333,14 @@ int amdgpu_amdkfd_reserve_mem_limit(struct amdgpu_device *adev, > void amdgpu_amdkfd_unreserve_mem_limit(struct amdgpu_device *adev, > uint64_t size, u32 alloc_flag, int8_t xcp_id); > > +u64 amdgpu_amdkfd_xcp_memory_size(struct amdgpu_device *adev, int xcp_id); > + > #define KFD_XCP_MEM_ID(adev, xcp_id) \ > ((adev)->xcp_mgr && (xcp_id) >= 0 ?\ > (adev)->xcp_mgr->xcp[(xcp_id)].mem_id : -1) > > -#define KFD_XCP_MEMORY_SIZE(adev, xcp_id)\ > - ((adev)->gmc.num_mem_partitions && (xcp_id) >= 0 ?\ > - (adev)->gmc.mem_partitions[KFD_XCP_MEM_ID((adev), (xcp_id))].size /\ > - (adev)->xcp_mgr->num_xcp_per_mem_partition :\ > - (adev)->gmc.real_vram_size) > +#define KFD_XCP_MEMORY_SIZE(adev, xcp_id) amdgpu_amdkfd_xcp_memory_size((adev), (xcp_id)) > + > > #if IS_ENABLED(CONFIG_HSA_AMD) > void amdgpu_amdkfd_gpuvm_init_mem_limits(void); > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > index ad664ef640ff..34724b771ace 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > @@ -818,11 +818,14 @@ static void amdgpu_ttm_gart_bind_gfx9_mqd(struct amdgpu_device *adev, > struct amdgpu_ttm_tt *gtt = (void *)ttm; > uint64_t total_pages = ttm->num_pages; > int num_xcc = max(1U, adev->gfx.num_xcc_per_xcp); > - uint64_t page_idx, pages_per_xcc = total_pages / num_xcc; > + uint64_t page_idx, pages_per_xcc; > int i; > uint64_t ctrl_flags = (flags & ~AMDGPU_PTE_MTYPE_VG10_MASK) | > AMDGPU_PTE_MTYPE_VG10(AMDGPU_MTYPE_NC); > > + pages_per_xcc = total_pages; > + do_div(pages_per_xcc, num_xcc); > + > for (i = 0, page_idx = 0; i < num_xcc; i++, page_idx += pages_per_xcc) { > /* MQD page: use default flags */ > amdgpu_gart_bind(adev, > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > index 938c8dba9057..d559e7bc0f09 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c > @@ -1919,9 +1919,10 @@ gmc_v9_0_init_acpi_mem_ranges(struct amdgpu_device *adev, > adev->gmc.num_mem_partitions = num_ranges; > > /* If there is only partition, don't use entire size */ > - if (adev->gmc.num_mem_partitions == 1) > - mem_ranges[0].size = > - (mem_ranges[0].size * (mem_groups - 1) / mem_groups); > + if (adev->gmc.num_mem_partitions == 1) { > + mem_ranges[0].size = mem_ranges[0].size * (mem_groups - 1); > + do_div(mem_ranges[0].size, mem_groups); > + } > } > > static void > @@ -1953,8 +1954,8 @@ gmc_v9_0_init_sw_mem_ranges(struct amdgpu_device *adev, > break; > } > > - size = (adev->gmc.real_vram_size >> AMDGPU_GPU_PAGE_SHIFT) / > - adev->gmc.num_mem_partitions; > + size = adev->gmc.real_vram_size >> AMDGPU_GPU_PAGE_SHIFT; > + size /= adev->gmc.num_mem_partitions; > > Did you miss using the do_div() here? It's not needed here. size is 32 bits IIRC. Alex > > Thanks, > Mukul > > for (i = 0; i < adev->gmc.num_mem_partitions; ++i) { > mem_ranges[i].range.fpfn = start_addr; > -- > 2.40.1 > </div> </div> </div> </body> </html>