[2/2] drm/amdgpu: disable vm fault irq during prt accessed

Submitted by Chunming Zhou on Jan. 3, 2019, 8:54 a.m.

Details

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

Not browsing as part of any series.

Commit Message

Chunming Zhou Jan. 3, 2019, 8:54 a.m.
For pre-gmc9, UMD can only access unmapped PRT tile from CB/TC without
firing VM fault. Kernel would still receive the VM fault interrupt
and output the error message if SDMA is the mc_client.
GMC9 don't need the same since it handle the PRT in different way.
We cannot just skip message for SDMA, as Christian pointed, VM fault
could crash mc block, so we disable vm fault irq during prt range is
accesed.
The nagative is normal vm fault could be ignored during that peroid
without enabling vm_debug kernel parameter.

Change-Id: Ic3c62393768eca90e3e45eaf81e7f26f2e91de84
Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 6 ++++++
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 6 ++++++
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 6 ++++++
 3 files changed, 18 insertions(+)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index dae73f6768c2..175c4b319559 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -486,6 +486,10 @@  static void gmc_v6_0_set_prt(struct amdgpu_device *adev, bool enable)
 		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
 		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
 		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
+		/* Note: when vm_debug enabled, vm fault from SDMAx accessing
+		 * PRT range is normal. */
+		if (!amdgpu_vm_debug)
+			amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
 	} else {
 		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
 		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
@@ -495,6 +499,8 @@  static void gmc_v6_0_set_prt(struct amdgpu_device *adev, bool enable)
 		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
 		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
 		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
+		if (!amdgpu_vm_debug)
+			amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
 	}
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 5bdeb358bfb5..a4d6d219f4e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -582,6 +582,10 @@  static void gmc_v7_0_set_prt(struct amdgpu_device *adev, bool enable)
 		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
 		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
 		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
+		/* Note: when vm_debug enabled, vm fault from SDMAx accessing
+		 * PRT range is normal. */
+		if (!amdgpu_vm_debug)
+			amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
 	} else {
 		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
 		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
@@ -591,6 +595,8 @@  static void gmc_v7_0_set_prt(struct amdgpu_device *adev, bool enable)
 		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
 		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
 		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
+		if (!amdgpu_vm_debug)
+			amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
 	}
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 5150ab614eaa..eea2eb7fc2f5 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -808,6 +808,10 @@  static void gmc_v8_0_set_prt(struct amdgpu_device *adev, bool enable)
 		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
 		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
 		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
+		/* Note: when vm_debug enabled, vm fault from SDMAx accessing
+		 * PRT range is normal. */
+		if (!amdgpu_vm_debug)
+			amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
 	} else {
 		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
 		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
@@ -817,6 +821,8 @@  static void gmc_v8_0_set_prt(struct amdgpu_device *adev, bool enable)
 		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
 		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
 		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
+		if (!amdgpu_vm_debug)
+			amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
 	}
 }
 

Comments

NAK, the problem is not the interrupt.

E.g. causing faults by accessing unmapped pages with the SDMA can still 
crash the MC.

The key point is that SDMA can't work with PRT tiles on pre-gmc9 and we 
need to forbid access on the application side.

Regards,
Christian.

Am 03.01.19 um 09:54 schrieb Chunming Zhou:
> For pre-gmc9, UMD can only access unmapped PRT tile from CB/TC without
> firing VM fault. Kernel would still receive the VM fault interrupt
> and output the error message if SDMA is the mc_client.
> GMC9 don't need the same since it handle the PRT in different way.
> We cannot just skip message for SDMA, as Christian pointed, VM fault
> could crash mc block, so we disable vm fault irq during prt range is
> accesed.
> The nagative is normal vm fault could be ignored during that peroid
> without enabling vm_debug kernel parameter.
>
> Change-Id: Ic3c62393768eca90e3e45eaf81e7f26f2e91de84
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 6 ++++++
>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 6 ++++++
>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 6 ++++++
>   3 files changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index dae73f6768c2..175c4b319559 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -486,6 +486,10 @@ static void gmc_v6_0_set_prt(struct amdgpu_device *adev, bool enable)
>   		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
>   		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
>   		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
> +		/* Note: when vm_debug enabled, vm fault from SDMAx accessing
> +		 * PRT range is normal. */
> +		if (!amdgpu_vm_debug)
> +			amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>   	} else {
>   		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
>   		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
> @@ -495,6 +499,8 @@ static void gmc_v6_0_set_prt(struct amdgpu_device *adev, bool enable)
>   		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
>   		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
>   		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
> +		if (!amdgpu_vm_debug)
> +			amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>   	}
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 5bdeb358bfb5..a4d6d219f4e8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -582,6 +582,10 @@ static void gmc_v7_0_set_prt(struct amdgpu_device *adev, bool enable)
>   		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
>   		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
>   		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
> +		/* Note: when vm_debug enabled, vm fault from SDMAx accessing
> +		 * PRT range is normal. */
> +		if (!amdgpu_vm_debug)
> +			amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>   	} else {
>   		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
>   		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
> @@ -591,6 +595,8 @@ static void gmc_v7_0_set_prt(struct amdgpu_device *adev, bool enable)
>   		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
>   		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
>   		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
> +		if (!amdgpu_vm_debug)
> +			amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>   	}
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 5150ab614eaa..eea2eb7fc2f5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -808,6 +808,10 @@ static void gmc_v8_0_set_prt(struct amdgpu_device *adev, bool enable)
>   		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
>   		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
>   		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
> +		/* Note: when vm_debug enabled, vm fault from SDMAx accessing
> +		 * PRT range is normal. */
> +		if (!amdgpu_vm_debug)
> +			amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>   	} else {
>   		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
>   		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
> @@ -817,6 +821,8 @@ static void gmc_v8_0_set_prt(struct amdgpu_device *adev, bool enable)
>   		WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
>   		WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
>   		WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
> +		if (!amdgpu_vm_debug)
> +			amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>   	}
>   }
>
need dummy page for that?


