Re: [PATCH 2/2] drm/amdgpu: check the GART table before invalidating TLB

[Christian König]

Am 07.02.22 um 07:30 schrieb Huang Rui:

On Mon, Feb 07, 2022 at 10:41:55AM +0800, Liu, Aaron wrote:

Bypass group programming (utcl2_harvest) aims to forbid UTCL2 to send
invalidation command to harvested SE/SA. Once invalidation command comes
into harvested SE/SA, SE/SA has no response and system hang.

This patch is to add checking if the GART table is already allocated before
invalidating TLB. The new procedure is as following:
1. Calling amdgpu_gtt_mgr_init() in amdgpu_ttm_init(). After this step GTT
BOs can be allocated, but GART mappings are still ignored.
2. Calling amdgpu_gart_table_vram_alloc() from the GMC code. This allocates
the GART backing store.
3. Initializing the hardware, and programming the backing store into VMID0
for all VMHUBs.
4. Calling amdgpu_gtt_mgr_recover() to make sure the table is updated with
the GTT allocations done before it was allocated.

Signed-off-by: Christian König 
Signed-off-by: Aaron Liu 

Acked-by: Huang Rui 


Reviewed-by: Christian König 




---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index 91d8207336c1..01cb89ffbd56 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -259,6 +259,9 @@ void amdgpu_gart_invalidate_tlb(struct amdgpu_device *adev)
  {
int i;
  
+	if (!adev->gart.ptr)

+   return;
+
mb();
amdgpu_device_flush_hdp(adev, NULL);
for (i = 0; i < adev->num_vmhubs; i++)
--
2.25.1

[PATCH 2/2] drm/amdkfd: use unmap all queues for poison consumption

[Tao Zhou]

Replace reset queue for specific PASID with unmap all queues, reset
queue could break CP scheduler.

Signed-off-by: Tao Zhou 
---
 drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
index 7a2b6342a8f2..68ee923a440b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
@@ -109,8 +109,7 @@ static void event_interrupt_poison_consumption(struct 
kfd_dev *dev,
 
switch (source_id) {
case SOC15_INTSRC_SQ_INTERRUPT_MSG:
-   if (dev->dqm->ops.reset_queues)
-   ret = dev->dqm->ops.reset_queues(dev->dqm, pasid);
+   kfd_dqm_evict_pasid(dev->dqm, pasid);
break;
case SOC15_INTSRC_SDMA_ECC:
default:
-- 
2.17.1

[PATCH 1/2] drm/amdkfd: rename kfd_process_vm_fault to kfd_dqm_evict_pasid

[Tao Zhou]

As the function is used in more different cases, use a more general
name.

Signed-off-by: Tao Zhou 
---
 drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c  | 2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c   | 2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c 
b/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c
index d60576ce10cd..5c8023cba196 100644
--- a/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c
+++ b/drivers/gpu/drm/amd/amdkfd/cik_event_interrupt.c
@@ -110,7 +110,7 @@ static void cik_event_interrupt_wq(struct kfd_dev *dev,
struct kfd_vm_fault_info info;
 
kfd_smi_event_update_vmfault(dev, pasid);
-   kfd_process_vm_fault(dev->dqm, pasid);
+   kfd_dqm_evict_pasid(dev->dqm, pasid);
 
memset(, 0, sizeof(info));
amdgpu_amdkfd_gpuvm_get_vm_fault_info(dev->adev, );
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 63b3c7af681b..219acc818eb4 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -2120,7 +2120,7 @@ void device_queue_manager_uninit(struct 
device_queue_manager *dqm)
kfree(dqm);
 }
 
-int kfd_process_vm_fault(struct device_queue_manager *dqm, u32 pasid)
+int kfd_dqm_evict_pasid(struct device_queue_manager *dqm, u32 pasid)
 {
struct kfd_process_device *pdd;
struct kfd_process *p = kfd_lookup_process_by_pasid(pasid);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
index e8bc28009c22..7a2b6342a8f2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_int_process_v9.c
@@ -308,7 +308,7 @@ static void event_interrupt_wq_v9(struct kfd_dev *dev,
info.prot_write = ring_id & 0x20;
 
kfd_smi_event_update_vmfault(dev, pasid);
-   kfd_process_vm_fault(dev->dqm, pasid);
+   kfd_dqm_evict_pasid(dev->dqm, pasid);
kfd_signal_vm_fault_event(dev, pasid, );
}
 }
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 74ff4132a163..74fb4762c66e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1174,7 +1174,7 @@ void device_queue_manager_uninit(struct 
device_queue_manager *dqm);
 struct kernel_queue *kernel_queue_init(struct kfd_dev *dev,
enum kfd_queue_type type);
 void kernel_queue_uninit(struct kernel_queue *kq, bool hanging);
-int kfd_process_vm_fault(struct device_queue_manager *dqm, u32 pasid);
+int kfd_dqm_evict_pasid(struct device_queue_manager *dqm, u32 pasid);
 
 /* Process Queue Manager */
 struct process_queue_node {
-- 
2.17.1

Re: [PATCH] drm/amdgpu: move lockdep assert to the right place.

[Christian König]

Ah, yes of course!

When the VM is freed we currently don't lock anything either because 
nobody should have a reference to that object any more.


Going to fix this as well.

Thanks,
Christian.

Am 04.02.22 um 20:15 schrieb Bhardwaj, Rajneesh:


On 2/4/2022 1:50 PM, Christian König wrote:

Am 04.02.22 um 19:47 schrieb Bhardwaj, Rajneesh:


On 2/4/2022 1:32 PM, Christian König wrote:

Am 04.02.22 um 19:12 schrieb Bhardwaj, Rajneesh:

[Sorry for top posting]

Hi Christian

I think you forgot the below hunk, without which the issue is not 
fixed completely on a multi GPU system.


No, that is perfectly intentional. While removing a bo_va structure 
it can happen that there are still mappings attached to it (for 
example because the application crashed).



OK. but for me, at boot time, I see flood of warning messages coming 
from  amdgpu_vm_bo_del so the previous patch doesn't help.


Do you have a backtrace? That should not happen.

Could be that we have this buggy at a couple of different places.



This is on Ubuntu 18.04.


[    8.392405] WARNING: CPU: 13 PID: 2732 at 
/home/rajneesh/git/compute/kernel/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:2673 
amdgpu_vm_bo_del+0x386/0x3b

0 [amdgpu]
[    8.392586] Modules linked in: amdgpu ast iommu_v2 gpu_sched 
drm_vram_helper drm_ttm_helper ttm drm_kms_helper cfbfillrect 
syscopyarea cfbimgblt sy
sfillrect sysimgblt fb_sys_fops cfbcopyarea drm 
drm_panel_orientation_quirks k10temp nf_tables nfnetlink ip_tables 
x_tables i2c_piix4
[    8.392628] CPU: 13 PID: 2732 Comm: plymouthd Not tainted 
5.13.0-kfd-rajneesh #1055
[    8.392632] Hardware name: GIGABYTE MZ01-CE0-00/MZ01-CE0-00, BIOS 
F02 08/29/2018

[    8.392635] RIP: 0010:amdgpu_vm_bo_del+0x386/0x3b0 [amdgpu]
[    8.392749] Code: 0f 85 56 fe ff ff 48 c7 c2 28 6b b3 c0 be 21 01 
00 00 48 c7 c7 58 6b b3 c0 c6 05 64 64 62 00 01 e8 5f be a4 d4 e9 32 
fe ff ff <0f

> 0b eb 8a 49 8b be 88 01 00 00 e9 af fc ff ff be 03 00 00 00 e8
[    8.392752] RSP: 0018:af33471d7d98 EFLAGS: 00010246
[    8.392756] RAX:  RBX: a0877160 RCX: 
0001
[    8.392758] RDX: 0001 RSI: a08772ae01f8 RDI: 
a0800a426f68
[    8.392761] RBP: a087691fd980 R08: c0a4e2e0 R09: 
000a
[    8.392763] R10: af33471d7d68 R11: 0001 R12: 
a0801d540010
[    8.392765] R13: a08771615c00 R14: a08024166618 R15: 
a08771615e60
[    8.392768] FS:  7f7b80179740() GS:a08f3dec() 
knlGS:

[    8.392771] CS:  0010 DS:  ES:  CR0: 80050033
[    8.392773] CR2: 55839db51eb8 CR3: 00010f49c000 CR4: 
003506e0

[    8.392775] Call Trace:
[    8.392779]  ? _raw_spin_unlock_irqrestore+0x30/0x40
[    8.392787]  amdgpu_driver_postclose_kms+0x94/0x270 [amdgpu]
[    8.392897]  drm_file_free.part.14+0x1e3/0x230 [drm]
[    8.392918]  drm_release+0x6e/0xf0 [drm]
[    8.392937]  __fput+0xa3/0x250
[    8.392942]  task_work_run+0x6d/0xb0
[    8.392949]  exit_to_user_mode_prepare+0x1c9/0x1d0
[    8.392953]  syscall_exit_to_user_mode+0x19/0x50
[    8.392957]  do_syscall_64+0x42/0x70
[    8.392960]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[    8.392964] RIP: 0033:0x7f7b7f8679e4
[    8.392967] Code: eb 89 e8 6f 44 02 00 66 2e 0f 1f 84 00 00 00 00 
00 0f 1f 44 00 00 48 8d 05 21 ff 2d 00 8b 00 85 c0 75 13 b8 03 00 00 
00 0f 05 <48> 3d 00 f0 ff ff 77 3c f3 c3 66 90 53 89 fb 48 83 ec 10 e8 
94 fe
[    8.392970] RSP: 002b:7ffe6bb0cdb8 EFLAGS: 0246 ORIG_RAX: 
0003
[    8.392973] RAX:  RBX: 55839db4b760 RCX: 
7f7b7f8679e4
[    8.392974] RDX: 55839db4aed0 RSI:  RDI: 
000b
[    8.392976] RBP: 000b R08: 55839db4aee0 R09: 
7f7b7fb42c40
[    8.392978] R10: 0007 R11: 0246 R12: 
e280
[    8.392979] R13: 000d R14: 7f7b7fb5b1e0 R15: 
7f7b7fb5b130

[    8.392988] irq event stamp: 1264799
[    8.392990] hardirqs last  enabled at (1264805): 
[] console_unlock+0x339/0x530
[    8.392994] hardirqs last disabled at (1264810): 
[] console_unlock+0x3a6/0x530





Regards,
Christian.






Because of this locking the VM before the remove is mandatory. Only 
while adding a bo_va structure we can avoid that.


Regards,
Christian.



diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index dcc80d6e099e..6f68fc9da56a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2670,8 +2670,6 @@ void amdgpu_vm_bo_del(struct amdgpu_device 
*adev,

    struct amdgpu_vm *vm = bo_va->base.vm;
    struct amdgpu_vm_bo_base **base;

- dma_resv_assert_held(vm->root.bo->tbo.base.resv);
-
    if (bo) {
dma_resv_assert_held(bo->tbo.base.resv);
    if (bo->tbo.base.resv == vm->root.bo->tbo.base.resv)


If you chose to include the above hunk, please feel free to add

Reviewed-and-tested-by: Rajneesh 

Re: [PATCH 1/2] drm/amdgpu: add utcl2_harvest to gc 10.3.1

[Huang Rui]

On Mon, Feb 07, 2022 at 10:41:54AM +0800, Liu, Aaron wrote:
> Confirmed with hardware team, there is harvesting for gc 10.3.1.
> 
> Signed-off-by: Aaron Liu 

Reviewed-by: Huang Rui 

> ---
>  drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c 
> b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> index b4eddf6e98a6..ff738e9725ee 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
> @@ -543,7 +543,9 @@ static void gfxhub_v2_1_utcl2_harvest(struct 
> amdgpu_device *adev)
>   adev->gfx.config.max_sh_per_se *
>   adev->gfx.config.max_shader_engines);
>  
> - if (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 3, 3)) {
> + switch (adev->ip_versions[GC_HWIP][0]) {
> + case IP_VERSION(10, 3, 1):
> + case IP_VERSION(10, 3, 3):
>   /* Get SA disabled bitmap from eFuse setting */
>   efuse_setting = RREG32_SOC15(GC, 0, mmCC_GC_SA_UNIT_DISABLE);
>   efuse_setting &= CC_GC_SA_UNIT_DISABLE__SA_DISABLE_MASK;
> @@ -566,6 +568,9 @@ static void gfxhub_v2_1_utcl2_harvest(struct 
> amdgpu_device *adev)
>   disabled_sa = tmp;
>  
>   WREG32_SOC15(GC, 0, 
> mmGCUTCL2_HARVEST_BYPASS_GROUPS_YELLOW_CARP, disabled_sa);
> + break;
> + default:
> + break;
>   }
>  }
>  
> -- 
> 2.25.1
> 

[PATCH 8/8] fsdax: depend on ZONE_DEVICE || FS_DAX_LIMITED

[Christoph Hellwig]

Add a depends on ZONE_DEVICE support or the s390-specific limited DAX
support, as one of the two is required at runtime for fsdax code to
actually work.

Signed-off-by: Christoph Hellwig 
---
 fs/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/Kconfig b/fs/Kconfig
index 05efea674bffa0..6e8818a5e53c45 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -48,6 +48,7 @@ config FS_DAX
bool "File system based Direct Access (DAX) support"
depends on MMU
depends on !(ARM || MIPS || SPARC)
+   depends on ZONE_DEVICE || FS_DAX_LIMITED
select FS_IOMAP
select DAX
help
-- 
2.30.2

[PATCH 7/8] mm: remove the extra ZONE_DEVICE struct page refcount

[Christoph Hellwig]

ZONE_DEVICE struct pages have an extra reference count that complicates
the code for put_page() and several places in the kernel that need to
check the reference count to see that a page is not being used (gup,
compaction, migration, etc.). Clean up the code so the reference count
doesn't need to be treated specially for ZONE_DEVICE pages.

Note that this excludes the special idle page wakeup for fsdax pages,
which still happens at refcount 1.  This is a separate issue and will
be sorted out later.  Given that only fsdax pages require the
notifiacation when the refcount hits 1 now, the PAGEMAP_OPS Kconfig
symbol can go away and be replaced with a FS_DAX check for this hook
in the put_page fastpath.

Based on an earlier patch from Ralph Campbell .

Signed-off-by: Christoph Hellwig 
---
 arch/powerpc/kvm/book3s_hv_uvmem.c   |  1 -
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c |  1 -
 drivers/gpu/drm/nouveau/nouveau_dmem.c   |  1 -
 fs/Kconfig   |  1 -
 include/linux/memremap.h | 12 +++--
 include/linux/mm.h   |  6 +--
 lib/test_hmm.c   |  1 -
 mm/Kconfig   |  4 --
 mm/internal.h|  2 +
 mm/memcontrol.c  | 11 ++---
 mm/memremap.c| 57 
 mm/migrate.c |  6 ---
 mm/swap.c| 16 ++-
 13 files changed, 36 insertions(+), 83 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c 
b/arch/powerpc/kvm/book3s_hv_uvmem.c
index e414ca44839fd1..8b6438fa18fc2b 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -712,7 +712,6 @@ static struct page *kvmppc_uvmem_get_page(unsigned long 
gpa, struct kvm *kvm)
 
dpage = pfn_to_page(uvmem_pfn);
dpage->zone_device_data = pvt;
-   get_page(dpage);
lock_page(dpage);
return dpage;
 out_clear:
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index cb835f95a76e66..e27ca375876230 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -225,7 +225,6 @@ svm_migrate_get_vram_page(struct svm_range *prange, 
unsigned long pfn)
page = pfn_to_page(pfn);
svm_range_bo_ref(prange->svm_bo);
page->zone_device_data = prange->svm_bo;
-   get_page(page);
lock_page(page);
 }
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c 
b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index a5cdfbe32b5e54..7ba66ad68a8a1e 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -326,7 +326,6 @@ nouveau_dmem_page_alloc_locked(struct nouveau_drm *drm)
return NULL;
}
 
-   get_page(page);
lock_page(page);
return page;
 }
diff --git a/fs/Kconfig b/fs/Kconfig
index 7a2b11c0b8036d..05efea674bffa0 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -48,7 +48,6 @@ config FS_DAX
bool "File system based Direct Access (DAX) support"
depends on MMU
depends on !(ARM || MIPS || SPARC)
-   select DEV_PAGEMAP_OPS if (ZONE_DEVICE && !FS_DAX_LIMITED)
select FS_IOMAP
select DAX
help
diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 514ab46f597e5c..d6a114dd5ea8b7 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -68,9 +68,9 @@ enum memory_type {
 
 struct dev_pagemap_ops {
/*
-* Called once the page refcount reaches 1.  (ZONE_DEVICE pages never
-* reach 0 refcount unless there is a refcount bug. This allows the
-* device driver to implement its own memory management.)
+* Called once the page refcount reaches 0.  The reference count will be
+* reset to one by the core code after the method is called to prepare
+* for handing out the page again.
 */
void (*page_free)(struct page *page);
 
@@ -133,16 +133,14 @@ static inline unsigned long pgmap_vmemmap_nr(struct 
dev_pagemap *pgmap)
 
 static inline bool is_device_private_page(const struct page *page)
 {
-   return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) &&
-   IS_ENABLED(CONFIG_DEVICE_PRIVATE) &&
+   return IS_ENABLED(CONFIG_DEVICE_PRIVATE) &&
is_zone_device_page(page) &&
page->pgmap->type == MEMORY_DEVICE_PRIVATE;
 }
 
 static inline bool is_pci_p2pdma_page(const struct page *page)
 {
-   return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) &&
-   IS_ENABLED(CONFIG_PCI_P2PDMA) &&
+   return IS_ENABLED(CONFIG_PCI_P2PDMA) &&
is_zone_device_page(page) &&
page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA;
 }
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 80fccfe31c3444..ff9f149ca2017e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1090,7 +1090,7 @@ 

[PATCH 6/8] mm: don't include in

[Christoph Hellwig]

Move the check for the actual pgmap types that need the free at refcount
one behavior into the out of line helper, and thus avoid the need to
pull memremap.h into mm.h.

Signed-off-by: Christoph Hellwig 
---
 arch/arm64/mm/mmu.c|  1 +
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h  |  1 +
 drivers/gpu/drm/drm_cache.c|  2 +-
 drivers/gpu/drm/nouveau/nouveau_dmem.c |  1 +
 drivers/gpu/drm/nouveau/nouveau_svm.c  |  1 +
 drivers/infiniband/core/rw.c   |  1 +
 drivers/nvdimm/pmem.h  |  1 +
 drivers/nvme/host/pci.c|  1 +
 drivers/nvme/target/io-cmd-bdev.c  |  1 +
 fs/fuse/virtio_fs.c|  1 +
 include/linux/memremap.h   | 18 ++
 include/linux/mm.h | 20 
 lib/test_hmm.c |  1 +
 mm/memremap.c  |  6 +-
 14 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index acfae9b41cc8c9..580abae6c0b93f 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index ea68f3b3a4e9cb..6d643b4b791d87 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -25,6 +25,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index f19d9acbe95936..50b8a088f763a6 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -27,11 +27,11 @@
 /*
  * Authors: Thomas Hellström 
  */
-
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c 
b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index e886a3b9e08c7d..a5cdfbe32b5e54 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -39,6 +39,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 /*
diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c 
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 266809e511e2c1..090b9b47708cca 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -35,6 +35,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 struct nouveau_svm {
diff --git a/drivers/infiniband/core/rw.c b/drivers/infiniband/core/rw.c
index 5a3bd41b331c93..4d98f931a13ddd 100644
--- a/drivers/infiniband/core/rw.c
+++ b/drivers/infiniband/core/rw.c
@@ -2,6 +2,7 @@
 /*
  * Copyright (c) 2016 HGST, a Western Digital Company.
  */
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/nvdimm/pmem.h b/drivers/nvdimm/pmem.h
index 59cfe13ea8a85c..1f51a23614299b 100644
--- a/drivers/nvdimm/pmem.h
+++ b/drivers/nvdimm/pmem.h
@@ -3,6 +3,7 @@
 #define __NVDIMM_PMEM_H__
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 6a99ed68091589..ab15bc72710dbe 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/nvme/target/io-cmd-bdev.c 
b/drivers/nvme/target/io-cmd-bdev.c
index 70ca9dfc1771a9..a141446db1bea3 100644
--- a/drivers/nvme/target/io-cmd-bdev.c
+++ b/drivers/nvme/target/io-cmd-bdev.c
@@ -6,6 +6,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include 
 #include 
+#include 
 #include 
 #include "nvmet.h"
 
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 9d737904d07c0b..86b7dbb6a0d43e 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 1fafcc38acbad6..514ab46f597e5c 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -1,6 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 #ifndef _LINUX_MEMREMAP_H_
 #define _LINUX_MEMREMAP_H_
+
+#include 
 #include 
 #include 
 #include 
@@ -129,6 +131,22 @@ static inline unsigned long pgmap_vmemmap_nr(struct 
dev_pagemap *pgmap)
return 1 << pgmap->vmemmap_shift;
 }
 
+static inline bool is_device_private_page(const struct page *page)
+{
+   return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) &&
+   IS_ENABLED(CONFIG_DEVICE_PRIVATE) &&
+   is_zone_device_page(page) &&
+   page->pgmap->type == MEMORY_DEVICE_PRIVATE;
+}
+
+static inline bool is_pci_p2pdma_page(const struct page *page)
+{
+   return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) &&
+   IS_ENABLED(CONFIG_PCI_P2PDMA) &&
+   is_zone_device_page(page) &&
+   page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA;
+}
+
 #ifdef CONFIG_ZONE_DEVICE
 void *memremap_pages(struct dev_pagemap *pgmap, 

[PATCH 5/8] mm: simplify freeing of devmap managed pages

[Christoph Hellwig]

Make put_devmap_managed_page return if it took charge of the page
or not and remove the separate page_is_devmap_managed helper.

Signed-off-by: Christoph Hellwig 
---
 include/linux/mm.h | 34 ++
 mm/memremap.c  | 20 +---
 mm/swap.c  | 10 +-
 3 files changed, 20 insertions(+), 44 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 91dd0bc786a9ec..26baadcef4556b 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1094,33 +1094,24 @@ static inline bool is_zone_movable_page(const struct 
page *page)
 #ifdef CONFIG_DEV_PAGEMAP_OPS
 DECLARE_STATIC_KEY_FALSE(devmap_managed_key);
 
-static inline bool page_is_devmap_managed(struct page *page)
+bool __put_devmap_managed_page(struct page *page);
+static inline bool put_devmap_managed_page(struct page *page)
 {
if (!static_branch_unlikely(_managed_key))
return false;
if (!is_zone_device_page(page))
return false;
-   switch (page->pgmap->type) {
-   case MEMORY_DEVICE_PRIVATE:
-   case MEMORY_DEVICE_FS_DAX:
-   return true;
-   default:
-   break;
-   }
-   return false;
+   if (page->pgmap->type != MEMORY_DEVICE_PRIVATE &&
+   page->pgmap->type != MEMORY_DEVICE_FS_DAX)
+   return false;
+   return __put_devmap_managed_page(page);
 }
 
-void put_devmap_managed_page(struct page *page);
-
 #else /* CONFIG_DEV_PAGEMAP_OPS */
-static inline bool page_is_devmap_managed(struct page *page)
+static inline bool put_devmap_managed_page(struct page *page)
 {
return false;
 }
-
-static inline void put_devmap_managed_page(struct page *page)
-{
-}
 #endif /* CONFIG_DEV_PAGEMAP_OPS */
 
 static inline bool is_device_private_page(const struct page *page)
@@ -1220,16 +1211,11 @@ static inline void put_page(struct page *page)
struct folio *folio = page_folio(page);
 
/*
-* For devmap managed pages we need to catch refcount transition from
-* 2 to 1, when refcount reach one it means the page is free and we
-* need to inform the device driver through callback. See
-* include/linux/memremap.h and HMM for details.
+* For some devmap managed pages we need to catch refcount transition
+* from 2 to 1:
 */
-   if (page_is_devmap_managed(>page)) {
-   put_devmap_managed_page(>page);
+   if (put_devmap_managed_page(>page))
return;
-   }
-
folio_put(folio);
 }
 
