All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Zeng, Oak" <Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
To: "Koenig,
	Christian" <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>,
	"Kuehling, Felix" <Felix.Kuehling-5C7GfCeVMHo@public.gmane.org>,
	"amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org"
	<amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>,
	"Zhang, Hawking" <Hawking.Zhang-5C7GfCeVMHo@public.gmane.org>,
	"Deucher,
	Alexander" <Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>
Cc: "Zhou1, Tao" <Tao.Zhou1-5C7GfCeVMHo@public.gmane.org>,
	"Liu, Shaoyun" <Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org>
Subject: RE: [PATCH 1/2] drm/amdgpu: Export function to flush TLB of specific vm hub
Date: Wed, 7 Aug 2019 15:17:21 +0000	[thread overview]
Message-ID: <BL0PR12MB2580FE8417C1C4AB87F4679780D40@BL0PR12MB2580.namprd12.prod.outlook.com> (raw)
In-Reply-To: <65784ad0-7693-9c98-82ee-66713c99f49b-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Thanks. I realized I didn't read the code careful enough...The workaround is only for navi10 and navi12 - I didn't read this correctly and was thinking gfxhub tlb invalidation was done twice.

I understand the codes now. I think the HW SDMA bug has been fixed in navi14 so we don't need that WA for 14.

Regards,
Oak

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Wednesday, August 7, 2019 4:51 AM
To: Zeng, Oak <Oak.Zeng@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; amd-gfx@lists.freedesktop.org; Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>
Cc: Zhou1, Tao <Tao.Zhou1@amd.com>; Liu, Shaoyun <Shaoyun.Liu@amd.com>
Subject: Re: [PATCH 1/2] drm/amdgpu: Export function to flush TLB of specific vm hub

> Does the coded below invalidate TLB on both mm and gfx hub?
No, just the gfx hub. The VMHUBs on Navi are unfortunately really buggy, so we had to do a lot of workarounds to get them into a state where they actually did what was expected from them.

One major issue for example is that you can't do MMIO based VM invalidation when the engine is busy. To work around this we do the invalidation with the (IIRC) SDMA engine as soon as that one is working.

The is the code you are noting below.

Regards,
Christian.

