[2/2] drm/amdgpu: cleanup PTE flag generation

Submitted by Christian König on Sept. 2, 2019, 2:57 p.m.

Details

Message ID 20190902145732.2567-2-christian.koenig@amd.com
State New
Headers show
Series "Series without cover letter" ( rev: 2 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Christian König Sept. 2, 2019, 2:57 p.m.
Move the ASIC specific code into a new callback function.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 29 ++-----------------------
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 22 ++++++++++++++++++-
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  9 ++++++++
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 11 +++++++++-
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 13 ++++++++++-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 24 +++++++++++++++++++-
 7 files changed, 82 insertions(+), 31 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index d5e4574afbc2..d3be51ba6349 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -104,6 +104,10 @@  struct amdgpu_gmc_funcs {
 	/* get the pde for a given mc addr */
 	void (*get_vm_pde)(struct amdgpu_device *adev, int level,
 			   u64 *dst, u64 *flags);
+	/* get the pte flags to use for a BO VA mapping */
+	void (*get_vm_pte)(struct amdgpu_device *adev,
+			   struct amdgpu_bo_va_mapping *mapping,
+			   uint64_t *flags);
 };
 
 struct amdgpu_xgmi {
@@ -185,6 +189,7 @@  struct amdgpu_gmc {
 #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))
 #define amdgpu_gmc_map_mtype(adev, flags) (adev)->gmc.gmc_funcs->map_mtype((adev),(flags))
 #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))
+#define amdgpu_gmc_get_vm_pte(adev, mapping, flags) (adev)->gmc.gmc_funcs->get_vm_pte((adev), (mapping), (flags))
 
 /**
  * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 2cb82b229802..b285ab25146d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1571,33 +1571,8 @@  static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
 	if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))
 		flags &= ~AMDGPU_PTE_WRITEABLE;
 
-	if (adev->asic_type >= CHIP_TONGA) {
-		flags &= ~AMDGPU_PTE_EXECUTABLE;
-		flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
-	}
-
-	if (adev->asic_type >= CHIP_NAVI10) {
-		flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
-		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
-	} else {
-		flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
-		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK);
-	}
-
-	if ((mapping->flags & AMDGPU_PTE_PRT) &&
-	    (adev->asic_type >= CHIP_VEGA10)) {
-		flags |= AMDGPU_PTE_PRT;
-		if (adev->asic_type >= CHIP_NAVI10) {
-			flags |= AMDGPU_PTE_SNOOPED;
-			flags |= AMDGPU_PTE_LOG;
-			flags |= AMDGPU_PTE_SYSTEM;
-		}
-		flags &= ~AMDGPU_PTE_VALID;
-	}
-	if (adev->asic_type == CHIP_ARCTURUS &&
-	    !(flags & AMDGPU_PTE_SYSTEM) &&
-	    mapping->bo_va->is_xgmi)
-		flags |= AMDGPU_PTE_SNOOPED;
+	/* Apply ASIC specific mapping flags */
+	amdgpu_gmc_get_vm_pte(adev, mapping, &flags);
 
 	trace_amdgpu_vm_bo_update(mapping);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 7eb0ba87fef9..1a8d8f528b01 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -440,12 +440,32 @@  static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	}
 }
 
+static void gmc_v10_0_get_vm_pte(struct amdgpu_device *adev,
+				 struct amdgpu_bo_va_mapping *mapping,
+				 uint64_t *flags)
+{
+	*flags &= ~AMDGPU_PTE_EXECUTABLE;
+	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
+
+	*flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
+	*flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
+
+	if (mapping->flags & AMDGPU_PTE_PRT) {
+		*flags |= AMDGPU_PTE_PRT;
+		*flags |= AMDGPU_PTE_SNOOPED;
+		*flags |= AMDGPU_PTE_LOG;
+		*flags |= AMDGPU_PTE_SYSTEM;
+		*flags &= ~AMDGPU_PTE_VALID;
+	}
+}
+
 static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {
 	.flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,
 	.emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,
 	.map_mtype = gmc_v10_0_map_mtype,
-	.get_vm_pde = gmc_v10_0_get_vm_pde
+	.get_vm_pde = gmc_v10_0_get_vm_pde,
+	.get_vm_pte = gmc_v10_0_get_vm_pte
 };
 
 static void gmc_v10_0_set_gmc_funcs(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index 2e305b21db69..ce591c995691 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -389,6 +389,14 @@  static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
 }
 
+static void gmc_v6_0_get_vm_pte(struct amdgpu_device *adev,
+				struct amdgpu_bo_va_mapping *mapping,
+				uint64_t *flags)
+{
+	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
+	BUG_ON(*flags & AMDGPU_PTE_PRT);
+}
+
 static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
 					      bool value)
 {
@@ -1144,6 +1152,7 @@  static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {
 	.emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,
 	.set_prt = gmc_v6_0_set_prt,
 	.get_vm_pde = gmc_v6_0_get_vm_pde,
+	.get_vm_pte = gmc_v6_0_get_vm_pte,
 };
 
 static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 3b77421234a7..e3f53fbfcd8f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -466,6 +466,14 @@  static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
 }
 
