drm/amdkfd: Do copy_to/from_user in general kfd_ioctl()
authorOded Gabbay <oded.gabbay@amd.com>
Mon, 29 Dec 2014 11:52:22 +0000 (13:52 +0200)
committerOded Gabbay <oded.gabbay@amd.com>
Tue, 6 Jan 2015 17:44:26 +0000 (19:44 +0200)
This patch moves the copy_to_user() and copy_from_user() calls from the
different ioctl functions in amdkfd to the general kfd_ioctl() function, as
this is a common code for all ioctls.

This was done according to example taken from drm_ioctl.c

Signed-off-by: Oded Gabbay <oded.gabbay@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c

index fe5c543599b0c0480cf932f146b27fc6f5a38905..249f4921f4a86c4666ed2caea00b7a04fd8b0c41 100644 (file)
@@ -126,17 +126,14 @@ static int kfd_open(struct inode *inode, struct file *filep)
        return 0;
 }
 
-static long kfd_ioctl_get_version(struct file *filep, struct kfd_process *p,
-                                       void __user *arg)
+static int kfd_ioctl_get_version(struct file *filep, struct kfd_process *p,
+                                       void *data)
 {
-       struct kfd_ioctl_get_version_args args;
+       struct kfd_ioctl_get_version_args *args = data;
        int err = 0;
 
-       args.major_version = KFD_IOCTL_MAJOR_VERSION;
-       args.minor_version = KFD_IOCTL_MINOR_VERSION;
-
-       if (copy_to_user(arg, &args, sizeof(args)))
-               err = -EFAULT;
+       args->major_version = KFD_IOCTL_MAJOR_VERSION;
+       args->minor_version = KFD_IOCTL_MINOR_VERSION;
 
        return err;
 }
@@ -220,10 +217,10 @@ static int set_queue_properties_from_user(struct queue_properties *q_properties,
        return 0;
 }
 
-static long kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
-                                       void __user *arg)
+static int kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
+                                       void *data)
 {
-       struct kfd_ioctl_create_queue_args args;
+       struct kfd_ioctl_create_queue_args *args = data;
        struct kfd_dev *dev;
        int err = 0;
        unsigned int queue_id;
@@ -232,16 +229,13 @@ static long kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
 
        memset(&q_properties, 0, sizeof(struct queue_properties));
 
-       if (copy_from_user(&args, arg, sizeof(args)))
-               return -EFAULT;
-
        pr_debug("kfd: creating queue ioctl\n");
 
-       err = set_queue_properties_from_user(&q_properties, &args);
+       err = set_queue_properties_from_user(&q_properties, args);
        if (err)
                return err;
 
-       dev = kfd_device_by_id(args.gpu_id);
+       dev = kfd_device_by_id(args->gpu_id);
        if (dev == NULL)
                return -EINVAL;
 
@@ -249,7 +243,7 @@ static long kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
 
        pdd = kfd_bind_process_to_device(dev, p);
        if (IS_ERR(pdd)) {
-               err = PTR_ERR(pdd);
+               err = -ESRCH;
                goto err_bind_process;
        }
 
@@ -262,33 +256,26 @@ static long kfd_ioctl_create_queue(struct file *filep, struct kfd_process *p,
        if (err != 0)
                goto err_create_queue;
 
-       args.queue_id = queue_id;
+       args->queue_id = queue_id;
 
        /* Return gpu_id as doorbell offset for mmap usage */
-       args.doorbell_offset = args.gpu_id << PAGE_SHIFT;
-
-       if (copy_to_user(arg, &args, sizeof(args))) {
-               err = -EFAULT;
-               goto err_copy_args_out;
-       }
+       args->doorbell_offset = args->gpu_id << PAGE_SHIFT;
 
        mutex_unlock(&p->mutex);
 
-       pr_debug("kfd: queue id %d was created successfully\n", args.queue_id);
+       pr_debug("kfd: queue id %d was created successfully\n", args->queue_id);
 
        pr_debug("ring buffer address == 0x%016llX\n",
-                       args.ring_base_address);
+                       args->ring_base_address);
 
        pr_debug("read ptr address    == 0x%016llX\n",
-                       args.read_pointer_address);
+                       args->read_pointer_address);
 
        pr_debug("write ptr address   == 0x%016llX\n",
-                       args.write_pointer_address);
+                       args->write_pointer_address);
 
        return 0;
 