diff --git a/mm/memremap.c b/mm/memremap.c
index 55d23e9f5c04ec..f41233a67edb12 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -502,24 +502,22 @@ void free_devmap_managed_page(struct page *page)
page->pgmap->ops->page_free(page);
 }
 
-void put_devmap_managed_page(struct page *page)
+bool __put_devmap_managed_page(struct page *page)
 {
-   int count;
-
-   if (WARN_ON_ONCE(!page_is_devmap_managed(page)))
-   return;
-
-   count = page_ref_dec_return(page);
-
/*
 * devmap page refcounts are 1-based, rather than 0-based: if
 * refcount is 1, then the page is free and the refcount is
 * stable because nobody holds a reference on the page.
 */
-   if (count == 1)
+   switch (page_ref_dec_return(page)) {
+   case 1:
free_devmap_managed_page(page);
-   else if (!count)
+   break;
+   case 0:
__put_page(page);
+   break;
+   }
+   return true;
 }
-EXPORT_SYMBOL(put_devmap_managed_page);
+EXPORT_SYMBOL(__put_devmap_managed_page);
 #endif /* CONFIG_DEV_PAGEMAP_OPS */
diff --git a/mm/swap.c b/mm/swap.c
index 08058f74cae23e..25b55c56614311 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -930,16 +930,8 @@ void release_pages(struct page **pages, int nr)
unlock_page_lruvec_irqrestore(lruvec, flags);
lruvec = NULL;
}
-   /*
-* ZONE_DEVICE pages that return 'false' from
-* page_is_devmap_managed() do not require special
-* processing, and instead, expect a call to
-* put_page_testzero().
-*/
-   if (page_is_devmap_managed(page)) {
-   put_devmap_managed_page(page);
+   if (put_devmap_managed_page(page))
continue;
-   }
if (put_page_testzero(page))
put_dev_pagemap(page->pgmap);
continue;
-- 
2.30.2

