[PATCH 1/2] drm/radeon: Assign VMID to PASID for IH in non-HWS mode

[PATCH 1/2] drm/radeon: Assign VMID to PASID for IH in non-HWS mode

[Oded Gabbay]

From: Ben Goz <ben.goz@amd.com>

This patch fixes a bug in kgd_set_pasid_vmid_mapping(), where the function
only updated the ATC registers (IOMMU) with the new VMID <--> PASID mapping,
but didn't update the IH (Interrupt) registers.

The bug only occurs when using non-HWS mode. In HWS mode, the CP automatically
does the VMID <--> PASID mapping.

Signed-off-by: Ben Goz <ben.goz@amd.com>
Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>
---
 drivers/gpu/drm/radeon/cikd.h       | 2 ++
 drivers/gpu/drm/radeon/radeon_kfd.c | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/radeon/cikd.h b/drivers/gpu/drm/radeon/cikd.h
index ba85986..03003f8 100644
--- a/drivers/gpu/drm/radeon/cikd.h
+++ b/drivers/gpu/drm/radeon/cikd.h
@@ -2156,4 +2156,6 @@
 #define ATC_VM_APERTURE1_HIGH_ADDR				0x330Cu
 #define ATC_VM_APERTURE1_LOW_ADDR				0x3304u
 
+#define IH_VMID_0_LUT						0x3D40u
+
 #endif
diff --git a/drivers/gpu/drm/radeon/radeon_kfd.c b/drivers/gpu/drm/radeon/radeon_kfd.c
index a55afba..8bf87f1 100644
--- a/drivers/gpu/drm/radeon/radeon_kfd.c
+++ b/drivers/gpu/drm/radeon/radeon_kfd.c
@@ -390,6 +390,10 @@ static int kgd_set_pasid_vmid_mapping(struct kgd_dev *kgd, unsigned int pasid,
 		cpu_relax();
 	write_register(kgd, ATC_VMID_PASID_MAPPING_UPDATE_STATUS, 1U << vmid);
 
+	/* Mapping vmid to pasid also for IH block */
+	write_register(kgd, IH_VMID_0_LUT + vmid * sizeof(uint32_t),
+			pasid_mapping);
+
 	return 0;
 }
 
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/amdkfd: unmap VMID<-->PASID when relesing VMID (non-HWS)

[Oded Gabbay]

From: Ben Goz <ben.goz@amd.com>

This patch fixes a bug where deallocate_vmid() didn't actually unmap the
VMID<-->PASID mapping (in the registers).
That can cause undefined behavior.

This bug only occurs in non-HWS mode.

Signed-off-by: Ben Goz <ben.goz@amd.com>
Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 3b08ed6..9c8961d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -161,6 +161,9 @@ static void deallocate_vmid(struct device_queue_manager *dqm,
 {
 	int bit = qpd->vmid - KFD_VMID_START_OFFSET;
 
+	/* Release the vmid mapping */
+	set_pasid_vmid_mapping(dqm, 0, qpd->vmid);
+
 	set_bit(bit, (unsigned long *)&dqm->vmid_bitmap);
 	qpd->vmid = 0;
 	q->properties.vmid = 0;
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/radeon: Assign VMID to PASID for IH in non-HWS mode

[Alex Deucher]

On Mon, Jan 5, 2015 at 8:52 AM, Oded Gabbay <oded.gabbay@amd.com> wrote:
> From: Ben Goz <ben.goz@amd.com>
>
> This patch fixes a bug in kgd_set_pasid_vmid_mapping(), where the function
> only updated the ATC registers (IOMMU) with the new VMID <--> PASID mapping,
> but didn't update the IH (Interrupt) registers.
>
> The bug only occurs when using non-HWS mode. In HWS mode, the CP automatically
> does the VMID <--> PASID mapping.
>
> Signed-off-by: Ben Goz <ben.goz@amd.com>
> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>

I'm not too familiar with how these registers work.  I'm assuming they
are just scratch registers that either the fw or the driver has to
update depending on the scheduling model?  For the series:

Acked-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/radeon/cikd.h       | 2 ++
>  drivers/gpu/drm/radeon/radeon_kfd.c | 4 ++++
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/radeon/cikd.h b/drivers/gpu/drm/radeon/cikd.h
> index ba85986..03003f8 100644
> --- a/drivers/gpu/drm/radeon/cikd.h
> +++ b/drivers/gpu/drm/radeon/cikd.h
> @@ -2156,4 +2156,6 @@
>  #define ATC_VM_APERTURE1_HIGH_ADDR                             0x330Cu
>  #define ATC_VM_APERTURE1_LOW_ADDR                              0x3304u
>
> +#define IH_VMID_0_LUT                                          0x3D40u
> +
>  #endif
> diff --git a/drivers/gpu/drm/radeon/radeon_kfd.c b/drivers/gpu/drm/radeon/radeon_kfd.c
> index a55afba..8bf87f1 100644
> --- a/drivers/gpu/drm/radeon/radeon_kfd.c
> +++ b/drivers/gpu/drm/radeon/radeon_kfd.c
> @@ -390,6 +390,10 @@ static int kgd_set_pasid_vmid_mapping(struct kgd_dev *kgd, unsigned int pasid,
>                 cpu_relax();
>         write_register(kgd, ATC_VMID_PASID_MAPPING_UPDATE_STATUS, 1U << vmid);
>
> +       /* Mapping vmid to pasid also for IH block */
> +       write_register(kgd, IH_VMID_0_LUT + vmid * sizeof(uint32_t),
> +                       pasid_mapping);
> +
>         return 0;
>  }
>
> --
> 1.9.1
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/radeon: Assign VMID to PASID for IH in non-HWS mode

[Oded Gabbay]

On 01/05/2015 08:41 PM, Alex Deucher wrote:
> On Mon, Jan 5, 2015 at 8:52 AM, Oded Gabbay <oded.gabbay@amd.com> wrote:
>> From: Ben Goz <ben.goz@amd.com>
>>
>> This patch fixes a bug in kgd_set_pasid_vmid_mapping(), where the function
>> only updated the ATC registers (IOMMU) with the new VMID <--> PASID mapping,
>> but didn't update the IH (Interrupt) registers.
>>
>> The bug only occurs when using non-HWS mode. In HWS mode, the CP automatically
>> does the VMID <--> PASID mapping.
>>
>> Signed-off-by: Ben Goz <ben.goz@amd.com>
>> Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>
>
> I'm not too familiar with how these registers work.  I'm assuming they
> are just scratch registers that either the fw or the driver has to
> update depending on the scheduling model?  For the series:
>
> Acked-by: Alex Deucher <alexander.deucher@amd.com>
>
Yes, those registers need to be updated by the driver, if we are working in 
non-HWS mode, or by the fw, if we are working in HWS mode, which is the default 
mode and the "production" mode.

	Oded

>> ---
>>   drivers/gpu/drm/radeon/cikd.h       | 2 ++
>>   drivers/gpu/drm/radeon/radeon_kfd.c | 4 ++++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/radeon/cikd.h b/drivers/gpu/drm/radeon/cikd.h
>> index ba85986..03003f8 100644
>> --- a/drivers/gpu/drm/radeon/cikd.h
>> +++ b/drivers/gpu/drm/radeon/cikd.h
>> @@ -2156,4 +2156,6 @@
>>   #define ATC_VM_APERTURE1_HIGH_ADDR                             0x330Cu
>>   #define ATC_VM_APERTURE1_LOW_ADDR                              0x3304u
>>
>> +#define IH_VMID_0_LUT                                          0x3D40u
>> +
>>   #endif
>> diff --git a/drivers/gpu/drm/radeon/radeon_kfd.c b/drivers/gpu/drm/radeon/radeon_kfd.c
>> index a55afba..8bf87f1 100644
>> --- a/drivers/gpu/drm/radeon/radeon_kfd.c
>> +++ b/drivers/gpu/drm/radeon/radeon_kfd.c
>> @@ -390,6 +390,10 @@ static int kgd_set_pasid_vmid_mapping(struct kgd_dev *kgd, unsigned int pasid,
>>                  cpu_relax();
>>          write_register(kgd, ATC_VMID_PASID_MAPPING_UPDATE_STATUS, 1U << vmid);
>>
>> +       /* Mapping vmid to pasid also for IH block */
>> +       write_register(kgd, IH_VMID_0_LUT + vmid * sizeof(uint32_t),
>> +                       pasid_mapping);
>> +
>>          return 0;
>>   }
>>
>> --
>> 1.9.1
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel