[PATCH 1/7] Revert "drm/amdgpu: mark A+A VF VRAM mappings as system memory"

[PATCH 1/7] Revert "drm/amdgpu: mark A+A VF VRAM mappings as system memory"

[Zhigang Luo]

This reverts commit 521bedfc94cc.

The patch sets AMDGPU_PTE_SYSTEM in amdgpu_ttm_tt_pde_flags() and
converts the VRAM address with amdgpu_gmc_vram_mc2pa() inside
amdgpu_gmc_get_pde_for_bo(). gmc_v12_1_get_vm_pde() already converts
an MC address to a physical address and skips that conversion when
the SYSTEM bit is set, so setting the bit first bypasses the
translation.

The bit belongs in the GFX 12.1 helpers, after that translation.
PDB0 is written directly and does not go through get_vm_pde(), and
NPA still requires PTE.S=0, so those two pieces come back in the
following patch.

Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    |  9 ---------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 14 +++-----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c |  1 -
 drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c     |  1 -
 4 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 4204c3c750a8..659e89e5e78f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -122,11 +122,6 @@ void amdgpu_gmc_get_pde_for_bo(struct amdgpu_bo *bo, int level,
 		break;
 	case TTM_PL_VRAM:
 		*addr = amdgpu_bo_gpu_offset(bo);
-		/* For SPM-backed VRAM, convert the MC address to a physical
-		 * address for system memory path access.
-		 */
-		if (amdgpu_virt_vram_is_spm(adev))
-			*addr = amdgpu_gmc_vram_mc2pa(adev, *addr);
 		break;
 	default:
 		*addr = 0;
@@ -1286,8 +1281,6 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
 	flags |= AMDGPU_PTE_VALID | AMDGPU_PTE_READABLE;
 	flags |= AMDGPU_PTE_WRITEABLE;
 	flags |= AMDGPU_PTE_SNOOPED;
-	if (amdgpu_virt_vram_is_spm(adev))
-		flags |= AMDGPU_PTE_SYSTEM;
 	flags |= AMDGPU_PTE_FRAG((adev->gmc.vmid0_page_table_block_size + 9*1));
 	flags |= AMDGPU_PDE_PTE_FLAG(adev);
 
@@ -1307,8 +1300,6 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
 	 * pointing to a 4K system page
 	 */
 	flags = AMDGPU_PTE_VALID;
-	if (amdgpu_virt_vram_is_spm(adev))
-		flags |= AMDGPU_PTE_SYSTEM;
 	flags |= AMDGPU_PTE_SNOOPED | AMDGPU_PDE_BFS_FLAG(adev, 0);
 	/* Requires gart_ptb_gpu_pa to be 4K aligned */
 	amdgpu_gmc_set_pte_pde(adev, adev->gmc.ptr_pdb0, i, gart_ptb_gpu_pa, flags);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 212d86adffd8..37f7f421b470 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1461,17 +1461,9 @@ uint64_t amdgpu_ttm_tt_pde_flags(struct ttm_tt *ttm, struct ttm_resource *mem)
 			flags |= AMDGPU_PTE_SNOOPED;
 	}
 
-	if (mem && mem->mem_type == TTM_PL_VRAM) {
-		if (mem->bo) {
-			struct amdgpu_device *adev = amdgpu_ttm_adev(mem->bo->bdev);
-
-			if (amdgpu_virt_vram_is_spm(adev))
-				flags |= AMDGPU_PTE_SYSTEM;
-		}
-
-		if (mem->bus.caching == ttm_cached)
-			flags |= AMDGPU_PTE_SNOOPED;
-	}
+	if (mem && mem->mem_type == TTM_PL_VRAM &&
+			mem->bus.caching == ttm_cached)
+		flags |= AMDGPU_PTE_SNOOPED;
 
 	return flags;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index 11f022c56667..3bb426b33af9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -6171,7 +6171,6 @@ static int amdgpu_ualink_peer_remote_init(struct amdgpu_device *adev)
 	flags = amdgpu_ttm_tt_pte_flags(adev, bo->tbo.ttm, bo->tbo.resource);
 	flags |= AMDGPU_PTE_SNOOPED | AMDGPU_PTE_PRT_GFX12 | AMDGPU_PTE_BUS_ATOMICS;
 	flags &= ~AMDGPU_PTE_VALID;
-	flags &= ~AMDGPU_PTE_SYSTEM;
 	/* PTE.X=0 turn off RPC checks for RBs, wptr and doorbell NPA address */
 	flags &= ~AMDGPU_PTE_EXECUTABLE;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c
index b67683012002..76391fca6c62 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c
@@ -616,7 +616,6 @@ static void gmc_v12_1_get_npa_flags(struct amdgpu_device *adev,
 	*flags |= AMDGPU_PTE_SNOOPED | AMDGPU_PTE_PRT_GFX12 |
 		   AMDGPU_PTE_BUS_ATOMICS;
 	*flags &= ~AMDGPU_PTE_VALID;
-	*flags &= ~AMDGPU_PTE_SYSTEM;
 	*flags &= ~AMDGPU_PTE_EXECUTABLE;
 }
 
-- 
2.43.0

[PATCH 2/7] Revert "drm/amdkfd: mark A+A VF VRAM PTEs as system memory"

[Zhigang Luo]

This reverts commit daf19be40e04.

