[PATCH v2] drm/amdkfd: Remove GPU ID in GWS queue creation

Greathouse, Joseph Joseph.Greathouse at amd.com
Thu Aug 1 21:14:33 UTC 2019


The gpu_id argument is not needed when enabling GWS on a queue.
The queue can only be associated with one device, so the only
possible situations for the call as previously defined were:
1) the gpu_id was for the device associated with the target queue
and things worked as expected, or 2) the gpu_id was for a device
not associated with the target queue and the request was undefined.

In particular, the previous result of the undefined operation is
that you would allocate the number of GWS entries available on the
gpu_id device, even if the queue was on a device with a different
number available. For example: a queue on a device without GWS
capability, but the user passes in a gpu_id for a device with GWS.
We would end up trying to allocate GWS on the device that does not
support it.

Rather than leaving this footgun around and making life more
difficult for user space, we can instead grab the gpu_id from the
target queue. The gpu_id argument being passed in is thus not
needed.

v2: Fixed styling, removed gpu_id since it never hit main release

Change-Id: I73c032d7115b62505a6e7c48d2c060fc3c6ee915
Signed-off-by: Joseph Greathouse <Joseph.Greathouse at amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      | 26 ++++++++++++++-----
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |  2 ++
 .../amd/amdkfd/kfd_process_queue_manager.c    |  9 +++++++
 include/uapi/linux/kfd_ioctl.h                |  3 +--
 4 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index f91126f5f1be..b464ad1e2c4c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1572,25 +1572,37 @@ static int kfd_ioctl_alloc_queue_gws(struct file *filep,
 {
 	int retval;
 	struct kfd_ioctl_alloc_queue_gws_args *args = data;
+	struct queue *q;
 	struct kfd_dev *dev;
 
 	if (!hws_gws_support)
 		return -ENODEV;
 
-	dev = kfd_device_by_id(args->gpu_id);
-	if (!dev) {
-		pr_debug("Could not find gpu id 0x%x\n", args->gpu_id);
-		return -ENODEV;
+	mutex_lock(&p->mutex);
+	q = pqm_get_user_queue(&p->pqm, args->queue_id);
+
+	if (q) {
+		dev = q->device;
+	}
+	else {
+		retval = -EINVAL;
+		goto out_unlock;
+	}
+
+	if (dev->dqm->sched_policy == KFD_SCHED_POLICY_NO_HWS) {
+		retval = -ENODEV;
+		goto out_unlock;
 	}
-	if (dev->dqm->sched_policy == KFD_SCHED_POLICY_NO_HWS)
-		return -ENODEV;
 
-	mutex_lock(&p->mutex);
 	retval = pqm_set_gws(&p->pqm, args->queue_id, args->num_gws ? dev->gws : NULL);
 	mutex_unlock(&p->mutex);
 
 	args->first_gws = 0;
 	return retval;
+
+out_unlock:
+	mutex_unlock(&p->mutex);
+	return retval;
 }
 
 static int kfd_ioctl_get_dmabuf_info(struct file *filep,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index aa7bf20d20f8..9b9a8da187c8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -915,6 +915,8 @@ int pqm_set_gws(struct process_queue_manager *pqm, unsigned int qid,
 			void *gws);
 struct kernel_queue *pqm_get_kernel_queue(struct process_queue_manager *pqm,
 						unsigned int qid);
+struct queue *pqm_get_user_queue(struct process_queue_manager *pqm,
+						unsigned int qid);
 int pqm_get_wave_state(struct process_queue_manager *pqm,
 		       unsigned int qid,
 		       void __user *ctl_stack,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index 7e6c3ee82f5b..7a61a5b09ed8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -473,6 +473,15 @@ struct kernel_queue *pqm_get_kernel_queue(
 	return NULL;
 }
 
+struct queue *pqm_get_user_queue(struct process_queue_manager *pqm,
+					unsigned int qid)
+{
+	struct process_queue_node *pqn;
+
+	pqn = get_queue_by_qid(pqm, qid);
+	return pqn ? pqn->q : NULL;
+}
+
 int pqm_get_wave_state(struct process_queue_manager *pqm,
 		       unsigned int qid,
 		       void __user *ctl_stack,
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index 070d1bc7e725..4f6676428c5c 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -412,17 +412,16 @@ struct kfd_ioctl_unmap_memory_from_gpu_args {
 
 /* Allocate GWS for specific queue
  *
- * @gpu_id:      device identifier
  * @queue_id:    queue's id that GWS is allocated for
  * @num_gws:     how many GWS to allocate
  * @first_gws:   index of the first GWS allocated.
  *               only support contiguous GWS allocation
  */
 struct kfd_ioctl_alloc_queue_gws_args {
-	__u32 gpu_id;		/* to KFD */
 	__u32 queue_id;		/* to KFD */
 	__u32 num_gws;		/* to KFD */
 	__u32 first_gws;	/* from KFD */
+	__u32 pad;
 };
 
 struct kfd_ioctl_get_dmabuf_info_args {
-- 
2.19.1



More information about the amd-gfx mailing list