-David


On 2019年01月03日 17:01, Christian König wrote:
> NAK, the problem is not the interrupt.
>
> E.g. causing faults by accessing unmapped pages with the SDMA can 
> still crash the MC.
>
> The key point is that SDMA can't work with PRT tiles on pre-gmc9 and 
> we need to forbid access on the application side.
>
> Regards,
> Christian.
>
> Am 03.01.19 um 09:54 schrieb Chunming Zhou:
>> For pre-gmc9, UMD can only access unmapped PRT tile from CB/TC without
>> firing VM fault. Kernel would still receive the VM fault interrupt
>> and output the error message if SDMA is the mc_client.
>> GMC9 don't need the same since it handle the PRT in different way.
>> We cannot just skip message for SDMA, as Christian pointed, VM fault
>> could crash mc block, so we disable vm fault irq during prt range is
>> accesed.
>> The nagative is normal vm fault could be ignored during that peroid
>> without enabling vm_debug kernel parameter.
>>
>> Change-Id: Ic3c62393768eca90e3e45eaf81e7f26f2e91de84
>> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 6 ++++++
>>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 6 ++++++
>>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 6 ++++++
>>   3 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> index dae73f6768c2..175c4b319559 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> @@ -486,6 +486,10 @@ static void gmc_v6_0_set_prt(struct 
>> amdgpu_device *adev, bool enable)
>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
>> +        /* Note: when vm_debug enabled, vm fault from SDMAx accessing
>> +         * PRT range is normal. */
>> +        if (!amdgpu_vm_debug)
>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>>       } else {
>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
>> @@ -495,6 +499,8 @@ static void gmc_v6_0_set_prt(struct amdgpu_device 
>> *adev, bool enable)
>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
>> +        if (!amdgpu_vm_debug)
>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>       }
>>   }
>>   diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> index 5bdeb358bfb5..a4d6d219f4e8 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -582,6 +582,10 @@ static void gmc_v7_0_set_prt(struct 
>> amdgpu_device *adev, bool enable)
>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
>> +        /* Note: when vm_debug enabled, vm fault from SDMAx accessing
>> +         * PRT range is normal. */
>> +        if (!amdgpu_vm_debug)
>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>>       } else {
>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
>> @@ -591,6 +595,8 @@ static void gmc_v7_0_set_prt(struct amdgpu_device 
>> *adev, bool enable)
>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
>> +        if (!amdgpu_vm_debug)
>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>       }
>>   }
>>   diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> index 5150ab614eaa..eea2eb7fc2f5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -808,6 +808,10 @@ static void gmc_v8_0_set_prt(struct 
>> amdgpu_device *adev, bool enable)
>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
>> +        /* Note: when vm_debug enabled, vm fault from SDMAx accessing
>> +         * PRT range is normal. */
>> +        if (!amdgpu_vm_debug)
>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>>       } else {
>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
>> @@ -817,6 +821,8 @@ static void gmc_v8_0_set_prt(struct amdgpu_device 
>> *adev, bool enable)
>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
>> +        if (!amdgpu_vm_debug)
>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>       }
>>   }
>
Yes, exactly.

Problem is that we then probably need two page tables. One for the CB/TC 
and one for the SDMA.

Christian.

Am 03.01.19 um 10:02 schrieb zhoucm1:
> need dummy page for that?
>
>
> -David
>
>
> On 2019年01月03日 17:01, Christian König wrote:
>> NAK, the problem is not the interrupt.
>>
>> E.g. causing faults by accessing unmapped pages with the SDMA can 
>> still crash the MC.
>>
>> The key point is that SDMA can't work with PRT tiles on pre-gmc9 and 
>> we need to forbid access on the application side.
>>
>> Regards,
>> Christian.
>>
>> Am 03.01.19 um 09:54 schrieb Chunming Zhou:
>>> For pre-gmc9, UMD can only access unmapped PRT tile from CB/TC without
>>> firing VM fault. Kernel would still receive the VM fault interrupt
>>> and output the error message if SDMA is the mc_client.
>>> GMC9 don't need the same since it handle the PRT in different way.
>>> We cannot just skip message for SDMA, as Christian pointed, VM fault
>>> could crash mc block, so we disable vm fault irq during prt range is
>>> accesed.
>>> The nagative is normal vm fault could be ignored during that peroid
>>> without enabling vm_debug kernel parameter.
>>>
>>> Change-Id: Ic3c62393768eca90e3e45eaf81e7f26f2e91de84
>>> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 6 ++++++
>>>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 6 ++++++
>>>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 6 ++++++
>>>   3 files changed, 18 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c 
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>> index dae73f6768c2..175c4b319559 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>> @@ -486,6 +486,10 @@ static void gmc_v6_0_set_prt(struct 
>>> amdgpu_device *adev, bool enable)
>>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
>>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
>>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx accessing
>>> +         * PRT range is normal. */
>>> +        if (!amdgpu_vm_debug)
>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>>>       } else {
>>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
>>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
>>> @@ -495,6 +499,8 @@ static void gmc_v6_0_set_prt(struct 
>>> amdgpu_device *adev, bool enable)
>>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
>>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
>>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
>>> +        if (!amdgpu_vm_debug)
>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>>       }
>>>   }
>>>   diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c 
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> index 5bdeb358bfb5..a4d6d219f4e8 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> @@ -582,6 +582,10 @@ static void gmc_v7_0_set_prt(struct 
>>> amdgpu_device *adev, bool enable)
>>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
>>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
>>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx accessing
>>> +         * PRT range is normal. */
>>> +        if (!amdgpu_vm_debug)
>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>>>       } else {
>>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
>>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
>>> @@ -591,6 +595,8 @@ static void gmc_v7_0_set_prt(struct 
>>> amdgpu_device *adev, bool enable)
>>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
>>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
>>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
>>> +        if (!amdgpu_vm_debug)
>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>>       }
>>>   }
>>>   diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c 
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> index 5150ab614eaa..eea2eb7fc2f5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> @@ -808,6 +808,10 @@ static void gmc_v8_0_set_prt(struct 
>>> amdgpu_device *adev, bool enable)
>>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
>>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
>>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx accessing
>>> +         * PRT range is normal. */
>>> +        if (!amdgpu_vm_debug)
>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>>>       } else {
>>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
>>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);
>>> @@ -817,6 +821,8 @@ static void gmc_v8_0_set_prt(struct 
>>> amdgpu_device *adev, bool enable)
>>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
>>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
>>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
>>> +        if (!amdgpu_vm_debug)
>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>>       }
>>>   }
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> -----Original Message-----