+static void gmc_v7_0_get_vm_pte(struct amdgpu_device *adev,
+				struct amdgpu_bo_va_mapping *mapping,
+				uint64_t *flags)
+{
+	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
+	BUG_ON(*flags & AMDGPU_PTE_PRT);
+}
+
 /**
  * gmc_v8_0_set_fault_enable_default - update VM fault handling
  *
@@ -1339,7 +1347,8 @@  static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
 	.emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,
 	.set_prt = gmc_v7_0_set_prt,
-	.get_vm_pde = gmc_v7_0_get_vm_pde
+	.get_vm_pde = gmc_v7_0_get_vm_pde,
+	.get_vm_pte = gmc_v7_0_get_vm_pte
 };
 
 static const struct amdgpu_irq_src_funcs gmc_v7_0_irq_funcs = {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 58444aa0af05..256d1faacada 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -689,6 +689,16 @@  static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
 }
 
+static void gmc_v8_0_get_vm_pte(struct amdgpu_device *adev,
+				struct amdgpu_bo_va_mapping *mapping,
+				uint64_t *flags)
+{
+	BUG_ON(*flags & AMDGPU_PTE_PRT);
+
+	*flags &= ~AMDGPU_PTE_EXECUTABLE;
+	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
+}
+
 /**
  * gmc_v8_0_set_fault_enable_default - update VM fault handling
  *
@@ -1705,7 +1715,8 @@  static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
 	.emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,
 	.set_prt = gmc_v8_0_set_prt,
-	.get_vm_pde = gmc_v8_0_get_vm_pde
+	.get_vm_pde = gmc_v8_0_get_vm_pde,
+	.get_vm_pte = gmc_v8_0_get_vm_pte
 };
 
 static const struct amdgpu_irq_src_funcs gmc_v8_0_irq_funcs = {
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 22660e1005db..b3d2d70e84c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -625,12 +625,34 @@  static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
 	}
 }
 
+static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
+				struct amdgpu_bo_va_mapping *mapping,
+				uint64_t *flags)
+{
+	*flags &= ~AMDGPU_PTE_EXECUTABLE;
+	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
+
+	*flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
+	*flags |= mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK;
+
+	if (mapping->flags & AMDGPU_PTE_PRT) {
+		*flags |= AMDGPU_PTE_PRT;
+		*flags &= ~AMDGPU_PTE_VALID;
+	}
+
+	if (adev->asic_type == CHIP_ARCTURUS &&
+	    !(*flags & AMDGPU_PTE_SYSTEM) &&
+	    mapping->bo_va->is_xgmi)
+		*flags |= AMDGPU_PTE_SNOOPED;
+}
+
 static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
 	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
 	.emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
 	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
 	.map_mtype = gmc_v9_0_map_mtype,
-	.get_vm_pde = gmc_v9_0_get_vm_pde
+	.get_vm_pde = gmc_v9_0_get_vm_pde,
+	.get_vm_pte = gmc_v9_0_get_vm_pte
 };
 
 static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)

Comments

On 2019-09-02 10:57 a.m., Christian König wrote:
> Move the ASIC specific code into a new callback function.


NAK. I believe the BUG_ONs you're adding will trigger with ROCm on 
Hawaii and Kaveri. See inline ...

ROCm user mode doesn't care that the page table doesn't have an 
executable bit on those chips. If the HW makes all memory executable, we 
should just ignore the flag.


>

> Signed-off-by: Christian König <christian.koenig@amd.com>

> ---

>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++++

>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 29 ++-----------------------

>   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 22 ++++++++++++++++++-

>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  9 ++++++++

>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 11 +++++++++-

>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 13 ++++++++++-

>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 24 +++++++++++++++++++-

>   7 files changed, 82 insertions(+), 31 deletions(-)

>

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h

> index d5e4574afbc2..d3be51ba6349 100644

> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h

> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h

> @@ -104,6 +104,10 @@ struct amdgpu_gmc_funcs {

>   	/* get the pde for a given mc addr */

>   	void (*get_vm_pde)(struct amdgpu_device *adev, int level,

>   			   u64 *dst, u64 *flags);

> +	/* get the pte flags to use for a BO VA mapping */

> +	void (*get_vm_pte)(struct amdgpu_device *adev,

> +			   struct amdgpu_bo_va_mapping *mapping,

> +			   uint64_t *flags);

>   };

>   

>   struct amdgpu_xgmi {

> @@ -185,6 +189,7 @@ struct amdgpu_gmc {

>   #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))

>   #define amdgpu_gmc_map_mtype(adev, flags) (adev)->gmc.gmc_funcs->map_mtype((adev),(flags))

>   #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))

> +#define amdgpu_gmc_get_vm_pte(adev, mapping, flags) (adev)->gmc.gmc_funcs->get_vm_pte((adev), (mapping), (flags))

>   

>   /**

>    * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR

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

> index 2cb82b229802..b285ab25146d 100644

> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

> @@ -1571,33 +1571,8 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,

>   	if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))

>   		flags &= ~AMDGPU_PTE_WRITEABLE;

>   

> -	if (adev->asic_type >= CHIP_TONGA) {

> -		flags &= ~AMDGPU_PTE_EXECUTABLE;

> -		flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;

> -	}

> -

> -	if (adev->asic_type >= CHIP_NAVI10) {

> -		flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;

> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);

> -	} else {

> -		flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;

> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK);

> -	}

> -

> -	if ((mapping->flags & AMDGPU_PTE_PRT) &&

> -	    (adev->asic_type >= CHIP_VEGA10)) {

> -		flags |= AMDGPU_PTE_PRT;

> -		if (adev->asic_type >= CHIP_NAVI10) {

> -			flags |= AMDGPU_PTE_SNOOPED;

> -			flags |= AMDGPU_PTE_LOG;

> -			flags |= AMDGPU_PTE_SYSTEM;

> -		}

> -		flags &= ~AMDGPU_PTE_VALID;

> -	}

> -	if (adev->asic_type == CHIP_ARCTURUS &&

> -	    !(flags & AMDGPU_PTE_SYSTEM) &&

> -	    mapping->bo_va->is_xgmi)

> -		flags |= AMDGPU_PTE_SNOOPED;

> +	/* Apply ASIC specific mapping flags */

> +	amdgpu_gmc_get_vm_pte(adev, mapping, &flags);

>   

>   	trace_amdgpu_vm_bo_update(mapping);

>   

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

> index 7eb0ba87fef9..1a8d8f528b01 100644

> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c

> @@ -440,12 +440,32 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,

>   	}

>   }