[PATCH 4/8] mm: move free_devmap_managed_page to memremap.c

[Christoph Hellwig]

free_devmap_managed_page has nothing to do with the code in swap.c,
move it to live with the rest of the code for devmap handling.

Signed-off-by: Christoph Hellwig 
---
 include/linux/mm.h |  1 -
 mm/memremap.c  | 21 +
 mm/swap.c  | 23 ---
 3 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7b46174989b086..91dd0bc786a9ec 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1092,7 +1092,6 @@ static inline bool is_zone_movable_page(const struct page 
*page)
 }
 
 #ifdef CONFIG_DEV_PAGEMAP_OPS
-void free_devmap_managed_page(struct page *page);
 DECLARE_STATIC_KEY_FALSE(devmap_managed_key);
 
 static inline bool page_is_devmap_managed(struct page *page)
diff --git a/mm/memremap.c b/mm/memremap.c
index 5f04a0709e436e..55d23e9f5c04ec 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -501,4 +501,25 @@ void free_devmap_managed_page(struct page *page)
page->mapping = NULL;
page->pgmap->ops->page_free(page);
 }
+
+void put_devmap_managed_page(struct page *page)
+{
+   int count;
+
+   if (WARN_ON_ONCE(!page_is_devmap_managed(page)))
+   return;
+
+   count = page_ref_dec_return(page);
+
+   /*
+* devmap page refcounts are 1-based, rather than 0-based: if
+* refcount is 1, then the page is free and the refcount is
+* stable because nobody holds a reference on the page.
+*/
+   if (count == 1)
+   free_devmap_managed_page(page);
+   else if (!count)
+   __put_page(page);
+}
+EXPORT_SYMBOL(put_devmap_managed_page);
 #endif /* CONFIG_DEV_PAGEMAP_OPS */
diff --git a/mm/swap.c b/mm/swap.c
index bcf3ac288b56d5..08058f74cae23e 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -1153,26 +1153,3 @@ void __init swap_setup(void)
 * _really_ don't want to cluster much more
 */
 }
-
-#ifdef CONFIG_DEV_PAGEMAP_OPS
-void put_devmap_managed_page(struct page *page)
-{
-   int count;
-
-   if (WARN_ON_ONCE(!page_is_devmap_managed(page)))
-   return;
-
-   count = page_ref_dec_return(page);
-
-   /*
-* devmap page refcounts are 1-based, rather than 0-based: if
-* refcount is 1, then the page is free and the refcount is
-* stable because nobody holds a reference on the page.
-*/
-   if (count == 1)
-   free_devmap_managed_page(page);
-   else if (!count)
-   __put_page(page);
-}
-EXPORT_SYMBOL(put_devmap_managed_page);
-#endif
-- 
2.30.2

[PATCH 3/8] mm: remove pointless includes from

[Christoph Hellwig]

hmm.h pulls in the world for no good reason at all.  Remove the
includes and push a few ones into the users instead.

Signed-off-by: Christoph Hellwig 
---
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 1 +
 drivers/gpu/drm/nouveau/nouveau_dmem.c   | 1 +
 include/linux/hmm.h  | 9 ++---
 lib/test_hmm.c   | 2 ++
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index ed5385137f4831..cb835f95a76e66 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "amdgpu_sync.h"
 #include "amdgpu_object.h"
 #include "amdgpu_vm.h"
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c 
b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 3828aafd3ac46f..e886a3b9e08c7d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -39,6 +39,7 @@
 
 #include 
 #include 
