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