Re: [PATCH v5 16/27] drm/amdgpu: Unmap all MMIO mappings

Am 28.04.21 um 17:11 schrieb Andrey Grodzovsky:

Access to those must be prevented post pci_remove

That is certainly a no-go. We want to get rid of the kernel pointers in BOs, not add another one.

Christian.

Signed-off-by: Andrey Grodzovsky andrey.grodzovsky@amd.com

drivers/gpu/drm/amd/amdgpu/amdgpu.h | 5 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 38 ++++++++++++++++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 28 ++++++++++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 5 +++ 4 files changed, 71 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 30a24db5f4d1..3e4755fc10c8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1056,6 +1056,11 @@ struct amdgpu_device { struct pci_saved_state *pci_state;

struct list_head device_bo_list;

• /* List of all MMIO BOs */

• struct list_head mmio_list;

• struct mutex mmio_list_lock;

• };

  static inline struct amdgpu_device *drm_to_adev(struct drm_device *ddev)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 22b09c4db255..3ddad6cba62d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -3320,6 +3320,9 @@ int amdgpu_device_init(struct amdgpu_device *adev, INIT_LIST_HEAD(&adev->shadow_list); mutex_init(&adev->shadow_list_lock);

• INIT_LIST_HEAD(&adev->mmio_list);
• mutex_init(&adev->mmio_list_lock);
• INIT_DELAYED_WORK(&adev->delayed_init_work, amdgpu_device_delayed_init_work_handler); INIT_DELAYED_WORK(&adev->gfx.gfx_off_delay_work,

@@ -3636,6 +3639,36 @@ static void amdgpu_clear_dma_mappings(struct amdgpu_device *adev) spin_unlock(&adev->mman.bdev.lru_lock); }

+static void amdgpu_device_unmap_mmio(struct amdgpu_device *adev) +{

• struct amdgpu_bo *bo;
• /* Clear all CPU mappings pointing to this device */
• unmap_mapping_range(adev->ddev.anon_inode->i_mapping, 0, 0, 1);
• /* Unmap all MMIO mapped kernel BOs */
• mutex_lock(&adev->mmio_list_lock);
• list_for_each_entry(bo, &adev->mmio_list, mmio_list) {

• amdgpu_bo_kunmap(bo);
  

• if (*bo->kmap_ptr)
  

• 	*bo->kmap_ptr = NULL;
  

• }
• mutex_unlock(&adev->mmio_list_lock);
• /* Unmap all mapped bars - Doorbell, registers and VRAM */
• amdgpu_device_doorbell_fini(adev);
• iounmap(adev->rmmio);
• adev->rmmio = NULL;
• if (adev->mman.aper_base_kaddr)

• iounmap(adev->mman.aper_base_kaddr);
  

• adev->mman.aper_base_kaddr = NULL;
• /* Memory manager related */
• arch_phys_wc_del(adev->gmc.vram_mtrr);
• arch_io_free_memtype_wc(adev->gmc.aper_base, adev->gmc.aper_size);

+}

• /**
  • amdgpu_device_fini - tear down the driver

@@ -3683,6 +3716,8 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev) amdgpu_clear_dma_mappings(adev);

amdgpu_gart_dummy_page_fini(adev);

• amdgpu_device_unmap_mmio(adev); }

  void amdgpu_device_fini_sw(struct amdgpu_device *adev)

@@ -3713,9 +3748,6 @@ void amdgpu_device_fini_sw(struct amdgpu_device *adev) if (adev->rio_mem) pci_iounmap(adev->pdev, adev->rio_mem); adev->rio_mem = NULL;

• iounmap(adev->rmmio);

• adev->rmmio = NULL;

• amdgpu_device_doorbell_fini(adev);

  if (IS_ENABLED(CONFIG_PERF_EVENTS)) amdgpu_pmu_fini(adev);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 62d829f5e62c..9b05e3b96fa0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -531,6 +531,9 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, return -ENOMEM; drm_gem_private_object_init(adev_to_drm(adev), &bo->tbo.base, size); INIT_LIST_HEAD(&bo->shadow_list);

• INIT_LIST_HEAD(&bo->mmio_list);
• bo->vm_bo = NULL; bo->preferred_domains = bp->preferred_domain ? bp->preferred_domain : bp->domain;

@@ -774,9 +777,21 @@ int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr) if (r) return r;

• if (ptr)

• if (bo->kmap.bo_kmap_type == ttm_bo_map_iomap) {

• struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
  

• mutex_lock(&adev->mmio_list_lock);
  

• list_add_tail(&bo->mmio_list, &adev->mmio_list);
  

• mutex_unlock(&adev->mmio_list_lock);
  

• }

• if (ptr) { *ptr = amdgpu_bo_kptr(bo);

• if (bo->kmap.bo_kmap_type == ttm_bo_map_iomap)
  

• 	bo->kmap_ptr = ptr;
  

• }

• return 0; }

@@ -804,8 +819,17 @@ void *amdgpu_bo_kptr(struct amdgpu_bo *bo) */ void amdgpu_bo_kunmap(struct amdgpu_bo *bo) {

• if (bo->kmap.bo)

• struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);

• if (bo->kmap.bo) {

• if (bo->kmap.bo_kmap_type == ttm_bo_map_iomap) {
  

• 	mutex_lock(&adev->mmio_list_lock);
  

• 	list_del_init(&bo->mmio_list);
  

• 	mutex_unlock(&adev->mmio_list_lock);
  

• }
  

• ttm_bo_kunmap(&bo->kmap);

• } }

  /**

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h index 5ae8555ef275..3129d9bbfa22 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h @@ -112,6 +112,11 @@ struct amdgpu_bo { struct kgd_mem *kfd_bo;

struct list_head bo;

• struct list_head mmio_list;

• /* Address of kernel VA pointer to MMIO so they can be updated post remap */

• void **kmap_ptr;

• };

  static inline struct amdgpu_bo *ttm_to_amdgpu_bo(struct ttm_buffer_object *tbo)