Thanks, can you give the patches a try as well? I don't have access to SRIOV hardware to test them. Christian. Am 23.01.2018 um 14:44 schrieb Liu, Monk: > > looks this is HSA feature to me that CPU and GPU share the same > virtual address .... > > > Ack-by: Monk Liu <monk.liu at amd.com> > > ------------------------------------------------------------------------ > *From:* Koenig, Christian > *Sent:* Tuesday, January 23, 2018 8:51:21 PM > *To:* Liu, Monk; He, Roger; amd-gfx at lists.freedesktop.org > *Subject:* Re: [PATCH 3/3] drm/amdgpu: move static CSA address to top > of address space > The ATC (or alternatively HMM) maps the CPU addresses into the GPU > address space. > > Now on Linux the lower range (0x0-0x7fffffffffff) is used for the > userspace address space on the CPU, so when we want to use that on the > GPU it must be free of other mappings. > > GFX8 and GFX7 hardware had a separate flag which address space to use > in the memory instructions, but on GFX9 it's all one big shared > address space and we need to make sure that we don't kick each others > feet. > > Had to move the addresses Mesa use as well because of this and older > VCE firmware unfortunately was buggy regarding this as well. > > Regards, > Christian. > > Am 23.01.2018 um 13:43 schrieb Liu, Monk: >> >> OK I see, but why HMM/ATC/SVN want low 8MB range ? why they cannot >> use high range address ? >> >> >> ------------------------------------------------------------------------ >> *From:* Christian König <ckoenig.leichtzumerken at gmail.com> >> <mailto:ckoenig.leichtzumerken at gmail.com> >> *Sent:* Tuesday, January 23, 2018 4:26:21 PM >> *To:* Liu, Monk; He, Roger; amd-gfx at lists.freedesktop.org >> <mailto:amd-gfx at lists.freedesktop.org> >> *Subject:* Re: [PATCH 3/3] drm/amdgpu: move static CSA address to top >> of address space >>> 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 ? >> 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. >> >>> Besides, please note that you also need modify gfx8/9 source to >>> align emit_cs/de_meta with your change >> 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: >>> >>> 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 >>> >>> ------------------------------------------------------------------------ >>> *From:* Liu, Monk >>> *Sent:* Tuesday, January 23, 2018 11:20:35 AM >>> *To:* He, Roger; Christian König; amd-gfx at lists.freedesktop.org >>> <mailto:amd-gfx at lists.freedesktop.org> >>> *Subject:* Re: [PATCH 3/3] drm/amdgpu: move static CSA address to >>> top of address space >>> >>> 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 >>> >>> >>> >>> ------------------------------------------------------------------------ >>> *From:* He, Roger >>> *Sent:* Tuesday, January 23, 2018 10:36:31 AM >>> *To:* Christian König; Liu, Monk; amd-gfx at lists.freedesktop.org >>> <mailto:amd-gfx at lists.freedesktop.org> >>> *Subject:* RE: [PATCH 3/3] drm/amdgpu: move static CSA address to >>> top of address space >>> >>> >>> -----Original Message----- >>> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On >>> Behalf Of Christian K?nig >>> Sent: Monday, January 22, 2018 6:44 PM >>> To: Liu, Monk <Monk.Liu at amd.com> <mailto:Monk.Liu at amd.com>; >>> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org> >>> 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 <christian.koenig at amd.com> >>> <mailto:christian.koenig at amd.com> >>> --- >>>  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 >>> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org> >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >> > -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180123/b5a28328/attachment-0001.html>