> From: Christian König <ckoenig.leichtzumerken@gmail.com>

> Sent: Thursday, January 03, 2019 5:05 PM

> To: Zhou, David(ChunMing) <David1.Zhou@amd.com>; Koenig, Christian

> <Christian.Koenig@amd.com>; Zhou, David(ChunMing)

> <David1.Zhou@amd.com>; amd-gfx@lists.freedesktop.org

> Subject: Re: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt

> accessed

> 

> Yes, exactly.

> 

> Problem is that we then probably need two page tables. One for the CB/TC

> and one for the SDMA.


But when setup page table, how can we know the client is CB/TC or SDMA?

-David

> 

> Christian.

> 

> Am 03.01.19 um 10:02 schrieb zhoucm1:

> > need dummy page for that?

> >

> >

> > -David

> >

> >

> > On 2019年01月03日 17:01, Christian König wrote:

> >> NAK, the problem is not the interrupt.

> >>

> >> E.g. causing faults by accessing unmapped pages with the SDMA can

> >> still crash the MC.

> >>

> >> The key point is that SDMA can't work with PRT tiles on pre-gmc9 and

> >> we need to forbid access on the application side.

> >>

> >> Regards,

> >> Christian.

> >>

> >> Am 03.01.19 um 09:54 schrieb Chunming Zhou:

> >>> For pre-gmc9, UMD can only access unmapped PRT tile from CB/TC

> >>> without firing VM fault. Kernel would still receive the VM fault

> >>> interrupt and output the error message if SDMA is the mc_client.

> >>> GMC9 don't need the same since it handle the PRT in different way.

> >>> We cannot just skip message for SDMA, as Christian pointed, VM fault

> >>> could crash mc block, so we disable vm fault irq during prt range is

> >>> accesed.

> >>> The nagative is normal vm fault could be ignored during that peroid

> >>> without enabling vm_debug kernel parameter.

> >>>

> >>> Change-Id: Ic3c62393768eca90e3e45eaf81e7f26f2e91de84

> >>> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>

> >>> ---

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

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

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

> >>>   3 files changed, 18 insertions(+)

> >>>

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

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

> >>> index dae73f6768c2..175c4b319559 100644

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

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

> >>> @@ -486,6 +486,10 @@ static void gmc_v6_0_set_prt(struct

> >>> amdgpu_device *adev, bool enable)

> >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

> >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

> >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

> >>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

> >>> +accessing

> >>> +         * PRT range is normal. */

> >>> +        if (!amdgpu_vm_debug)

> >>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

> >>>       } else {

> >>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

> >>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -

> 495,6

> >>> +499,8 @@ static void gmc_v6_0_set_prt(struct amdgpu_device *adev,

> >>> bool enable)

> >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

> >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

> >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

> >>> +        if (!amdgpu_vm_debug)

> >>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

> >>>       }

> >>>   }

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

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

> >>> index 5bdeb358bfb5..a4d6d219f4e8 100644

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

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

> >>> @@ -582,6 +582,10 @@ static void gmc_v7_0_set_prt(struct

> >>> amdgpu_device *adev, bool enable)

> >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

> >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

> >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

> >>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

> >>> +accessing

> >>> +         * PRT range is normal. */

> >>> +        if (!amdgpu_vm_debug)

> >>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

> >>>       } else {

> >>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

> >>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -

> 591,6

> >>> +595,8 @@ static void gmc_v7_0_set_prt(struct amdgpu_device *adev,

> >>> bool enable)

> >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

> >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

> >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

> >>> +        if (!amdgpu_vm_debug)

> >>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

> >>>       }

> >>>   }

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

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

> >>> index 5150ab614eaa..eea2eb7fc2f5 100644

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

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

> >>> @@ -808,6 +808,10 @@ static void gmc_v8_0_set_prt(struct

> >>> amdgpu_device *adev, bool enable)

> >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

> >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

> >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

> >>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

> >>> +accessing

> >>> +         * PRT range is normal. */

> >>> +        if (!amdgpu_vm_debug)

> >>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

> >>>       } else {

> >>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

> >>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -

> 817,6

> >>> +821,8 @@ static void gmc_v8_0_set_prt(struct amdgpu_device *adev,

> >>> bool enable)

> >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

> >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

> >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

> >>> +        if (!amdgpu_vm_debug)

> >>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

> >>>       }

> >>>   }

> >>

> >

> > _______________________________________________

> > amd-gfx mailing list

> > amd-gfx@lists.freedesktop.org

> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Am 03.01.19 um 10:23 schrieb Zhou, David(ChunMing):
>

>> -----Original Message-----

>> From: Christian König <ckoenig.leichtzumerken@gmail.com>

>> Sent: Thursday, January 03, 2019 5:05 PM

>> To: Zhou, David(ChunMing) <David1.Zhou@amd.com>; Koenig, Christian

>> <Christian.Koenig@amd.com>; Zhou, David(ChunMing)

>> <David1.Zhou@amd.com>; amd-gfx@lists.freedesktop.org

>> Subject: Re: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt

>> accessed

>>

>> Yes, exactly.

>>

>> Problem is that we then probably need two page tables. One for the CB/TC

>> and one for the SDMA.

> But when setup page table, how can we know the client is CB/TC or SDMA?


