Re: [PATCH 4/6] drm/amdgpu: address remove from fault filter

[philip yang <yangp@xxxxxxx>]

On 2021-04-21 3:22 a.m., Christian König wrote:

> Am
>       20.04.21 um 22:21 schrieb Philip Yang:
>       
> > Add interface to remove address from fault
> >         filter ring by resetting
> >         
> >         fault ring entry of the fault address timestamp to 0, then
> >         future vm
> >         
> >         fault on the address will be processed to recover.
> >         
> >         
> >         Check fault address from fault ring, add address into fault ring
> >         and
> >         
> >         remove address from fault ring are serialized in same interrupt
> >         deferred
> >         
> >         work, don't have race condition.
> >         
> >       
>       
>       That might not work on Vega20.
>       
>       
>       We call amdgpu_gmc_filter_faults() from the the IH while the fault
>       handling id done from the delegated IH processing.
>       
>     
    Added spinlock for VG20.
>       
>       More comments below.
>       
>       
> > Signed-off-by: Philip Yang
> >         <Philip.Yang@xxxxxxx>
> >         
> >         ---
> >         
> >           drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 24
> >         ++++++++++++++++++++++++
> >         
> >           drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  2 ++
> >         
> >           2 files changed, 26 insertions(+)
> >         
> >         
> >         diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> >         b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> >         
> >         index c39ed9eb0987..338e45fa66cb 100644
> >         
> >         --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> >         
> >         +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> >         
> >         @@ -387,6 +387,30 @@ bool amdgpu_gmc_filter_faults(struct
> >         amdgpu_device *adev, uint64_t addr,
> >         
> >               return false;
> >         
> >           }
> >         
> >           +/**
> >         
> >         + * amdgpu_gmc_filter_faults_remove - remove address from VM
> >         faults filter
> >         
> >         + *
> >         
> >         + * @adev: amdgpu device structure
> >         
> >         + * @addr: address of the VM fault
> >         
> >         + * @pasid: PASID of the process causing the fault
> >         
> >         + *
> >         
> >         + * Remove the address from fault filter, then future vm fault
> >         on this address
> >         
> >         + * will pass to retry fault handler to recover.
> >         
> >         + */
> >         
> >         +void amdgpu_gmc_filter_faults_remove(struct amdgpu_device
> >         *adev, uint64_t addr,
> >         
> >         +                     uint16_t pasid)
> >         
> >         +{
> >         
> >         +    struct amdgpu_gmc *gmc = &adev->gmc;
> >         
> >         +
> >         
> >         +    uint64_t key = addr << 4 | pasid;
> >         
> >       
>       
>       We should probably have a function for this now.
>       
>     
    add function fault_key in v2.
>       
> > +    struct amdgpu_gmc_fault *fault;
> >         
> >         +    uint32_t hash;
> >         
> >         +
> >         
> >         +    hash = hash_64(key, AMDGPU_GMC_FAULT_HASH_ORDER);
> >         
> >         +    fault =
> >         &gmc->fault_ring[gmc->fault_hash[hash].idx];
> >         
> >         +    fault->timestamp = 0;
> >         
> >       
>       
>       There is no guarantee that the ring entry you found for the fault
>       is the one for this address.
>       
>       
>       After all that is just an 8 bit hash for a 64bit values :)
>       
>       
>       You need to double check the key and walk the chain by looking at
>       the next entry to eventually find the right one.
>       
>       
>

I am not completely understand how fault->next and gmc->last_fault works, as it keep increasing. Please help review patch v2.

Thanks,

Philip

> Christian.
>       
>       
> > +}
> >         
> >         +
> >         
> >           int amdgpu_gmc_ras_late_init(struct amdgpu_device *adev)
> >         
> >           {
> >         
> >               int r;
> >         
> >         diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> >         b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> >         
> >         index 9d11c02a3938..498a7a0d5a9e 100644
> >         
> >         --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> >         
> >         +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> >         
> >         @@ -318,6 +318,8 @@ void amdgpu_gmc_agp_location(struct
> >         amdgpu_device *adev,
> >         
> >                            struct amdgpu_gmc *mc);
> >         
> >           bool amdgpu_gmc_filter_faults(struct amdgpu_device *adev,
> >         uint64_t addr,
> >         
> >                             uint16_t pasid, uint64_t timestamp);
> >         
> >         +void amdgpu_gmc_filter_faults_remove(struct amdgpu_device
> >         *adev, uint64_t addr,
> >         
> >         +                     uint16_t pasid);
> >         
> >           int amdgpu_gmc_ras_late_init(struct amdgpu_device *adev);
> >         
> >           void amdgpu_gmc_ras_fini(struct amdgpu_device *adev);
> >         
> >           int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device
> >         *adev);
> >         
> >       
>       
>     
  

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