Re: [PATCH 1/2] drm/amdgpu: Fix MMIO access page fault

[Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>]

Note that it already has this protection.

Andrey

On 2021-09-17 8:04 a.m., James Zhu wrote:
> typo. vcn_v3_0_sw_init   -->  vcn_v3_0_sw_fini
>
> On 2021-09-17 8:00 a.m., James Zhu wrote:
> > Hi Andrey
> >
> > Can you apply this improvement  on vcn_v3_0_sw_init also?
> >
> > With this adding, This patch is Reviewed-by: James Zhu 
> > <James.Zhu@xxxxxxx>
> >
> > Thanks & Best Regards!
> >
> > James
> >
> > On 2021-09-17 7:30 a.m., Andrey Grodzovsky wrote:
> > > Add more guards to MMIO access post device
> > > unbind/unplug
> > >
> > > Bug:https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.archlinux.org%2Ftask%2F72092%3Fproject%3D1%26order%3Ddateopened%26sort%3Ddesc%26pagenum%3D1&amp;data=04%7C01%7Candrey.grodzovsky%40amd.com%7C209112865fef455dba0208d979d35e93%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637674771021422447%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=pskUESxJBYWMjGxbTqb5W%2FwpXUpui9c%2FyUEl7HX9PA8%3D&amp;reserved=0 
> > >
> > > Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c |  8 ++++++--
> > >   drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 17 +++++++++++------
> > >   2 files changed, 17 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c 
> > > b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> > > index e6e9ef50719e..a03c0fc8338f 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> > > @@ -22,6 +22,7 @@
> > >    */
> > >     #include <linux/firmware.h>
> > > +#include <drm/drm_drv.h>
> > >     #include "amdgpu.h"
> > >   #include "amdgpu_vcn.h"
> > > @@ -194,11 +195,14 @@ static int vcn_v2_0_sw_init(void *handle)
> > >    */
> > >   static int vcn_v2_0_sw_fini(void *handle)
> > >   {
> > > -    int r;
> > > +    int r, idx;
> > >       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> > >       volatile struct amdgpu_fw_shared *fw_shared = 
> > > adev->vcn.inst->fw_shared_cpu_addr;
> > >   -    fw_shared->present_flag_0 = 0;
> > > +    if (drm_dev_enter(&adev->ddev, &idx)) {
> > > +        fw_shared->present_flag_0 = 0;
> > > +        drm_dev_exit(idx);
> > > +    }
> > >         amdgpu_virt_free_mm_table(adev);
> > >   diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c 
> > > b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > > index 2e6b7913bf6c..1780ad1eacd6 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > > @@ -22,6 +22,7 @@
> > >    */
> > >     #include <linux/firmware.h>
> > > +#include <drm/drm_drv.h>
> > >     #include "amdgpu.h"
> > >   #include "amdgpu_vcn.h"
> > > @@ -235,17 +236,21 @@ static int vcn_v2_5_sw_init(void *handle)
> > >    */
> > >   static int vcn_v2_5_sw_fini(void *handle)
> > >   {
> > > -    int i, r;
> > > +    int i, r, idx;
> > >       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> > >       volatile struct amdgpu_fw_shared *fw_shared;
> > >   -    for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
> > > -        if (adev->vcn.harvest_config & (1 << i))
> > > -            continue;
> > > -        fw_shared = adev->vcn.inst[i].fw_shared_cpu_addr;
> > > -        fw_shared->present_flag_0 = 0;
> > > +    if (drm_dev_enter(&adev->ddev, &idx)) {
> > > +        for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
> > > +            if (adev->vcn.harvest_config & (1 << i))
> > > +                continue;
> > > +            fw_shared = adev->vcn.inst[i].fw_shared_cpu_addr;
> > > +            fw_shared->present_flag_0 = 0;
> > > +        }
> > > +        drm_dev_exit(idx);
> > >       }
> > >   +
> > >       if (amdgpu_sriov_vf(adev))
> > >           amdgpu_virt_free_mm_table(adev);