| 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: <CAF6AEGsHY+kh-k+=FqNzf=d=60JqkfFC=+U039XYK-7dTkXZmA@mail.gmail.com>
Date: Fri, 14 Feb 2025 09:45:50 -0800
From: Rob Clark <robdclark@...il.com>
To: Dmitry Osipenko <dmitry.osipenko@...labora.com>
Cc: David Airlie <airlied@...hat.com>, Gerd Hoffmann <kraxel@...hat.com>,
Gurchetan Singh <gurchetansingh@...omium.org>, Chia-I Wu <olvaffe@...il.com>,
Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@....com>, Asahi Lina <lina@...hilina.net>,
Alyssa Rosenzweig <alyssa@...enzweig.io>, dri-devel@...ts.freedesktop.org,
virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org,
kernel@...labora.com
Subject: Re: [PATCH v1] drm/virtio: Extend blob UAPI with deferred-mapping hinting
On Sun, Jan 26, 2025 at 12:43 PM Dmitry Osipenko
<dmitry.osipenko@...labora.com> wrote:
>
> If userspace never maps GEM object, then BO wastes hostmem space
> because VirtIO-GPU driver maps VRAM BO at the BO's creating time.
>
> Make mappings on-demand by adding new RESOURCE_CREATE_BLOB IOCTL/UAPI
> hinting flag telling that host mapping should be deferred until first
> mapping is made when the flag is set by userspace.
>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@...labora.com>
I suppose we could have just added a new blob_flag, but then userspace
would need to probe the kernel to see whether the new flag was
supported. (Which isn't a big deal, that sort of thing is done in
many places.) But extending the struct does make it more transparent
to userspace. Either way,
Reviewed-by: Rob Clark <robdclark@...il.com
> ---
> drivers/gpu/drm/virtio/virtgpu_drv.h | 2 ++
> drivers/gpu/drm/virtio/virtgpu_gem.c | 9 ++++++++
> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 1 +
> drivers/gpu/drm/virtio/virtgpu_vram.c | 30 +++++++++++++++++++++-----
> include/uapi/drm/virtgpu_drm.h | 4 ++++
> 5 files changed, 41 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index 64c236169db8..e5db91a3e8ba 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -85,6 +85,7 @@ struct virtio_gpu_object_params {
> uint32_t blob_mem;
> uint32_t blob_flags;
> uint64_t blob_id;
> + uint32_t blob_hints;
> };
>
> struct virtio_gpu_object {
> @@ -483,6 +484,7 @@ struct sg_table *virtio_gpu_vram_map_dma_buf(struct virtio_gpu_object *bo,
> void virtio_gpu_vram_unmap_dma_buf(struct device *dev,
> struct sg_table *sgt,
> enum dma_data_direction dir);
> +void virtio_gpu_vram_map_deferred(struct virtio_gpu_object_vram *vram);
>
> /* virtgpu_submit.c */
> int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
> index 7db48d17ee3a..c902b68cc25a 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_gem.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
> @@ -103,12 +103,21 @@ int virtio_gpu_mode_dumb_mmap(struct drm_file *file_priv,
> struct drm_device *dev,
> uint32_t handle, uint64_t *offset_p)
> {
> + struct virtio_gpu_object_vram *vram;
> + struct virtio_gpu_object *bo;
> struct drm_gem_object *gobj;
>
> BUG_ON(!offset_p);
> gobj = drm_gem_object_lookup(file_priv, handle);
> if (gobj == NULL)
> return -ENOENT;
> +
> + bo = gem_to_virtio_gpu_obj(gobj);
> + if (virtio_gpu_is_vram(bo)) {
> + vram = to_virtio_gpu_vram(bo);
> + virtio_gpu_vram_map_deferred(vram);
> + }
> +
> *offset_p = drm_vma_node_offset_addr(&gobj->vma_node);
> drm_gem_object_put(gobj);
> return 0;
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index e4f76f315550..51544ee8d3c3 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -489,6 +489,7 @@ static int verify_blob(struct virtio_gpu_device *vgdev,
> params->size = rc_blob->size;
> params->blob = true;
> params->blob_flags = rc_blob->blob_flags;
> + params->blob_hints = rc_blob->blob_hints;
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vram.c b/drivers/gpu/drm/virtio/virtgpu_vram.c
> index 25df81c02783..d72c93e06a4a 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vram.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vram.c
> @@ -41,6 +41,11 @@ static int virtio_gpu_vram_mmap(struct drm_gem_object *obj,
> if (!(bo->blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE))
> return -EINVAL;
>
> + virtio_gpu_vram_map_deferred(vram);
> +
> + if (vram->map_state == STATE_INITIALIZING)
> + virtio_gpu_notify(vgdev);
> +
> wait_event(vgdev->resp_wq, vram->map_state != STATE_INITIALIZING);
> if (vram->map_state != STATE_OK)
> return -EINVAL;
> @@ -215,14 +220,29 @@ int virtio_gpu_vram_create(struct virtio_gpu_device *vgdev,
>
> virtio_gpu_cmd_resource_create_blob(vgdev, &vram->base, params, NULL,
> 0);
> - if (params->blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE) {
> - ret = virtio_gpu_vram_map(&vram->base);
> - if (ret) {
> - virtio_gpu_vram_free(obj);
> - return ret;
> + if (!(params->blob_hints & DRM_VIRTGPU_BLOB_FLAG_HINT_DEFER_MAPPING)) {
> + if (params->blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE) {
> + ret = virtio_gpu_vram_map(&vram->base);
> + if (ret) {
> + virtio_gpu_vram_free(obj);
> + return ret;
> + }
> }
> }
>
> *bo_ptr = &vram->base;
> return 0;
> }
> +
> +void virtio_gpu_vram_map_deferred(struct virtio_gpu_object_vram *vram)
> +{
> + static DEFINE_MUTEX(map_lock);
> +
> + if (!(vram->base.blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE))
> + return;
> +
> + mutex_lock(&map_lock);
> + if (!drm_mm_node_allocated(&vram->vram_node))
> + virtio_gpu_vram_map(&vram->base);
> + mutex_unlock(&map_lock);
> +}
> diff --git a/include/uapi/drm/virtgpu_drm.h b/include/uapi/drm/virtgpu_drm.h
> index c2ce71987e9b..3004a1d08570 100644
> --- a/include/uapi/drm/virtgpu_drm.h
> +++ b/include/uapi/drm/virtgpu_drm.h
> @@ -194,6 +194,10 @@ struct drm_virtgpu_resource_create_blob {
> __u32 cmd_size;
> __u64 cmd;
> __u64 blob_id;
> +
> +#define DRM_VIRTGPU_BLOB_FLAG_HINT_DEFER_MAPPING 0x0001
> + __u32 blob_hints;
> + __u32 pad2;
> };
>
> #define VIRTGPU_CONTEXT_PARAM_CAPSET_ID 0x0001
> --
> 2.47.1
>
Powered by blists - more mailing lists