+#include 
 
 /*
  * FIXME: this is ugly right now we are using TTM to allocate vram and we pin
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 2fd2e91d5107c0..d5a6f101f843e6 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -9,14 +9,9 @@
 #ifndef LINUX_HMM_H
 #define LINUX_HMM_H
 
-#include 
-#include 
+#include 
 
-#include 
-#include 
-#include 
-#include 
-#include 
+struct mmu_interval_notifier;
 
 /*
  * On output:
diff --git a/lib/test_hmm.c b/lib/test_hmm.c
index 767538089a62e4..396beee6b061d4 100644
--- a/lib/test_hmm.c
+++ b/lib/test_hmm.c
@@ -26,6 +26,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "test_hmm_uapi.h"
 
-- 
2.30.2

[PATCH 2/8] mm: remove the __KERNEL__ guard from

[Christoph Hellwig]

__KERNEL__ ifdefs don't make sense outside of include/uapi/.

Signed-off-by: Christoph Hellwig 
---
 include/linux/mm.h | 4 
 1 file changed, 4 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 213cc569b19223..7b46174989b086 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3,9 +3,6 @@
 #define _LINUX_MM_H
 
 #include 
-
-#ifdef __KERNEL__
-
 #include 
 #include 
 #include 
@@ -3381,5 +3378,4 @@ madvise_set_anon_name(struct mm_struct *mm, unsigned long 
start,
 }
 #endif
 
-#endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
-- 
2.30.2

start sorting out the ZONE_DEVICE refcount mess

[Christoph Hellwig]

Hi all,

this series removes the offset by one refcount for ZONE_DEVICE pages
that are freed back to the driver owning them, which is just device
private ones for now, but also the planned device coherent pages
and the ehanced p2p ones pending.

It does not address the fsdax pages yet, which will be attacked in a
follow on series.

Diffstat:
 arch/arm64/mm/mmu.c  |1 
 arch/powerpc/kvm/book3s_hv_uvmem.c   |1 
 drivers/gpu/drm/amd/amdkfd/kfd_migrate.c |2 
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h|1 
 drivers/gpu/drm/drm_cache.c  |2 
 drivers/gpu/drm/nouveau/nouveau_dmem.c   |3 -
 drivers/gpu/drm/nouveau/nouveau_svm.c|1 
 drivers/infiniband/core/rw.c |1 
 drivers/nvdimm/pmem.h|1 
 drivers/nvme/host/pci.c  |1 
 drivers/nvme/target/io-cmd-bdev.c|1 
 fs/Kconfig   |2 
 fs/fuse/virtio_fs.c  |1 
 include/linux/hmm.h  |9 
 include/linux/memremap.h |   22 +-
 include/linux/mm.h   |   59 -
 lib/test_hmm.c   |4 +
 mm/Kconfig   |4 -
 mm/internal.h|2 
 mm/memcontrol.c  |   11 +
 mm/memremap.c|   63 ---
 mm/migrate.c |6 --
 mm/swap.c|   49 ++--
 23 files changed, 90 insertions(+), 157 deletions(-)

[PATCH 1/8] mm: remove a pointless CONFIG_ZONE_DEVICE check in memremap_pages

[Christoph Hellwig]

memremap.c is only built when CONFIG_ZONE_DEVICE is set, so remove
the superflous extra check.

Signed-off-by: Christoph Hellwig 
---
 mm/memremap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/memremap.c b/mm/memremap.c
index 6aa5f0c2d11fda..5f04a0709e436e 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -328,8 +328,7 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid)
}
break;
case MEMORY_DEVICE_FS_DAX:
-   if (!IS_ENABLED(CONFIG_ZONE_DEVICE) ||
-   IS_ENABLED(CONFIG_FS_DAX_LIMITED)) {
+   if (IS_ENABLED(CONFIG_FS_DAX_LIMITED)) {
WARN(1, "File system DAX not supported\n");
return ERR_PTR(-EINVAL);
}
-- 
2.30.2

RE: [PATCH v1] drm/amdgpu: Print once if RAS unsupported

[Zhou1, Tao]

[AMD Official Use Only]

Reviewed-by: Tao Zhou 

> -Original Message-
> From: Tuikov, Luben 
> Sent: Friday, February 4, 2022 7:13 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Tuikov, Luben ; Deucher, Alexander
> ; Zhang, Hawking ;
> Clements, John ; Zhou1, Tao
> ; Chai, Thomas 
> Subject: [PATCH v1] drm/amdgpu: Print once if RAS unsupported
> 
> MESA polls for errors every 2-3 seconds. Printing with dev_info() causes the
> dmesg log to fill up with the same message, e.g,
> 
> [18028.206676] amdgpu :0b:00.0: amdgpu: df doesn't config ras function.
> 
> Make it dev_dbg_once(), as it isn't something correctible during boot or
> thereafter, so printing just once is sufficient. Also sanitize the message.
> 
> Cc: Alex Deucher 
> Cc: Hawking Zhang 
> Cc: John Clements 
> Cc: Tao Zhou 
> Cc: yipechai 
> Fixes: e93ea3d0cf434b ("drm/amdgpu: Modify gfx block to fit for the unified 
> ras
> block data and ops")
> Signed-off-by: Luben Tuikov 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index 9d7c778c1a2d8e..e440a5268acecf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -952,8 +952,8 @@ int amdgpu_ras_query_error_status(struct
> amdgpu_device *adev,
>   } else {
>   block_obj = amdgpu_ras_get_ras_block(adev, info->head.block,
> 0);
>   if (!block_obj || !block_obj->hw_ops)   {
> - dev_info(adev->dev, "%s doesn't config ras function.\n",
> - get_ras_block_str(>head));
> + dev_dbg_once(adev->dev, "%s doesn't config RAS
> function\n",
> +  get_ras_block_str(>head));
>   return -EINVAL;
>   }
> 
> @@ -1028,8 +1028,8 @@ int amdgpu_ras_reset_error_status(struct
> amdgpu_device *adev,
>   return -EINVAL;
> 
>   if (!block_obj || !block_obj->hw_ops)   {
> - dev_info(adev->dev, "%s doesn't config ras function.\n",
> - ras_block_str(block));
> + dev_dbg_once(adev->dev, "%s doesn't config RAS function\n",
> +  ras_block_str(block));
>   return -EINVAL;
>   }
> 
> @@ -1066,8 +1066,8 @@ int amdgpu_ras_error_inject(struct amdgpu_device
> *adev,
>   return -EINVAL;
> 
>   if (!block_obj || !block_obj->hw_ops)   {
> - dev_info(adev->dev, "%s doesn't config ras function.\n",
> - get_ras_block_str(>head));
> + dev_dbg_once(adev->dev, "%s doesn't config RAS function\n",
> +  get_ras_block_str(>head));
>   return -EINVAL;
>   }
> 
> @@ -1717,8 +1717,8 @@ static void amdgpu_ras_error_status_query(struct
> amdgpu_device *adev,
>   info->head.sub_block_index);
> 
>   if (!block_obj || !block_obj->hw_ops) {
> - dev_info(adev->dev, "%s doesn't config ras function.\n",
> - get_ras_block_str(>head));
> + dev_dbg_once(adev->dev, "%s doesn't config RAS function\n",
> +  get_ras_block_str(>head));
>   return;
>   }
> 
> 
> base-commit: cf33ae90884f254d683436fc2538b99dc4932447
> --
> 2.35.0.3.gb23dac905b

RE: [PATCH] drm/amd/pm: add missing prototypes to amdgpu_dpm_internal

[Quan, Evan]

[AMD Official Use Only]

Thanks for the fix!
Reviewed-by: Evan Quan 

> -Original Message-
> From: Maíra Canal 
> Sent: Thursday, February 3, 2022 8:40 AM
> To: Quan, Evan ; Deucher, Alexander
> ; Koenig, Christian
> ; Pan, Xinhui ;
> airl...@linux.ie; dan...@ffwll.ch; nat...@kernel.org;
> ndesaulni...@google.com; Lazar, Lijo ; Tuikov, Luben
> ; Chen, Guchun ;
> Zhang, Hawking ;
> jiapeng.ch...@linux.alibaba.com
> Cc: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH] drm/amd/pm: add missing prototypes to
> amdgpu_dpm_internal
> 
> Include the header with the prototype to silence the following clang
> warnings:
> 
> drivers/gpu/drm/amd/amdgpu/../pm/amdgpu_dpm_internal.c:29:6:
> warning: no
> previous prototype for function 'amdgpu_dpm_get_active_displays'
> [-Wmissing-prototypes]
> void amdgpu_dpm_get_active_displays(struct amdgpu_device *adev)
>  ^
> drivers/gpu/drm/amd/amdgpu/../pm/amdgpu_dpm_internal.c:29:1: note:
> declare
> 'static' if the function is not intended to be used outside of this
> translation unit
> void amdgpu_dpm_get_active_displays(struct amdgpu_device *adev)
> ^
> static
> drivers/gpu/drm/amd/amdgpu/../pm/amdgpu_dpm_internal.c:76:5:
> warning: no
> previous prototype for function 'amdgpu_dpm_get_vrefresh'
> [-Wmissing-prototypes]
> u32 amdgpu_dpm_get_vrefresh(struct amdgpu_device *adev)
> ^
> drivers/gpu/drm/amd/amdgpu/../pm/amdgpu_dpm_internal.c:76:1: note:
> declare
> 'static' if the function is not intended to be used outside of this
> translation unit
> u32 amdgpu_dpm_get_vrefresh(struct amdgpu_device *adev)
> ^
> static
> 2 warnings generated.
> 
> Besides that, remove the duplicated prototype of the function
> amdgpu_dpm_get_vblank_time in order to keep the consistency of the
> headers.
> 
> fixes: 6ddbd37f ("drm/amd/pm: optimize the amdgpu_pm_compute_clocks()
> implementations")
> 
> Signed-off-by: Maíra Canal 
> ---
>  drivers/gpu/drm/amd/pm/amdgpu_dpm_internal.c | 1 +
>  drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h  | 1 -
>  drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c   | 1 +
>  3 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm_internal.c
> b/drivers/gpu/drm/amd/pm/amdgpu_dpm_internal.c
> index ba5f6413412d..42efe838fa85 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm_internal.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm_internal.c
> @@ -25,6 +25,7 @@
>  #include "amdgpu_display.h"
>  #include "hwmgr.h"
>  #include "amdgpu_smu.h"
> +#include "amdgpu_dpm_internal.h"
> 
>  void amdgpu_dpm_get_active_displays(struct amdgpu_device *adev)
>  {
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> index 5cc05110cdae..09790413cbc4 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> @@ -343,7 +343,6 @@ struct amdgpu_pm {
>   struct amdgpu_ctx   *stable_pstate_ctx;
>  };
> 
> -u32 amdgpu_dpm_get_vblank_time(struct amdgpu_device *adev);
>  int amdgpu_dpm_read_sensor(struct amdgpu_device *adev, enum
> amd_pp_sensors sensor,
>  void *data, uint32_t *size);
> 
> diff --git a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> index 7427c50409d4..caae54487f9c 100644
> --- a/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c
> @@ -28,6 +28,7 @@
>  #include "amdgpu_pm.h"
>  #include "amdgpu_dpm.h"
>  #include "amdgpu_atombios.h"
> +#include "amdgpu_dpm_internal.h"
>  #include "amd_pcie.h"
>  #include "sid.h"
>  #include "r600_dpm.h"
> --
> 2.34.1

Re: [RFC v2 4/8] drm/amdgpu: Serialize non TDR gpu recovery with TDRs

[Grodzovsky, Andrey]

I already did, thanks to Shayun I already tested on XGMI SRIOV and it looks ok. 
What I need now is code review, mostly on the new patches (8-12). I hope you, 
Monk, Shayun, Lijo and Christian can help with that.

Andrey

From: Chen, JingWen 
Sent: 06 February 2022 21:41
To: Grodzovsky, Andrey ; Christian König 
; Koenig, Christian 
; Lazar, Lijo ; 
dri-de...@lists.freedesktop.org ; 
amd-gfx@lists.freedesktop.org ; Chen, JingWen 

Cc: Chen, Horace ; Liu, Monk 
Subject: Re: [RFC v2 4/8] drm/amdgpu: Serialize non TDR gpu recovery with TDRs


Hi Andrey,

I don't have any XGMI machines here, maybe you can reach out shaoyun for help.

On 2022/1/29 上午12:57, Grodzovsky, Andrey wrote:
Just a gentle ping.

Andrey

From: Grodzovsky, Andrey
Sent: 26 January 2022 10:52
To: Christian König 
; 
Koenig, Christian ; 
Lazar, Lijo ; 
dri-de...@lists.freedesktop.org 
; 
amd-gfx@lists.freedesktop.org 
; Chen, 
JingWen 
Cc: Chen, Horace ; Liu, Monk 

Subject: Re: [RFC v2 4/8] drm/amdgpu: Serialize non TDR gpu recovery with TDRs


JingWen - could you maybe give those patches a try on SRIOV XGMI system ? If 
you see issues maybe you could let me connect and debug. My SRIOV XGMI system 
which Shayun kindly arranged for me is not loading the driver with my 
drm-misc-next branch even without my patches.

Andrey

On 2022-01-17 14:21, Andrey Grodzovsky wrote:


On 2022-01-17 2:17 p.m., Christian König wrote:
Am 17.01.22 um 20:14 schrieb Andrey Grodzovsky:

Ping on the question

Oh, my! That was already more than a week ago and is completely swapped out of 
my head again.


Andrey

On 2022-01-05 1:11 p.m., Andrey Grodzovsky wrote:
Also, what about having the reset_active or in_reset flag in the reset_domain 
itself?

Of hand that sounds like a good idea.


What then about the adev->reset_sem semaphore ? Should we also move this to 
reset_domain ?  Both of the moves have functional
implications only for XGMI case because there will be contention over accessing 
those single instance variables from multiple devices
while now each device has it's own copy.

Since this is a rw semaphore that should be unproblematic I think. It could 
just be that the cache line of the lock then plays ping/pong between the CPU 
cores.


What benefit the centralization into reset_domain gives - is it for example to 
prevent one device in a hive trying to access through MMIO another one's
VRAM (shared FB memory) while the other one goes through reset ?

I think that this is the killer argument for a centralized lock, yes.


np, i will add a patch with centralizing both flag into reset domain and resend.

Andrey


Christian.


Andrey

RE: [PATCH] drm/amd/pm: fix error handling

[Quan, Evan]

[AMD Official Use Only]

Reviewed-by: Evan Quan 

> -Original Message-
> From: t...@redhat.com 
> Sent: Saturday, February 5, 2022 11:00 PM
> To: Quan, Evan ; Deucher, Alexander
> ; Koenig, Christian
> ; Pan, Xinhui ;
> airl...@linux.ie; dan...@ffwll.ch; nat...@kernel.org;
> ndesaulni...@google.com; Lazar, Lijo ; Powell, Darren
> ; Chen, Guchun ;
> Grodzovsky, Andrey 
> Cc: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; linux-
> ker...@vger.kernel.org; l...@lists.linux.dev; Tom Rix 
> Subject: [PATCH] drm/amd/pm: fix error handling
> 
> From: Tom Rix 
> 
> clang static analysis reports this error
> amdgpu_smu.c:2289:9: warning: Called function pointer
>   is null (null dereference)
> return smu->ppt_funcs->emit_clk_levels(
>^~~~
> 
> There is a logic error in the earlier check of
> emit_clk_levels.  The error value is set to
> the ret variable but ret is never used.  Return
> directly and remove the unneeded ret variable.
> 
> Fixes: 5d64f9bbb628 ("amdgpu/pm: Implement new API function "emit" that
> accepts buffer base and write offset")
> Signed-off-by: Tom Rix 
> ---
>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index af368aa1fd0ae..5f3b3745a9b7a 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -2274,7 +2274,6 @@ static int smu_emit_ppclk_levels(void *handle,
> enum pp_clock_type type, char *bu
>  {
>   struct smu_context *smu = handle;
>   enum smu_clk_type clk_type;
> - int ret = 0;
> 
>   clk_type = smu_convert_to_smuclk(type);
>   if (clk_type == SMU_CLK_COUNT)
> @@ -2284,7 +2283,7 @@ static int smu_emit_ppclk_levels(void *handle,
> enum pp_clock_type type, char *bu
>   return -EOPNOTSUPP;
> 
>   if (!smu->ppt_funcs->emit_clk_levels)
> - ret = -ENOENT;
> + return -ENOENT;
> 
>   return smu->ppt_funcs->emit_clk_levels(smu, clk_type, buf, offset);
> 
> --
> 2.26.3

[PATCH 2/2] drm/amdgpu: check the GART table before invalidating TLB

[Aaron Liu]

Bypass group programming (utcl2_harvest) aims to forbid UTCL2 to send
invalidation command to harvested SE/SA. Once invalidation command comes
into harvested SE/SA, SE/SA has no response and system hang.

This patch is to add checking if the GART table is already allocated before
invalidating TLB. The new procedure is as following:
1. Calling amdgpu_gtt_mgr_init() in amdgpu_ttm_init(). After this step GTT
   BOs can be allocated, but GART mappings are still ignored.
2. Calling amdgpu_gart_table_vram_alloc() from the GMC code. This allocates
   the GART backing store.
3. Initializing the hardware, and programming the backing store into VMID0
   for all VMHUBs.
4. Calling amdgpu_gtt_mgr_recover() to make sure the table is updated with
   the GTT allocations done before it was allocated.

Signed-off-by: Christian König 
Signed-off-by: Aaron Liu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index 91d8207336c1..01cb89ffbd56 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -259,6 +259,9 @@ void amdgpu_gart_invalidate_tlb(struct amdgpu_device *adev)
 {
int i;
 
+   if (!adev->gart.ptr)
+   return;
+
mb();
amdgpu_device_flush_hdp(adev, NULL);
for (i = 0; i < adev->num_vmhubs; i++)
-- 
2.25.1

[PATCH 1/2] drm/amdgpu: add utcl2_harvest to gc 10.3.1

[Aaron Liu]

Confirmed with hardware team, there is harvesting for gc 10.3.1.

Signed-off-by: Aaron Liu 
---
 drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c 
b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
index b4eddf6e98a6..ff738e9725ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_1.c
@@ -543,7 +543,9 @@ static void gfxhub_v2_1_utcl2_harvest(struct amdgpu_device 
*adev)
adev->gfx.config.max_sh_per_se *
adev->gfx.config.max_shader_engines);
 
-   if (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 3, 3)) {
+   switch (adev->ip_versions[GC_HWIP][0]) {
+   case IP_VERSION(10, 3, 1):
+   case IP_VERSION(10, 3, 3):
/* Get SA disabled bitmap from eFuse setting */
efuse_setting = RREG32_SOC15(GC, 0, mmCC_GC_SA_UNIT_DISABLE);
efuse_setting &= CC_GC_SA_UNIT_DISABLE__SA_DISABLE_MASK;
@@ -566,6 +568,9 @@ static void gfxhub_v2_1_utcl2_harvest(struct amdgpu_device 
*adev)
disabled_sa = tmp;
 
