[PATCH 2/2] [RFC] drm/virtgpu: modify uapi with stride/layer_stride fix

[Gerd Hoffmann]

On Tue, Oct 01, 2019 at 06:49:35PM -0700, Gurchetan Singh wrote:
> This doesn't really break userspace, since it always passes down
> 0 for stride/layer_stride currently. We could:
> 
> (1) modify UAPI now and add a VIRTGPU_PARAM_STRIDE_FIX feature

This I think.
But IMO it's not a fix, it is an added feature ...

Also missing the big picture here.  Why do we need this?

For guest object we don't have strides (virtio_gpu_resource_create_3d
doesn't allow this).

For host objects the host should know the strides.

Which I think is the reason why the stride and layer_stride fields in
the transfer commands are effectively unused ...

> -	/* TODO: add the correct stride / layer_stride. */
>  	virtio_gpu_cmd_transfer_from_host_3d
> -		(vgdev, vfpriv->ctx_id, offset, args->level, 0, 0,
> -		 &box, objs, fence);
> +		(vgdev, vfpriv->ctx_id, offset, args->level, args->stride,
> +		 args->layer_stride, &box, objs, fence);

What happens with old userspace running on a new kernel?

I don't think we can simply use the args here without checking we
actually got something from userspace ...

> diff --git a/include/uapi/drm/virtgpu_drm.h b/include/uapi/drm/virtgpu_drm.h
> index f06a789f34cd..b2fc92c3d2be 100644
> --- a/include/uapi/drm/virtgpu_drm.h
> +++ b/include/uapi/drm/virtgpu_drm.h
> @@ -117,6 +117,8 @@ struct drm_virtgpu_3d_transfer_to_host {
>  	struct drm_virtgpu_3d_box box;
>  	__u32 level;
>  	__u32 offset;
> +	__u32 stride;
> +	__u32 layer_stride;
>  };

cheers,
  Gerd