Well we can't, that's why this approach won't work (It was just a 
theoretically thought).

What we could do on pre-gmc9 is to use the address of the dummy page and 
set the valid and readonly bit.

This way SDMA reads would return zero (as expected), but writes would 
still cause a fault.

Not sure if that helps, but it at least mitigates the problem.

Christian.

>

> -David

>

>> Christian.

>>

>> Am 03.01.19 um 10:02 schrieb zhoucm1:

>>> need dummy page for that?

>>>

>>>

>>> -David

>>>

>>>

>>> On 2019年01月03日 17:01, Christian König wrote:

>>>> NAK, the problem is not the interrupt.

>>>>

>>>> E.g. causing faults by accessing unmapped pages with the SDMA can

>>>> still crash the MC.

>>>>

>>>> The key point is that SDMA can't work with PRT tiles on pre-gmc9 and

>>>> we need to forbid access on the application side.

>>>>

>>>> Regards,

>>>> Christian.

>>>>

>>>> Am 03.01.19 um 09:54 schrieb Chunming Zhou:

>>>>> For pre-gmc9, UMD can only access unmapped PRT tile from CB/TC

>>>>> without firing VM fault. Kernel would still receive the VM fault

>>>>> interrupt and output the error message if SDMA is the mc_client.

>>>>> GMC9 don't need the same since it handle the PRT in different way.

>>>>> We cannot just skip message for SDMA, as Christian pointed, VM fault

>>>>> could crash mc block, so we disable vm fault irq during prt range is

>>>>> accesed.

>>>>> The nagative is normal vm fault could be ignored during that peroid

>>>>> without enabling vm_debug kernel parameter.

>>>>>

>>>>> Change-Id: Ic3c62393768eca90e3e45eaf81e7f26f2e91de84

>>>>> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>

>>>>> ---

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

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

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

>>>>>    3 files changed, 18 insertions(+)

>>>>>

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

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

>>>>> index dae73f6768c2..175c4b319559 100644

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

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

>>>>> @@ -486,6 +486,10 @@ static void gmc_v6_0_set_prt(struct

>>>>> amdgpu_device *adev, bool enable)

>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

>>>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

>>>>> +accessing

>>>>> +         * PRT range is normal. */

>>>>> +        if (!amdgpu_vm_debug)

>>>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

>>>>>        } else {

>>>>>            WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

>>>>>            WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -

>> 495,6

>>>>> +499,8 @@ static void gmc_v6_0_set_prt(struct amdgpu_device *adev,

>>>>> bool enable)

>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

>>>>> +        if (!amdgpu_vm_debug)

>>>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

>>>>>        }

>>>>>    }

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

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

>>>>> index 5bdeb358bfb5..a4d6d219f4e8 100644

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

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

>>>>> @@ -582,6 +582,10 @@ static void gmc_v7_0_set_prt(struct

>>>>> amdgpu_device *adev, bool enable)

>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

>>>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

>>>>> +accessing

>>>>> +         * PRT range is normal. */

>>>>> +        if (!amdgpu_vm_debug)

>>>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

>>>>>        } else {

>>>>>            WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

>>>>>            WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -

>> 591,6

>>>>> +595,8 @@ static void gmc_v7_0_set_prt(struct amdgpu_device *adev,

>>>>> bool enable)

>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

>>>>> +        if (!amdgpu_vm_debug)

>>>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

>>>>>        }

>>>>>    }

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

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

>>>>> index 5150ab614eaa..eea2eb7fc2f5 100644

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

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

>>>>> @@ -808,6 +808,10 @@ static void gmc_v8_0_set_prt(struct

>>>>> amdgpu_device *adev, bool enable)

>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

>>>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

>>>>> +accessing

>>>>> +         * PRT range is normal. */

>>>>> +        if (!amdgpu_vm_debug)

>>>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

>>>>>        } else {

>>>>>            WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

>>>>>            WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -

>> 817,6

>>>>> +821,8 @@ static void gmc_v8_0_set_prt(struct amdgpu_device *adev,

>>>>> bool enable)

>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

>>>>> +        if (!amdgpu_vm_debug)

>>>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

>>>>>        }

>>>>>    }

>>> _______________________________________________

>>> amd-gfx mailing list

>>> amd-gfx@lists.freedesktop.org

>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Seems we don't need two page table, we just map every prt range to dummy page, any problem?

-David

> -----Original Message-----

> From: Zhou, David(ChunMing)

> Sent: Thursday, January 03, 2019 5:23 PM

> To: Koenig, Christian <Christian.Koenig@amd.com>; amd-

> gfx@lists.freedesktop.org

> Subject: RE: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt

> accessed

> 

> 

> 

> > -----Original Message-----

> > From: Christian König <ckoenig.leichtzumerken@gmail.com>

> > Sent: Thursday, January 03, 2019 5:05 PM

> > To: Zhou, David(ChunMing) <David1.Zhou@amd.com>; Koenig, Christian

> > <Christian.Koenig@amd.com>; Zhou, David(ChunMing)

> > <David1.Zhou@amd.com>; amd-gfx@lists.freedesktop.org

> > Subject: Re: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt

> > accessed

> >

> > Yes, exactly.

> >

> > Problem is that we then probably need two page tables. One for the

> > CB/TC and one for the SDMA.

> 

> But when setup page table, how can we know the client is CB/TC or SDMA?

> 

> -David

> 

> >

> > Christian.

> >

> > Am 03.01.19 um 10:02 schrieb zhoucm1:

> > > need dummy page for that?

> > >

> > >

> > > -David

> > >

> > >

> > > On 2019年01月03日 17:01, Christian König wrote:

> > >> NAK, the problem is not the interrupt.

> > >>

> > >> E.g. causing faults by accessing unmapped pages with the SDMA can

> > >> still crash the MC.

> > >>

> > >> The key point is that SDMA can't work with PRT tiles on pre-gmc9