-err_copy_args_out:
-       pqm_destroy_queue(&p->pqm, queue_id);
 err_create_queue:
 err_bind_process:
        mutex_unlock(&p->mutex);
@@ -296,99 +283,90 @@ err_bind_process:
 }
 
 static int kfd_ioctl_destroy_queue(struct file *filp, struct kfd_process *p,
-                                       void __user *arg)
+                                       void *data)
 {
        int retval;
-       struct kfd_ioctl_destroy_queue_args args;
-
-       if (copy_from_user(&args, arg, sizeof(args)))
-               return -EFAULT;
+       struct kfd_ioctl_destroy_queue_args *args = data;
 
        pr_debug("kfd: destroying queue id %d for PASID %d\n",
-                               args.queue_id,
+                               args->queue_id,
                                p->pasid);
 
        mutex_lock(&p->mutex);
 
-       retval = pqm_destroy_queue(&p->pqm, args.queue_id);
+       retval = pqm_destroy_queue(&p->pqm, args->queue_id);
 
        mutex_unlock(&p->mutex);
        return retval;
 }
 
 static int kfd_ioctl_update_queue(struct file *filp, struct kfd_process *p,
-                                       void __user *arg)
+                                       void *data)
 {
        int retval;
-       struct kfd_ioctl_update_queue_args args;
+       struct kfd_ioctl_update_queue_args *args = data;
        struct queue_properties properties;
 
-       if (copy_from_user(&args, arg, sizeof(args)))
-               return -EFAULT;
-
-       if (args.queue_percentage > KFD_MAX_QUEUE_PERCENTAGE) {
+       if (args->queue_percentage > KFD_MAX_QUEUE_PERCENTAGE) {
                pr_err("kfd: queue percentage must be between 0 to KFD_MAX_QUEUE_PERCENTAGE\n");
                return -EINVAL;
        }
 
-       if (args.queue_priority > KFD_MAX_QUEUE_PRIORITY) {
+       if (args->queue_priority > KFD_MAX_QUEUE_PRIORITY) {
                pr_err("kfd: queue priority must be between 0 to KFD_MAX_QUEUE_PRIORITY\n");
                return -EINVAL;
        }
 
-       if ((args.ring_base_address) &&
+       if ((args->ring_base_address) &&
                (!access_ok(VERIFY_WRITE,
-                       (const void __user *) args.ring_base_address,
+                       (const void __user *) args->ring_base_address,
                        sizeof(uint64_t)))) {
                pr_err("kfd: can't access ring base address\n");
                return -EFAULT;
        }
 
-       if (!is_power_of_2(args.ring_size) && (args.ring_size != 0)) {
+       if (!is_power_of_2(args->ring_size) && (args->ring_size != 0)) {
                pr_err("kfd: ring size must be a power of 2 or 0\n");
                return -EINVAL;
        }
 
-       properties.queue_address = args.ring_base_address;
-       properties.queue_size = args.ring_size;
-       properties.queue_percent = args.queue_percentage;
-       properties.priority = args.queue_priority;
+       properties.queue_address = args->ring_base_address;
+       properties.queue_size = args->ring_size;
+       properties.queue_percent = args->queue_percentage;
+       properties.priority = args->queue_priority;
 
        pr_debug("kfd: updating queue id %d for PASID %d\n",
-                       args.queue_id, p->pasid);
+                       args->queue_id, p->pasid);
 
        mutex_lock(&p->mutex);
 
-       retval = pqm_update_queue(&p->pqm, args.queue_id, &properties);
+       retval = pqm_update_queue(&p->pqm, args->queue_id, &properties);
 
        mutex_unlock(&p->mutex);
 
        return retval;
 }
 
-static long kfd_ioctl_set_memory_policy(struct file *filep,
-                               struct kfd_process *p, void __user *arg)
+static int kfd_ioctl_set_memory_policy(struct file *filep,
+                                       struct kfd_process *p, void *data)
 {
-       struct kfd_ioctl_set_memory_policy_args args;
+       struct kfd_ioctl_set_memory_policy_args *args = data;
        struct kfd_dev *dev;
        int err = 0;
        struct kfd_process_device *pdd;
        enum cache_policy default_policy, alternate_policy;
 
-       if (copy_from_user(&args, arg, sizeof(args)))
-               return -EFAULT;
-
-       if (args.default_policy != KFD_IOC_CACHE_POLICY_COHERENT
-           && args.default_policy != KFD_IOC_CACHE_POLICY_NONCOHERENT) {
+       if (args->default_policy != KFD_IOC_CACHE_POLICY_COHERENT
+           && args->default_policy != KFD_IOC_CACHE_POLICY_NONCOHERENT) {
                return -EINVAL;
        }
 
-       if (args.alternate_policy != KFD_IOC_CACHE_POLICY_COHERENT
-           && args.alternate_policy != KFD_IOC_CACHE_POLICY_NONCOHERENT) {
+       if (args->alternate_policy != KFD_IOC_CACHE_POLICY_COHERENT
+           && args->alternate_policy != KFD_IOC_CACHE_POLICY_NONCOHERENT) {
                return -EINVAL;
        }
 
-       dev = kfd_device_by_id(args.gpu_id);
+       dev = kfd_device_by_id(args->gpu_id);
        if (dev == NULL)
                return -EINVAL;
 
@@ -396,23 +374,23 @@ static long kfd_ioctl_set_memory_policy(struct file *filep,
 
        pdd = kfd_bind_process_to_device(dev, p);
        if (IS_ERR(pdd)) {
-               err = PTR_ERR(pdd);
+               err = -ESRCH;
                goto out;
        }
 
-       default_policy = (args.default_policy == KFD_IOC_CACHE_POLICY_COHERENT)
+       default_policy = (args->default_policy == KFD_IOC_CACHE_POLICY_COHERENT)
                         ? cache_policy_coherent : cache_policy_noncoherent;
 
        alternate_policy =
-               (args.alternate_policy == KFD_IOC_CACHE_POLICY_COHERENT)
+               (args->alternate_policy == KFD_IOC_CACHE_POLICY_COHERENT)
                   ? cache_policy_coherent : cache_policy_noncoherent;
 
        if (!dev->dqm->set_cache_memory_policy(dev->dqm,
                                &pdd->qpd,
                                default_policy,
                                alternate_policy,
-                               (void __user *)args.alternate_aperture_base,
-                               args.alternate_aperture_size))
+                               (void __user *)args->alternate_aperture_base,
+                               args->alternate_aperture_size))
                err = -EINVAL;
 
 out:
@@ -421,53 +399,44 @@ out:
        return err;
 }
 
-static long kfd_ioctl_get_clock_counters(struct file *filep,
-                               struct kfd_process *p, void __user *arg)
+static int kfd_ioctl_get_clock_counters(struct file *filep,
+                               struct kfd_process *p, void *data)
 {
-       struct kfd_ioctl_get_clock_counters_args args;
+       struct kfd_ioctl_get_clock_counters_args *args = data;
        struct kfd_dev *dev;
        struct timespec time;
 
-       if (copy_from_user(&args, arg, sizeof(args)))
-               return -EFAULT;
-
-       dev = kfd_device_by_id(args.gpu_id);
+       dev = kfd_device_by_id(args->gpu_id);
        if (dev == NULL)
                return -EINVAL;
 
        /* Reading GPU clock counter from KGD */
-       args.gpu_clock_counter = kfd2kgd->get_gpu_clock_counter(dev->kgd);
+       args->gpu_clock_counter = kfd2kgd->get_gpu_clock_counter(dev->kgd);
 
        /* No access to rdtsc. Using raw monotonic time */
        getrawmonotonic(&time);
-       args.cpu_clock_counter = (uint64_t)timespec_to_ns(&time);
+       args->cpu_clock_counter = (uint64_t)timespec_to_ns(&time);
 
        get_monotonic_boottime(&time);
-       args.system_clock_counter = (uint64_t)timespec_to_ns(&time);
+       args->system_clock_counter = (uint64_t)timespec_to_ns(&time);
 
        /* Since the counter is in nano-seconds we use 1GHz frequency */
-       args.system_clock_freq = 1000000000;
-
-       if (copy_to_user(arg, &args, sizeof(args)))
-               return -EFAULT;
+       args->system_clock_freq = 1000000000;
 
        return 0;
 }
 
 
 static int kfd_ioctl_get_process_apertures(struct file *filp,
-                               struct kfd_process *p, void __user *arg)
+                               struct kfd_process *p, void *data)
 {
-       struct kfd_ioctl_get_process_apertures_args args;
+       struct kfd_ioctl_get_process_apertures_args *args = data;
        struct kfd_process_device_apertures *pAperture;
        struct kfd_process_device *pdd;
 
        dev_dbg(kfd_device, "get apertures for PASID %d", p->pasid);
 
-       if (copy_from_user(&args, arg, sizeof(args)))
-               return -EFAULT;
-
-       args.num_of_nodes = 0;
+       args->num_of_nodes = 0;
 
        mutex_lock(&p->mutex);
 
@@ -476,7 +445,8 @@ static int kfd_ioctl_get_process_apertures(struct file *filp,
                /* Run over all pdd of the process */
                pdd = kfd_get_first_process_device_data(p);
                do {
-                       pAperture = &args.process_apertures[args.num_of_nodes];
+                       pAperture =
+                               &args->process_apertures[args->num_of_nodes];
                        pAperture->gpu_id = pdd->dev->id;
                        pAperture->lds_base = pdd->lds_base;
                        pAperture->lds_limit = pdd->lds_limit;
@@ -486,7 +456,7 @@ static int kfd_ioctl_get_process_apertures(struct file *filp,
                        pAperture->scratch_limit = pdd->scratch_limit;
 
                        dev_dbg(kfd_device,
-                               "node id %u\n", args.num_of_nodes);
+                               "node id %u\n", args->num_of_nodes);
                        dev_dbg(kfd_device,
                                "gpu id %u\n", pdd->dev->id);
                        dev_dbg(kfd_device,
@@ -502,23 +472,23 @@ static int kfd_ioctl_get_process_apertures(struct file *filp,
                        dev_dbg(kfd_device,
                                "scratch_limit %llX\n", pdd->scratch_limit);
 
-                       args.num_of_nodes++;
+                       args->num_of_nodes++;
                } while ((pdd = kfd_get_next_process_device_data(p, pdd)) != NULL &&
-                               (args.num_of_nodes < NUM_OF_SUPPORTED_GPUS));
+                               (args->num_of_nodes < NUM_OF_SUPPORTED_GPUS));
        }
 
        mutex_unlock(&p->mutex);
 
-       if (copy_to_user(arg, &args, sizeof(args)))
-               return -EFAULT;
-
        return 0;
 }
 
 static long kfd_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
 {
        struct kfd_process *process;
-       long err = -EINVAL;
+       char stack_kdata[128];
+       char *kdata = NULL;
+       unsigned int usize, asize;
+       int retcode = -EINVAL;
 
        dev_dbg(kfd_device,
                "ioctl cmd 0x%x (#%d), arg 0x%lx\n",
@@ -528,54 +498,84 @@ static long kfd_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
        if (IS_ERR(process))
                return PTR_ERR(process);
 
+       if (cmd & (IOC_IN | IOC_OUT)) {
+               if (asize <= sizeof(stack_kdata)) {
+                       kdata = stack_kdata;
+               } else {
+                       kdata = kmalloc(asize, GFP_KERNEL);
+                       if (!kdata) {
+                               retcode = -ENOMEM;
+                               goto err_i1;
+                       }
+               }
+               if (asize > usize)
+                       memset(kdata + usize, 0, asize - usize);
+       }
+
+       if (cmd & IOC_IN) {
+               if (copy_from_user(kdata, (void __user *)arg, usize) != 0) {
+                       retcode = -EFAULT;
+                       goto err_i1;
+               }
+       } else if (cmd & IOC_OUT) {
+               memset(kdata, 0, usize);
+       }
+
+
        switch (cmd) {
        case KFD_IOC_GET_VERSION:
-               err = kfd_ioctl_get_version(filep, process, (void __user *)arg);
+               retcode = kfd_ioctl_get_version(filep, process, kdata);
                break;
        case KFD_IOC_CREATE_QUEUE:
-               err = kfd_ioctl_create_queue(filep, process,
-                                               (void __user *)arg);
+               retcode = kfd_ioctl_create_queue(filep, process,
+                                               kdata);
                break;
 
        case KFD_IOC_DESTROY_QUEUE:
-               err = kfd_ioctl_destroy_queue(filep, process,
-                                               (void __user *)arg);
+               retcode = kfd_ioctl_destroy_queue(filep, process,
+                                               kdata);
                break;
 
        case KFD_IOC_SET_MEMORY_POLICY:
-               err = kfd_ioctl_set_memory_policy(filep, process,
-                                               (void __user *)arg);
+               retcode = kfd_ioctl_set_memory_policy(filep, process,
+                                               kdata);
                break;
 
        case KFD_IOC_GET_CLOCK_COUNTERS:
-               err = kfd_ioctl_get_clock_counters(filep, process,
-                                               (void __user *)arg);
+               retcode = kfd_ioctl_get_clock_counters(filep, process,
+                                               kdata);
                break;
 
        case KFD_IOC_GET_PROCESS_APERTURES:
-               err = kfd_ioctl_get_process_apertures(filep, process,
-                                               (void __user *)arg);
+               retcode = kfd_ioctl_get_process_apertures(filep, process,
+                                               kdata);
                break;
 
        case KFD_IOC_UPDATE_QUEUE:
-               err = kfd_ioctl_update_queue(filep, process,
-                                               (void __user *)arg);
+               retcode = kfd_ioctl_update_queue(filep, process,
+                                               kdata);
                break;
 
        default:
-               dev_err(kfd_device,
+               dev_dbg(kfd_device,
                        "unknown ioctl cmd 0x%x, arg 0x%lx)\n",
                        cmd, arg);
-               err = -EINVAL;
+               retcode = -EINVAL;
                break;
        }
 
-       if (err < 0)
-               dev_err(kfd_device,
-                       "ioctl error %ld for ioctl cmd 0x%x (#%d)\n",
-                       err, cmd, _IOC_NR(cmd));
+       if (cmd & IOC_OUT)
+               if (copy_to_user((void __user *)arg, kdata, usize) != 0)
+                       retcode = -EFAULT;
 
-       return err;
+err_i1:
+       if (kdata != stack_kdata)
+               kfree(kdata);
+
+       if (retcode)
+               dev_dbg(kfd_device, "ret = %d\n", retcode);
+
+       return retcode;
 }
 
 static int kfd_mmap(struct file *filp, struct vm_area_struct *vma)