<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> </head> <body text="#000000" bgcolor="#FFFFFF"> <div class="moz-cite-prefix"> <blockquote type="cite">Any bug introduced by original design ? I don't see why 8MB- 8KB as the start vm address of CSA has any trouble compared with top range ?</blockquote> On gfx9 we need the lower address range (0x0-0x7fffffffffff) for HMM/ATC/SVM. If we map the 8k CSA in there and hit it by accident then the GPU will not do what it is supposed to do :) I could limit the change to only gfx9, but I think the code will be simpler if we change both gfx8 and gfx9. <blockquote type="cite">Besides, please note that you also need modify gfx8/9 source to align emit_cs/de_meta with your change</blockquote> That was the design flaw I was talking about. In other words I couldn't find the code where that address was actually used. We should use the macro define in those functions as well. Going to send fixed for this in a minute. Thanks, Christian. Am 23.01.2018 um 04:23 schrieb Liu, Monk: </div> <blockquote type="cite" cite="mid:BLUPR12MB0449A76D5FB31F4C46AD8BB384E30@BLUPR12MB0449.namprd12.prod.outlook.com"> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <style type="text/css" style="display:none;"></style> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> anyway I prefer no change on that part unless there issues or bug need to fix by the change. the CSA address is for use by CPG h/w not s/w, and since 8MB is a reserved range for each VM I don't see it's a design flaw with it </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Liu, Monk Sent: Tuesday, January 23, 2018 11:20:35 AM To: He, Roger; Christian König; <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> Subject: Re: [PATCH 3/3] drm/amdgpu: move static CSA address to top of address space <div> </div> </div> <style type="text/css" style="display:none">  </style> <div dir="ltr"> <div id="x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Helvetica,sans-serif"> Any bug introduced by original design ? I don't see why 8MB- 8KB as the start vm address of CSA has any trouble compared with top range ? Besides, please note that you also need modify gfx8/9 source to align emit_cs/de_meta with your change </div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr">From: He, Roger Sent: Tuesday, January 23, 2018 10:36:31 AM To: Christian König; Liu, Monk; <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> Subject: RE: [PATCH 3/3] drm/amdgpu: move static CSA address to top of address space <div> </div> </div> <div class="x_BodyFragment"> <div class="x_PlainText"> -----Original Message----- From: amd-gfx [<a href="mailto:amd-gfx-bounces@lists.freedesktop.org" moz-do-not-send="true">mailto:amd-gfx-bounces@lists.freedesktop.org</a>] On Behalf Of Christian K?nig Sent: Monday, January 22, 2018 6:44 PM To: Liu, Monk <a class="moz-txt-link-rfc2396E" href="mailto:Monk.Liu@amd.com"><Monk.Liu@amd.com></a>; <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> Subject: [PATCH 3/3] drm/amdgpu: move static CSA address to top of address space There seems to be a design flaw with this since the address of the static CSA is never exported anywhere. Signed-off-by: Christian König <a class="moz-txt-link-rfc2396E" href="mailto:christian.koenig@amd.com"><christian.koenig@amd.com></a> --- drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 15 +++++++-------- drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 6 ++++-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c index e7dfb7b44b4b..c7d24af03e3e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c @@ -55,11 +55,10 @@ void amdgpu_free_static_csa(struct amdgpu_device *adev) { /* * amdgpu_map_static_csa should be called during amdgpu_vm_init - * it maps virtual address "AMDGPU_VA_RESERVED_SIZE - AMDGPU_CSA_SIZE" - * to this VM, and each command submission of GFX should use this virtual - * address within META_DATA init package to support SRIOV gfx preemption. + * it maps virtual address "AMDGPU_CSA_VADDR" to this VM, and each + command + * submission of GFX should use this virtual address within META_DATA + init + * package to support SRIOV gfx preemption. */ - int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm, struct amdgpu_bo_va **bo_va) { @@ -90,7 +89,7 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm, return -ENOMEM; } - r = amdgpu_vm_alloc_pts(adev, (*bo_va)->base.vm, AMDGPU_CSA_VADDR, + r = amdgpu_vm_alloc_pts(adev, (*bo_va)->base.vm, +AMDGPU_CSA_VADDR(adev), AMDGPU_CSA_SIZE); if (r) { DRM_ERROR("failed to allocate pts for static CSA, err=%d\n", r); @@ -99,9 +98,9 @@ int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm, return r; } - r = amdgpu_vm_bo_map(adev, *bo_va, AMDGPU_CSA_VADDR, 0, AMDGPU_CSA_SIZE, - AMDGPU_PTE_READABLE | AMDGPU_PTE_WRITEABLE | - AMDGPU_PTE_EXECUTABLE); + r = amdgpu_vm_bo_map(adev, *bo_va, AMDGPU_CSA_VADDR(adev), 0, + AMDGPU_CSA_SIZE, AMDGPU_PTE_READABLE | + AMDGPU_PTE_WRITEABLE | AMDGPU_PTE_EXECUTABLE); if (r) { DRM_ERROR("failed to do bo_map on static CSA, err=%d\n", r); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h index 6a83425aa9ed..499362b55e45 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h @@ -251,8 +251,10 @@ struct amdgpu_virt { uint32_t gim_feature; }; -#define AMDGPU_CSA_SIZE (8 * 1024) -#define AMDGPU_CSA_VADDR (AMDGPU_VA_RESERVED_SIZE - AMDGPU_CSA_SIZE) +#define AMDGPU_CSA_SIZE (8 * 1024) +#define AMDGPU_CSA_VADDR(adev) \ + (((adev)->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT) + \ + AMDGPU_VA_RESERVED_SIZE) If I understand your intention correctly, it should be that: + (((adev)->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT) - \ + AMDGPU_VA_RESERVED_SIZE) Thanks Roger(Hongbo.He) #define amdgpu_sriov_enabled(adev) \ ((adev)->virt.caps & AMDGPU_SRIOV_CAPS_ENABLE_IOV) -- 2.14.1 _______________________________________________ amd-gfx mailing list <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" moz-do-not-send="true">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </div> </div> </div> </blockquote> </body> </html>