> > >> and we need to forbid access on the application side.

> > >>

> > >> Regards,

> > >> Christian.

> > >>

> > >> Am 03.01.19 um 09:54 schrieb Chunming Zhou:

> > >>> For pre-gmc9, UMD can only access unmapped PRT tile from CB/TC

> > >>> without firing VM fault. Kernel would still receive the VM fault

> > >>> interrupt and output the error message if SDMA is the mc_client.

> > >>> GMC9 don't need the same since it handle the PRT in different way.

> > >>> We cannot just skip message for SDMA, as Christian pointed, VM

> > >>> fault could crash mc block, so we disable vm fault irq during prt

> > >>> range is accesed.

> > >>> The nagative is normal vm fault could be ignored during that

> > >>> peroid without enabling vm_debug kernel parameter.

> > >>>

> > >>> Change-Id: Ic3c62393768eca90e3e45eaf81e7f26f2e91de84

> > >>> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>

> > >>> ---

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

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

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

> > >>>   3 files changed, 18 insertions(+)

> > >>>

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

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

> > >>> index dae73f6768c2..175c4b319559 100644

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

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

> > >>> @@ -486,6 +486,10 @@ static void gmc_v6_0_set_prt(struct

> > >>> amdgpu_device *adev, bool enable)

> > >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

> > >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

> > >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

> > >>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

> > >>> +accessing

> > >>> +         * PRT range is normal. */

> > >>> +        if (!amdgpu_vm_debug)

> > >>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

> > >>>       } else {

> > >>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

> > >>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -

> > 495,6

> > >>> +499,8 @@ static void gmc_v6_0_set_prt(struct amdgpu_device

> *adev,

> > >>> bool enable)

> > >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

> > >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

> > >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

> > >>> +        if (!amdgpu_vm_debug)

> > >>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

> > >>>       }

> > >>>   }

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

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

> > >>> index 5bdeb358bfb5..a4d6d219f4e8 100644

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

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

> > >>> @@ -582,6 +582,10 @@ static void gmc_v7_0_set_prt(struct

> > >>> amdgpu_device *adev, bool enable)

> > >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

> > >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

> > >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

> > >>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

> > >>> +accessing

> > >>> +         * PRT range is normal. */

> > >>> +        if (!amdgpu_vm_debug)

> > >>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

> > >>>       } else {

> > >>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

> > >>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -

> > 591,6

> > >>> +595,8 @@ static void gmc_v7_0_set_prt(struct amdgpu_device

> *adev,

> > >>> bool enable)

> > >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

> > >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

> > >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

> > >>> +        if (!amdgpu_vm_debug)

> > >>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

> > >>>       }

> > >>>   }

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

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

> > >>> index 5150ab614eaa..eea2eb7fc2f5 100644

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

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

> > >>> @@ -808,6 +808,10 @@ static void gmc_v8_0_set_prt(struct

> > >>> amdgpu_device *adev, bool enable)

> > >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

> > >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

> > >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

> > >>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

> > >>> +accessing

> > >>> +         * PRT range is normal. */

> > >>> +        if (!amdgpu_vm_debug)

> > >>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

> > >>>       } else {

> > >>>           WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

> > >>>           WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -

> > 817,6

> > >>> +821,8 @@ static void gmc_v8_0_set_prt(struct amdgpu_device

> *adev,

> > >>> bool enable)

> > >>>           WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

> > >>>           WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

> > >>>           WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

> > >>> +        if (!amdgpu_vm_debug)

> > >>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

> > >>>       }

> > >>>   }

> > >>

> > >

> > > _______________________________________________

> > > amd-gfx mailing list

> > > amd-gfx@lists.freedesktop.org

> > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Writes are then not ignored and garbage the dummy page.

Christian.