>   

> +static void gmc_v10_0_get_vm_pte(struct amdgpu_device *adev,

> +				 struct amdgpu_bo_va_mapping *mapping,

> +				 uint64_t *flags)

> +{

> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;

> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;

> +

> +	*flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;

> +	*flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);

> +

> +	if (mapping->flags & AMDGPU_PTE_PRT) {

> +		*flags |= AMDGPU_PTE_PRT;

> +		*flags |= AMDGPU_PTE_SNOOPED;

> +		*flags |= AMDGPU_PTE_LOG;

> +		*flags |= AMDGPU_PTE_SYSTEM;

> +		*flags &= ~AMDGPU_PTE_VALID;

> +	}

> +}

> +

>   static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {

>   	.flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,

>   	.emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,

>   	.emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,

>   	.map_mtype = gmc_v10_0_map_mtype,

> -	.get_vm_pde = gmc_v10_0_get_vm_pde

> +	.get_vm_pde = gmc_v10_0_get_vm_pde,

> +	.get_vm_pte = gmc_v10_0_get_vm_pte

>   };

>   

>   static void gmc_v10_0_set_gmc_funcs(struct amdgpu_device *adev)

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

> index 2e305b21db69..ce591c995691 100644

> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c

> @@ -389,6 +389,14 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,

>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);

>   }

>   

> +static void gmc_v6_0_get_vm_pte(struct amdgpu_device *adev,

> +				struct amdgpu_bo_va_mapping *mapping,

> +				uint64_t *flags)

> +{

> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);

> +	BUG_ON(*flags & AMDGPU_PTE_PRT);

> +}

> +

>   static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,

>   					      bool value)

>   {

> @@ -1144,6 +1152,7 @@ static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {

>   	.emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,

>   	.set_prt = gmc_v6_0_set_prt,

>   	.get_vm_pde = gmc_v6_0_get_vm_pde,

> +	.get_vm_pte = gmc_v6_0_get_vm_pte,

>   };

>   

>   static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {

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

> index 3b77421234a7..e3f53fbfcd8f 100644

> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c

> @@ -466,6 +466,14 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,

>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);

>   }

>   

> +static void gmc_v7_0_get_vm_pte(struct amdgpu_device *adev,

> +				struct amdgpu_bo_va_mapping *mapping,

> +				uint64_t *flags)

> +{

> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);


NAK. This is going to break ROCm on Kaveri and Hawaii.

Regards,
   Felix


> +	BUG_ON(*flags & AMDGPU_PTE_PRT);

> +}

> +

