Re: [PATCH 1/2] virtio-gpu: add resource create blob

[Gerd Hoffmann <kraxel@redhat.com>]

  Hi,

> > Do we need separate items (HOSTSYS/HOST3D) for these?  Shouldn't this be
> > a host implementation detail the guest doesn't need to worry about?
> 
> Yes, separate items are not needed and could indeed be an host
> implementation detail.  Right now, the drm uapi has VIRTGPU_BLOB_MEM_HOST /
> VIRTIO_GPU_BLOB_MEM_HOST_GUEST and the kernel separates to HOST{SYS,3D} and
> does various checks.
> 
> https://gitlab.freedesktop.org/virgl/drm-misc-next/-/commit/0ccb47d5150a2978d179d0c44ef3285541c8964c
> 
> 
> The reasoning is since there's no RESOURCE_CREATE_BLOB2D
> or RESOURCE_CREATE_BLOB3D, the separation captures that difference in an
> explicit API level.  That way, the blob id is only allowed with HOST3D.

I think we can also simply define "blob id is only allowed virgl=on", no
need for a separate command.

> A HOST3D blob has no guest memory associated with it, and can only be
> mapped through the shm region.  Even then, a linear (mappable) view of a
> host3d blob may not be possible/desirable -- like shareable device local
> memory, or maybe even a protected content blob.
> 
> HOST3D_GUEST is always mappable, while HOST3D isn't.

Ok.  So _GUEST postfix means there is a guest shadow and we TRANSFER
data instead of mapping the host resource directly.  Ok, makes sense,
needs a better description in the spec though.

> > > +If VIRTIO_GPU_F_VIRGL is set, both VIRTIO_GPU_CMD_TRANSFER_TO_HOST_3D
> > > +and VIRTIO_GPU_CMD_TRANSFER_FROM_HOST_3D may be used to update the
> > > +resource. There is no restriction on the image/buffer view the driver
> > > +has on the blob resource.
> >
> > I don't think VIRTIO_GPU_CMD_TRANSFER_TO_HOST_3D is going to work if
> > the resource has no format attached.  I had a TRANSFER_BLOB command
> > because of that.
> >
> 
> I was thinking about GL/VK interop here.
> 
> Say VK allocates the blob resource, but a GL texture is subsequently
> created from the blob resource using GL/VK interop extensions and a format
> is attached.  For this reason, VIRTIO_GPU_CMD_TRANSFER_TO_HOST_3D is
> allowed on blob resources.

Makes sense to allow the old transfer commands for the cases where the
resource has a format attached.

> Similarly, virtgpu_kms always attaches XR24 and AB24 to resources[1], so
> VIRTIO_GPU_CMD_TRANSFER_TO_HOST_2D can be reused if size = 4 * width for
> dumb/sys blob resources (which I think will always be the case?).

If the kernel uses BLOB resources for dumb BOs (so we can have a shared
mapping for them and avoid copying fbdev data) they will not have a
format attached.

Imported dma-bufs (inside the guest) don't have a format attached
either.  With BLOB resources we can support this, but we'll likewise get
a resource without format attached.

So we need ways to work with those resources ...

> Ack.  I definitely see the logic behind VIRTIO_GPU_CMD_SET_SCANOUT_BLOB,
> but current display integration (dpy_gl_scanout_texture(..) in QEMU)
> assumes the presence of a GL texture (which can be created from a blob
> resource).

I have a qemu VIRTIO_GPU_CMD_SET_SCANOUT_BLOB implementation for
virgl=off only, covering dumb bo / fbdev as outlined above.

With virgl=on this should be doable too, you only have to create the
texture from the blob resource at SET_SCANOUT_BLOB time (when you know
the format the guest wants scanout), not at resource creation time.

take care,
  Gerd