Am 03.01.19 um 10:46 schrieb Zhou, David(ChunMing):
> Seems we don't need two page table, we just map every prt range to dummy page, any problem?
>
> -David
>
>> -----Original Message-----
>> From: Zhou, David(ChunMing)
>> Sent: Thursday, January 03, 2019 5:23 PM
>> To: Koenig, Christian <Christian.Koenig@amd.com>; amd-
>> gfx@lists.freedesktop.org
>> Subject: RE: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt
>> accessed
>>
>>
>>
>>> -----Original Message-----
>>> From: Christian König <ckoenig.leichtzumerken@gmail.com>
>>> Sent: Thursday, January 03, 2019 5:05 PM
>>> To: Zhou, David(ChunMing) <David1.Zhou@amd.com>; Koenig, Christian
>>> <Christian.Koenig@amd.com>; Zhou, David(ChunMing)
>>> <David1.Zhou@amd.com>; amd-gfx@lists.freedesktop.org
>>> Subject: Re: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt
>>> accessed
>>>
>>> Yes, exactly.
>>>
>>> Problem is that we then probably need two page tables. One for the
>>> CB/TC and one for the SDMA.
>> But when setup page table, how can we know the client is CB/TC or SDMA?
>>
>> -David
>>
>>> Christian.
>>>
>>> Am 03.01.19 um 10:02 schrieb zhoucm1:
>>>> need dummy page for that?
>>>>
>>>>
>>>> -David
>>>>
>>>>
>>>> On 2019年01月03日 17:01, Christian König wrote:
>>>>> NAK, the problem is not the interrupt.
>>>>>
>>>>> E.g. causing faults by accessing unmapped pages with the SDMA can
>>>>> still crash the MC.
>>>>>
>>>>> The key point is that SDMA can't work with PRT tiles on pre-gmc9
>>>>> and we need to forbid access on the application side.
>>>>>
>>>>> Regards,
>>>>> Christian.
>>>>>
>>>>> Am 03.01.19 um 09:54 schrieb Chunming Zhou:
>>>>>> For pre-gmc9, UMD can only access unmapped PRT tile from CB/TC
>>>>>> without firing VM fault. Kernel would still receive the VM fault
>>>>>> interrupt and output the error message if SDMA is the mc_client.
>>>>>> GMC9 don't need the same since it handle the PRT in different way.
>>>>>> We cannot just skip message for SDMA, as Christian pointed, VM
>>>>>> fault could crash mc block, so we disable vm fault irq during prt
>>>>>> range is accesed.
>>>>>> The nagative is normal vm fault could be ignored during that
>>>>>> peroid without enabling vm_debug kernel parameter.
>>>>>>
>>>>>> Change-Id: Ic3c62393768eca90e3e45eaf81e7f26f2e91de84
>>>>>> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
>>>>>> ---
>>>>>>    drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 6 ++++++
>>>>>>    drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 6 ++++++
>>>>>>    drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 6 ++++++
>>>>>>    3 files changed, 18 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>>>>> index dae73f6768c2..175c4b319559 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>>>>> @@ -486,6 +486,10 @@ static void gmc_v6_0_set_prt(struct
>>>>>> amdgpu_device *adev, bool enable)
>>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
>>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
>>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
>>>>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx
>>>>>> +accessing
>>>>>> +         * PRT range is normal. */
>>>>>> +        if (!amdgpu_vm_debug)
>>>>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>>>>>>        } else {
>>>>>>            WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
>>>>>>            WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -
>>> 495,6
>>>>>> +499,8 @@ static void gmc_v6_0_set_prt(struct amdgpu_device
>> *adev,
>>>>>> bool enable)
>>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
>>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
>>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
>>>>>> +        if (!amdgpu_vm_debug)
>>>>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>>>>>        }
>>>>>>    }
>>>>>>    diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>>>> index 5bdeb358bfb5..a4d6d219f4e8 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>>>>> @@ -582,6 +582,10 @@ static void gmc_v7_0_set_prt(struct
>>>>>> amdgpu_device *adev, bool enable)
>>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
>>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
>>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
>>>>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx
>>>>>> +accessing
>>>>>> +         * PRT range is normal. */
>>>>>> +        if (!amdgpu_vm_debug)
>>>>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>>>>>>        } else {
>>>>>>            WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
>>>>>>            WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -
>>> 591,6
>>>>>> +595,8 @@ static void gmc_v7_0_set_prt(struct amdgpu_device
>> *adev,
>>>>>> bool enable)
>>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
>>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
>>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
>>>>>> +        if (!amdgpu_vm_debug)
>>>>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>>>>>        }
>>>>>>    }
>>>>>>    diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>>>> index 5150ab614eaa..eea2eb7fc2f5 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>>>>> @@ -808,6 +808,10 @@ static void gmc_v8_0_set_prt(struct
>>>>>> amdgpu_device *adev, bool enable)
>>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);
>>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);
>>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);
>>>>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx
>>>>>> +accessing
>>>>>> +         * PRT range is normal. */
>>>>>> +        if (!amdgpu_vm_debug)
>>>>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>>>>>>        } else {
>>>>>>            WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);
>>>>>>            WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff); @@ -
>>> 817,6
>>>>>> +821,8 @@ static void gmc_v8_0_set_prt(struct amdgpu_device
>> *adev,
>>>>>> bool enable)
>>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);
>>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);
>>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);
>>>>>> +        if (!amdgpu_vm_debug)
>>>>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>>>>>        }
>>>>>>    }
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Doesn't gpu check PTE prt bit first and then access va range?

Even wrte to dummy page, seem there still is no problem, we don't care that content at all.

-David

> -----Original Message-----

> From: Christian König <ckoenig.leichtzumerken@gmail.com>

> Sent: Thursday, January 03, 2019 5:54 PM

> To: Zhou, David(ChunMing) <David1.Zhou@amd.com>; Koenig, Christian

> <Christian.Koenig@amd.com>; amd-gfx@lists.freedesktop.org

> Subject: Re: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt

> accessed

> 

> Writes are then not ignored and garbage the dummy page.

> 

> Christian.

> 

> Am 03.01.19 um 10:46 schrieb Zhou, David(ChunMing):

> > Seems we don't need two page table, we just map every prt range to

> dummy page, any problem?

> >

> > -David

> >

> >> -----Original Message-----

> >> From: Zhou, David(ChunMing)

> >> Sent: Thursday, January 03, 2019 5:23 PM

> >> To: Koenig, Christian <Christian.Koenig@amd.com>; amd-

> >> gfx@lists.freedesktop.org

> >> Subject: RE: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt

> >> accessed

> >>

> >>

> >>

> >>> -----Original Message-----

> >>> From: Christian König <ckoenig.leichtzumerken@gmail.com>

> >>> Sent: Thursday, January 03, 2019 5:05 PM

> >>> To: Zhou, David(ChunMing) <David1.Zhou@amd.com>; Koenig, Christian

> >>> <Christian.Koenig@amd.com>; Zhou, David(ChunMing)

> >>> <David1.Zhou@amd.com>; amd-gfx@lists.freedesktop.org

> >>> Subject: Re: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt

> >>> accessed

> >>>

> >>> Yes, exactly.

> >>>

> >>> Problem is that we then probably need two page tables. One for the

> >>> CB/TC and one for the SDMA.

> >> But when setup page table, how can we know the client is CB/TC or SDMA?

> >>

> >> -David

> >>

> >>> Christian.

> >>>

> >>> Am 03.01.19 um 10:02 schrieb zhoucm1:

> >>>> need dummy page for that?

> >>>>

> >>>>

> >>>> -David

> >>>>

> >>>>

> >>>> On 2019年01月03日 17:01, Christian König wrote:

> >>>>> NAK, the problem is not the interrupt.

> >>>>>

> >>>>> E.g. causing faults by accessing unmapped pages with the SDMA can

> >>>>> still crash the MC.

> >>>>>

> >>>>> The key point is that SDMA can't work with PRT tiles on pre-gmc9

> >>>>> and we need to forbid access on the application side.

> >>>>>

> >>>>> Regards,

> >>>>> Christian.