>   /**

>    * gmc_v8_0_set_fault_enable_default - update VM fault handling

>    *

> @@ -1339,7 +1347,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {

>   	.emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,

>   	.emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,

>   	.set_prt = gmc_v7_0_set_prt,

> -	.get_vm_pde = gmc_v7_0_get_vm_pde

> +	.get_vm_pde = gmc_v7_0_get_vm_pde,

> +	.get_vm_pte = gmc_v7_0_get_vm_pte

>   };

>   

>   static const struct amdgpu_irq_src_funcs gmc_v7_0_irq_funcs = {

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

> index 58444aa0af05..256d1faacada 100644

> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c

> @@ -689,6 +689,16 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,

>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);

>   }

>   

> +static void gmc_v8_0_get_vm_pte(struct amdgpu_device *adev,

> +				struct amdgpu_bo_va_mapping *mapping,

> +				uint64_t *flags)

> +{

> +	BUG_ON(*flags & AMDGPU_PTE_PRT);

> +

> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;

> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;

> +}

> +

>   /**

>    * gmc_v8_0_set_fault_enable_default - update VM fault handling

>    *

> @@ -1705,7 +1715,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {

>   	.emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,

>   	.emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,

>   	.set_prt = gmc_v8_0_set_prt,

> -	.get_vm_pde = gmc_v8_0_get_vm_pde

> +	.get_vm_pde = gmc_v8_0_get_vm_pde,

> +	.get_vm_pte = gmc_v8_0_get_vm_pte

>   };

>   

>   static const struct amdgpu_irq_src_funcs gmc_v8_0_irq_funcs = {

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

> index 22660e1005db..b3d2d70e84c9 100644

> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c

> @@ -625,12 +625,34 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,

>   	}

>   }

>   

> +static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,

> +				struct amdgpu_bo_va_mapping *mapping,

> +				uint64_t *flags)

> +{

> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;

> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;

> +

> +	*flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;

> +	*flags |= mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK;

> +

> +	if (mapping->flags & AMDGPU_PTE_PRT) {

> +		*flags |= AMDGPU_PTE_PRT;

> +		*flags &= ~AMDGPU_PTE_VALID;

> +	}

> +

> +	if (adev->asic_type == CHIP_ARCTURUS &&

> +	    !(*flags & AMDGPU_PTE_SYSTEM) &&

> +	    mapping->bo_va->is_xgmi)

> +		*flags |= AMDGPU_PTE_SNOOPED;

> +}

> +

>   static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {

>   	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,

>   	.emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,

>   	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,

>   	.map_mtype = gmc_v9_0_map_mtype,

> -	.get_vm_pde = gmc_v9_0_get_vm_pde

> +	.get_vm_pde = gmc_v9_0_get_vm_pde,

> +	.get_vm_pte = gmc_v9_0_get_vm_pte

>   };

>   

>   static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)
Maybe  it's not necessary to separate the mtype from the get_vm_pte 
function , so we just need one  asic specific functions (get_vm_pte) .

What make me confusing originally is  the  the  logic  to setup the  PTE 
flag.   We first map the  UAPI flags to HW specific PTE flag , save it 
into mapping  structure  and  in amdgpu_bo_split_mapping adjust the  
flag again before set the value to HW . Is it possible we just have one 
place to set the asic specific PTE flag ,  ex, the  mapping  structure 
still keep the  UAPI flag and  all the HW specific PTE setting is in the 
last step before real set to HW ?

Regards

shaoyun.liu

On 2019-09-02 10:57 a.m., Christian König wrote:
> Move the ASIC specific code into a new callback function.

>

> Signed-off-by: Christian König <christian.koenig@amd.com>

> ---

>   drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++++

>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 29 ++-----------------------

>   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 22 ++++++++++++++++++-

>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  9 ++++++++

>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 11 +++++++++-

>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 13 ++++++++++-

>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 24 +++++++++++++++++++-

>   7 files changed, 82 insertions(+), 31 deletions(-)

>

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h

> index d5e4574afbc2..d3be51ba6349 100644

> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h

> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h

> @@ -104,6 +104,10 @@ struct amdgpu_gmc_funcs {

>   	/* get the pde for a given mc addr */

>   	void (*get_vm_pde)(struct amdgpu_device *adev, int level,

>   			   u64 *dst, u64 *flags);

> +	/* get the pte flags to use for a BO VA mapping */

> +	void (*get_vm_pte)(struct amdgpu_device *adev,

> +			   struct amdgpu_bo_va_mapping *mapping,

> +			   uint64_t *flags);

>   };

>   

>   struct amdgpu_xgmi {

> @@ -185,6 +189,7 @@ struct amdgpu_gmc {

>   #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))

>   #define amdgpu_gmc_map_mtype(adev, flags) (adev)->gmc.gmc_funcs->map_mtype((adev),(flags))

>   #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))

> +#define amdgpu_gmc_get_vm_pte(adev, mapping, flags) (adev)->gmc.gmc_funcs->get_vm_pte((adev), (mapping), (flags))

>   

>   /**

>    * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR

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

> index 2cb82b229802..b285ab25146d 100644

> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

> @@ -1571,33 +1571,8 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,

>   	if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))

>   		flags &= ~AMDGPU_PTE_WRITEABLE;

>   

> -	if (adev->asic_type >= CHIP_TONGA) {

> -		flags &= ~AMDGPU_PTE_EXECUTABLE;

> -		flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;

> -	}

> -

> -	if (adev->asic_type >= CHIP_NAVI10) {

> -		flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;

> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);

> -	} else {

> -		flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;

> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK);

> -	}

> -

> -	if ((mapping->flags & AMDGPU_PTE_PRT) &&

> -	    (adev->asic_type >= CHIP_VEGA10)) {

> -		flags |= AMDGPU_PTE_PRT;

> -		if (adev->asic_type >= CHIP_NAVI10) {

> -			flags |= AMDGPU_PTE_SNOOPED;

> -			flags |= AMDGPU_PTE_LOG;

> -			flags |= AMDGPU_PTE_SYSTEM;

> -		}

> -		flags &= ~AMDGPU_PTE_VALID;

> -	}

> -	if (adev->asic_type == CHIP_ARCTURUS &&

> -	    !(flags & AMDGPU_PTE_SYSTEM) &&

> -	    mapping->bo_va->is_xgmi)

> -		flags |= AMDGPU_PTE_SNOOPED;

> +	/* Apply ASIC specific mapping flags */

> +	amdgpu_gmc_get_vm_pte(adev, mapping, &flags);

>   

>   	trace_amdgpu_vm_bo_update(mapping);

>   

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

> index 7eb0ba87fef9..1a8d8f528b01 100644

> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c

> @@ -440,12 +440,32 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,

>   	}

>   }

>   

> +static void gmc_v10_0_get_vm_pte(struct amdgpu_device *adev,

> +				 struct amdgpu_bo_va_mapping *mapping,

> +				 uint64_t *flags)

> +{

> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;

> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;

> +

> +	*flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;

> +	*flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);

> +

> +	if (mapping->flags & AMDGPU_PTE_PRT) {

> +		*flags |= AMDGPU_PTE_PRT;

> +		*flags |= AMDGPU_PTE_SNOOPED;

> +		*flags |= AMDGPU_PTE_LOG;

> +		*flags |= AMDGPU_PTE_SYSTEM;

> +		*flags &= ~AMDGPU_PTE_VALID;

> +	}

