Seems we don't need two page table, we just map every prt range to dummy page, 
any problem?

-David

> -----Original Message-----
> From: Zhou, David(ChunMing)
> Sent: Thursday, January 03, 2019 5:23 PM
> To: Koenig, Christian <christian.koe...@amd.com>; amd-
> g...@lists.freedesktop.org
> Subject: RE: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt
> accessed
> 
> 
> 
> > -----Original Message-----
> > From: Christian König <ckoenig.leichtzumer...@gmail.com>
> > Sent: Thursday, January 03, 2019 5:05 PM
> > To: Zhou, David(ChunMing) <david1.z...@amd.com>; Koenig, Christian
> > <christian.koe...@amd.com>; Zhou, David(ChunMing)
> > <david1.z...@amd.com>; amd-gfx@lists.freedesktop.org
> > Subject: Re: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt
> > accessed
> >
> > Yes, exactly.
> >
> > Problem is that we then probably need two page tables. One for the
> > CB/TC and one for the SDMA.
> 
> But when setup page table, how can we know the client is CB/TC or SDMA?
> 
> -David
> 
> >
> > Christian.
> >
> > Am 03.01.19 um 10:02 schrieb zhoucm1:
> > > need dummy page for that?
> > >
> > >
> > > -David
> > >
> > >
> > > On 2019年01月03日 17:01, Christian König wrote:
> > >> NAK, the problem is not the interrupt.
> > >>
> > >> E.g. causing faults by accessing unmapped pages with the SDMA can
> > >> still crash the MC.
> > >>
> > >> The key point is that SDMA can't work with PRT tiles on pre-gmc9
> > >> and we need to forbid access on the application side.
> > >>
> > >> Regards,
> > >> Christian.
> > >>
> > >> Am 03.01.19 um 09:54 schrieb Chunming Zhou:
> > >>> For pre-gmc9, UMD can only access unmapped PRT tile from CB/TC
> > >>> without firing VM fault. Kernel would still receive the VM fault
> > >>> interrupt and output the error message if SDMA is the mc_client.
> > >>> GMC9 don't need the same since it handle the PRT in different way.
> > >>> We cannot just skip message for SDMA, as Christian pointed, VM
> > >>> fault could crash mc block, so we disable vm fault irq during prt
> > >>> range is accesed.
> > >>> The nagative is normal vm fault could be ignored during that
> > >>> peroid without enabling vm_debug kernel parameter.
> > >>>
> > >>> Change-Id: Ic3c62393768eca90e3e45eaf81e7f26f2e91de84
> > >>> Signed-off-by: Chunming Zhou <david1.z...@amd.com>
> > >>> ---
> > >>>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 6 ++++++
> > >>>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 6 ++++++
> > >>>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 6 ++++++
> > >>>   3 files changed, 18 insertions(+)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> > >>> b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> > >>> index dae73f6768c2..175c4b319559 100644
> > >>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> > >>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> > >>> @@ -486,6 +486,10 @@ static void gmc_v6_0_set_prt(struct
> > >>> amdgpu_device *adev, bool enable)
> > >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
> > >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
> > >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
> > >>> +        /* Note: when vm_debug enabled, vm fault from SDMAx
> > >>> +accessing
> > >>> +         * PRT range is normal. */
> > >>> +        if (!amdgpu_vm_debug)
> > >>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
> > >>>       } else {
> > >>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
> > >>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -
> > 495,6
> > >>> +499,8 @@ static void gmc_v6_0_set_prt(struct amdgpu_device
> *adev,
> > >>> bool enable)
> > >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
> > >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
> > >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
> > >>> +        if (!amdgpu_vm_debug)
> > >>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
> > >>>       }
> > >>>   }
> > >>>   diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > >>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > >>> index 5bdeb358bfb5..a4d6d219f4e8 100644
> > >>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > >>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > >>> @@ -582,6 +582,10 @@ static void gmc_v7_0_set_prt(struct
> > >>> amdgpu_device *adev, bool enable)
> > >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
> > >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
> > >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
> > >>> +        /* Note: when vm_debug enabled, vm fault from SDMAx
> > >>> +accessing
> > >>> +         * PRT range is normal. */
> > >>> +        if (!amdgpu_vm_debug)
> > >>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
> > >>>       } else {
> > >>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
> > >>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -
> > 591,6
> > >>> +595,8 @@ static void gmc_v7_0_set_prt(struct amdgpu_device
> *adev,
> > >>> bool enable)
> > >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
> > >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
> > >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
> > >>> +        if (!amdgpu_vm_debug)
> > >>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
> > >>>       }
> > >>>   }
> > >>>   diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > >>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > >>> index 5150ab614eaa..eea2eb7fc2f5 100644
> > >>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > >>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > >>> @@ -808,6 +808,10 @@ static void gmc_v8_0_set_prt(struct
> > >>> amdgpu_device *adev, bool enable)
> > >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
> > >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
> > >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
> > >>> +        /* Note: when vm_debug enabled, vm fault from SDMAx
> > >>> +accessing
> > >>> +         * PRT range is normal. */
> > >>> +        if (!amdgpu_vm_debug)
> > >>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
> > >>>       } else {
> > >>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
> > >>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -
> > 817,6
> > >>> +821,8 @@ static void gmc_v8_0_set_prt(struct amdgpu_device
> *adev,
> > >>> bool enable)
> > >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
> > >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
> > >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
> > >>> +        if (!amdgpu_vm_debug)
> > >>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
> > >>>       }
> > >>>   }
> > >>
> > >
> > > _______________________________________________
> > > 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

Reply via email to