WREG32_SOC15(GC, 0, 
mmGCUTCL2_HARVEST_BYPASS_GROUPS_YELLOW_CARP, disabled_sa);
+   break;
+   default:
+   break;
}
 }
 
-- 
2.25.1

Re: [RFC v2 4/8] drm/amdgpu: Serialize non TDR gpu recovery with TDRs

[JingWen Chen]

Hi Andrey,

I don't have any XGMI machines here, maybe you can reach out shaoyun for help.

On 2022/1/29 上午12:57, Grodzovsky, Andrey wrote:
> Just a gentle ping.
>
> Andrey
> --
> *From:* Grodzovsky, Andrey
> *Sent:* 26 January 2022 10:52
> *To:* Christian König ; Koenig, Christian 
> ; Lazar, Lijo ; 
> dri-de...@lists.freedesktop.org ; 
> amd-gfx@lists.freedesktop.org ; Chen, JingWen 
> 
> *Cc:* Chen, Horace ; Liu, Monk 
> *Subject:* Re: [RFC v2 4/8] drm/amdgpu: Serialize non TDR gpu recovery with 
> TDRs
>  
>
> JingWen - could you maybe give those patches a try on SRIOV XGMI system ? If 
> you see issues maybe you could let me connect and debug. My SRIOV XGMI system 
> which Shayun kindly arranged for me is not loading the driver with my 
> drm-misc-next branch even without my patches.
>
> Andrey
>
> On 2022-01-17 14:21, Andrey Grodzovsky wrote:
>>
>>
>> On 2022-01-17 2:17 p.m., Christian König wrote:
>>> Am 17.01.22 um 20:14 schrieb Andrey Grodzovsky:

 Ping on the question