> +}

> +

>   static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {

>   	.flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,

>   	.emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,

>   	.emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,

>   	.map_mtype = gmc_v10_0_map_mtype,

> -	.get_vm_pde = gmc_v10_0_get_vm_pde

> +	.get_vm_pde = gmc_v10_0_get_vm_pde,

> +	.get_vm_pte = gmc_v10_0_get_vm_pte

>   };

>   

>   static void gmc_v10_0_set_gmc_funcs(struct amdgpu_device *adev)

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

> index 2e305b21db69..ce591c995691 100644

> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c

> @@ -389,6 +389,14 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,

>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);

>   }

>   

> +static void gmc_v6_0_get_vm_pte(struct amdgpu_device *adev,

> +				struct amdgpu_bo_va_mapping *mapping,

> +				uint64_t *flags)

> +{

> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);

> +	BUG_ON(*flags & AMDGPU_PTE_PRT);

> +}

> +

>   static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,

>   					      bool value)

>   {

> @@ -1144,6 +1152,7 @@ static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {

>   	.emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,

>   	.set_prt = gmc_v6_0_set_prt,

>   	.get_vm_pde = gmc_v6_0_get_vm_pde,

> +	.get_vm_pte = gmc_v6_0_get_vm_pte,

>   };

>   

>   static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {

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

> index 3b77421234a7..e3f53fbfcd8f 100644

> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c

> @@ -466,6 +466,14 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,

>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);

>   }

>   

> +static void gmc_v7_0_get_vm_pte(struct amdgpu_device *adev,

> +				struct amdgpu_bo_va_mapping *mapping,

> +				uint64_t *flags)

> +{

> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);

> +	BUG_ON(*flags & AMDGPU_PTE_PRT);

> +}

> +