> >>>>>

> >>>>> Am 03.01.19 um 09:54 schrieb Chunming Zhou:

> >>>>>> For pre-gmc9, UMD can only access unmapped PRT tile from CB/TC

> >>>>>> without firing VM fault. Kernel would still receive the VM fault

> >>>>>> interrupt and output the error message if SDMA is the mc_client.

> >>>>>> GMC9 don't need the same since it handle the PRT in different way.

> >>>>>> We cannot just skip message for SDMA, as Christian pointed, VM

> >>>>>> fault could crash mc block, so we disable vm fault irq during prt

> >>>>>> range is accesed.

> >>>>>> The nagative is normal vm fault could be ignored during that

> >>>>>> peroid without enabling vm_debug kernel parameter.

> >>>>>>

> >>>>>> Change-Id: Ic3c62393768eca90e3e45eaf81e7f26f2e91de84

> >>>>>> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>

> >>>>>> ---

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

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

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

> >>>>>>    3 files changed, 18 insertions(+)

> >>>>>>

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

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

> >>>>>> index dae73f6768c2..175c4b319559 100644

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

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

> >>>>>> @@ -486,6 +486,10 @@ static void gmc_v6_0_set_prt(struct

> >>>>>> amdgpu_device *adev, bool enable)

> >>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

> >>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

> >>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

> >>>>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

> >>>>>> +accessing

> >>>>>> +         * PRT range is normal. */

> >>>>>> +        if (!amdgpu_vm_debug)

> >>>>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

> >>>>>>        } else {

> >>>>>>            WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

> >>>>>>            WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);

> @@ -

> >>> 495,6

> >>>>>> +499,8 @@ static void gmc_v6_0_set_prt(struct amdgpu_device

> >> *adev,

> >>>>>> bool enable)

> >>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

> >>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

> >>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

> >>>>>> +        if (!amdgpu_vm_debug)

> >>>>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

> >>>>>>        }

> >>>>>>    }

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

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

> >>>>>> index 5bdeb358bfb5..a4d6d219f4e8 100644

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

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

> >>>>>> @@ -582,6 +582,10 @@ static void gmc_v7_0_set_prt(struct

> >>>>>> amdgpu_device *adev, bool enable)

> >>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

> >>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

> >>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

> >>>>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

> >>>>>> +accessing

> >>>>>> +         * PRT range is normal. */

> >>>>>> +        if (!amdgpu_vm_debug)

> >>>>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

> >>>>>>        } else {

> >>>>>>            WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

> >>>>>>            WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);

> @@ -

> >>> 591,6

> >>>>>> +595,8 @@ static void gmc_v7_0_set_prt(struct amdgpu_device

> >> *adev,

> >>>>>> bool enable)

> >>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

> >>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

> >>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

> >>>>>> +        if (!amdgpu_vm_debug)

> >>>>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

> >>>>>>        }

> >>>>>>    }

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

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

> >>>>>> index 5150ab614eaa..eea2eb7fc2f5 100644

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

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

> >>>>>> @@ -808,6 +808,10 @@ static void gmc_v8_0_set_prt(struct

> >>>>>> amdgpu_device *adev, bool enable)

> >>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

> >>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

> >>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

> >>>>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

> >>>>>> +accessing

> >>>>>> +         * PRT range is normal. */

> >>>>>> +        if (!amdgpu_vm_debug)

> >>>>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

> >>>>>>        } else {

> >>>>>>            WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

> >>>>>>            WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);

> @@ -

> >>> 817,6

> >>>>>> +821,8 @@ static void gmc_v8_0_set_prt(struct amdgpu_device

> >> *adev,

> >>>>>> bool enable)

> >>>>>>            WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

> >>>>>>            WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

> >>>>>>            WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

> >>>>>> +        if (!amdgpu_vm_debug)

> >>>>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

> >>>>>>        }

> >>>>>>    }

> >>>> _______________________________________________

> >>>> amd-gfx mailing list

> >>>> amd-gfx@lists.freedesktop.org

> >>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

> > _______________________________________________

> > amd-gfx mailing list

> > amd-gfx@lists.freedesktop.org

> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On pre-gmc9 PTE prt bit is completely ignored.

Check the OpenGL spec for PRTs, writes needs to be ignored and reads 
must return zero.

Not sure about Vulkan and OpenCL.

Christian.

Am 03.01.19 um 10:56 schrieb Zhou, David(ChunMing):
> Doesn't gpu check PTE prt bit first and then access va range?

>

> Even wrte to dummy page, seem there still is no problem, we don't care that content at all.

>

> -David

>

>> -----Original Message-----

>> From: Christian König <ckoenig.leichtzumerken@gmail.com>

>> Sent: Thursday, January 03, 2019 5:54 PM

>> To: Zhou, David(ChunMing) <David1.Zhou@amd.com>; Koenig, Christian

>> <Christian.Koenig@amd.com>; amd-gfx@lists.freedesktop.org

>> Subject: Re: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt

>> accessed

>>

>> Writes are then not ignored and garbage the dummy page.

>>

>> Christian.

>>

>> Am 03.01.19 um 10:46 schrieb Zhou, David(ChunMing):

>>> Seems we don't need two page table, we just map every prt range to

>> dummy page, any problem?

>>> -David

>>>

>>>> -----Original Message-----

>>>> From: Zhou, David(ChunMing)

>>>> Sent: Thursday, January 03, 2019 5:23 PM

>>>> To: Koenig, Christian <Christian.Koenig@amd.com>; amd-

>>>> gfx@lists.freedesktop.org

>>>> Subject: RE: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt

>>>> accessed

>>>>

>>>>

>>>>

>>>>> -----Original Message-----

>>>>> From: Christian König <ckoenig.leichtzumerken@gmail.com>

>>>>> Sent: Thursday, January 03, 2019 5:05 PM

>>>>> To: Zhou, David(ChunMing) <David1.Zhou@amd.com>; Koenig, Christian