>>>
>>> Oh, my! That was already more than a week ago and is completely swapped out 
>>> of my head again.
>>>
 Andrey

 On 2022-01-05 1:11 p.m., Andrey Grodzovsky wrote:
>>> Also, what about having the reset_active or in_reset flag in the 
>>> reset_domain itself?
>>
>> Of hand that sounds like a good idea.
>
>
> What then about the adev->reset_sem semaphore ? Should we also move this 
> to reset_domain ?  Both of the moves have functional
> implications only for XGMI case because there will be contention over 
> accessing those single instance variables from multiple devices
> while now each device has it's own copy.
>>>
>>> Since this is a rw semaphore that should be unproblematic I think. It could 
>>> just be that the cache line of the lock then plays ping/pong between the 
>>> CPU cores.
>>>
>
> What benefit the centralization into reset_domain gives - is it for 
> example to prevent one device in a hive trying to access through MMIO 
> another one's
> VRAM (shared FB memory) while the other one goes through reset ?
>>>
>>> I think that this is the killer argument for a centralized lock, yes.
>>
>>
>> np, i will add a patch with centralizing both flag into reset domain and 
>> resend.
>>
>> Andrey
>>
>>
>>>
>>> Christian.
>>>
>
> Andrey 
>>>

RE: [PATCH 1/2] drm/amdgpu: Fixed the defect of soft lock caused by infinite loop

[Chai, Thomas]

OK

-Original Message-
From: Kuehling, Felix  
Sent: Tuesday, February 1, 2022 12:24 AM
To: Zhou1, Tao ; Chai, Thomas ; 
amd-gfx@lists.freedesktop.org
Cc: Clements, John ; Zhang, Hawking 

Subject: Re: [PATCH 1/2] drm/amdgpu: Fixed the defect of soft lock caused by 
infinite loop


