| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <46c0feb8-3a0d-eeac-180c-cb0f3147fe11@tronnes.org>
Date: Thu, 31 Jan 2019 11:40:00 +0100
From: Noralf Trønnes <noralf@...nnes.org>
To: Gerd Hoffmann <kraxel@...hat.com>, dri-devel@...ts.freedesktop.org
Cc: David Airlie <airlied@...ux.ie>,
open list <linux-kernel@...r.kernel.org>,
"open list:VIRTIO GPU DRIVER"
<virtualization@...ts.linux-foundation.org>
Subject: Re: [PATCH v2 2/6] drm/virtio: use struct to pass params to
virtio_gpu_object_create()
Den 30.01.2019 10.43, skrev Gerd Hoffmann:
> Create virtio_gpu_object_params, use that to pass object parameters to
> virtio_gpu_object_create. This is just the first step, followup patches
> will add more parameters to the struct. The plan is to use the struct
> for all object parameters.
>
> Also drop unused "kernel" parameter for virtio_gpu_alloc_object(), it is
> unused and always false.
>
> Signed-off-by: Gerd Hoffmann <kraxel@...hat.com>
> ---
> drivers/gpu/drm/virtio/virtgpu_drv.h | 15 ++++++++++-----
> drivers/gpu/drm/virtio/virtgpu_gem.c | 17 ++++++++++-------
> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 11 ++++++-----
> drivers/gpu/drm/virtio/virtgpu_object.c | 22 +++++++++-------------
> 4 files changed, 35 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index d577cb76f5..40928980a2 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -50,6 +50,11 @@
> #define DRIVER_MINOR 1
> #define DRIVER_PATCHLEVEL 0
>
> +struct virtio_gpu_object_params {
> + unsigned long size;
> + bool pinned;
> +};
> +
> struct virtio_gpu_object {
> struct drm_gem_object gem_base;
> uint32_t hw_res_handle;
> @@ -217,16 +222,16 @@ int virtio_gpu_gem_init(struct virtio_gpu_device *vgdev);
> void virtio_gpu_gem_fini(struct virtio_gpu_device *vgdev);
> int virtio_gpu_gem_create(struct drm_file *file,
> struct drm_device *dev,
> - uint64_t size,
> + struct virtio_gpu_object_params *params,
> struct drm_gem_object **obj_p,
> uint32_t *handle_p);
> int virtio_gpu_gem_object_open(struct drm_gem_object *obj,
> struct drm_file *file);
> void virtio_gpu_gem_object_close(struct drm_gem_object *obj,
> struct drm_file *file);
> -struct virtio_gpu_object *virtio_gpu_alloc_object(struct drm_device *dev,
> - size_t size, bool kernel,
> - bool pinned);
> +struct virtio_gpu_object*
> +virtio_gpu_alloc_object(struct drm_device *dev,
> + struct virtio_gpu_object_params *params);
> int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
> struct drm_device *dev,
> struct drm_mode_create_dumb *args);
> @@ -342,7 +347,7 @@ void virtio_gpu_fence_event_process(struct virtio_gpu_device *vdev,
>
> /* virtio_gpu_object */
> int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
> - unsigned long size, bool kernel, bool pinned,
> + struct virtio_gpu_object_params *params,
> struct virtio_gpu_object **bo_ptr);
> void virtio_gpu_object_kunmap(struct virtio_gpu_object *bo);
> int virtio_gpu_object_kmap(struct virtio_gpu_object *bo);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
> index f065863939..b5f2d94ce5 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_gem.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
> @@ -34,15 +34,15 @@ void virtio_gpu_gem_free_object(struct drm_gem_object *gem_obj)
> virtio_gpu_object_unref(&obj);
> }
>
> -struct virtio_gpu_object *virtio_gpu_alloc_object(struct drm_device *dev,
> - size_t size, bool kernel,
> - bool pinned)
> +struct virtio_gpu_object*
> +virtio_gpu_alloc_object(struct drm_device *dev,
> + struct virtio_gpu_object_params *params)
> {
> struct virtio_gpu_device *vgdev = dev->dev_private;
> struct virtio_gpu_object *obj;
> int ret;
>
> - ret = virtio_gpu_object_create(vgdev, size, kernel, pinned, &obj);
> + ret = virtio_gpu_object_create(vgdev, params, &obj);
> if (ret)
> return ERR_PTR(ret);
>
> @@ -51,7 +51,7 @@ struct virtio_gpu_object *virtio_gpu_alloc_object(struct drm_device *dev,
>
> int virtio_gpu_gem_create(struct drm_file *file,
> struct drm_device *dev,
> - uint64_t size,
> + struct virtio_gpu_object_params *params,
> struct drm_gem_object **obj_p,
> uint32_t *handle_p)
> {
> @@ -59,7 +59,7 @@ int virtio_gpu_gem_create(struct drm_file *file,
> int ret;
> u32 handle;
>
> - obj = virtio_gpu_alloc_object(dev, size, false, false);
> + obj = virtio_gpu_alloc_object(dev, params);
> if (IS_ERR(obj))
> return PTR_ERR(obj);
>
> @@ -85,6 +85,7 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
> struct virtio_gpu_device *vgdev = dev->dev_private;
> struct drm_gem_object *gobj;
> struct virtio_gpu_object *obj;
> + struct virtio_gpu_object_params params = { 0 };
> int ret;
> uint32_t pitch;
> uint32_t format;
> @@ -96,7 +97,9 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv,
> args->size = pitch * args->height;
> args->size = ALIGN(args->size, PAGE_SIZE);
>
> - ret = virtio_gpu_gem_create(file_priv, dev, args->size, &gobj,
> + params.pinned = false,
You have a comma here, but assigning to false isn't really necessary
since the struct is zeroed. Same goes for the same assignment further down.
With this fixed in some way:
Acked-by: Noralf Trønnes <noralf@...nnes.org>
> + params.size = args->size;
> + ret = virtio_gpu_gem_create(file_priv, dev, ¶ms, &gobj,
> &args->handle);
> if (ret)
> goto fail;
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 14ce8188c0..fa7b958ca2 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -279,12 +279,12 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
> struct virtio_gpu_object *qobj;
> struct drm_gem_object *obj;
> uint32_t handle = 0;
> - uint32_t size;
> struct list_head validate_list;
> struct ttm_validate_buffer mainbuf;
> struct virtio_gpu_fence *fence = NULL;
> struct ww_acquire_ctx ticket;
> struct virtio_gpu_resource_create_3d rc_3d;
> + struct virtio_gpu_object_params params = { 0 };
>
> if (vgdev->has_virgl_3d == false) {
> if (rc->depth > 1)
> @@ -302,13 +302,14 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data,
> INIT_LIST_HEAD(&validate_list);
> memset(&mainbuf, 0, sizeof(struct ttm_validate_buffer));
>
> - size = rc->size;
> + params.pinned = false,
> + params.size = rc->size;
>
> /* allocate a single page size object */
> - if (size == 0)
> - size = PAGE_SIZE;
> + if (params.size == 0)
> + params.size = PAGE_SIZE;
>
> - qobj = virtio_gpu_alloc_object(dev, size, false, false);
> + qobj = virtio_gpu_alloc_object(dev, ¶ms);
> if (IS_ERR(qobj))
> return PTR_ERR(qobj);
> obj = &qobj->gem_base;
> diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
> index f39a183d59..62367e3f80 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_object.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_object.c
> @@ -79,21 +79,16 @@ static void virtio_gpu_init_ttm_placement(struct virtio_gpu_object *vgbo,
> }
>
> int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
> - unsigned long size, bool kernel, bool pinned,
> + struct virtio_gpu_object_params *params,
> struct virtio_gpu_object **bo_ptr)
> {
> struct virtio_gpu_object *bo;
> - enum ttm_bo_type type;
> size_t acc_size;
> int ret;
>
> - if (kernel)
> - type = ttm_bo_type_kernel;
> - else
> - type = ttm_bo_type_device;
> *bo_ptr = NULL;
>
> - acc_size = ttm_bo_dma_acc_size(&vgdev->mman.bdev, size,
> + acc_size = ttm_bo_dma_acc_size(&vgdev->mman.bdev, params->size,
> sizeof(struct virtio_gpu_object));
>
> bo = kzalloc(sizeof(struct virtio_gpu_object), GFP_KERNEL);
> @@ -104,19 +99,20 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
> kfree(bo);
> return ret;
> }
> - size = roundup(size, PAGE_SIZE);
> - ret = drm_gem_object_init(vgdev->ddev, &bo->gem_base, size);
> + params->size = roundup(params->size, PAGE_SIZE);
> + ret = drm_gem_object_init(vgdev->ddev, &bo->gem_base, params->size);
> if (ret != 0) {
> virtio_gpu_resource_id_put(vgdev, bo->hw_res_handle);
> kfree(bo);
> return ret;
> }
> bo->dumb = false;
> - virtio_gpu_init_ttm_placement(bo, pinned);
> + virtio_gpu_init_ttm_placement(bo, params->pinned);
>
> - ret = ttm_bo_init(&vgdev->mman.bdev, &bo->tbo, size, type,
> - &bo->placement, 0, !kernel, acc_size,
> - NULL, NULL, &virtio_gpu_ttm_bo_destroy);
> + ret = ttm_bo_init(&vgdev->mman.bdev, &bo->tbo, params->size,
> + ttm_bo_type_device, &bo->placement, 0,
> + true, acc_size, NULL, NULL,
> + &virtio_gpu_ttm_bo_destroy);
> /* ttm_bo_init failure will call the destroy */
> if (ret != 0)
> return ret;
>
Powered by blists - more mailing lists