Hi,
I am still new to virgl, and missed the last round of discussion about resource_create_v2.
From the discussion below, semantically resource_create_v2 creates a host resource object _without_ any storage; memory_create creates a host memory object which provides the storage. Is that correct?
And this version of memory_create is probably the most special one among its other potential variants, because it is the only(?) one who imports the pre-allocated guest pages.
Do we expect these new commands to be supported by OpenGL, which does not separate resources and memories?
On Thu, Apr 11, 2019 at 10:49 PM Gerd Hoffmann <kraxel@xxxxxxxxxx> wrote:
On Thu, Apr 11, 2019 at 06:36:15PM -0700, Gurchetan Singh wrote:
> On Wed, Apr 10, 2019 at 10:03 PM Gerd Hoffmann <kraxel@xxxxxxxxxx> wrote:
> >
> > > > +/* VIRTIO_GPU_CMD_RESOURCE_CREATE_V2 */
> > > > +struct virtio_gpu_cmd_resource_create_v2 {
> > > > + struct virtio_gpu_ctrl_hdr hdr;
> > > > + __le32 resource_id;
> > > > + __le32 format;
> > > > + __le32 width;
> > > > + __le32 height;
> > > > + /* 3d only */
> > > > + __le32 target;
> > > > + __le32 bind;
> > > > + __le32 depth;
> > > > + __le32 array_size;
> > > > + __le32 last_level;
> > > > + __le32 nr_samples;
> > > > + __le32 flags;
> > > > +};
> > >
> > >
> > > I assume this is always backed by some host side allocation, without any
> > > guest side pages associated with it?
> >
> > No. It is not backed at all yet. Workflow would be like this:
> >
> > (1) VIRTIO_GPU_CMD_RESOURCE_CREATE_V2
> > (2) VIRTIO_GPU_CMD_MEMORY_CREATE (see patch 2)
> > (3) VIRTIO_GPU_CMD_RESOURCE_MEMORY_ATTACH (see patch 2)
>
> Thanks for the clarification.
>
> >
> > You could also create a larger pool with VIRTIO_GPU_CMD_MEMORY_CREATE,
> > then go attach multiple resources to it.
> >
> > > If so, do we want the option for the guest allocate?
> >
> > Allocation options are handled by VIRTIO_GPU_CMD_MEMORY_CREATE
> > (initially guest allocated only, i.e. what virtio-gpu supports today,
> > the plan is to add other allocation types later on).
>
> You want to cover Vulkan, host-allocated dma-bufs, and guest-allocated
> dma-bufs with this, correct? Let me know if it's a non-goal :-)
Yes, even though it is not clear yet how we are going to handle
host-allocated buffers in the vhost-user case ...
This might be another dumb question, but is this only an issue for vhost-user(-gpu) case? What mechanisms are used to map host dma-buf into the guest address space?
> If so, we might want to distinguish between memory types (kind of like
> memoryTypeIndex in Vulkan). [Assuming memory_id is like resource_id]
For the host-allocated buffers we surely want that, yes.
For guest-allocated memory regions it isn't useful I think ...
Guest-allocated memory regions can be just another memory type.
But one needs to create the resource first to know which memory types can be attached to it. I think the metadata needs to be returned with resource_create_v2.
> 1) Vulkan seems the most straightforward
>
> virtio_gpu_cmd_memory_create --> create kernel data structure,
> vkAllocateMemory on the host or import guest memory into Vulkan,
> depending on the memory type
> virtio_gpu_cmd_resource_create_v2 --> vkCreateImage +
> vkGetImageMemoryRequirements on host
> virtio_gpu_cmd_resource_attach_memory --> vkBindImageMemory on host
Yes.
Note 1: virtio_gpu_cmd_memory_create + virtio_gpu_cmd_resource_create_v2
ordering doesn't matter, so you can virtio_gpu_cmd_resource_create_v2
first to figure stride and size, then adjust memory size accordingly.
Note 2: The old virtio_gpu_cmd_resource_create variants can be used
too if you don't need the _v2 features.
Note 3: If I understand things correctly it would be valid to create a
memory pool (allocate one big chunk of memory) with vkAllocateMemory,
then bind multiple images at different offsets to it.
> 2) With a guest allocated dma-buf using some new allocation library,
>
> virtio_gpu_cmd_resource_create_v2 --> host returns metadata describing
> optimal allocation
> virtio_gpu_cmd_memory_create --> allocate guest memory pages since
> it's guest memory type
> virtio_gpu_cmd_resource_attach_memory --> associate guest pages with
> resource in kernel, send iovecs to host for bookkeeping
virtio_gpu_cmd_memory_create sends the iovecs. Otherwise correct.
> 3) With gbm it's a little trickier,
>
> virtio_gpu_cmd_resource_create_v2 --> gbm_bo_create_with_modifiers,
> get metadata in return
Only get metadata in return.
> virtio_gpu_cmd_memory_create --> create kernel data structure, but
> don't allocate pages, nothing on the host
Memory allocation happens here. Probably makes sense to have a
virtio_gpu_cmd_memory_create_host command here, because the parameters
we need are quite different from the guest-allocated case.
If we follow Vulkan, we only need the size and the memory type in most cases. The current version of memory_create is a special case because it is an import operation and needs the guest mem_entry. Perhaps memory_create (for host) and memory_import_guest (to replace the current version)?
Maybe we even need a virtio_gpu_cmd_memory_create_host_for_resource
variant, given that gbm doesn't have raw memory buffers without any
format attached to it.
And the memory will only be attachable to the given (or compatible) resource, right?
Vulkan is much more explicit than any pre-existing API. I guess we will have to add this to cover APIs beyond Vulkan.
> > > > +/* VIRTIO_GPU_RESP_OK_RESOURCE_INFO */
> > > > +struct virtio_gpu_resp_resource_info {
> > > > + struct virtio_gpu_ctrl_hdr hdr;
> > > > + __le32 stride[4];
> > > > + __le32 size[4];
> > > > +};
> > >
> > > offsets[4] needed too
> >
> > That is in VIRTIO_GPU_CMD_RESOURCE_MEMORY_ATTACH ...
>
> I assume the offsets aren't returned by
> VIRTIO_GPU_CMD_RESOURCE_MEMORY_ATTACH.
Yes, they are send by the guest.
Gurchetan probably means alignment[4].
> How does the guest know at which offsets in memory will be compatible
> to share with display, camera, etc?
Is is good enough to align offsets to page boundaries?
That should be good enough. But by returning alignments, we can minimize the gaps when attaching multiple resources, especially when the resources are only used by GPU.
> Also, do you want to cover the case where the resource is backed by
> three separate memory regions (VK_IMAGE_CREATE_DISJOINT_BIT)?
Good point. I guess we should make memory_id in
virtio_gpu_cmd_resource_attach_memory an array then,
so you can specify a different memory region for each plane.
cheers,
Gerd
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