Am 07.08.19 um 04:40 schrieb Zeng, Oak:
> Ok, will do it.
>
> BTW, does those codes below really needed, in function 
> gmc_v10_0_flush_gpu_tlb. I think if we have the bug, then before below 
> codes, when we flush TLB of gfxhub through mmio, it has already 
> triggered the bug. Also as we already invalidated tlb on both mm and 
> gfx hub (in the same function gmc_v10_0_flush_gpu_tlb), what is the 
> point of below codes? Does the coded below invalidate TLB on both mm 
> and gfx hub? Also @Zhang, Hawking@Deucher, Alexander
>
> 	/* The SDMA on Navi has a bug which can theoretically result in memory
> 	 * corruption if an invalidation happens at the same time as an VA
> 	 * translation. Avoid this by doing the invalidation from the SDMA
> 	 * itself.
> 	 */
> 	r = amdgpu_job_alloc_with_ib(adev, 16 * 4, &job);
> 	if (r)
> 		goto error_alloc;
>
> 	job->vm_pd_addr = amdgpu_gmc_pd_addr(adev->gart.bo);
> 	job->vm_needs_flush = true;
> 	amdgpu_ring_pad_ib(ring, &job->ibs[0]);
> 	r = amdgpu_job_submit(job, &adev->mman.entity,
> 			      AMDGPU_FENCE_OWNER_UNDEFINED, &fence);
>
> Regards,
> Oak
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Monday, August 5, 2019 5:37 AM
> To: Zeng, Oak <Oak.Zeng@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Kuehling, Felix <Felix.Kuehling@amd.com>; Zhou1, Tao 
> <Tao.Zhou1@amd.com>; Liu, Shaoyun <Shaoyun.Liu@amd.com>
> Subject: Re: [PATCH 1/2] drm/amdgpu: Export function to flush TLB of 
> specific vm hub
>
> Am 02.08.19 um 18:04 schrieb Zeng, Oak:
>> This is for kfd to reuse amdgpu TLB invalidation function. There is 
>> already a gmc function flush_gpu_tlb to flush TLB on all vm hub. On 
>> gfx10, kfd only needs to flush TLB on gfx hub but not on mm hub. So 
>> export a function for KFD flush TLB only on gfx hub.
> I would rather go ahead and add another parameter to flush_gpu_tlb to note which hub needs flushing.
>
> We can probably easily extend the few callers to flush all hubs needed.
>
> Christian.
>
>> Change-Id: I58ff00969f88438cfd3dc7e9deb7bff0c1bb4133
>> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 4 ++++
>>    drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 1 +
>>    2 files changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> index 071145a..0bd4a4f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> @@ -88,6 +88,9 @@ struct amdgpu_vmhub {
>>     * GPU MC structures, functions & helpers
>>     */
>>    struct amdgpu_gmc_funcs {
>> +	/* flush vm tlb of specific hub */
>> +	void (*flush_vm_hub)(struct amdgpu_device *adev, uint32_t vmid,
>> +				   unsigned int vmhub, uint32_t flush_type);
>>    	/* flush the vm tlb via mmio */
>>    	void (*flush_gpu_tlb)(struct amdgpu_device *adev,
>>    			      uint32_t vmid, uint32_t flush_type); @@ -180,6 +183,7 @@ 
>> struct amdgpu_gmc {
>>    	struct ras_common_if    *ras_if;
>>    };
>>    
>> +#define amdgpu_gmc_flush_vm_hub(adev, vmid, vmhub, type) 
>> +((adev)->gmc.gmc_funcs->flush_vm_hub((adev), (vmid), (vmhub),
>> +(type)))
>>    #define amdgpu_gmc_flush_gpu_tlb(adev, vmid, type) (adev)->gmc.gmc_funcs->flush_gpu_tlb((adev), (vmid), (type))
>>    #define amdgpu_gmc_emit_flush_gpu_tlb(r, vmid, addr) (r)->adev->gmc.gmc_funcs->emit_flush_gpu_tlb((r), (vmid), (addr))
>>    #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) 
>> (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid)) 
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> index 4e3ac10..247515d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> @@ -416,6 +416,7 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    }
>>    
>>    static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {
>> +	.flush_vm_hub = gmc_v10_0_flush_vm_hub,
>>    	.flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,
>>    	.emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  parent reply	other threads:[~2019-08-07 15:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-02 16:04 [PATCH 1/2] drm/amdgpu: Export function to flush TLB of specific vm hub Zeng, Oak
     [not found] ` <1564761834-19210-1-git-send-email-Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
2019-08-02 16:04   ` [PATCH 2/2] drm/amdkfd/gfx10: Calling amdgpu functions to invalidate TLB Zeng, Oak
2019-08-05  9:37   ` [PATCH 1/2] drm/amdgpu: Export function to flush TLB of specific vm hub Christian König
     [not found]     ` <855377a1-69ca-891e-1dad-5b93f57671da-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2019-08-07  2:40       ` Zeng, Oak
     [not found]         ` <BL0PR12MB2580778BFD71E211AC18CFAF80D40-b4cIHhjg/p/XzH18dTCKOgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2019-08-07  8:50           ` Christian König
     [not found]             ` <65784ad0-7693-9c98-82ee-66713c99f49b-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2019-08-07 15:17               ` Zeng, Oak [this message]
2019-08-09  4:21 Zeng, Oak
     [not found] ` <1565324504-4445-1-git-send-email-Oak.Zeng-5C7GfCeVMHo@public.gmane.org>
2019-08-13 19:17   ` Zeng, Oak
2019-08-14  8:10   ` Koenig, Christian

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BL0PR12MB2580FE8417C1C4AB87F4679780D40@BL0PR12MB2580.namprd12.prod.outlook.com \
    --to=oak.zeng-5c7gfcevmho@public.gmane.org \
    --cc=Alexander.Deucher-5C7GfCeVMHo@public.gmane.org \
    --cc=Christian.Koenig-5C7GfCeVMHo@public.gmane.org \
    --cc=Felix.Kuehling-5C7GfCeVMHo@public.gmane.org \
    --cc=Hawking.Zhang-5C7GfCeVMHo@public.gmane.org \
    --cc=Shaoyun.Liu-5C7GfCeVMHo@public.gmane.org \
    --cc=Tao.Zhou1-5C7GfCeVMHo@public.gmane.org \
    --cc=amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.