Am 2022-01-29 um 22:19 schrieb Zhou1, Tao:
> [AMD Official Use Only]
>
>
>
>> -Original Message-
>> From: Chai, Thomas 
>> Sent: Saturday, January 29, 2022 8:34 PM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Chai, Thomas ; Zhang, Hawking 
>> ; Zhou1, Tao ; Clements, 
>> John ; Chai, Thomas 
>> Subject: [PATCH 1/2] drm/amdgpu: Fixed the defect of soft lock caused 
>> by infinite loop
>>
>> 1. The infinite loop case only occurs on multiple cards support
>> ras functions.
>> 2. The explanation of root cause refer to 76641cbbf196523b5752c6cf68f86.
>> 3. Create new node to manage each unique ras instance to guarantee
>> each device .ras_list is completely independent.
>> 4. Fixes:7a6b8ab3231b511915cb94cac1debabf093.
>> 5. The soft locked logs are as follows:
>> [  262.165690] CPU: 93 PID: 758 Comm: kworker/93:1 Tainted: G   OE
>> 5.13.0-27-generic #29~20.04.1-Ubuntu
>> [  262.165695] Hardware name: Supermicro AS -4124GS-TNR/H12DSG-O-CPU, 
>> BIOS T20200717143848 07/17/2020 [  262.165698] Workqueue: events 
>> amdgpu_ras_do_recovery [amdgpu] [  262.165980] RIP:
>> 0010:amdgpu_ras_get_ras_block+0x86/0xd0 [amdgpu] [  262.166239] Code: 
>> 68
>> d8 4c 8d 71 d8 48 39 c3 74 54 49 8b 45 38 48 85 c0 74 32 44 89 fa 44 
>> 89 e6 4c 89 ef e8 82 e4 9b dc 85 c0 74 3c 49 8b 46 28 <49> 8d 56 28 
>> 4d 89 f5 48 83 e8 28 48
>> 39 d3 74 25 49 89 c6 49 8b 45 [  262.166243] RSP: 
>> 0018:ac908fa87d80
>> EFLAGS: 0202 [  262.166247] RAX: c1394248 RBX: 
>> 91e4ab8d6e20
>> RCX: c1394248 [  262.166249] RDX: 91e4aa356e20 RSI:
>> 000e RDI: 91e4ab8c [  262.166252] RBP:
>> ac908fa87da8 R08: 0007 R09: 0001 [  
>> 262.166254] R10: 91e4930b64ec R11:  R12:
>> 000e [  262.166256] R13: 91e4aa356df8 R14: 
>> c1394320
>> R15: 0003 [  262.166258] FS:  ()
>> GS:92238fb4() knlGS: [  262.166261] CS:  
>> 0010
>> DS:  ES:  CR0: 80050033 [  262.166264] CR2:
>> 0001004865d0 CR3: 00406d796000 CR4: 00350ee0 [  
>> 262.166267] Call Trace:
>> [  262.166272]  amdgpu_ras_do_recovery+0x130/0x290 [amdgpu] [  
>> 262.166529]  ? psi_task_switch+0xd2/0x250 [  262.166537]  ?
>> __switch_to+0x11d/0x460 [  262.166542]  ? __switch_to_asm+0x36/0x70 [  
>> 262.166549]  process_one_work+0x220/0x3c0 [  262.166556]
>> worker_thread+0x4d/0x3f0 [  262.166560]  ? 
>> process_one_work+0x3c0/0x3c0 [  262.166563]  kthread+0x12b/0x150 [  
>> 262.166568]  ?
>> set_kthread_struct+0x40/0x40 [  262.166571]  ret_from_fork+0x22/0x30
>>
>> Signed-off-by: yipechai 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 37 
>> ++--
>> -  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h |  3 --
>>   2 files changed, 33 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
>> index 9d7c778c1a2d..b0aa67308c31 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
>> @@ -75,6 +75,13 @@ const char *ras_mca_block_string[] = {
>>  "mca_iohc",
>>   };
>>
>> +struct amdgpu_ras_block_list {
>> +/* ras block link */
>> +struct list_head node;
>> +
>> +struct amdgpu_ras_block_object *ras_obj; };
>> +
>>   const char *get_ras_block_str(struct ras_common_if *ras_block)  {
>>  if (!ras_block)
>> @@ -880,7 +887,8 @@ static struct amdgpu_ras_block_object 
>> *amdgpu_ras_get_ras_block(struct amdgpu_de
>>  enum amdgpu_ras_block block,
>> uint32_t sub_block_index)  {
>>  int loop_cnt = 0;
>> -struct amdgpu_ras_block_object *obj, *tmp;
>> +struct amdgpu_ras_block_list *node, *tmp;
>> +struct amdgpu_ras_block_object *obj;
>>
>>  if (block >= AMDGPU_RAS_BLOCK__LAST)
>>  return NULL;
>> @@ -888,7 +896,13 @@ static struct amdgpu_ras_block_object 
>> *amdgpu_ras_get_ras_block(struct amdgpu_de
>>  if (!amdgpu_ras_is_supported(adev, block))
>>  return NULL;
>>
>> -list_for_each_entry_safe(obj, tmp, >ras_list, node) {
>> +list_for_each_entry_safe(node, tmp, >ras_list, node) {
>> +if (!node->ras_obj) {
>> +DRM_ERROR("Warning: abnormal ras list node");
> [Tao]: dev_warn is recommended.
>
>> +continue;
>> +}
>> +
>> +obj = node->ras_obj;
>>  if (obj->ras_block_match) {
>>  if (obj->ras_block_match(obj, block, sub_block_index) 
>> == 0)
>>  return obj;
>> @@ -2527,6 +2541,7 @@ int amdgpu_ras_pre_fini(struct 

Re: [BUG] gpu: drm: radeon: two possible deadlocks involving locking and waiting

[Christian König]

Am 05.02.22 um 02:55 schrieb Jia-Ju Bai:

Hi Christian,

Thanks for the reply :)

On 2022/2/1 15:56, Christian König wrote:

Hi Jia-Ju,

interesting that you have found those issues with an automated tool.

And yes that is a well design flaw within the radeon driver which can 
happen on hardware faults, e.g. when radeon_ring_backup() needs to be 
called.


In fact, my tool finds dozens of similar possible deadlocks caused by 
wait_event_timeout() in radeon_fence_wait_seq_timeout().


Those are false positives.

The call to radeon_fence_process() from radeon_fence_count_emitted() for 
example is just to speed things up, it's not mandatory for correct 
operation and so it doesn't matter if it isn't called because of the 
thread is blocked on the pm.mutex.


But I also don't see how your tool should be able to figure that out 
automated.


Regards,
Christian.


There are three other examples in Linux 5.16:

#BUG 1
radeon_dpm_change_power_state_locked()
  mutex_lock(>ring_lock); --> Line 1133 (Lock A)
  radeon_fence_wait_empty()
    radeon_fence_wait_seq_timeout()
      wait_event_timeout(rdev->fence_queue, ...) --> Line 504 (Wait X)

radeon_fence_driver_fini()
  mutex_lock(>ring_lock); --> Line 917 (Lock A)
  wake_up_all(>fence_queue); --> Line 927 (Wake X)

#BUG 2
radeon_set_pm_profile()
  mutex_lock(>pm.mutex); --> Line 382 (Lock A)
  radeon_pm_set_clocks()
    radeon_fence_wait_empty()
  radeon_fence_wait_seq_timeout()
        wait_event_timeout(rdev->fence_queue, ...) --> Line 504 (Wait X)

radeon_dynpm_idle_work_handler()
  mutex_lock(>pm.mutex); --> Line 1861 (Lock A)
  radeon_fence_count_emitted()
    radeon_fence_process()
  wake_up_all(>fence_queue); --> Line 323 (Wake X)

#BUG 3
radeon_pm_fini_old()
  mutex_lock(>pm.mutex); --> Line 1642 (Lock A)
  radeon_pm_set_clocks()
    radeon_fence_wait_empty()
  radeon_fence_wait_seq_timeout()
        wait_event_timeout(rdev->fence_queue, ...) --> Line 504 (Wait X)

radeon_dynpm_idle_work_handler()
  mutex_lock(>pm.mutex); --> Line 1861 (Lock A)
  radeon_fence_count_emitted()
    radeon_fence_process()
  wake_up_all(>fence_queue); --> Line 323 (Wake X)

Thus, to fix these possible deadlocks, we could moditify the code 
related to radeon_fence_wait_seq_timeout().
But I am not quite familar with the radeon driver, so I am not sure 
how to moditify the code properly.




But that happens so rarely and the driver is not developed further 
that we decided to not address this any more.


Ah, okay.



Regards,
Christian.

Am 01.02.22 um 08:40 schrieb Jia-Ju Bai:

Hello,

My static analysis tool reports a possible deadlock in the radeon 
driver in Linux 5.16:


#BUG 1
radeon_dpm_change_power_state_locked()
  mutex_lock(>ring_lock); --> Line 1133 (Lock A)
  radeon_fence_wait_empty()
    radeon_fence_wait_seq_timeout()
      wait_event_timeout(rdev->fence_queue, ...) --> Line 504 (Wait X)

radeon_ring_backup()
  mutex_lock(>ring_lock); --> Line 289(Lock A)
  radeon_fence_count_emitted()
    radeon_fence_process()
  wake_up_all(>fence_queue); --> Line 323 (Wake X)

When radeon_dpm_change_power_state_locked() is executed, "Wait X" is 
performed by holding "Lock A". If radeon_ring_backup() is executed 
at this time, "Wake X" cannot be performed to wake up "Wait X" in 
radeon_dpm_change_power_state_locked(), because "Lock A" has been 
already hold by radeon_dpm_change_power_state_locked(), causing a 
possible deadlock.
I find that "Wait X" is performed with a timeout 
MAX_SCHEDULE_TIMEOUT, to relieve the possible deadlock; but I think 
this timeout can cause inefficient execution.


#BUG 2
radeon_ring_lock()
  mutex_lock(>ring_lock); --> Line 147 (Lock A)
  radeon_ring_alloc()
    radeon_fence_wait_next()
  radeon_fence_wait_seq_timeout()
        wait_event_timeout(rdev->fence_queue, ...) --> Line 504 
(Wait X)


radeon_ring_backup()
  mutex_lock(>ring_lock); --> Line 289(Lock A)
  radeon_fence_count_emitted()
    radeon_fence_process()
  wake_up_all(>fence_queue); --> Line 323 (Wake X)

When radeon_ring_lock() is executed, "Wait X" is performed by 
holding "Lock A". If radeon_ring_backup() is executed at this time, 
"Wake X" cannot be performed to wake up "Wait X" in 
radeon_ring_lock(), because "Lock A" has been already hold by 
radeon_ring_lock(), causing a possible deadlock.
I find that "Wait X" is performed with a timeout 
MAX_SCHEDULE_TIMEOUT, to relieve the possible deadlock; but I think 
this timeout can cause inefficient execution.


I am not quite sure whether these possible problems are real and how 
to fix them if they are real.

Any feedback would be appreciated, thanks :)


Best wishes,
Jia-Ju Bai

Re: Minimal GPU setup

[Christian König]

Hi Amol,

Am 05.02.22 um 10:47 schrieb Amol:

Hello,

I am learning to program Radeon HD 7350 by reading the radeon
driver source in Linux, and the guides/manuals from AMD.

I understand the general flow of initialization the driver performs. I
have also been able to understand and re-implement the ATOM
BIOS virtual machine.


That sounds like you already came pretty far.


I am trying to program the device up from scratch (i.e. bare-metal).
Do I need to perform all those steps that the driver does? Reading
the evergreen_gpu_init function is demotivating; it initializes many
fields and registers which I suspect may not be required for a minimal
setup.

Is posting the BIOS and loading the microcode enough to get me started
with running basic tasks (DMA transfers, simple packet processing, etc.)?


Well yes and no. As bare minimum you need the following:
1. Firmware loading
2. Memory management
3. Ring buffer setup
4. Hardware initialization

When that is done you can write commands into the ring buffers of the CP 
or SDMA and see if they are executed (see the *_ring_test() functions in 
the driver). SDMA is usually easier to get working.


When you got that working you can worry about IB (indirect buffers) 
which are basically subroutines calls written into the ring buffers.


Most commands (like copy from A to B, fill something, write value X to 
memory or write X into register Y) can be used from the ring buffers 
directly, but IIRC some context switching commands which are part of the 
rendering process require special handling.


But keep in mind that all of this will just be horrible slow because the 
ASIC runs with the bootup clocks which are something like 100Mhz or even 
only 17Mhz on very old models. To change that you need to implement 
power management, interrupt handling etc etc


Good luck,
Christian.



Thanks,
Amol