SVM VRAM ranges leave AMDGPU_PTE_SYSTEM clear and then call
amdgpu_gmc_get_vm_pte() with a NULL buffer object. The GFX 12.1
PTE helper sets the bit for an A+A VF, so the special case in
svm_range_get_pte_flags() is the wrong place.

Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 876476046926..a5aed7853775 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1398,13 +1398,7 @@ svm_range_get_pte_flags(struct kfd_node *node, struct amdgpu_vm *vm,
 		mapping_flags |= AMDGPU_VM_PAGE_EXECUTABLE;
 
 	pte_flags = AMDGPU_PTE_VALID;
-	/* SPM-backed VRAM is accessed through the system memory path and
-	 * requires the SYSTEM bit.
-	 */
-	if (amdgpu_virt_vram_is_spm(node->adev))
-		pte_flags |= AMDGPU_PTE_SYSTEM;
-	else
-		pte_flags |= (domain == SVM_RANGE_VRAM_DOMAIN) ? 0 : AMDGPU_PTE_SYSTEM;
+	pte_flags |= (domain == SVM_RANGE_VRAM_DOMAIN) ? 0 : AMDGPU_PTE_SYSTEM;
 	pte_flags |= snoop ? AMDGPU_PTE_SNOOPED : 0;
 	if (gc_ip_version >= IP_VERSION(12, 0, 0))
 		pte_flags |= AMDGPU_PTE_IS_PTE;
-- 
2.43.0

[PATCH 3/7] Revert "drm/amdgpu: take the A+A VF GMC aperture from SRAT"

[Zhigang Luo]

This reverts commit ea7147d613a0.

The aperture was copied into vram_base_offset and node_segment_size.
Those values are the sources, and the aperture is filled from them.
The last patch reads the SRAT address as the FB offset and the SRAT
length as the segment size. mc_init already copies that offset into
aper_base for a device connected to the CPU.

Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c    | 57 +++++------------------
 drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c |  5 +-
 2 files changed, 13 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
index cd276a138e10..7e0b83b1015f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
@@ -639,7 +639,6 @@ static void gmc_v12_0_set_gfxhub_funcs(struct amdgpu_device *adev)
 static int gmc_v12_0_early_init(struct amdgpu_ip_block *ip_block)
 {
 	struct amdgpu_device *adev = ip_block->adev;
-	int r;
 
 	if (adev->smuio.funcs &&
 	    adev->smuio.funcs->is_host_gpu_xgmi_supported)
@@ -662,18 +661,6 @@ static int gmc_v12_0_early_init(struct amdgpu_ip_block *ip_block)
 	gmc_v12_0_set_mmhub_funcs(adev);
 	gmc_v12_0_set_umc_funcs(adev);
 
-	if (amdgpu_virt_vram_is_spm(adev)) {
-		u64 aper_base, aper_size;
-
-		r = amdgpu_acpi_find_gpu_memory_in_srat(adev, &aper_base,
-							&aper_size);
-		if (r)
-			return r;
-
-		adev->gmc.aper_base = aper_base;
-		adev->gmc.aper_size = aper_size;
-	}
-
 	adev->gmc.shared_aperture_start = 0x2000000000000000ULL;
 	adev->gmc.shared_aperture_end =
 		adev->gmc.shared_aperture_start + (4ULL << 30) - 1;
@@ -720,19 +707,10 @@ static void gmc_v12_0_vram_gtt_location(struct amdgpu_device *adev,
 			amdgpu_gmc_agp_location(adev, mc);
 	}
 	/* base offset of vram pages */
-	if (amdgpu_virt_vram_is_spm(adev)) {
-		/*
-		 * For SPM-backed VRAM, PDB0 entries use PTE_SYSTEM, so the
-		 * address is a system physical address. vram_base_offset must
-		 * be aper_base so PDB0 maps GPU address 0 onto the HBM system
-		 * physical address.
-		 */
-		adev->vm_manager.vram_base_offset = adev->gmc.aper_base;
-	} else if (amdgpu_sriov_vf(adev)) {
+	if (amdgpu_sriov_vf(adev))
 		adev->vm_manager.vram_base_offset = 0;
-	} else {
+	else
 		adev->vm_manager.vram_base_offset = adev->mmhub.funcs->get_mc_fb_offset(adev);
-	}
 
 	adev->vm_manager.vram_base_offset +=
 		adev->gmc.xgmi.physical_node_id * adev->gmc.xgmi.node_segment_size;
@@ -769,28 +747,17 @@ static int gmc_v12_0_mc_init(struct amdgpu_device *adev)
 			return r;
 	}
 
+	adev->gmc.aper_base = pci_resource_start(adev->pdev, 0);
+	adev->gmc.aper_size = pci_resource_len(adev->pdev, 0);
+
 #ifdef CONFIG_X86_64
-	/*
-	 * SPM-backed devices resolve the aperture from SRAT during early_init.
-	 * For native A+A (non-VF) or APU, override aper_base using mmhub
-	 * offset. Otherwise derive the aperture from BAR 0.
-	 */
-	if (!amdgpu_virt_vram_is_spm(adev)) {
-		if (!amdgpu_sriov_vf(adev) &&
-		    (((adev->flags & AMD_IS_APU) &&
-		      !amdgpu_passthrough(adev)) ||
-		     adev->gmc.xgmi.connected_to_cpu)) {
-			adev->gmc.aper_base =
-				adev->mmhub.funcs->get_mc_fb_offset(adev) +
-				adev->gmc.xgmi.physical_node_id *
-				adev->gmc.xgmi.node_segment_size;
-			adev->gmc.aper_size = adev->gmc.real_vram_size;
-		} else {
-			adev->gmc.aper_base =
-				pci_resource_start(adev->pdev, 0);
-			adev->gmc.aper_size =
-				pci_resource_len(adev->pdev, 0);
-		}
+	if (((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) ||
+	    (adev->gmc.xgmi.connected_to_cpu)) {
+		adev->gmc.aper_base =
+			adev->mmhub.funcs->get_mc_fb_offset(adev) +
+			adev->gmc.xgmi.physical_node_id *
+			adev->gmc.xgmi.node_segment_size;
+		adev->gmc.aper_size = adev->gmc.real_vram_size;
 	}
 #endif
 	/* In case the PCI BAR is larger than the actual amount of vram */
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c
index 4c0a2d671cfc..4495d52b4b07 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c
@@ -154,10 +154,7 @@ static int mmhub_v4_2_0_get_xgmi_info(struct amdgpu_device *adev)
 	if (adev->gmc.xgmi.physical_node_id > max_physical_node_id)
 		return -EINVAL;
 
-	if (amdgpu_virt_vram_is_spm(adev) && adev->gmc.aper_size)
-		adev->gmc.xgmi.node_segment_size = adev->gmc.aper_size;
-	else
-		adev->gmc.xgmi.node_segment_size = seg_size;
+	adev->gmc.xgmi.node_segment_size = seg_size;
 
 	return 0;
 }
-- 
2.43.0

[PATCH 4/7] drm/amdgpu: set the A+A VF SYSTEM bit in the GFX 12.1 helpers

[Zhigang Luo]

gmc_v12_1_get_vm_pde() converts a VRAM MC address to a physical
address and skips that conversion when AMDGPU_PTE_SYSTEM is already
set. Set the bit after the translation so the address stays physical.

gmc_v12_1_get_vm_pte() sets the bit for a VRAM buffer object. SVM
calls it with a NULL buffer object, and svm_range_get_pte_flags()
sets the bit only for system memory, so the NULL case sets it for
SPM VRAM. NPA mappings are left unchanged.

v2:
- Set the bit for a VRAM BO inside if (bo), and for the SVM NULL BO
  in the else.

Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c
index 76391fca6c62..7ac0d9c6f280 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c
@@ -586,6 +586,12 @@ static void gmc_v12_1_get_vm_pde(struct amdgpu_device *adev, int level,
 		*addr = adev->vm_manager.vram_base_offset + *addr -
 			adev->gmc.vram_start;
 
+	/* SPM VRAM is reached at a system physical address. Set S after the
+	 * translation above so that translation is not skipped.
+	 */
+	if (amdgpu_virt_vram_is_spm(adev))
+		*flags |= AMDGPU_PTE_SYSTEM;
+
 	if (*addr & ~adev->gmc.pte_addr_mask) {
 		dev_err(adev->dev, "Invalid PDE address: %llx\n", *addr);
 		*addr = 0;
@@ -758,6 +764,10 @@ static void gmc_v12_1_get_vm_pte(struct amdgpu_device *adev,
 
 	if (bo) {
 		mem = bo->tbo.resource;
+		if (amdgpu_virt_vram_is_spm(adev) &&
+		    mem && mem->mem_type == TTM_PL_VRAM)
+			*flags |= AMDGPU_PTE_SYSTEM;
+
 		if (mem && mem->mem_type == AMDGPU_PL_NPA) {
 			dev_dbg(adev->dev,
 				"Setting PTE for NPA BO, mem->type: %d, mem->start: %lx, mem->size: %u, cur_flags: %llx\n",
@@ -766,6 +776,9 @@ static void gmc_v12_1_get_vm_pte(struct amdgpu_device *adev,
 		} else if (*flags & AMDGPU_PTE_VALID) {
 			gmc_v12_1_get_coherence_flags(adev, bo, flags);
 		}
+	} else if (amdgpu_virt_vram_is_spm(adev)) {
+		/* SVM has no BO. System memory already has S. VRAM does not. */
+		*flags |= AMDGPU_PTE_SYSTEM;
 	}
 }
 
-- 
2.43.0

[PATCH 5/7] drm/amdgpu: fill the A+A VF PDB0 through get_vm_pde

[Zhigang Luo]

PDB0 is written directly and does not go through get_vm_pte(). Route
both entries through amdgpu_gmc_get_vm_pde() so they take the same
SYSTEM-bit path as every other GFX 12.1 mapping.

The VRAM entries are PDE_PTEs and already hold a physical address, so
the translation is skipped and S is set for SPM. The GART PDE is given
the GART BO's MC address and translated once.

Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 659e89e5e78f..04cf02dd22ac 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -1272,7 +1272,7 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
 	u64 vram_size = adev->gmc.xgmi.node_segment_size * adev->gmc.xgmi.num_physical_nodes;
 	u64 pde0_page_size = (1ULL<<adev->gmc.vmid0_page_table_block_size)<<21;
 	u64 vram_addr, vram_end;
-	u64 gart_ptb_gpu_pa = amdgpu_gmc_vram_pa(adev, adev->gart.bo);
+	u64 pde_addr, pde_flags;
 	int idx;
 
 	if (!drm_dev_enter(adev_to_drm(adev), &idx))
@@ -1290,10 +1290,16 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
 	vram_end = vram_addr + vram_size;
 
 	/* The first n PDE0 entries are used as PTE,
-	 * pointing to vram
+	 * pointing to vram. PDE_PTE already holds a physical address, so
+	 * get_vm_pde() skips translation and sets S for SPM.
 	 */
-	for (i = 0; vram_addr < vram_end; i++, vram_addr += pde0_page_size)
-		amdgpu_gmc_set_pte_pde(adev, adev->gmc.ptr_pdb0, i, vram_addr, flags);
+	for (i = 0; vram_addr < vram_end; i++, vram_addr += pde0_page_size) {
+		pde_addr = vram_addr;
+		pde_flags = flags;
+		amdgpu_gmc_get_vm_pde(adev, AMDGPU_VM_PDB0, &pde_addr, &pde_flags);
+		amdgpu_gmc_set_pte_pde(adev, adev->gmc.ptr_pdb0, i, pde_addr,
+				       pde_flags);
+	}
 
 	/* The n+1'th PDE0 entry points to a huge
 	 * PTB who has more than 512 entries each
@@ -1301,8 +1307,12 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
 	 */
 	flags = AMDGPU_PTE_VALID;
 	flags |= AMDGPU_PTE_SNOOPED | AMDGPU_PDE_BFS_FLAG(adev, 0);
-	/* Requires gart_ptb_gpu_pa to be 4K aligned */
-	amdgpu_gmc_set_pte_pde(adev, adev->gmc.ptr_pdb0, i, gart_ptb_gpu_pa, flags);
+	/* MC address. get_vm_pde() translates it once and sets S for SPM.
+	 * Requires the GART PT to be 4K aligned.
+	 */
+	pde_addr = amdgpu_bo_gpu_offset(adev->gart.bo);
+	amdgpu_gmc_get_vm_pde(adev, AMDGPU_VM_PDB0, &pde_addr, &flags);
+	amdgpu_gmc_set_pte_pde(adev, adev->gmc.ptr_pdb0, i, pde_addr, flags);
 	drm_dev_exit(idx);
 }
 
-- 
2.43.0

[PATCH 6/7] drm/amdgpu: align the sysvm GART start to 8GB

[Zhigang Luo]

PDB0 block size 12 covers one 8GB entry. A GART start aligned only to
4GB can land in the middle of that entry. Align the sysvm GART start
to 8GB, and use SZ_4G and SZ_8G in place of the local size constants.

Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 04cf02dd22ac..8a62f4f3aace 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -25,6 +25,7 @@
  */
 
 #include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/sizes.h>
 #ifdef CONFIG_X86
 #include <asm/hypervisor.h>
 #endif
@@ -41,8 +42,6 @@
 #include <drm/drm_drv.h>
 #include <drm/ttm/ttm_tt.h>
 
-static const u64 four_gb = 0x100000000ULL;
-
 bool amdgpu_gmc_is_pdb0_enabled(struct amdgpu_device *adev)
 {
 	return adev->gmc.xgmi.connected_to_cpu || amdgpu_virt_xgmi_migrate_enabled(adev);
@@ -261,8 +260,8 @@ void amdgpu_gmc_sysvm_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc
 	u64 hive_vram_end = mc->xgmi.node_segment_size * mc->xgmi.num_physical_nodes - 1;
 	mc->vram_start = mc->xgmi.node_segment_size * mc->xgmi.physical_node_id;
 	mc->vram_end = mc->vram_start + mc->xgmi.node_segment_size - 1;
-	/* node_segment_size may not 4GB aligned on SRIOV, align up is needed. */
-	mc->gart_start = ALIGN(hive_vram_end + 1, four_gb);
+	/* Round up to a PDB0 entry so GART starts on the page-table PDE. */
+	mc->gart_start = ALIGN(hive_vram_end + 1, SZ_8G);
 	mc->gart_end = mc->gart_start + mc->gart_size - 1;
 	if (amdgpu_virt_xgmi_migrate_enabled(adev)) {
 		/* set mc->vram_start to 0 to switch the returned GPU address of
@@ -313,7 +312,7 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
 	 * the GART base on a 4GB boundary as well.
 	 */
 	size_bf = mc->fb_start;
-	size_af = max_mc_address + 1 - ALIGN(mc->fb_end + 1, four_gb);
+	size_af = max_mc_address + 1 - ALIGN(mc->fb_end + 1, SZ_4G);
 
 	if (mc->gart_size > max(size_bf, size_af)) {
 		dev_warn(adev->dev, "limiting GART\n");
@@ -328,7 +327,7 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
 		if (size_bf >= mc->gart_size)
 			mc->gart_start = 0;
 		else
-			mc->gart_start = ALIGN(mc->fb_end, four_gb);
+			mc->gart_start = ALIGN(mc->fb_end, SZ_4G);
 		break;
 	case AMDGPU_GART_PLACEMENT_BEST_FIT:
 	default:
@@ -340,7 +339,7 @@ void amdgpu_gmc_gart_location(struct amdgpu_device *adev, struct amdgpu_gmc *mc,
 		break;
 	}
 
-	mc->gart_start &= ~(four_gb - 1);
+	mc->gart_start &= ~(SZ_4G - 1);
 	mc->gart_end = mc->gart_start + mc->gart_size - 1;
 	dev_info(adev->dev, "GART: %lluM 0x%016llX - 0x%016llX\n",
 			mc->gart_size >> 20, mc->gart_start, mc->gart_end);
-- 
2.43.0

[PATCH 7/7] drm/amdgpu: keep the A+A VF VRAM base at the SRAT address

[Zhigang Luo]

On the A+A VF the hypervisor reports each VF's VRAM address and length
in the ACPI SRAT table. The XGMI LFB region and segment registers
describe the physical GPU, not the VF, so the node id and segment read
from those registers are not a valid VRAM starting address.

Read the SRAT entry in mmhub_v4_2_0_get_xgmi_info() and report the VF
as one physical node at id 0 whose segment is the SRAT length. Save
the SRAT address as the FB offset. get_mc_fb_offset() returns that
address, and mc_init copies it, plus the node product, into aper_base
and sets aper_size from the segment. Node id 0 makes the product zero,
so the PTE base stays at the SRAT address.

v2:
- Explain that the SRAT address is already the VF VRAM base and must
  not be adjusted by the XGMI node segment.

v3:
- Report the VF as XGMI node 0 of 1 instead of special-casing the
  node offset and the PDB0 size.

v4:
- Read SRAT in get_xgmi_info() and derive aper_base and aper_size from
  the FB offset and the segment size.
- Move the 8GB GART alignment into its own patch.

v5:
- Revert the SRAT aperture commit with the other two reverts. Leave
  mc_init on the restored connected_to_cpu path.

Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h   |  4 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h  |  8 +++++---
 drivers/gpu/drm/amd/amdgpu/gfxhub_v12_1.c |  4 ++++
 drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c | 25 +++++++++++++++++++++++
 5 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index d17055d4c50a..1772c9f5c940 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -231,6 +231,10 @@ struct amdgpu_gmc {
 	 */
 	resource_size_t		aper_size;
 	resource_size_t		aper_base;
+	/* SPM VF framebuffer offset from SRAT. get_mc_fb_offset() returns
+	 * this, and mc_init copies it into aper_base.
+	 */
+	u64			fb_offset;
 	/* for some chips with <= 32MB we need to lie
 	 * about vram size near mc fb location */
 	u64			mc_vram_size;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
index 5f36aff17e79..47092ec6dff2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.h
@@ -79,11 +79,13 @@ struct amdgpu_xgmi {
 	/* from psp */
 	u64 node_id;
 	u64 hive_id;
-	/* fixed per family */
+	/* Segment size. Fixed per family on a hive. On an SPM VF this is
+	 * the SRAT length of that one VF.
+	 */
 	u64 node_segment_size;
-	/* physical node (0-3) */
+	/* Physical node, 0-3. An SPM VF is node 0. */
 	unsigned physical_node_id;
-	/* number of nodes (0-4) */
+	/* Number of nodes, 0-4. An SPM VF reports one node. */
 	unsigned num_physical_nodes;
 	/* gpu list in the same hive */
 	struct list_head head;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v12_1.c b/drivers/gpu/drm/amd/amdgpu/gfxhub_v12_1.c
index b13967a1c8fb..b19f5b601d2c 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v12_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v12_1.c
@@ -53,6 +53,10 @@ static u64 gfxhub_v12_1_get_fb_location(struct amdgpu_device *adev)
 
 static u64 gfxhub_v12_1_get_mc_fb_offset(struct amdgpu_device *adev)
 {
+	/* SPM VRAM starts at the SRAT address saved as the FB offset. */
+	if (amdgpu_virt_vram_is_spm(adev))
+		return adev->gmc.fb_offset;
+
 	return (u64)(RREG32_SOC15(GC, GET_INST(GC, 0),
 				  regGCMC_VM_FB_OFFSET) << 24);
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
index 7e0b83b1015f..14ef841d7036 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
@@ -707,7 +707,7 @@ static void gmc_v12_0_vram_gtt_location(struct amdgpu_device *adev,
 			amdgpu_gmc_agp_location(adev, mc);
 	}
 	/* base offset of vram pages */
-	if (amdgpu_sriov_vf(adev))
+	if (amdgpu_sriov_vf(adev) && !amdgpu_virt_vram_is_spm(adev))
 		adev->vm_manager.vram_base_offset = 0;
 	else
 		adev->vm_manager.vram_base_offset = adev->mmhub.funcs->get_mc_fb_offset(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c
index 4495d52b4b07..8593453445e3 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c
@@ -132,6 +132,27 @@ static int mmhub_v4_2_0_get_xgmi_info(struct amdgpu_device *adev)
 	if (!adev->gmc.xgmi.connected_to_cpu)
 		return 0;
 
+	/* The SRAT entry is the VF's whole VRAM. PF_LFB_REGION describes
+	 * the physical GPU, not the VF, so report a single node. The
+	 * length is the segment size; the address is the FB offset.
+	 * aper_base is filled from those later.
+	 */
+	if (amdgpu_virt_vram_is_spm(adev)) {
+		u64 fb_offset;
+		int r;
+
+		r = amdgpu_acpi_find_gpu_memory_in_srat(adev, &fb_offset,
+							&seg_size);
+		if (r)
+			return r;
+
+		adev->gmc.fb_offset = fb_offset;
+		adev->gmc.xgmi.num_physical_nodes = 1;
+		adev->gmc.xgmi.physical_node_id = 0;
+		adev->gmc.xgmi.node_segment_size = seg_size;
+		return 0;
+	}
+
 	xgmi_lfb_cntl = RREG32_SOC15(MMHUB, GET_INST(MMHUB, 0),
 				     regMMMC_VM_XGMI_LFB_CNTL);
 	seg_size = REG_GET_FIELD(
@@ -177,6 +198,10 @@ static u64 mmhub_v4_2_0_get_fb_location(struct amdgpu_device *adev)
 
 static u64 mmhub_v4_2_0_get_mc_fb_offset(struct amdgpu_device *adev)
 {
+	/* SPM VRAM starts at the SRAT address saved as the FB offset. */
+	if (amdgpu_virt_vram_is_spm(adev))
+		return adev->gmc.fb_offset;
+
 	return (u64)RREG32_SOC15(MMHUB, GET_INST(MMHUB, 0),
 			         regMMMC_VM_FB_OFFSET) << 24;
 }
-- 
2.43.0

Re: [PATCH 6/7] drm/amdgpu: align the sysvm GART start to 8GB

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] drm/amdgpu: missing Fixes trailer for sysvm GART 8GB alignment
  fix
--

commit ac03025c9ec955f71abbea2e6a6172fcff3e1a6c
Author: Zhigang Luo <Zhigang.Luo@amd.com>

> drm/amdgpu: align the sysvm GART start to 8GB
>
> PDB0 block size 12 covers one 8GB entry. A GART start aligned only to
> 4GB can land in the middle of that entry. Align the sysvm GART start
> to 8GB, and use SZ_4G and SZ_8G in place of the local size constants.
>
> Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>

[Severity: Low]
This fixes the 4GB GART start alignment in amdgpu_gmc_sysvm_location().
Should this include:

  Fixes: 2f405eb45c55 ("drm/amdgpu: enable pdb0 for hibernation on SRIOV")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113145702.323968-1-Zhigang.Luo@amd.com?part=6

Re: [PATCH 1/7] Revert "drm/amdgpu: mark A+A VF VRAM mappings as system memory"

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] drm/amdgpu: revert commit message cites unmerged SHA and lacks
  Fixes tag
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Critical] drm/amdgpu: use-after-free on
  amdgpu_ualink_peer_remote_init() error [1]
--

commit 6fcaa0941d3baaab33a7b2b5dd02cf2d392bc896
Author: Zhigang Luo <Zhigang.Luo@amd.com>

> Revert "drm/amdgpu: mark A+A VF VRAM mappings as system memory"
>
> This reverts commit 521bedfc94cc.
>
> The patch sets AMDGPU_PTE_SYSTEM in amdgpu_ttm_tt_pde_flags() and
> converts the VRAM address with amdgpu_gmc_vram_mc2pa() inside
> amdgpu_gmc_get_pde_for_bo(). gmc_v12_1_get_vm_pde() already converts
> an MC address to a physical address and skips that conversion when
> the SYSTEM bit is set, so setting the bit first bypasses the
> translation.
>
> The bit belongs in the GFX 12.1 helpers, after that translation.
> PDB0 is written directly and does not go through get_vm_pde(), and
> NPA still requires PTE.S=0, so those two pieces come back in the
> following patch.
>
> Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>

[Severity: Low]
In the merged tree, the reverted change was committed as 010e510347f5
("drm/amdgpu: mark A+A VF VRAM mappings as system memory") rather than the
pre-signoff commit cited above.  Since this revert fixes broken PDE address
translation, should the commit message reference 010e510347f5 ("drm/amdgpu:
mark A+A VF VRAM mappings as system memory") and include:

  Fixes: 010e510347f5 ("drm/amdgpu: mark A+A VF VRAM mappings as system memory")

[ ... ]

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index d025b4183b296..9655758fa11ad 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -6173,7 +6173,6 @@ static int amdgpu_ualink_peer_remote_init(struct amdgpu_device *adev)
>  	flags = amdgpu_ttm_tt_pte_flags(adev, bo->tbo.ttm, bo->tbo.resource);
>  	flags |= AMDGPU_PTE_SNOOPED | AMDGPU_PTE_PRT_GFX12 | AMDGPU_PTE_BUS_ATOMICS;
>  	flags &= ~AMDGPU_PTE_VALID;
> -	flags &= ~AMDGPU_PTE_SYSTEM;
>  	/* PTE.X=0 turn off RPC checks for RBs, wptr and doorbell NPA address */
>  	flags &= ~AMDGPU_PTE_EXECUTABLE;

[Severity: Critical]
This problem wasn't introduced by this patch, but can a failure in
amdgpu_ualink_peer_remote_init() cause a use-after-free on
adev->ualink.remote [1]?

For each active remote peer, amdgpu_ualink_peer_remote_init() calls
amdgpu_ualink_gart_map() several times, inserting the embedded drm_mm_node
fields (mm_node_rb, mm_node_wptr and mm_node_doorbell) into
adev->mman.gtt_mgr.mm.  If a later amdgpu_ualink_gart_map() call fails,
amdgpu_ualink_peer_remote_init() breaks out and returns the error without
unmapping the earlier nodes:

drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c:amdgpu_ualink_peer_remote_init() {
	...
	for_each_set_bit(dst_accel_id, remote->active_accel_bits, AMDGPU_UALINK_ACCEL_MAX) {
		...
		r = amdgpu_ualink_gart_map(adev, rb_pages, npa, &interrupt->mm_node_rb, flags);
		if (r)
			break;
		...
		r = amdgpu_ualink_gart_map(adev, rb_pages, npa, &shootdown->mm_node_rb, flags);
		if (r)
			break;
	...
}

amdgpu_ualink_sw_init() then jumps to out_sdma_entities_fini and frees
adev->ualink.remote with kfree(), leaving the freed drm_mm_node objects
linked in adev->mman.gtt_mgr.mm and corrupting the GTT allocator on later
GTT operations or driver teardown.

[1] https://sashiko.dev/#/bug/linux-104150f2-9bfd-48ac-8dd1-d6f510b9e3c1

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113145702.323968-1-Zhigang.Luo@amd.com?part=1

Re: [PATCH 5/7] drm/amdgpu: fill the A+A VF PDB0 through get_vm_pde

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] drm/amdgpu: amdgpu_gmc_init_pdb0() corrupts GFX9 PDB0 via
  get_vm_pde()
--

commit 94fec17ddae148fbc91f8d2688eb078363097ddd
Author: Zhigang Luo <Zhigang.Luo@amd.com>

drm/amdgpu: fill the A+A VF PDB0 through get_vm_pde

This commit routes the VRAM PDE_PTE entries and the GART PTB PDE0 entry in
amdgpu_gmc_init_pdb0() through amdgpu_gmc_get_vm_pde().

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 659e89e5e78f1..04cf02dd22acb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -1272,7 +1272,7 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
>  	u64 vram_size = adev->gmc.xgmi.node_segment_size * adev->gmc.xgmi.num_physical_nodes;
>  	u64 pde0_page_size = (1ULL<<adev->gmc.vmid0_page_table_block_size)<<21;
>  	u64 vram_addr, vram_end;
> -	u64 gart_ptb_gpu_pa = amdgpu_gmc_vram_pa(adev, adev->gart.bo);
> +	u64 pde_addr, pde_flags;
>  	int idx;
>  
>  	if (!drm_dev_enter(adev_to_drm(adev), &idx))
> @@ -1290,10 +1290,16 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
>  	vram_end = vram_addr + vram_size;
>  
>  	/* The first n PDE0 entries are used as PTE,
> -	 * pointing to vram
> +	 * pointing to vram. PDE_PTE already holds a physical address, so
> +	 * get_vm_pde() skips translation and sets S for SPM.
>  	 */
> -	for (i = 0; vram_addr < vram_end; i++, vram_addr += pde0_page_size)
> -		amdgpu_gmc_set_pte_pde(adev, adev->gmc.ptr_pdb0, i, vram_addr, flags);
> +	for (i = 0; vram_addr < vram_end; i++, vram_addr += pde0_page_size) {
> +		pde_addr = vram_addr;
> +		pde_flags = flags;
> +		amdgpu_gmc_get_vm_pde(adev, AMDGPU_VM_PDB0, &pde_addr, &pde_flags);
> +		amdgpu_gmc_set_pte_pde(adev, adev->gmc.ptr_pdb0, i, pde_addr,
> +				       pde_flags);
> +	}
>  
>  	/* The n+1'th PDE0 entry points to a huge
>  	 * PTB who has more than 512 entries each
> @@ -1301,8 +1307,12 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
>  	 */
>  	flags = AMDGPU_PTE_VALID;
>  	flags |= AMDGPU_PTE_SNOOPED | AMDGPU_PDE_BFS_FLAG(adev, 0);
> -	/* Requires gart_ptb_gpu_pa to be 4K aligned */
> -	amdgpu_gmc_set_pte_pde(adev, adev->gmc.ptr_pdb0, i, gart_ptb_gpu_pa, flags);
> +	/* MC address. get_vm_pde() translates it once and sets S for SPM.
> +	 * Requires the GART PT to be 4K aligned.
> +	 */
> +	pde_addr = amdgpu_bo_gpu_offset(adev->gart.bo);
> +	amdgpu_gmc_get_vm_pde(adev, AMDGPU_VM_PDB0, &pde_addr, &flags);
> +	amdgpu_gmc_set_pte_pde(adev, adev->gmc.ptr_pdb0, i, pde_addr, flags);

[Severity: High]
Does passing AMDGPU_VM_PDB0 to amdgpu_gmc_get_vm_pde() corrupt the VMID0
PDB0 table on GFX9 devices?

amdgpu_gmc_init_pdb0() is also called from gmc_v9_0_gart_enable() on GFX9
ASICs with PDB0 enabled (such as Aldebaran and Aqua Vanjaram), where
adev->gmc.translate_further is true for 4-level user VMs:

drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:gmc_v9_0_get_vm_pde() {
	...
	if (!adev->gmc.translate_further)
		return;

	if (level == AMDGPU_VM_PDB1) {
		...
	} else if (level == AMDGPU_VM_PDB0) {
		if (*flags & AMDGPU_PDE_PTE) {
			*flags &= ~AMDGPU_PDE_PTE;
			if (!(*flags & AMDGPU_PTE_VALID))
				*addr |= 1 << PAGE_SHIFT;
		} else {
			*flags |= AMDGPU_PTE_TF;
		}
	}
}

Unlike VM_CONTEXT1..15, VM_CONTEXT0_CNTL.PAGE_TABLE_DEPTH is programmed to
vmid0_page_table_depth (1) without subtracting 1 for translate_further.
Clearing AMDGPU_PDE_PTE on the VRAM entries causes the hardware walker to
treat each VRAM physical address as a child PTB pointer instead of a leaf
PTE, and setting AMDGPU_PTE_TF on the GART entry sets a PTE-only bit in a
depth-1 PDE0 entry, breaking system-domain VRAM and GART translation on
GFX9.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113145702.323968-1-Zhigang.Luo@amd.com?part=5

Re: [PATCH 2/7] Revert "drm/amdkfd: mark A+A VF VRAM PTEs as system memory"

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] drm/amdkfd: revert commit message cites non-ancestor SHA
  daf19be40e04
--

commit 3e660e5d230b226dd7bb0de7b064fd710bf0e2a4
Author: Zhigang Luo <Zhigang.Luo@amd.com>

> Revert "drm/amdkfd: mark A+A VF VRAM PTEs as system memory"
>
> This reverts commit daf19be40e04.
>
> SVM VRAM ranges leave AMDGPU_PTE_SYSTEM clear and then call
> amdgpu_gmc_get_vm_pte() with a NULL buffer object. The GFX 12.1
> PTE helper sets the bit for an A+A VF, so the special case in
> svm_range_get_pte_flags() is the wrong place.
>
> Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>

[Severity: Low]
Commit daf19be40e04 ("drm/amdkfd: mark A+A VF VRAM PTEs as system memory")
is not an ancestor of this branch; the commit in the branch history that
added the amdgpu_virt_vram_is_spm() check in svm_range_get_pte_flags() is
a69f4d4076ea ("drm/amdkfd: mark A+A VF VRAM PTEs as system memory").
Should the revert reference be updated to cite a69f4d4076ea ("drm/amdkfd:
mark A+A VF VRAM PTEs as system memory")?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113145702.323968-1-Zhigang.Luo@amd.com?part=2

Re: [PATCH 3/7] Revert "drm/amdgpu: take the A+A VF GMC aperture from SRAT"

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] drm/amdgpu: undefined guard in
  amdgpu_acpi_find_gpu_memory_in_srat() [1]
--

commit d687d56267f159cb114f4a53091a5e9a32dc7973
Author: Zhigang Luo <Zhigang.Luo@amd.com>

Revert "drm/amdgpu: take the A+A VF GMC aperture from SRAT"

This commit reverts the direct SRAT aperture lookup in gmc_v12_0_early_init()
and the SPM overrides in gmc_v12_0_vram_gtt_location(), gmc_v12_0_mc_init(),
and mmhub_v4_2_0_get_xgmi_info() so that the GMC aperture is derived from the
FB offset and XGMI node segment size instead.

> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> index cd276a138e101..7e0b83b1015fc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> @@ -639,7 +639,6 @@ static void gmc_v12_0_set_gfxhub_funcs(struct amdgpu_device *adev)
>  static int gmc_v12_0_early_init(struct amdgpu_ip_block *ip_block)
>  {
>  	struct amdgpu_device *adev = ip_block->adev;
> -	int r;
>  
>  	if (adev->smuio.funcs &&
>  	    adev->smuio.funcs->is_host_gpu_xgmi_supported)
> @@ -662,18 +661,6 @@ static int gmc_v12_0_early_init(struct amdgpu_ip_block *ip_block)
>  	gmc_v12_0_set_mmhub_funcs(adev);
>  	gmc_v12_0_set_umc_funcs(adev);
>  
> -	if (amdgpu_virt_vram_is_spm(adev)) {
> -		u64 aper_base, aper_size;
> -
> -		r = amdgpu_acpi_find_gpu_memory_in_srat(adev, &aper_base,
> -							&aper_size);
> -		if (r)
> -			return r;
> -
> -		adev->gmc.aper_base = aper_base;
> -		adev->gmc.aper_size = aper_size;
> -	}

[Severity: High]
This problem wasn't introduced by this patch, but does
amdgpu_acpi_find_gpu_memory_in_srat() [1] always return -ENOENT?

In drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c, the SRAT lookup implementation
is guarded by #ifdef HAVE_ACPI_SRAT_GENERIC_AFFINITY, an out-of-tree autoconf
macro that is never defined in the kernel tree even though struct
acpi_srat_generic_affinity is unconditionally defined in
include/acpi/actbl3.h.  Only the #else fallback stub is compiled, causing
SRAT lookups for SPM-backed VRAM to fail with -ENOENT during device probe.

[1] https://sashiko.dev/#/bug/linux-e20f9f1a-d616-45e0-9734-058df15f4c59

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113145702.323968-1-Zhigang.Luo@amd.com?part=3

Re: [PATCH 4/7] drm/amdgpu: set the A+A VF SYSTEM bit in the GFX 12.1 helpers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] drm/amdgpu: gmc_v12_1_get_vm_pde() sets AMDGPU_PTE_SYSTEM on
  invalid and NPA PDEs
- [Medium] drm/amdgpu: gmc_v12_1_get_vm_pte() sets AMDGPU_PTE_SYSTEM
  when PTE is invalid
--

commit 914948e43f4fe117dbe355b73ada0eb4485a66d2
Author: Zhigang Luo <Zhigang.Luo@amd.com>

drm/amdgpu: set the A+A VF SYSTEM bit in the GFX 12.1 helpers

This commit sets AMDGPU_PTE_SYSTEM in gmc_v12_1_get_vm_pde() and
gmc_v12_1_get_vm_pte() when VRAM is backed by system physical memory on
SR-IOV VFs.

> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c
> index 57aac3a9a3b0f..1c5998fa3ad6e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c
> @@ -586,6 +586,12 @@ static void gmc_v12_1_get_vm_pde(struct amdgpu_device *adev, int level,
>  		*addr = adev->vm_manager.vram_base_offset + *addr -
>  			adev->gmc.vram_start;
>  
> +	/* SPM VRAM is reached at a system physical address. Set S after the
> +	 * translation above so that translation is not skipped.
> +	 */
> +	if (amdgpu_virt_vram_is_spm(adev))
> +		*flags |= AMDGPU_PTE_SYSTEM;
> +
>  	if (*addr & ~adev->gmc.pte_addr_mask) {
>  		dev_err(adev->dev, "Invalid PDE address: %llx\n", *addr);
>  		*addr = 0;

[Severity: High]
Does setting AMDGPU_PTE_SYSTEM here without checking (*flags &
AMDGPU_PTE_VALID) corrupt cleared PDEs and 2MB+ NPA leaf PDEs?

On GFX 12, gmc_v12_1_get_vm_pde() always sets AMDGPU_PTE_SNOOPED, and
(AMDGPU_PTE_SYSTEM | AMDGPU_PTE_SNOOPED) on an invalid entry is
AMDGPU_VM_NORETRY_FLAGS_GFX12.  When amdgpu_vm_pt_clear() clears a
non-PTB page directory (with AMDGPU_PTE_VALID == 0), this turns every
cleared PDE in a normal VM into a fatal no-retry fault entry, breaking
SVM demand paging on unpopulated ranges.

For 2MB+ AMDGPU_PL_NPA mappings updated at a leaf PDE level in
amdgpu_vm_pte_update_flags(), gmc_v12_1_get_npa_flags() clears
AMDGPU_PTE_VALID and leaves AMDGPU_PTE_SYSTEM clear (VSCT = 0011), which
amdgpu_gmc_get_vm_pde() then overwrites with AMDGPU_PTE_SYSTEM.

> @@ -753,6 +759,10 @@ static void gmc_v12_1_get_vm_pte(struct amdgpu_device *adev,
>  
>  	if (bo) {
>  		mem = bo->tbo.resource;
> +		if (amdgpu_virt_vram_is_spm(adev) &&
> +		    mem && mem->mem_type == TTM_PL_VRAM)
> +			*flags |= AMDGPU_PTE_SYSTEM;
> +
>  		if (mem && mem->mem_type == AMDGPU_PL_NPA) {
>  			dev_dbg(adev->dev,
>  				"Setting PTE for NPA BO, mem->type: %d, mem->start: %lx, mem->size: %u, cur_flags: %llx\n",
> @@ -761,6 +771,9 @@ static void gmc_v12_1_get_vm_pte(struct amdgpu_device *adev,
>  		} else if (*flags & AMDGPU_PTE_VALID) {
>  			gmc_v12_1_get_coherence_flags(adev, bo, flags);
>  		}
> +	} else if (amdgpu_virt_vram_is_spm(adev)) {
> +		/* SVM has no BO. System memory already has S. VRAM does not. */
> +		*flags |= AMDGPU_PTE_SYSTEM;
>  	}
>  }

[Severity: Medium]
Should both branches check (*flags & AMDGPU_PTE_VALID) before setting
AMDGPU_PTE_SYSTEM?

When amdgpu_vm_handle_moved() calls amdgpu_vm_bo_update() with clear =
true after dma_resv_trylock() fails, bo->tbo.base.resv is not held and
update_flags has AMDGPU_PTE_VALID == 0.  Checking mem->mem_type before
(*flags & AMDGPU_PTE_VALID) reads bo->tbo.resource without holding the
reservation lock and sets AMDGPU_PTE_SYSTEM on cleared PTEs.

In the else branch, amdgpu_vm_bo_update() also passes bo == NULL with
AMDGPU_PTE_VALID == 0 for PRT/sparse mappings (fpriv->prt_va), so
setting AMDGPU_PTE_SYSTEM unconditionally pollutes invalid non-SVM PTEs
(and forms AMDGPU_VM_NORETRY_FLAGS_GFX12 if a cleared or PRT mapping
spans a 2MB+ leaf PDE).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113145702.323968-1-Zhigang.Luo@amd.com?part=4

Re: [PATCH 7/7] drm/amdgpu: keep the A+A VF VRAM base at the SRAT address

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] drm/amdgpu: amdgpu_acpi_find_gpu_memory_in_srat() always
  returns -ENOENT
--

commit eababb17e2adcd526ed509506729f9c07f446351
Author: Zhigang Luo <Zhigang.Luo@amd.com>

drm/amdgpu: keep the A+A VF VRAM base at the SRAT address

This commit reads the ACPI SRAT entry in mmhub_v4_2_0_get_xgmi_info() on A+A
SPM VFs to set the framebuffer offset and single-node XGMI segment size from
the SRAT address and length.

> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c
> index 4495d52b4b07c..8593453445e38 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v4_2_0.c
> @@ -132,6 +132,27 @@ static int mmhub_v4_2_0_get_xgmi_info(struct amdgpu_device *adev)
>  	if (!adev->gmc.xgmi.connected_to_cpu)
>  		return 0;
>  
> +	/* The SRAT entry is the VF's whole VRAM. PF_LFB_REGION describes
> +	 * the physical GPU, not the VF, so report a single node. The
> +	 * length is the segment size; the address is the FB offset.
> +	 * aper_base is filled from those later.
> +	 */
> +	if (amdgpu_virt_vram_is_spm(adev)) {
> +		u64 fb_offset;
> +		int r;
> +
> +		r = amdgpu_acpi_find_gpu_memory_in_srat(adev, &fb_offset,
> +							&seg_size);
> +		if (r)
> +			return r;

[Severity: High]
Does calling amdgpu_acpi_find_gpu_memory_in_srat() here cause
mmhub_v4_2_0_get_xgmi_info() to always fail with -ENOENT on mainline builds?
The real implementation of amdgpu_acpi_find_gpu_memory_in_srat() is guarded
by HAVE_ACPI_SRAT_GENERIC_AFFINITY, which is never defined in the kernel
tree:

drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c:
#ifdef HAVE_ACPI_SRAT_GENERIC_AFFINITY
...
int amdgpu_acpi_find_gpu_memory_in_srat(struct amdgpu_device *adev,
					uint64_t *base_addr, uint64_t *length)
{
...
}
#else
int amdgpu_acpi_find_gpu_memory_in_srat(struct amdgpu_device *adev,
					uint64_t *base_addr, uint64_t *length)
{
	if (!base_addr || !length)
		return -EINVAL;

	return -ENOENT;
}
#endif

Because only the #else stub is compiled, mmhub_v4_2_0_get_xgmi_info() returns
-ENOENT and aborts amdgpu_device_init() on all A+A SPM VFs.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113145702.323968-1-Zhigang.Luo@amd.com?part=7