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

[PATCH 1/4] 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/4] 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/4] 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() covers the graphics and SVM PTE paths. SVM
VRAM ranges call it with a NULL buffer object, so set the bit there
before the NPA flags, which still clear it.

PDB0 is written directly and does not go through get_vm_pde(). Set
AMDGPU_PTE_SYSTEM on the SPM VRAM entries and the GART PDE. NPA
still requires PTE.S=0, so clear the bit in gmc_v12_1_get_npa_flags()
and amdgpu_ualink_peer_remote_init().

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 659e89e5e78f..f76da74abfec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -1281,6 +1281,8 @@ 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);
 
@@ -1300,6 +1302,8 @@ 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_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index 3bb426b33af9..11f022c56667 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -6171,6 +6171,7 @@ 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 76391fca6c62..3bd86028f304 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;
@@ -616,6 +622,7 @@ 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;
 }
 
@@ -756,6 +763,9 @@ static void gmc_v12_1_get_vm_pte(struct amdgpu_device *adev,
 		break;
 	}
 
+	if (amdgpu_virt_vram_is_spm(adev))
+		*flags |= AMDGPU_PTE_SYSTEM;
+
 	if (bo) {
 		mem = bo->tbo.resource;
 		if (mem && mem->mem_type == AMDGPU_PL_NPA) {
-- 
2.43.0

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

[Christian König]

On 11/9/26 17:26, Zhigang Luo wrote:
> 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() covers the graphics and SVM PTE paths. SVM
> VRAM ranges call it with a NULL buffer object, so set the bit there
> before the NPA flags, which still clear it.
> 
> PDB0 is written directly and does not go through get_vm_pde(). Set
> AMDGPU_PTE_SYSTEM on the SPM VRAM entries and the GART PDE. NPA
> still requires PTE.S=0, so clear the bit in gmc_v12_1_get_npa_flags()
> and amdgpu_ualink_peer_remote_init().
> 
> Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    |  4 ++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c |  1 +
>  drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c     | 10 ++++++++++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 659e89e5e78f..f76da74abfec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -1281,6 +1281,8 @@ 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;

Yeah again, hacks like that are a pretty big NO-GO.

We should probably start to use amdgpu_gmc_get_vm_pde() here as well. The reason we haven't done that previously is potentially because the address parameter is in the wrong address space.

Old HW generations expected the MC address space in the PDEs, so that is what amdgpu_gmc_get_vm_pde() takes as parameter. But newer generations expect the PA and because of this we call amdgpu_gmc_vram_mc2pa() in the get_vm_pde callback.

So that results in a PA->MC->PA conversion which doesn't make to much sense.

This needs to be restructured to completely support VRAM domain in system memory as well.

>  	flags |= AMDGPU_PTE_FRAG((adev->gmc.vmid0_page_table_block_size + 9*1));
>  	flags |= AMDGPU_PDE_PTE_FLAG(adev);
>  
> @@ -1300,6 +1302,8 @@ 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;

Same here.

>  	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_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index 3bb426b33af9..11f022c56667 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -6171,6 +6171,7 @@ 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;

And that here actually breaks thing, clear NAK to that!

>  	/* 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 76391fca6c62..3bd86028f304 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;
> +

That looks correct to me.

>  	if (*addr & ~adev->gmc.pte_addr_mask) {
>  		dev_err(adev->dev, "Invalid PDE address: %llx\n", *addr);
>  		*addr = 0;
> @@ -616,6 +622,7 @@ 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

That should be superflous.
;
>  	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>  }
>  
> @@ -756,6 +763,9 @@ static void gmc_v12_1_get_vm_pte(struct amdgpu_device *adev,
>  		break;
>  	}
>  
> +	if (amdgpu_virt_vram_is_spm(adev))
> +		*flags |= AMDGPU_PTE_SYSTEM;
> +

Here you need to check if the destination is actually VRAM and not NPA.

Regards,
Christian.

>  	if (bo) {
>  		mem = bo->tbo.resource;
>  		if (mem && mem->mem_type == AMDGPU_PL_NPA) {

[PATCH 4/4] 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 in the
ACPI SRAT table, and that address is stored in aper_base. The XGMI
node segment does not describe the VF, so physical_node_id *
node_segment_size is not a valid VRAM starting address.

gmc_v12_0_vram_gtt_location() and amdgpu_virt_resume() still added
that offset on top of aper_base, which moved the SPM PDB0 system PTEs
off the VF's HBM. Leave vram_base_offset at aper_base on both paths.
Size PDB0 from the VF's local VRAM and place GART after that window,
aligned to 8GB. That alignment is one PDB0 entry when
vmid0_page_table_block_size is 12, and it applies to every PDB0
placement.

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

Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    | 22 ++++++++++++++++------
 drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c     |  6 ++++--
 3 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 9326f811c500..7a664b7b1ba3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4677,9 +4677,15 @@ static inline int amdgpu_virt_resume(struct amdgpu_device *adev)
 	dev_info(adev->dev, "xgmi node, old id %d, new id %d\n",
 		prev_physical_node_id, adev->gmc.xgmi.physical_node_id);
 
-	adev->vm_manager.vram_base_offset = adev->gfxhub.funcs->get_mc_fb_offset(adev);
-	adev->vm_manager.vram_base_offset +=
-		adev->gmc.xgmi.physical_node_id * adev->gmc.xgmi.node_segment_size;
+	if (amdgpu_virt_vram_is_spm(adev)) {
+		adev->vm_manager.vram_base_offset = adev->gmc.aper_base;
+	} else {
+		adev->vm_manager.vram_base_offset =
+			adev->gfxhub.funcs->get_mc_fb_offset(adev);
+		adev->vm_manager.vram_base_offset +=
+			adev->gmc.xgmi.physical_node_id *
+			adev->gmc.xgmi.node_segment_size;
+	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index f76da74abfec..96075db3b9ac 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -42,6 +42,8 @@
 #include <drm/ttm/ttm_tt.h>
 
 static const u64 four_gb = 0x100000000ULL;
+/* PDB0 block size 12 covers one 8GB entry. */
+static const u64 eight_gb = 0x200000000ULL;
 
 bool amdgpu_gmc_is_pdb0_enabled(struct amdgpu_device *adev)
 {
@@ -60,7 +62,8 @@ int amdgpu_gmc_pdb0_alloc(struct amdgpu_device *adev)
 {
 	int r;
 	struct amdgpu_bo_param bp;
-	u64 vram_size = adev->gmc.xgmi.node_segment_size * adev->gmc.xgmi.num_physical_nodes;
+	u64 vram_size = amdgpu_virt_vram_is_spm(adev) ? adev->gmc.mc_vram_size :
+		adev->gmc.xgmi.node_segment_size * adev->gmc.xgmi.num_physical_nodes;
 	uint32_t pde0_page_shift = adev->gmc.vmid0_page_table_block_size + 21;
 	uint32_t npdes = (vram_size + (1ULL << pde0_page_shift) - 1) >> pde0_page_shift;
 
@@ -261,16 +264,21 @@ 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, eight_gb);
 	mc->gart_end = mc->gart_start + mc->gart_size - 1;
-	if (amdgpu_virt_xgmi_migrate_enabled(adev)) {
+	if (amdgpu_virt_xgmi_migrate_enabled(adev) ||
+	    amdgpu_virt_vram_is_spm(adev)) {
 		/* set mc->vram_start to 0 to switch the returned GPU address of
 		 * amdgpu_bo_create_reserved() from FB aperture to GART aperture.
 		 */
 		mc->vram_start = 0;
 		mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
 		mc->visible_vram_size = min(mc->visible_vram_size, mc->real_vram_size);
+		if (amdgpu_virt_vram_is_spm(adev)) {
+			mc->gart_start = ALIGN(mc->vram_end + 1, eight_gb);
+			mc->gart_end = mc->gart_start + mc->gart_size - 1;
+		}
 	} else {
 		mc->fb_start = hive_vram_start;
 		mc->fb_end = hive_vram_end;
@@ -1269,7 +1277,8 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
 	uint64_t flags = adev->gart.gart_pte_flags; //TODO it is UC. explore NC/RW?
 	/* Each PDE0 (used as PTE) covers (2^vmid0_page_table_block_size)*2M
 	 */
-	u64 vram_size = adev->gmc.xgmi.node_segment_size * adev->gmc.xgmi.num_physical_nodes;
+	u64 vram_size = amdgpu_virt_vram_is_spm(adev) ? adev->gmc.mc_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);
@@ -1287,7 +1296,8 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
 	flags |= AMDGPU_PDE_PTE_FLAG(adev);
 
 	vram_addr = adev->vm_manager.vram_base_offset;
-	if (!amdgpu_virt_xgmi_migrate_enabled(adev))
+	if (!amdgpu_virt_xgmi_migrate_enabled(adev) &&
+	    !amdgpu_virt_vram_is_spm(adev))
 		vram_addr -= adev->gmc.xgmi.physical_node_id * adev->gmc.xgmi.node_segment_size;
 	vram_end = vram_addr + vram_size;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
index cd276a138e10..e37c84f10709 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
@@ -734,8 +734,10 @@ static void gmc_v12_0_vram_gtt_location(struct amdgpu_device *adev,
 		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;
+	if (!amdgpu_virt_vram_is_spm(adev))
+		adev->vm_manager.vram_base_offset +=
+			adev->gmc.xgmi.physical_node_id *
+			adev->gmc.xgmi.node_segment_size;
 }
 
 /**
-- 
2.43.0

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

[Christian König]

On 11/9/26 17:26, Zhigang Luo wrote:
> On the A+A VF the hypervisor reports each VF's VRAM address in the
> ACPI SRAT table, and that address is stored in aper_base. The XGMI
> node segment does not describe the VF, so physical_node_id *
> node_segment_size is not a valid VRAM starting address.
> 
> gmc_v12_0_vram_gtt_location() and amdgpu_virt_resume() still added
> that offset on top of aper_base, which moved the SPM PDB0 system PTEs
> off the VF's HBM. Leave vram_base_offset at aper_base on both paths.
> Size PDB0 from the VF's local VRAM and place GART after that window,
> aligned to 8GB. That alignment is one PDB0 entry when
> vmid0_page_table_block_size is 12, and it applies to every PDB0
> placement.
> 
> v2:
> - Explain that the SRAT address is already the VF VRAM base and must
>   not be adjusted by the XGMI node segment.
> 
> Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++++++++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    | 22 ++++++++++++++++------
>  drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c     |  6 ++++--
>  3 files changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 9326f811c500..7a664b7b1ba3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -4677,9 +4677,15 @@ static inline int amdgpu_virt_resume(struct amdgpu_device *adev)
>  	dev_info(adev->dev, "xgmi node, old id %d, new id %d\n",
>  		prev_physical_node_id, adev->gmc.xgmi.physical_node_id);
>  
> -	adev->vm_manager.vram_base_offset = adev->gfxhub.funcs->get_mc_fb_offset(adev);
> -	adev->vm_manager.vram_base_offset +=
> -		adev->gmc.xgmi.physical_node_id * adev->gmc.xgmi.node_segment_size;
> +	if (amdgpu_virt_vram_is_spm(adev)) {
> +		adev->vm_manager.vram_base_offset = adev->gmc.aper_base;
> +	} else {
> +		adev->vm_manager.vram_base_offset =
> +			adev->gfxhub.funcs->get_mc_fb_offset(adev);
> +		adev->vm_manager.vram_base_offset +=
> +			adev->gmc.xgmi.physical_node_id *
> +			adev->gmc.xgmi.node_segment_size;

This is a massive NAK. That needs to be handled in get_mc_fb_offset().

See how aper_base is initialized on APUS as well:

#ifdef CONFIG_X86_64
        if ((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) {
                adev->gmc.aper_base = adev->mmhub.funcs->get_mc_fb_offset(adev);
                adev->gmc.aper_size = adev->gmc.real_vram_size;
        }               
#endif



> +	}
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index f76da74abfec..96075db3b9ac 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -42,6 +42,8 @@
>  #include <drm/ttm/ttm_tt.h>
>  
>  static const u64 four_gb = 0x100000000ULL;
> +/* PDB0 block size 12 covers one 8GB entry. */
> +static const u64 eight_gb = 0x200000000ULL;
>  
>  bool amdgpu_gmc_is_pdb0_enabled(struct amdgpu_device *adev)
>  {
> @@ -60,7 +62,8 @@ int amdgpu_gmc_pdb0_alloc(struct amdgpu_device *adev)
>  {
>  	int r;
>  	struct amdgpu_bo_param bp;
> -	u64 vram_size = adev->gmc.xgmi.node_segment_size * adev->gmc.xgmi.num_physical_nodes;
> +	u64 vram_size = amdgpu_virt_vram_is_spm(adev) ? adev->gmc.mc_vram_size :
> +		adev->gmc.xgmi.node_segment_size * adev->gmc.xgmi.num_physical_nodes;

Same here. I need to double check but we probably need a new callback for this.

Regards,
Christian.

>  	uint32_t pde0_page_shift = adev->gmc.vmid0_page_table_block_size + 21;
>  	uint32_t npdes = (vram_size + (1ULL << pde0_page_shift) - 1) >> pde0_page_shift;
>  
> @@ -261,16 +264,21 @@ 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, eight_gb);
>  	mc->gart_end = mc->gart_start + mc->gart_size - 1;
> -	if (amdgpu_virt_xgmi_migrate_enabled(adev)) {
> +	if (amdgpu_virt_xgmi_migrate_enabled(adev) ||
> +	    amdgpu_virt_vram_is_spm(adev)) {
>  		/* set mc->vram_start to 0 to switch the returned GPU address of
>  		 * amdgpu_bo_create_reserved() from FB aperture to GART aperture.
>  		 */
>  		mc->vram_start = 0;
>  		mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
>  		mc->visible_vram_size = min(mc->visible_vram_size, mc->real_vram_size);
> +		if (amdgpu_virt_vram_is_spm(adev)) {
> +			mc->gart_start = ALIGN(mc->vram_end + 1, eight_gb);
> +			mc->gart_end = mc->gart_start + mc->gart_size - 1;
> +		}
>  	} else {
>  		mc->fb_start = hive_vram_start;
>  		mc->fb_end = hive_vram_end;
> @@ -1269,7 +1277,8 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
>  	uint64_t flags = adev->gart.gart_pte_flags; //TODO it is UC. explore NC/RW?
>  	/* Each PDE0 (used as PTE) covers (2^vmid0_page_table_block_size)*2M
>  	 */
> -	u64 vram_size = adev->gmc.xgmi.node_segment_size * adev->gmc.xgmi.num_physical_nodes;
> +	u64 vram_size = amdgpu_virt_vram_is_spm(adev) ? adev->gmc.mc_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);
> @@ -1287,7 +1296,8 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device *adev)
>  	flags |= AMDGPU_PDE_PTE_FLAG(adev);
>  
>  	vram_addr = adev->vm_manager.vram_base_offset;
> -	if (!amdgpu_virt_xgmi_migrate_enabled(adev))
> +	if (!amdgpu_virt_xgmi_migrate_enabled(adev) &&
> +	    !amdgpu_virt_vram_is_spm(adev))
>  		vram_addr -= adev->gmc.xgmi.physical_node_id * adev->gmc.xgmi.node_segment_size;
>  	vram_end = vram_addr + vram_size;
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> index cd276a138e10..e37c84f10709 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> @@ -734,8 +734,10 @@ static void gmc_v12_0_vram_gtt_location(struct amdgpu_device *adev,
>  		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;
> +	if (!amdgpu_virt_vram_is_spm(adev))
> +		adev->vm_manager.vram_base_offset +=
> +			adev->gmc.xgmi.physical_node_id *
> +			adev->gmc.xgmi.node_segment_size;
>  }
>  
>  /**

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

[Luo, Zhigang]

AMD General

Hi Christian,

Thanks for the review. I will drop the SPM branch in amdgpu_virt_resume() and keep the common calculation:

vram_base_offset = get_mc_fb_offset() +
                   physical_node_id * node_segment_size

On an SPM VF the SRAT address is already the VRAM base, in the same role as get_mc_fb_offset() on an APU. get_mc_fb_offset() will return aper_base in that case.

PF_LFB_REGION and the XGMI segment do not describe this VF. Instead of adjusting the node product at each caller, mmhub_v4_2_0_get_xgmi_info() will report the SPM VF as a single node:

num_physical_nodes = 1
physical_node_id   = 0
node_segment_size  = SRAT aperture size
The node product is then 0, so the resume calculation leaves vram_base_offset at the SRAT address.

This also covers the PDB0 size. node_segment_size * num_physical_nodes becomes the VF's local VRAM, so the mc_vram_size ternary in amdgpu_gmc_pdb0_alloc() and amdgpu_gmc_init_pdb0() can go away. The existing hive-size formula stays.

Does this look right to you?

Regards,
Zhigang


> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Tuesday, November 10, 2026 3:42 AM
> To: Luo, Zhigang <Zhigang.Luo@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Zhang, Hawking
> <Hawking.Zhang@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>
> Subject: Re: [PATCH 4/4] drm/amdgpu: keep the A+A VF VRAM base at the
> SRAT address
>
>
>
> On 11/9/26 17:26, Zhigang Luo wrote:
> > On the A+A VF the hypervisor reports each VF's VRAM address in the
> > ACPI SRAT table, and that address is stored in aper_base. The XGMI
> > node segment does not describe the VF, so physical_node_id *
> > node_segment_size is not a valid VRAM starting address.
> >
> > gmc_v12_0_vram_gtt_location() and amdgpu_virt_resume() still added
> > that offset on top of aper_base, which moved the SPM PDB0 system PTEs
> > off the VF's HBM. Leave vram_base_offset at aper_base on both paths.
> > Size PDB0 from the VF's local VRAM and place GART after that window,
> > aligned to 8GB. That alignment is one PDB0 entry when
> > vmid0_page_table_block_size is 12, and it applies to every PDB0
> > placement.
> >
> > v2:
> > - Explain that the SRAT address is already the VF VRAM base and must
> >   not be adjusted by the XGMI node segment.
> >
> > Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++++++++---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    | 22 ++++++++++++++++--
> ----
> >  drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c     |  6 ++++--
> >  3 files changed, 29 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 9326f811c500..7a664b7b1ba3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -4677,9 +4677,15 @@ static inline int amdgpu_virt_resume(struct
> amdgpu_device *adev)
> >     dev_info(adev->dev, "xgmi node, old id %d, new id %d\n",
> >             prev_physical_node_id, adev->gmc.xgmi.physical_node_id);
> >
> > -   adev->vm_manager.vram_base_offset = adev->gfxhub.funcs-
> >get_mc_fb_offset(adev);
> > -   adev->vm_manager.vram_base_offset +=
> > -           adev->gmc.xgmi.physical_node_id * adev-
> >gmc.xgmi.node_segment_size;
> > +   if (amdgpu_virt_vram_is_spm(adev)) {
> > +           adev->vm_manager.vram_base_offset = adev-
> >gmc.aper_base;
> > +   } else {
> > +           adev->vm_manager.vram_base_offset =
> > +                   adev->gfxhub.funcs->get_mc_fb_offset(adev);
> > +           adev->vm_manager.vram_base_offset +=
> > +                   adev->gmc.xgmi.physical_node_id *
> > +                   adev->gmc.xgmi.node_segment_size;
>
> This is a massive NAK. That needs to be handled in get_mc_fb_offset().
>
> See how aper_base is initialized on APUS as well:
>
> #ifdef CONFIG_X86_64
>         if ((adev->flags & AMD_IS_APU) && !amdgpu_passthrough(adev)) {
>                 adev->gmc.aper_base = adev->mmhub.funcs-
> >get_mc_fb_offset(adev);
>                 adev->gmc.aper_size = adev->gmc.real_vram_size;
>         }
> #endif
>
>
>
> > +   }
> >
> >     return 0;
> >  }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> > index f76da74abfec..96075db3b9ac 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> > @@ -42,6 +42,8 @@
> >  #include <drm/ttm/ttm_tt.h>
> >
> >  static const u64 four_gb = 0x100000000ULL;
> > +/* PDB0 block size 12 covers one 8GB entry. */ static const u64
> > +eight_gb = 0x200000000ULL;
> >
> >  bool amdgpu_gmc_is_pdb0_enabled(struct amdgpu_device *adev)  { @@
> > -60,7 +62,8 @@ int amdgpu_gmc_pdb0_alloc(struct amdgpu_device
> *adev)
> > {
> >     int r;
> >     struct amdgpu_bo_param bp;
> > -   u64 vram_size = adev->gmc.xgmi.node_segment_size * adev-
> >gmc.xgmi.num_physical_nodes;
> > +   u64 vram_size = amdgpu_virt_vram_is_spm(adev) ? adev-
> >gmc.mc_vram_size :
> > +           adev->gmc.xgmi.node_segment_size *
> > +adev->gmc.xgmi.num_physical_nodes;
>
> Same here. I need to double check but we probably need a new callback for
> this.
>
> Regards,
> Christian.
>
> >     uint32_t pde0_page_shift = adev->gmc.vmid0_page_table_block_size
> + 21;
> >     uint32_t npdes = (vram_size + (1ULL << pde0_page_shift) - 1) >>
> > pde0_page_shift;
> >
> > @@ -261,16 +264,21 @@ 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, eight_gb);
> >     mc->gart_end = mc->gart_start + mc->gart_size - 1;
> > -   if (amdgpu_virt_xgmi_migrate_enabled(adev)) {
> > +   if (amdgpu_virt_xgmi_migrate_enabled(adev) ||
> > +       amdgpu_virt_vram_is_spm(adev)) {
> >             /* set mc->vram_start to 0 to switch the returned GPU address
> of
> >              * amdgpu_bo_create_reserved() from FB aperture to GART
> aperture.
> >              */
> >             mc->vram_start = 0;
> >             mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
> >             mc->visible_vram_size = min(mc->visible_vram_size,
> > mc->real_vram_size);
> > +           if (amdgpu_virt_vram_is_spm(adev)) {
> > +                   mc->gart_start = ALIGN(mc->vram_end + 1, eight_gb);
> > +                   mc->gart_end = mc->gart_start + mc->gart_size - 1;
> > +           }
> >     } else {
> >             mc->fb_start = hive_vram_start;
> >             mc->fb_end = hive_vram_end;
> > @@ -1269,7 +1277,8 @@ void amdgpu_gmc_init_pdb0(struct
> amdgpu_device *adev)
> >     uint64_t flags = adev->gart.gart_pte_flags; //TODO it is UC. explore
> NC/RW?
> >     /* Each PDE0 (used as PTE) covers
> (2^vmid0_page_table_block_size)*2M
> >      */
> > -   u64 vram_size = adev->gmc.xgmi.node_segment_size * adev-
> >gmc.xgmi.num_physical_nodes;
> > +   u64 vram_size = amdgpu_virt_vram_is_spm(adev) ? adev-
> >gmc.mc_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);
> @@
> > -1287,7 +1296,8 @@ void amdgpu_gmc_init_pdb0(struct amdgpu_device
> *adev)
> >     flags |= AMDGPU_PDE_PTE_FLAG(adev);
> >
> >     vram_addr = adev->vm_manager.vram_base_offset;
> > -   if (!amdgpu_virt_xgmi_migrate_enabled(adev))
> > +   if (!amdgpu_virt_xgmi_migrate_enabled(adev) &&
> > +       !amdgpu_virt_vram_is_spm(adev))
> >             vram_addr -= adev->gmc.xgmi.physical_node_id * adev-
> >gmc.xgmi.node_segment_size;
> >     vram_end = vram_addr + vram_size;
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> > index cd276a138e10..e37c84f10709 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c
> > @@ -734,8 +734,10 @@ static void gmc_v12_0_vram_gtt_location(struct
> amdgpu_device *adev,
> >             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;
> > +   if (!amdgpu_virt_vram_is_spm(adev))
> > +           adev->vm_manager.vram_base_offset +=
> > +                   adev->gmc.xgmi.physical_node_id *
> > +                   adev->gmc.xgmi.node_segment_size;
> >  }
> >
> >  /**

[PATCH 1/4] 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 3/4] 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.

PDB0 is filled through amdgpu_gmc_get_vm_pde() so those entries take
the same path. The VRAM entries are PDE_PTEs and already hold a
physical address, so the translation is skipped. The GART PDE is
given the GART BO's MC address and translated once.

gmc_v12_1_get_vm_pte() sets the bit only for VRAM. SVM VRAM ranges
call it with a NULL buffer object and leave the bit clear. NPA
mappings are left unchanged.

Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 22 ++++++++++++++++------
 drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c  | 12 +++++++++++-
 2 files changed, 27 insertions(+), 7 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);
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c
index 76391fca6c62..0fbf4525947a 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;
@@ -756,8 +762,12 @@ static void gmc_v12_1_get_vm_pte(struct amdgpu_device *adev,
 		break;
 	}
 
+	mem = bo ? bo->tbo.resource : NULL;
+	if (amdgpu_virt_vram_is_spm(adev) &&
+	    (!mem || mem->mem_type == TTM_PL_VRAM))
+		*flags |= AMDGPU_PTE_SYSTEM;
+
 	if (bo) {
-		mem = bo->tbo.resource;
 		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",
-- 
2.43.0

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

[Christian König]

On 11/11/26 20:23, Zhigang Luo wrote:
> 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.
> 
> PDB0 is filled through amdgpu_gmc_get_vm_pde() so those entries take
> the same path. The VRAM entries are PDE_PTEs and already hold a
> physical address, so the translation is skipped. The GART PDE is
> given the GART BO's MC address and translated once.
> 
> gmc_v12_1_get_vm_pte() sets the bit only for VRAM. SVM VRAM ranges
> call it with a NULL buffer object and leave the bit clear. NPA
> mappings are left unchanged.
> 
> Signed-off-by: Zhigang Luo <zhigang.luo@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 22 ++++++++++++++++------
>  drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c  | 12 +++++++++++-
>  2 files changed, 27 insertions(+), 7 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);
> +	}

That is still a little bit hacky but certainly better than before.

>  
>  	/* 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);

Please move those two changes into a separate patch.

>  	drm_dev_exit(idx);
>  }
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c b/drivers/gpu/drm/amd/amdgpu/gmc_v12_1.c
> index 76391fca6c62..0fbf4525947a 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;
> @@ -756,8 +762,12 @@ static void gmc_v12_1_get_vm_pte(struct amdgpu_device *adev,
>  		break;
>  	}
>  
> +	mem = bo ? bo->tbo.resource : NULL;
> +	if (amdgpu_virt_vram_is_spm(adev) &&
> +	    (!mem || mem->mem_type == TTM_PL_VRAM))
> +		*flags |= AMDGPU_PTE_SYSTEM;
> +
>  	if (bo) {
> -		mem = bo->tbo.resource;

Move the check here, this way you don't need to check if there is a BO any more and already have mem.

Apart from those minor comments that looks good to me now.

Regards,
Christian.

>  		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",