>   /**

>    * gmc_v8_0_set_fault_enable_default - update VM fault handling

>    *

> @@ -1339,7 +1347,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {

>   	.emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,

>   	.emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,

>   	.set_prt = gmc_v7_0_set_prt,

> -	.get_vm_pde = gmc_v7_0_get_vm_pde

> +	.get_vm_pde = gmc_v7_0_get_vm_pde,

> +	.get_vm_pte = gmc_v7_0_get_vm_pte

>   };

>   

>   static const struct amdgpu_irq_src_funcs gmc_v7_0_irq_funcs = {

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

> index 58444aa0af05..256d1faacada 100644

> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c

> @@ -689,6 +689,16 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,

>   	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);

>   }

>   

> +static void gmc_v8_0_get_vm_pte(struct amdgpu_device *adev,

> +				struct amdgpu_bo_va_mapping *mapping,

> +				uint64_t *flags)

> +{

> +	BUG_ON(*flags & AMDGPU_PTE_PRT);

> +

> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;

> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;

> +}

> +

>   /**

>    * gmc_v8_0_set_fault_enable_default - update VM fault handling

>    *

> @@ -1705,7 +1715,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {

>   	.emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,

>   	.emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,

>   	.set_prt = gmc_v8_0_set_prt,

> -	.get_vm_pde = gmc_v8_0_get_vm_pde

> +	.get_vm_pde = gmc_v8_0_get_vm_pde,

> +	.get_vm_pte = gmc_v8_0_get_vm_pte

>   };

>   

>   static const struct amdgpu_irq_src_funcs gmc_v8_0_irq_funcs = {

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

> index 22660e1005db..b3d2d70e84c9 100644

> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c

> @@ -625,12 +625,34 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,

>   	}

>   }

>   

> +static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,

> +				struct amdgpu_bo_va_mapping *mapping,

> +				uint64_t *flags)

> +{

> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;

> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;

> +

> +	*flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;

> +	*flags |= mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK;

> +

> +	if (mapping->flags & AMDGPU_PTE_PRT) {

> +		*flags |= AMDGPU_PTE_PRT;

> +		*flags &= ~AMDGPU_PTE_VALID;

> +	}

> +

> +	if (adev->asic_type == CHIP_ARCTURUS &&

> +	    !(*flags & AMDGPU_PTE_SYSTEM) &&

> +	    mapping->bo_va->is_xgmi)

> +		*flags |= AMDGPU_PTE_SNOOPED;

> +}

> +

>   static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {

>   	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,

>   	.emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,

>   	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,

>   	.map_mtype = gmc_v9_0_map_mtype,

> -	.get_vm_pde = gmc_v9_0_get_vm_pde

> +	.get_vm_pde = gmc_v9_0_get_vm_pde,

> +	.get_vm_pte = gmc_v9_0_get_vm_pte

>   };

>   

>   static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)
Am 03.09.19 um 22:28 schrieb Kuehling, Felix:
> On 2019-09-02 10:57 a.m., Christian König wrote:
>> Move the ASIC specific code into a new callback function.
> NAK. I believe the BUG_ONs you're adding will trigger with ROCm on
> Hawaii and Kaveri. See inline ...

Those are the flags as generated by the TTM code, they shouldn't contain 
the executable bit.

The mapping->flags are the one generated by ROCm.

> ROCm user mode doesn't care that the page table doesn't have an
> executable bit on those chips. If the HW makes all memory executable, we
> should just ignore the flag.

I've added this because I'm not sure how the hardware reacts when it 
sees a reserved bit set.

The alternative is to just clear the flag, because it isn't really 
supported in any way.

Christian.

>
>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++++
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 29 ++-----------------------
>>    drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 22 ++++++++++++++++++-
>>    drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  9 ++++++++
>>    drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 11 +++++++++-
>>    drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 13 ++++++++++-
>>    drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 24 +++++++++++++++++++-
>>    7 files changed, 82 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> index d5e4574afbc2..d3be51ba6349 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> @@ -104,6 +104,10 @@ struct amdgpu_gmc_funcs {
>>    	/* get the pde for a given mc addr */
>>    	void (*get_vm_pde)(struct amdgpu_device *adev, int level,
>>    			   u64 *dst, u64 *flags);
>> +	/* get the pte flags to use for a BO VA mapping */
>> +	void (*get_vm_pte)(struct amdgpu_device *adev,
>> +			   struct amdgpu_bo_va_mapping *mapping,
>> +			   uint64_t *flags);
>>    };
>>    
>>    struct amdgpu_xgmi {
>> @@ -185,6 +189,7 @@ struct amdgpu_gmc {
>>    #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))
>>    #define amdgpu_gmc_map_mtype(adev, flags) (adev)->gmc.gmc_funcs->map_mtype((adev),(flags))
>>    #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))
>> +#define amdgpu_gmc_get_vm_pte(adev, mapping, flags) (adev)->gmc.gmc_funcs->get_vm_pte((adev), (mapping), (flags))
>>    
>>    /**
>>     * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 2cb82b229802..b285ab25146d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1571,33 +1571,8 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>>    	if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))
>>    		flags &= ~AMDGPU_PTE_WRITEABLE;
>>    
>> -	if (adev->asic_type >= CHIP_TONGA) {
>> -		flags &= ~AMDGPU_PTE_EXECUTABLE;
>> -		flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> -	}
>> -
>> -	if (adev->asic_type >= CHIP_NAVI10) {
>> -		flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
>> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
>> -	} else {
>> -		flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
>> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK);
>> -	}
>> -
>> -	if ((mapping->flags & AMDGPU_PTE_PRT) &&
>> -	    (adev->asic_type >= CHIP_VEGA10)) {
>> -		flags |= AMDGPU_PTE_PRT;
>> -		if (adev->asic_type >= CHIP_NAVI10) {
>> -			flags |= AMDGPU_PTE_SNOOPED;
>> -			flags |= AMDGPU_PTE_LOG;
>> -			flags |= AMDGPU_PTE_SYSTEM;
>> -		}
>> -		flags &= ~AMDGPU_PTE_VALID;
>> -	}
>> -	if (adev->asic_type == CHIP_ARCTURUS &&
>> -	    !(flags & AMDGPU_PTE_SYSTEM) &&
>> -	    mapping->bo_va->is_xgmi)
>> -		flags |= AMDGPU_PTE_SNOOPED;
>> +	/* Apply ASIC specific mapping flags */
>> +	amdgpu_gmc_get_vm_pte(adev, mapping, &flags);
>>    
>>    	trace_amdgpu_vm_bo_update(mapping);
>>    
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> index 7eb0ba87fef9..1a8d8f528b01 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> @@ -440,12 +440,32 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	}
>>    }
>>    
>> +static void gmc_v10_0_get_vm_pte(struct amdgpu_device *adev,
>> +				 struct amdgpu_bo_va_mapping *mapping,
>> +				 uint64_t *flags)
>> +{
>> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +
>> +	*flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
>> +	*flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
>> +
>> +	if (mapping->flags & AMDGPU_PTE_PRT) {
>> +		*flags |= AMDGPU_PTE_PRT;
>> +		*flags |= AMDGPU_PTE_SNOOPED;
>> +		*flags |= AMDGPU_PTE_LOG;
>> +		*flags |= AMDGPU_PTE_SYSTEM;
>> +		*flags &= ~AMDGPU_PTE_VALID;
>> +	}
>> +}
>> +
>>    static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {
>>    	.flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,
>>    	.emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,
>>    	.map_mtype = gmc_v10_0_map_mtype,
>> -	.get_vm_pde = gmc_v10_0_get_vm_pde
>> +	.get_vm_pde = gmc_v10_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v10_0_get_vm_pte
>>    };
>>    
>>    static void gmc_v10_0_set_gmc_funcs(struct amdgpu_device *adev)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> index 2e305b21db69..ce591c995691 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> @@ -389,6 +389,14 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>>    }
>>    
>> +static void gmc_v6_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
>> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +}
>> +
>>    static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
>>    					      bool value)
>>    {
>> @@ -1144,6 +1152,7 @@ static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {
>>    	.emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,
>>    	.set_prt = gmc_v6_0_set_prt,
>>    	.get_vm_pde = gmc_v6_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v6_0_get_vm_pte,
>>    };
>>    
>>    static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> index 3b77421234a7..e3f53fbfcd8f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -466,6 +466,14 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>>    }
>>    
>> +static void gmc_v7_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
> NAK. This is going to break ROCm on Kaveri and Hawaii.
>
> Regards,
>     Felix
>
>
>> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +}
>> +
>>    /**
>>     * gmc_v8_0_set_fault_enable_default - update VM fault handling
>>     *
>> @@ -1339,7 +1347,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
>>    	.emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,
>>    	.set_prt = gmc_v7_0_set_prt,
>> -	.get_vm_pde = gmc_v7_0_get_vm_pde
>> +	.get_vm_pde = gmc_v7_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v7_0_get_vm_pte
>>    };
>>    
>>    static const struct amdgpu_irq_src_funcs gmc_v7_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> index 58444aa0af05..256d1faacada 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -689,6 +689,16 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>>    }
>>    
>> +static void gmc_v8_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +
>> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +}
>> +
>>    /**
>>     * gmc_v8_0_set_fault_enable_default - update VM fault handling
>>     *
>> @@ -1705,7 +1715,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
>>    	.emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,
>>    	.set_prt = gmc_v8_0_set_prt,
>> -	.get_vm_pde = gmc_v8_0_get_vm_pde
>> +	.get_vm_pde = gmc_v8_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v8_0_get_vm_pte
>>    };
>>    
>>    static const struct amdgpu_irq_src_funcs gmc_v8_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index 22660e1005db..b3d2d70e84c9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -625,12 +625,34 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	}
>>    }
>>    
>> +static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +
>> +	*flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
>> +	*flags |= mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK;
>> +
>> +	if (mapping->flags & AMDGPU_PTE_PRT) {
>> +		*flags |= AMDGPU_PTE_PRT;
>> +		*flags &= ~AMDGPU_PTE_VALID;
>> +	}
>> +
>> +	if (adev->asic_type == CHIP_ARCTURUS &&
>> +	    !(*flags & AMDGPU_PTE_SYSTEM) &&
>> +	    mapping->bo_va->is_xgmi)
>> +		*flags |= AMDGPU_PTE_SNOOPED;
>> +}
>> +
>>    static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
>>    	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
>>    	.emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
>>    	.map_mtype = gmc_v9_0_map_mtype,
>> -	.get_vm_pde = gmc_v9_0_get_vm_pde
>> +	.get_vm_pde = gmc_v9_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v9_0_get_vm_pte
>>    };
>>    
>>    static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)
> the  mapping  structure still keep the  UAPI flag and  all the HW specific PTE setting is in the
> last step before real set to HW ?

I thought about that approach as well, but that would unfortunately not 
work correctly.

The problem is that we have some mapping flags which don't have an 
userspace representation.

Additional to that I want to avoid leaking the UAPI defines into the VM 
subsystem.

We shouldn't have created the UAPI defines in the first place and should 
have used the hardware flags directly.

Regards,
Christian.

Am 03.09.19 um 23:32 schrieb Liu, Shaoyun:
> Maybe  it's not necessary to separate the mtype from the get_vm_pte
> function , so we just need one  asic specific functions (get_vm_pte) .
>
> What make me confusing originally is  the  the  logic  to setup the  PTE
> flag.   We first map the  UAPI flags to HW specific PTE flag , save it
> into mapping  structure  and  in amdgpu_bo_split_mapping adjust the
> flag again before set the value to HW . Is it possible we just have one
> place to set the asic specific PTE flag ,  ex, the  mapping  structure
> still keep the  UAPI flag and  all the HW specific PTE setting is in the
> last step before real set to HW ?
>
> Regards
>
> shaoyun.liu
>
> On 2019-09-02 10:57 a.m., Christian König wrote:
>> Move the ASIC specific code into a new callback function.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  5 +++++
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 29 ++-----------------------
>>    drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c  | 22 ++++++++++++++++++-
>>    drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  9 ++++++++
>>    drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 11 +++++++++-
>>    drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 13 ++++++++++-
>>    drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 24 +++++++++++++++++++-
>>    7 files changed, 82 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> index d5e4574afbc2..d3be51ba6349 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> @@ -104,6 +104,10 @@ struct amdgpu_gmc_funcs {
>>    	/* get the pde for a given mc addr */
>>    	void (*get_vm_pde)(struct amdgpu_device *adev, int level,
>>    			   u64 *dst, u64 *flags);
>> +	/* get the pte flags to use for a BO VA mapping */
>> +	void (*get_vm_pte)(struct amdgpu_device *adev,
>> +			   struct amdgpu_bo_va_mapping *mapping,
>> +			   uint64_t *flags);
>>    };
>>    
>>    struct amdgpu_xgmi {
>> @@ -185,6 +189,7 @@ struct amdgpu_gmc {
>>    #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))
>>    #define amdgpu_gmc_map_mtype(adev, flags) (adev)->gmc.gmc_funcs->map_mtype((adev),(flags))
>>    #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))
>> +#define amdgpu_gmc_get_vm_pte(adev, mapping, flags) (adev)->gmc.gmc_funcs->get_vm_pte((adev), (mapping), (flags))
>>    
>>    /**
>>     * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 2cb82b229802..b285ab25146d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1571,33 +1571,8 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>>    	if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))
>>    		flags &= ~AMDGPU_PTE_WRITEABLE;
>>    
>> -	if (adev->asic_type >= CHIP_TONGA) {
>> -		flags &= ~AMDGPU_PTE_EXECUTABLE;
>> -		flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> -	}
>> -
>> -	if (adev->asic_type >= CHIP_NAVI10) {
>> -		flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
>> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
>> -	} else {
>> -		flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
>> -		flags |= (mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK);
>> -	}
>> -
>> -	if ((mapping->flags & AMDGPU_PTE_PRT) &&
>> -	    (adev->asic_type >= CHIP_VEGA10)) {
>> -		flags |= AMDGPU_PTE_PRT;
>> -		if (adev->asic_type >= CHIP_NAVI10) {
>> -			flags |= AMDGPU_PTE_SNOOPED;
>> -			flags |= AMDGPU_PTE_LOG;
>> -			flags |= AMDGPU_PTE_SYSTEM;
>> -		}
>> -		flags &= ~AMDGPU_PTE_VALID;
>> -	}
>> -	if (adev->asic_type == CHIP_ARCTURUS &&
>> -	    !(flags & AMDGPU_PTE_SYSTEM) &&
>> -	    mapping->bo_va->is_xgmi)
>> -		flags |= AMDGPU_PTE_SNOOPED;
>> +	/* Apply ASIC specific mapping flags */
>> +	amdgpu_gmc_get_vm_pte(adev, mapping, &flags);
>>    
>>    	trace_amdgpu_vm_bo_update(mapping);
>>    
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> index 7eb0ba87fef9..1a8d8f528b01 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> @@ -440,12 +440,32 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	}
>>    }
>>    
>> +static void gmc_v10_0_get_vm_pte(struct amdgpu_device *adev,
>> +				 struct amdgpu_bo_va_mapping *mapping,
>> +				 uint64_t *flags)
>> +{
>> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +
>> +	*flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
>> +	*flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
>> +
>> +	if (mapping->flags & AMDGPU_PTE_PRT) {
>> +		*flags |= AMDGPU_PTE_PRT;
>> +		*flags |= AMDGPU_PTE_SNOOPED;
>> +		*flags |= AMDGPU_PTE_LOG;
>> +		*flags |= AMDGPU_PTE_SYSTEM;
>> +		*flags &= ~AMDGPU_PTE_VALID;
>> +	}
>> +}
>> +
>>    static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {
>>    	.flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,
>>    	.emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,
>>    	.map_mtype = gmc_v10_0_map_mtype,
>> -	.get_vm_pde = gmc_v10_0_get_vm_pde
>> +	.get_vm_pde = gmc_v10_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v10_0_get_vm_pte
>>    };
>>    
>>    static void gmc_v10_0_set_gmc_funcs(struct amdgpu_device *adev)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> index 2e305b21db69..ce591c995691 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> @@ -389,6 +389,14 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>>    }
>>    
>> +static void gmc_v6_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
>> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +}
>> +
>>    static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
>>    					      bool value)
>>    {
>> @@ -1144,6 +1152,7 @@ static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {
>>    	.emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,
>>    	.set_prt = gmc_v6_0_set_prt,
>>    	.get_vm_pde = gmc_v6_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v6_0_get_vm_pte,
>>    };
>>    
>>    static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> index 3b77421234a7..e3f53fbfcd8f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -466,6 +466,14 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>>    }
>>    
>> +static void gmc_v7_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
>> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +}
>> +
>>    /**
>>     * gmc_v8_0_set_fault_enable_default - update VM fault handling
>>     *
>> @@ -1339,7 +1347,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
>>    	.emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,
>>    	.set_prt = gmc_v7_0_set_prt,
>> -	.get_vm_pde = gmc_v7_0_get_vm_pde
>> +	.get_vm_pde = gmc_v7_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v7_0_get_vm_pte
>>    };
>>    
>>    static const struct amdgpu_irq_src_funcs gmc_v7_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> index 58444aa0af05..256d1faacada 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -689,6 +689,16 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>>    }
>>    
>> +static void gmc_v8_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +
>> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +}
>> +
>>    /**
>>     * gmc_v8_0_set_fault_enable_default - update VM fault handling
>>     *
>> @@ -1705,7 +1715,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
>>    	.emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,
>>    	.set_prt = gmc_v8_0_set_prt,
>> -	.get_vm_pde = gmc_v8_0_get_vm_pde
>> +	.get_vm_pde = gmc_v8_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v8_0_get_vm_pte
>>    };
>>    
>>    static const struct amdgpu_irq_src_funcs gmc_v8_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index 22660e1005db..b3d2d70e84c9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -625,12 +625,34 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
>>    	}
>>    }
>>    
>> +static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
>> +				struct amdgpu_bo_va_mapping *mapping,
>> +				uint64_t *flags)
>> +{
>> +	*flags &= ~AMDGPU_PTE_EXECUTABLE;
>> +	*flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +
>> +	*flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
>> +	*flags |= mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK;
>> +
>> +	if (mapping->flags & AMDGPU_PTE_PRT) {
>> +		*flags |= AMDGPU_PTE_PRT;
>> +		*flags &= ~AMDGPU_PTE_VALID;
>> +	}
>> +
>> +	if (adev->asic_type == CHIP_ARCTURUS &&
>> +	    !(*flags & AMDGPU_PTE_SYSTEM) &&
>> +	    mapping->bo_va->is_xgmi)
>> +		*flags |= AMDGPU_PTE_SNOOPED;
>> +}
>> +
>>    static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
>>    	.flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
>>    	.emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
>>    	.emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
>>    	.map_mtype = gmc_v9_0_map_mtype,
>> -	.get_vm_pde = gmc_v9_0_get_vm_pde
>> +	.get_vm_pde = gmc_v9_0_get_vm_pde,
>> +	.get_vm_pte = gmc_v9_0_get_vm_pte
>>    };
>>    
>>    static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)