>>>>> <Christian.Koenig@amd.com>; Zhou, David(ChunMing)

>>>>> <David1.Zhou@amd.com>; amd-gfx@lists.freedesktop.org

>>>>> Subject: Re: [PATCH 2/2] drm/amdgpu: disable vm fault irq during prt

>>>>> accessed

>>>>>

>>>>> Yes, exactly.

>>>>>

>>>>> Problem is that we then probably need two page tables. One for the

>>>>> CB/TC and one for the SDMA.

>>>> But when setup page table, how can we know the client is CB/TC or SDMA?

>>>>

>>>> -David

>>>>

>>>>> Christian.

>>>>>

>>>>> Am 03.01.19 um 10:02 schrieb zhoucm1:

>>>>>> need dummy page for that?

>>>>>>

>>>>>>

>>>>>> -David

>>>>>>

>>>>>>

>>>>>> On 2019年01月03日 17:01, Christian König wrote:

>>>>>>> NAK, the problem is not the interrupt.

>>>>>>>

>>>>>>> E.g. causing faults by accessing unmapped pages with the SDMA can

>>>>>>> still crash the MC.

>>>>>>>

>>>>>>> The key point is that SDMA can't work with PRT tiles on pre-gmc9

>>>>>>> and we need to forbid access on the application side.

>>>>>>>

>>>>>>> Regards,

>>>>>>> Christian.

>>>>>>>

>>>>>>> Am 03.01.19 um 09:54 schrieb Chunming Zhou:

>>>>>>>> For pre-gmc9, UMD can only access unmapped PRT tile from CB/TC

>>>>>>>> without firing VM fault. Kernel would still receive the VM fault

>>>>>>>> interrupt and output the error message if SDMA is the mc_client.

>>>>>>>> GMC9 don't need the same since it handle the PRT in different way.

>>>>>>>> We cannot just skip message for SDMA, as Christian pointed, VM

>>>>>>>> fault could crash mc block, so we disable vm fault irq during prt

>>>>>>>> range is accesed.

>>>>>>>> The nagative is normal vm fault could be ignored during that

>>>>>>>> peroid without enabling vm_debug kernel parameter.

>>>>>>>>

>>>>>>>> Change-Id: Ic3c62393768eca90e3e45eaf81e7f26f2e91de84

>>>>>>>> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>

>>>>>>>> ---

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

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

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

>>>>>>>>     3 files changed, 18 insertions(+)

>>>>>>>>

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

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

>>>>>>>> index dae73f6768c2..175c4b319559 100644

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

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

>>>>>>>> @@ -486,6 +486,10 @@ static void gmc_v6_0_set_prt(struct

>>>>>>>> amdgpu_device *adev, bool enable)

>>>>>>>>             WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

>>>>>>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

>>>>>>>> +accessing

>>>>>>>> +         * PRT range is normal. */

>>>>>>>> +        if (!amdgpu_vm_debug)

>>>>>>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

>>>>>>>>         } else {

>>>>>>>>             WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);

>> @@ -

>>>>> 495,6

>>>>>>>> +499,8 @@ static void gmc_v6_0_set_prt(struct amdgpu_device

>>>> *adev,

>>>>>>>> bool enable)

>>>>>>>>             WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

>>>>>>>> +        if (!amdgpu_vm_debug)

>>>>>>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

>>>>>>>>         }

>>>>>>>>     }

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

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

>>>>>>>> index 5bdeb358bfb5..a4d6d219f4e8 100644

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

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

>>>>>>>> @@ -582,6 +582,10 @@ static void gmc_v7_0_set_prt(struct

>>>>>>>> amdgpu_device *adev, bool enable)

>>>>>>>>             WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

>>>>>>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

>>>>>>>> +accessing

>>>>>>>> +         * PRT range is normal. */

>>>>>>>> +        if (!amdgpu_vm_debug)

>>>>>>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

>>>>>>>>         } else {

>>>>>>>>             WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);

>> @@ -

>>>>> 591,6

>>>>>>>> +595,8 @@ static void gmc_v7_0_set_prt(struct amdgpu_device

>>>> *adev,

>>>>>>>> bool enable)

>>>>>>>>             WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

>>>>>>>> +        if (!amdgpu_vm_debug)

>>>>>>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

>>>>>>>>         }

>>>>>>>>     }

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

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

>>>>>>>> index 5150ab614eaa..eea2eb7fc2f5 100644

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

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

>>>>>>>> @@ -808,6 +808,10 @@ static void gmc_v8_0_set_prt(struct

>>>>>>>> amdgpu_device *adev, bool enable)

>>>>>>>>             WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, high);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, high);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, high);

>>>>>>>> +        /* Note: when vm_debug enabled, vm fault from SDMAx

>>>>>>>> +accessing

>>>>>>>> +         * PRT range is normal. */

>>>>>>>> +        if (!amdgpu_vm_debug)

>>>>>>>> +            amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);

>>>>>>>>         } else {

>>>>>>>>             WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, 0xfffffff);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, 0xfffffff);

>> @@ -

>>>>> 817,6

>>>>>>>> +821,8 @@ static void gmc_v8_0_set_prt(struct amdgpu_device

>>>> *adev,

>>>>>>>> bool enable)

>>>>>>>>             WREG32(mmVM_PRT_APERTURE1_HIGH_ADDR, 0x0);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE2_HIGH_ADDR, 0x0);

>>>>>>>>             WREG32(mmVM_PRT_APERTURE3_HIGH_ADDR, 0x0);

>>>>>>>> +        if (!amdgpu_vm_debug)

>>>>>>>> +            amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);

>>>>>>>>         }

>>>>>>>>     }

>>>>>> _______________________________________________

>>>>>> amd-gfx mailing list

>>>>>> amd-gfx@lists.freedesktop.org

>>>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

>>> _______________________________________________

>>> amd-gfx mailing list

>>> amd-gfx@lists.freedesktop.org

>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx