[PATCH 1/2] drm/amdkfd: Initialize HSA_CAP_ATS_PRESENT capability in topology codes

[PATCH 1/2] drm/amdkfd: Initialize HSA_CAP_ATS_PRESENT capability in topology codes

[Zeng, Oak]

Move HSA_CAP_ATS_PRESENT initialization logic from kfd iommu codes to
kfd topology codes. This removes kfd_iommu_device_init's dependency
on kfd_topology_add_device. Also remove duplicate code setting the
same.

Change-Id: I2bf3c000d4795b41afe1b39fe679677c1f86bfbe
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_iommu.c    | 10 +---------
 drivers/gpu/drm/amd/amdkfd/kfd_topology.c |  6 +++++-
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c b/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
index 0149475..5f35df2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
@@ -66,16 +66,8 @@ int kfd_iommu_device_init(struct kfd_dev *kfd)
 
 	top_dev = kfd_topology_device_by_id(kfd->id);
 
-	/*
-	 * Overwrite ATS capability according to needs_iommu_device to fix
-	 * potential missing corresponding bit in CRAT of BIOS.
-	 */
-	if (!kfd->device_info->needs_iommu_device) {
-		top_dev->node_props.capability &= ~HSA_CAP_ATS_PRESENT;
+	if (!kfd->device_info->needs_iommu_device)
 		return 0;
-	}
-
-	top_dev->node_props.capability |= HSA_CAP_ATS_PRESENT;
 
 	iommu_info.flags = 0;
 	err = amd_iommu_device_info(kfd->pdev, &iommu_info);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index d241a86..1c184b3 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -1330,6 +1330,11 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
 		     dev->gpu->device_info->asic_family);
 	}
 
+	if (dev->gpu->device_info->needs_iommu_device)
+		dev->node_props.capability |= HSA_CAP_ATS_PRESENT;
+	else
+		dev->node_props.capability &= ~HSA_CAP_ATS_PRESENT;
+
 	/* Fix errors in CZ CRAT.
 	 * simd_count: Carrizo CRAT reports wrong simd_count, probably
 	 *		because it doesn't consider masked out CUs
@@ -1340,7 +1345,6 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
 		dev->node_props.simd_count =
 			cu_info.simd_per_cu * cu_info.cu_active_number;
 		dev->node_props.max_waves_per_simd = 10;
-		dev->node_props.capability |= HSA_CAP_ATS_PRESENT;
 	}
 
 	ctx = amdgpu_ras_get_context((struct amdgpu_device *)(dev->gpu->kgd));
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amdkfd: Add device to topology after it is completely inited

[Zeng, Oak]

We can't have devices that are not completely initialized in kfd topology.
Otherwise it is a race condition when user access not completely
initialized device. This also addresses a kfd_topology_add_device accessing
NULL dqm pointer issue.

Change-Id: I53a0fe498e6daaeb63aa8d78362df01955f14a2f
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 9d1b026..ebac7d7 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -603,11 +603,6 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
 	if (kfd->kfd2kgd->get_hive_id)
 		kfd->hive_id = kfd->kfd2kgd->get_hive_id(kfd->kgd);
 
-	if (kfd_topology_add_device(kfd)) {
-		dev_err(kfd_device, "Error adding device to topology\n");
-		goto kfd_topology_add_device_error;
-	}
-
 	if (kfd_interrupt_init(kfd)) {
 		dev_err(kfd_device, "Error initializing interrupts\n");
 		goto kfd_interrupt_error;
@@ -631,6 +626,11 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
 
 	kfd->dbgmgr = NULL;
 
+	if (kfd_topology_add_device(kfd)) {
+		dev_err(kfd_device, "Error adding device to topology\n");
+		goto kfd_topology_add_device_error;
+	}
+
 	kfd->init_complete = true;
 	dev_info(kfd_device, "added device %x:%x\n", kfd->pdev->vendor,
 		 kfd->pdev->device);
@@ -640,14 +640,13 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
 
 	goto out;
 
+kfd_topology_add_device_error:
 kfd_resume_error:
 device_iommu_error:
 	device_queue_manager_uninit(kfd->dqm);
 device_queue_manager_error:
 	kfd_interrupt_exit(kfd);
 kfd_interrupt_error:
-	kfd_topology_remove_device(kfd);
-kfd_topology_add_device_error:
 	kfd_doorbell_fini(kfd);
 kfd_doorbell_error:
 	kfd_gtt_sa_fini(kfd);
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdkfd: Initialize HSA_CAP_ATS_PRESENT capability in topology codes

[Kuehling, Felix]

On 2019-06-10 13:48, Zeng, Oak wrote:
> Move HSA_CAP_ATS_PRESENT initialization logic from kfd iommu codes to
> kfd topology codes. This removes kfd_iommu_device_init's dependency
> on kfd_topology_add_device. Also remove duplicate code setting the
> same.
>
> Change-Id: I2bf3c000d4795b41afe1b39fe679677c1f86bfbe
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_iommu.c    | 10 +---------
>   drivers/gpu/drm/amd/amdkfd/kfd_topology.c |  6 +++++-
>   2 files changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c b/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
> index 0149475..5f35df2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
> @@ -66,16 +66,8 @@ int kfd_iommu_device_init(struct kfd_dev *kfd)
>   
>   	top_dev = kfd_topology_device_by_id(kfd->id);
>   
> -	/*
> -	 * Overwrite ATS capability according to needs_iommu_device to fix
> -	 * potential missing corresponding bit in CRAT of BIOS.
> -	 */

It would be good to preserve this comment by moving it to 
kfd_topology_add_device.


> -	if (!kfd->device_info->needs_iommu_device) {
> -		top_dev->node_props.capability &= ~HSA_CAP_ATS_PRESENT;
> +	if (!kfd->device_info->needs_iommu_device)
>   		return 0;
> -	}
> -
> -	top_dev->node_props.capability |= HSA_CAP_ATS_PRESENT;
>   
>   	iommu_info.flags = 0;
>   	err = amd_iommu_device_info(kfd->pdev, &iommu_info);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> index d241a86..1c184b3 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> @@ -1330,6 +1330,11 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
>   		     dev->gpu->device_info->asic_family);
>   	}
>   
> +	if (dev->gpu->device_info->needs_iommu_device)
> +		dev->node_props.capability |= HSA_CAP_ATS_PRESENT;
> +	else
> +		dev->node_props.capability &= ~HSA_CAP_ATS_PRESENT;
> +
>   	/* Fix errors in CZ CRAT.
>   	 * simd_count: Carrizo CRAT reports wrong simd_count, probably
>   	 *		because it doesn't consider masked out CUs
> @@ -1340,7 +1345,6 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
>   		dev->node_props.simd_count =
>   			cu_info.simd_per_cu * cu_info.cu_active_number;
>   		dev->node_props.max_waves_per_simd = 10;
> -		dev->node_props.capability |= HSA_CAP_ATS_PRESENT;

There is a comment above the if-block that should be updated when you 
remove this.

With these two comments fixed, the series is Reviewed-by: Felix Kuehling 
<Felix.Kuehling@amd.com>

Regards,
   Felix


>   	}
>   
>   	ctx = amdgpu_ras_get_context((struct amdgpu_device *)(dev->gpu->kgd));
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx