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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 4 Jan 2024 00:42:48 +0300
From: Dmitry Osipenko <dmitry.osipenko@...labora.com>
To: Julia Zhang <julia.zhang@....com>,
 Gurchetan Singh <gurchetansingh@...omium.org>, Chia-I Wu
 <olvaffe@...il.com>, David Airlie <airlied@...hat.com>,
 Gerd Hoffmann <kraxel@...hat.com>, linux-kernel@...r.kernel.org,
 dri-devel@...ts.freedesktop.org, amd-gfx@...ts.freedesktop.org,
 virtualization@...ts.linux-foundation.org
Cc: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@....com>,
 Daniel Stone <daniels@...labora.com>, Erik Faye-Lund <kusmabite@...il.com>,
 Marek Olšák <marek.olsak@....com>,
 Chen Jiqian <Jiqian.Chen@....com>, Huang Rui <ray.huang@....com>,
 Honglei Huang <honglei1.huang@....com>,
 Alex Deucher <alexander.deucher@....com>, David Airlie <airlied@...il.com>,
 Christian König <christian.koenig@....com>
Subject: Re: [PATCH 1/1] drm/virtio: Implement RESOURCE_GET_LAYOUT ioctl

On 12/21/23 13:00, Julia Zhang wrote:
> From: Daniel Stone <daniels@...labora.com>
> 
> Add a new ioctl to allow the guest VM to discover how the guest
> actually allocated the underlying buffer, which allows buffers to
> be used for GL<->Vulkan interop and through standard window systems.
> It's also a step towards properly supporting modifiers in the guest.
> 
> Signed-off-by: Daniel Stone <daniels@...labora.com>
> Co-developed-by: Julia Zhang <julia.zhang@....com> # support query
> stride before it's created
> Signed-off-by: Julia Zhang <julia.zhang@....com>
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.c   |  1 +
>  drivers/gpu/drm/virtio/virtgpu_drv.h   | 22 ++++++++-
>  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 66 ++++++++++++++++++++++++++
>  drivers/gpu/drm/virtio/virtgpu_kms.c   |  8 +++-
>  drivers/gpu/drm/virtio/virtgpu_vq.c    | 63 ++++++++++++++++++++++++
>  include/uapi/drm/virtgpu_drm.h         | 21 ++++++++
>  include/uapi/linux/virtio_gpu.h        | 30 ++++++++++++
>  7 files changed, 208 insertions(+), 3 deletions(-)
...
> +static int virtio_gpu_resource_query_layout_ioctl(struct drm_device *dev,
> +						  void *data,
> +						  struct drm_file *file)
> +{
> +	struct drm_virtgpu_resource_query_layout *args = data;
> +	struct virtio_gpu_device *vgdev = dev->dev_private;
> +	struct drm_gem_object *obj = NULL;
> +	struct virtio_gpu_object *bo = NULL;
> +	struct virtio_gpu_query_info bo_info = {0};
> +	int ret = 0;
> +	int i;
> +
> +	if (!vgdev->has_resource_query_layout) {
> +		DRM_ERROR("failing: no RQL on host\n");

Please remove this message

> +		return -EINVAL;

return -ENOSYS

> +	}
> +
> +	if (args->handle > 0) {
> +		obj = drm_gem_object_lookup(file, args->handle);
> +		if (obj == NULL) {
> +			DRM_ERROR("invalid handle 0x%x\n", args->handle);
> +			return -ENOENT;
> +		}
> +		bo = gem_to_virtio_gpu_obj(obj);
> +	}
> +
> +	ret = virtio_gpu_cmd_get_resource_layout(vgdev, &bo_info, args->width,
> +						 args->height, args->format,
> +						 args->bind, bo ? bo->hw_res_handle : 0);

What this special hw_res_handle=0 is doing? Why is it needed?

> +	if (ret)
> +		goto out;
> +
> +	ret = wait_event_timeout(vgdev->resp_wq,
> +				 atomic_read(&bo_info.is_valid),
> +				 5 * HZ);
> +	if (!ret)
> +		goto out;
> +
> +valid:
> +	smp_rmb();
> +	WARN_ON(atomic_read(&bo_info.is_valid));

Please remove this WARN_ON and fix the kernelbot report

> +	args->num_planes = bo_info.num_planes;
> +	args->modifier = bo_info.modifier;
> +	for (i = 0; i < args->num_planes; i++) {
> +		args->planes[i].offset = bo_info.planes[i].offset;
> +		args->planes[i].stride = bo_info.planes[i].stride;
> +	}
> +	for (; i < VIRTIO_GPU_MAX_RESOURCE_PLANES; i++) {
> +		args->planes[i].offset = 0;
> +		args->planes[i].stride = 0;
> +	}
> +	ret = 0;

ret is already 0 here

> +out:
> +	if (obj)
> +		drm_gem_object_put(obj);
> +	return ret;
> +}

...
> diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
> index f556fde07b76..547575232376 100644
> --- a/include/uapi/linux/virtio_gpu.h
> +++ b/include/uapi/linux/virtio_gpu.h
> @@ -65,6 +65,11 @@
>   */
>  #define VIRTIO_GPU_F_CONTEXT_INIT        4
>  
> +/*
> + * VIRTIO_GPU_CMD_RESOURCE_QUERY_LAYOUT
> + */
> +#define VIRTIO_GPU_F_RESOURCE_QUERY_LAYOUT 5
> +
>  enum virtio_gpu_ctrl_type {
>  	VIRTIO_GPU_UNDEFINED = 0,
>  
> @@ -95,6 +100,7 @@ enum virtio_gpu_ctrl_type {
>  	VIRTIO_GPU_CMD_SUBMIT_3D,
>  	VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB,
>  	VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB,
> +	VIRTIO_GPU_CMD_RESOURCE_QUERY_LAYOUT,
>  
>  	/* cursor commands */
>  	VIRTIO_GPU_CMD_UPDATE_CURSOR = 0x0300,
> @@ -108,6 +114,7 @@ enum virtio_gpu_ctrl_type {
>  	VIRTIO_GPU_RESP_OK_EDID,
>  	VIRTIO_GPU_RESP_OK_RESOURCE_UUID,
>  	VIRTIO_GPU_RESP_OK_MAP_INFO,
> +	VIRTIO_GPU_RESP_OK_RESOURCE_LAYOUT,
>  
>  	/* error responses */
>  	VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200,
> @@ -453,4 +460,27 @@ struct virtio_gpu_resource_unmap_blob {
>  	__le32 padding;
>  };
>  
> +/* VIRTIO_GPU_CMD_RESOURCE_QUERY_LAYOUT */
> +struct virtio_gpu_resource_query_layout {
> +	struct virtio_gpu_ctrl_hdr hdr;
> +	__le32 resource_id;
> +	__le32 width;
> +	__le32 height;
> +	__le32 format;
> +	__le32 bind;

64b pad missing

> +};
> +
> +
> +/* VIRTIO_GPU_RESP_OK_RESOURCE_LAYOUT */
> +#define VIRTIO_GPU_RES_MAX_PLANES 4
> +struct virtio_gpu_resp_resource_layout {
> +	struct virtio_gpu_ctrl_hdr hdr;
> +	__le64 modifier;
> +	__le32 num_planes;
> +	struct virtio_gpu_resource_plane {
> +		__le64 offset;
> +		__le32 stride;
> +	} planes[VIRTIO_GPU_RES_MAX_PLANES];
> +};

Virto-spec changes should have a corresponding doc update in [1].

[1]
https://github.com/oasis-tcs/virtio-spec/blob/master/device-types/gpu/description.tex

-- 
Best regards,
Dmitry


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