| Message ID | 20210406203625.1727955-1-vivek.kasireddy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/virtio: Create Dumb BOs as guest Blobs (v2) | expand |
On Tue, Apr 6, 2021 at 1:47 PM Vivek Kasireddy <vivek.kasireddy@intel.com> wrote: > If support for Blob resources is available, then dumb BOs created > by the driver can be considered as guest Blobs. > > v2: Don't skip transfer and flush commands as part of plane update > as the device may have created a shared mapping. (Gerd) > > Cc: Gerd Hoffmann <kraxel@redhat.com> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> > --- > drivers/gpu/drm/virtio/virtgpu_gem.c | 8 ++++++++ > drivers/gpu/drm/virtio/virtgpu_object.c | 3 +++ > 2 files changed, 11 insertions(+) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c > b/drivers/gpu/drm/virtio/virtgpu_gem.c > index 8502400b2f9c..5f49fb1cce65 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_gem.c > +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c > @@ -64,6 +64,7 @@ int virtio_gpu_mode_dumb_create(struct drm_file > *file_priv, > { > struct drm_gem_object *gobj; > struct virtio_gpu_object_params params = { 0 }; > + struct virtio_gpu_device *vgdev = dev->dev_private; > int ret; > uint32_t pitch; > > @@ -79,6 +80,13 @@ int virtio_gpu_mode_dumb_create(struct drm_file > *file_priv, > params.height = args->height; > params.size = args->size; > params.dumb = true; > + > + if (vgdev->has_resource_blob) { > + params.blob_mem = VIRTGPU_BLOB_MEM_GUEST; > + params.blob_flags = VIRTGPU_BLOB_FLAG_USE_SHAREABLE; > This creates some log spam with crosvm + virgl_3d + vanilla linux, since transfers don't work for guest blobs. Two options: a) Add vgdev->has_virgl_3d check and don't create a guest blob in that case. b) The interactions between TRANSFER_TO_HOST_2D and VIRTGPU_BLOB_MEM_GUEST are a bit under-defined in the spec. Though since you don't have a host side resource, you can safely skip the transfer and crosvm can be modified to do the right thing in case of RESOURCE_FLUSH. It makes a ton of sense to have a explicit udmabuf-like flag ("BLOB_FLAG_CREATE_GUEST_HANDLE" or "BLOB_FLAG_HANDLE_FROM_GUEST" -- want to host OS agnostic -- any other ideas?), especially with 3d mode. For now, implicit udmabuf + dumb should be fine since the QEMU patches have been floating around for a while and should land soon for future use cases. > + params.blob = true; > + } > > + > ret = virtio_gpu_gem_create(file_priv, dev, ¶ms, &gobj, > &args->handle); > if (ret) > diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c > b/drivers/gpu/drm/virtio/virtgpu_object.c > index 4ff1ec28e630..f648b0e24447 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_object.c > +++ b/drivers/gpu/drm/virtio/virtgpu_object.c > @@ -254,6 +254,9 @@ int virtio_gpu_object_create(struct virtio_gpu_device > *vgdev, > } > > if (params->blob) { > + if (params->blob_mem == VIRTGPU_BLOB_MEM_GUEST) > + bo->guest_blob = true; > + > virtio_gpu_cmd_resource_create_blob(vgdev, bo, params, > ents, nents); > } else if (params->virgl) { > -- > 2.26.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
> > + > > + if (vgdev->has_resource_blob) { > > + params.blob_mem = VIRTGPU_BLOB_MEM_GUEST; > > + params.blob_flags = VIRTGPU_BLOB_FLAG_USE_SHAREABLE; > > > > This creates some log spam with crosvm + virgl_3d + vanilla linux, since > transfers don't work for guest blobs. Two options: > > a) Add vgdev->has_virgl_3d check and don't create a guest blob in that case. > b) The interactions between TRANSFER_TO_HOST_2D and VIRTGPU_BLOB_MEM_GUEST > are a bit under-defined in the spec. Indeed. > Though since you don't have a host > side resource, you can safely skip the transfer and crosvm can be modified > to do the right thing in case of RESOURCE_FLUSH. IIRC the VIRTGPU_BLOB_FLAG_USE_SHAREABLE flag means that the host *can* create a shared mapping (i.e. the host seeing guest-side changes without explicit transfer doesn't cause problems for the guest). It doesn not mean the host *must* create a shared mapping (note that there is no negotiation whenever the host supports shared mappings or not). So the transfer calls are still needed, and the host can decide to shortcut them in case it can create a shared mapping. In case there is no shared mapping (say due to missing udmabuf support) the host can fallback to copying. So I think crosvm should be fixed to not consider transfer commands for VIRTGPU_BLOB_MEM_GUEST resources an error. > It makes a ton of sense to have a explicit udmabuf-like flag > ("BLOB_FLAG_CREATE_GUEST_HANDLE" or "BLOB_FLAG_HANDLE_FROM_GUEST" -- want > to host OS agnostic -- any other ideas?), especially with 3d mode. Why? Can't this be simply an host implementation detail which the guest doesn't need to worry about? take care, Gerd
On Thu, Apr 8, 2021 at 2:27 AM Gerd Hoffmann <kraxel@redhat.com> wrote: > > > + > > > + if (vgdev->has_resource_blob) { > > > + params.blob_mem = VIRTGPU_BLOB_MEM_GUEST; > > > + params.blob_flags = VIRTGPU_BLOB_FLAG_USE_SHAREABLE; > > > > > > > This creates some log spam with crosvm + virgl_3d + vanilla linux, since > > transfers don't work for guest blobs. Two options: > > > > a) Add vgdev->has_virgl_3d check and don't create a guest blob in that > case. > > b) The interactions between TRANSFER_TO_HOST_2D and > VIRTGPU_BLOB_MEM_GUEST > > are a bit under-defined in the spec. > > Indeed. > > > Though since you don't have a host > > side resource, you can safely skip the transfer and crosvm can be > modified > > to do the right thing in case of RESOURCE_FLUSH. > > IIRC the VIRTGPU_BLOB_FLAG_USE_SHAREABLE flag means that the host *can* > create a shared mapping (i.e. the host seeing guest-side changes without > explicit transfer doesn't cause problems for the guest). It doesn not > mean the host *must* create a shared mapping (note that there is no > negotiation whenever the host supports shared mappings or not). > VIRTGPU_BLOB_FLAG_USE_SHAREABLE means guest userspace intends to share the blob resource with another virtgpu driver instance via drmPrimeHandleToFd. It's a rough analogue to VkExportMemoryAllocateInfoKHR or PIPE_BIND_USE_SHARED. The dumb case is a bit interesting because there is no userspace to provide that information. Though I think even VIRTGPU_BLOB_FLAG_USE_MAPPABLE is fine, since for my vanilla Linux setup, I'm seeing the guest blob is mapped only and drmPrimeHandleToFd(..) isn't called on it. We can also modify the virtio-gpu spec to say "blob_flags may be undefined/zero for BLOB_MEM_GUEST when 3D mode is not on". Though all options work for me. The implicit dumb blob udmabuf case for me is more about advancing blob development rather than being super rigorous. > > So the transfer calls are still needed, and the host can decide to > shortcut them in case it can create a shared mapping. In case there is > no shared mapping (say due to missing udmabuf support) the host can > fallback to copying. > Transfers are a bit under-defined for BLOB_MEM_GUEST. Even without udmabuf on the host, there is no host side resource for guest-only blobs? Before blob resources, the dumb flow was: 1) update guest side resource 2) TRANSFER_TO_HOST_2D to copy guest side contents to host side private resource [Pixman??] 3) RESOURCE_FLUSH to copy the host-side contents to the framebuffer and page-flip At least for crosvm, this is possible: 1) update guest side resource 2) RESOURCE_FLUSH to copy the guest-side contents to the framebuffer and pageflip With implicit udmabuf, it may be possible to do this: 1) update guest side resource 2) RESOURCE_FLUSH to page-flip So I think crosvm should be fixed to not consider transfer commands for > VIRTGPU_BLOB_MEM_GUEST resources an error. > It's a simple change to make and we can definitely do it, if TRANSFER_2D is helpful for the QEMU case. I haven't looked at the QEMU side patches. > > It makes a ton of sense to have a explicit udmabuf-like flag > > ("BLOB_FLAG_CREATE_GUEST_HANDLE" or "BLOB_FLAG_HANDLE_FROM_GUEST" -- want > > to host OS agnostic -- any other ideas?), especially with 3d mode. > > Why? Can't this be simply an host implementation detail which the guest > doesn't need to worry about? > For 3D mode, it's desirable to create an {EGL image}/{VkDeviceMemory} from guest memory for certain zero-copy use cases. If no explicit guarantee exists for the paravirtualized user-space that there will be a host side OS-specific handle associated with guest memory, then guest user space must fall-back to old-style transfers. For the PCI-passthrough + guest blob case, the end goal is to share it with the host compositor. If there is no guarantee the guest memory can be converted to an OS-handle (to share with the host compositor), then I think the guest user space should fallback to another technique involving memcpy() to share the memory. So essentially, thinking for two new protocol additions: F_CREATE_GUEST_HANDLE (or F_HANDLE_FROM_GUEST) --> means an OS-specific udmabuf-like mechanism exists on the host. BLOB_FLAG_CREATE_GUEST_HANDLE (or BLOB_FLAG_HANDLE_FROM_GUEST)--> tells host userspace "you must create a udmabuf" [or OS-specific equivalent] upon success Though much testing/work remains (both with the PCI passthough case + virgl3d case), could be a good chance to float the nomenclature by everyone. Happy to collaborate further with Tina/Vivek on making such a thing happen. > > take care, > Gerd > >
Hi, > > IIRC the VIRTGPU_BLOB_FLAG_USE_SHAREABLE flag means that the host *can* > > create a shared mapping (i.e. the host seeing guest-side changes without > > explicit transfer doesn't cause problems for the guest). It doesn not > > mean the host *must* create a shared mapping (note that there is no > > negotiation whenever the host supports shared mappings or not). > > > > VIRTGPU_BLOB_FLAG_USE_SHAREABLE means guest userspace intends to share the > blob resource with another virtgpu driver instance via drmPrimeHandleToFd. > It's a rough analogue to VkExportMemoryAllocateInfoKHR or > PIPE_BIND_USE_SHARED. Oh. My memory was failing me then. We should *really* clarify the spec for BLOB_MEM_GUEST. So shared mappings are allowed for all BLOB_MEM_GUEST resources, right? > > So the transfer calls are still needed, and the host can decide to > > shortcut them in case it can create a shared mapping. In case there is > > no shared mapping (say due to missing udmabuf support) the host can > > fallback to copying. > > Transfers are a bit under-defined for BLOB_MEM_GUEST. Even without udmabuf > on the host, there is no host side resource for guest-only blobs? Before > blob resources, the dumb flow was: > > 1) update guest side resource > 2) TRANSFER_TO_HOST_2D to copy guest side contents to host side private > resource [Pixman??] > 3) RESOURCE_FLUSH to copy the host-side contents to the framebuffer and > page-flip Yes. > At least for crosvm, this is possible: > > 1) update guest side resource > 2) RESOURCE_FLUSH to copy the guest-side contents to the framebuffer and > pageflip > > With implicit udmabuf, it may be possible to do this: > > 1) update guest side resource > 2) RESOURCE_FLUSH to page-flip > > > So I think crosvm should be fixed to not consider transfer commands for > > VIRTGPU_BLOB_MEM_GUEST resources an error. > > It's a simple change to make and we can definitely do it, if TRANSFER_2D is > helpful for the QEMU case. I haven't looked at the QEMU side patches. Well, we have two different cases: (1) No udmabuf available. qemu will have a host-side shadow then and the workflow will be largely identical to the non-blob resource workflow. (2) With udmabuf support. qemu can create udmabufs for the resources, mmap() the dmabuf to get a linear mapping, create a pixman buffer backed by that dmabuf (no copying needed then). Depending on capabilities pass either the pixman image (gl=off) or the dmabuf handle (gl=on) to the UI code to actually show the guest display. The guest doesn't need to know any of this, it'll just send transfer and flush commands. In case (1) qemu must process the transfer commands and for case (2) qemu can simply ignore them. > For the PCI-passthrough + guest blob case, the end goal is to share it with > the host compositor. If there is no guarantee the guest memory can be > converted to an OS-handle (to share with the host compositor), then I think > the guest user space should fallback to another technique involving > memcpy() to share the memory. This is what happens today (using non-blob resources). > So essentially, thinking for two new protocol additions: > > F_CREATE_GUEST_HANDLE (or F_HANDLE_FROM_GUEST) --> means an OS-specific > udmabuf-like mechanism exists on the host. > > BLOB_FLAG_CREATE_GUEST_HANDLE (or BLOB_FLAG_HANDLE_FROM_GUEST)--> tells > host userspace "you must create a udmabuf" [or OS-specific equivalent] upon > success Again: Why do we actually need that? Is there any benefit other than the guest knowing it doesn't need to send transfer commands? I see the whole udmabuf thing as a host-side performance optimization and I think this should be fully transparent to the guest as the host can easily just ignore the transfer commands. Given we batch commands the extra commands don't lead to extra context switches, so there shouldn't be much overhead. If we really want make the guest aware of the hosts udmabuf state I think this should be designed the other way around: Add some way for the host to tell the guest transfer commands are not needed for a specific BLOB_MEM_GUEST resource. take care, Gerd
On Fri, Apr 9, 2021 at 12:48 AM Gerd Hoffmann <kraxel@redhat.com> wrote: > Hi, > > > > IIRC the VIRTGPU_BLOB_FLAG_USE_SHAREABLE flag means that the host *can* > > > create a shared mapping (i.e. the host seeing guest-side changes > without > > > explicit transfer doesn't cause problems for the guest). It doesn not > > > mean the host *must* create a shared mapping (note that there is no > > > negotiation whenever the host supports shared mappings or not). > > > > > > > VIRTGPU_BLOB_FLAG_USE_SHAREABLE means guest userspace intends to share > the > > blob resource with another virtgpu driver instance via > drmPrimeHandleToFd. > > It's a rough analogue to VkExportMemoryAllocateInfoKHR or > > PIPE_BIND_USE_SHARED. > > Oh. My memory was failing me then. We should *really* clarify the spec > for BLOB_MEM_GUEST. > So shared mappings are allowed for all BLOB_MEM_GUEST resources, right? > The guest iovecs are always shared with the host, so they may be copied to/from directly depending on the operation. In the case of RESOURCE_FLUSH + BLOB_MEM_GUEST, it could be a copy from the guest iovecs to the host framebuffer [host framebuffer != host shadow memory]. > > > > So the transfer calls are still needed, and the host can decide to > > > shortcut them in case it can create a shared mapping. In case there is > > > no shared mapping (say due to missing udmabuf support) the host can > > > fallback to copying. > > > > Transfers are a bit under-defined for BLOB_MEM_GUEST. Even without > udmabuf > > on the host, there is no host side resource for guest-only blobs? Before > > blob resources, the dumb flow was: > > > > 1) update guest side resource > > 2) TRANSFER_TO_HOST_2D to copy guest side contents to host side private > > resource [Pixman??] > > 3) RESOURCE_FLUSH to copy the host-side contents to the framebuffer and > > page-flip > > Yes. > > > At least for crosvm, this is possible: > > > > 1) update guest side resource > > 2) RESOURCE_FLUSH to copy the guest-side contents to the framebuffer and > > pageflip > > > > With implicit udmabuf, it may be possible to do this: > > > > 1) update guest side resource > > 2) RESOURCE_FLUSH to page-flip > > > > > So I think crosvm should be fixed to not consider transfer commands for > > > VIRTGPU_BLOB_MEM_GUEST resources an error. > > > > It's a simple change to make and we can definitely do it, if TRANSFER_2D > is > > helpful for the QEMU case. I haven't looked at the QEMU side patches. > > Well, we have two different cases: > > (1) No udmabuf available. qemu will have a host-side shadow then and > the workflow will be largely identical to the non-blob resource > workflow. > I think this is the key difference. With BLOB_MEM_GUEST, crosvm can only have a guest side iovecs and no host-side shadow memory. With BLOB_MEM_GUEST_HOST3D, host-side shadow memory will exist. I guess it boils down the Pixman dependency. crosvm sits right on top of display APIs (X, wayland) rather than having intermediary layers. Adding a new Pixman API takes time too. There's a bunch of options: 1) Don't use BLOB_MEM_GUEST dumb buffers in 3D mode. 2) virglrenderer or crosvm modified to implicitly ignore TRANSFER_TO_HOST_2D for BLOB_MEM_GUEST when in 3D mode. 3) It's probably possible to create an implicit udmabuf for RESOURCE_CREATE_2D resources and ignore the transfer there too. The benefit of this is TRANSFER_TO_HOST_2D makes a ton of sense for non-blob resources. No kernel side change needed here, just QEMU. 4) modify QEMU display integration I would choose (1) since it solves the log spam problem and it advances blob support in QEMU. Though I leave the decision to QEMU devs. > > (2) With udmabuf support. qemu can create udmabufs for the resources, > mmap() the dmabuf to get a linear mapping, create a pixman buffer > backed by that dmabuf (no copying needed then). Depending on > capabilities pass either the pixman image (gl=off) or the dmabuf > handle (gl=on) to the UI code to actually show the guest display. > > The guest doesn't need to know any of this, it'll just send transfer and > flush commands. In case (1) qemu must process the transfer commands and > for case (2) qemu can simply ignore them. > > > For the PCI-passthrough + guest blob case, the end goal is to share it > with > > the host compositor. If there is no guarantee the guest memory can be > > converted to an OS-handle (to share with the host compositor), then I > think > > the guest user space should fallback to another technique involving > > memcpy() to share the memory. > > This is what happens today (using non-blob resources). > > > So essentially, thinking for two new protocol additions: > > > > F_CREATE_GUEST_HANDLE (or F_HANDLE_FROM_GUEST) --> means an OS-specific > > udmabuf-like mechanism exists on the host. > > > > BLOB_FLAG_CREATE_GUEST_HANDLE (or BLOB_FLAG_HANDLE_FROM_GUEST)--> tells > > host userspace "you must create a udmabuf" [or OS-specific equivalent] > upon > > success > > Again: Why do we actually need that? Is there any benefit other than > the guest knowing it doesn't need to send transfer commands? I see the whole udmabuf thing as a host-side performance optimization > and I think this should be fully transparent to the guest as the host > can easily just ignore the transfer commands. So the use case I'm most interested in (and Vivek/Tina?) is tiled/compressed udmabufs, so they may be eventually shared with the host compositor via the DRM modifier API. Transfers to linear udmabufs make sense. Maybe transfers to tiled/compressed udmabufs shouldn't even be attempted. It's a complicated case with many ambiguities, especially with PCI passthrough involved. Explicit tiled/compressed udmabufs are just an idea, will have to think more about it / have some proof of concept [with virgl and PCI passthrough], before making any concrete proposals. Will keep your idea of just ignoring transfers on the host in mind. > Given we batch commands > the extra commands don't lead to extra context switches, so there > shouldn't be much overhead. > > If we really want make the guest aware of the hosts udmabuf state I > think this should be designed the other way around: Add some way for > the host to tell the guest transfer commands are not needed for a > specific BLOB_MEM_GUEST resource. > > take care, > Gerd > >
> -----Original Message----- > From: Gurchetan Singh <gurchetansingh@chromium.org> > Sent: Tuesday, April 13, 2021 8:58 AM > To: Gerd Hoffmann <kraxel@redhat.com> > Cc: Kasireddy, Vivek <vivek.kasireddy@intel.com>; ML dri-devel <dri- > devel@lists.freedesktop.org>; Zhang, Tina <tina.zhang@intel.com> > Subject: Re: [PATCH] drm/virtio: Create Dumb BOs as guest Blobs (v2) > > > > On Fri, Apr 9, 2021 at 12:48 AM Gerd Hoffmann <mailto:kraxel@redhat.com> > wrote: > Hi, > > > > IIRC the VIRTGPU_BLOB_FLAG_USE_SHAREABLE flag means that the host > *can* > > > create a shared mapping (i.e. the host seeing guest-side changes without > > > explicit transfer doesn't cause problems for the guest). It doesn not > > > mean the host *must* create a shared mapping (note that there is no > > > negotiation whenever the host supports shared mappings or not). > > > > > > > VIRTGPU_BLOB_FLAG_USE_SHAREABLE means guest userspace intends to > share the > > blob resource with another virtgpu driver instance via > drmPrimeHandleToFd. > > It's a rough analogue to VkExportMemoryAllocateInfoKHR or > > PIPE_BIND_USE_SHARED. > > Oh. My memory was failing me then. We should *really* clarify the spec > for BLOB_MEM_GUEST. > > So shared mappings are allowed for all BLOB_MEM_GUEST resources, right? > > The guest iovecs are always shared with the host, so they may be copied > to/from directly depending on the operation. In the case of > RESOURCE_FLUSH + BLOB_MEM_GUEST, it could be a copy from the guest > iovecs to the host framebuffer [host framebuffer != host shadow memory]. > > > > > So the transfer calls are still needed, and the host can decide to > > > shortcut them in case it can create a shared mapping. In case there is > > > no shared mapping (say due to missing udmabuf support) the host can > > > fallback to copying. > > > > Transfers are a bit under-defined for BLOB_MEM_GUEST. Even without > udmabuf > > on the host, there is no host side resource for guest-only blobs? Before > > blob resources, the dumb flow was: > > > > 1) update guest side resource > > 2) TRANSFER_TO_HOST_2D to copy guest side contents to host side private > > resource [Pixman??] > > 3) RESOURCE_FLUSH to copy the host-side contents to the framebuffer and > > page-flip > > Yes. > > > At least for crosvm, this is possible: > > > > 1) update guest side resource > > 2) RESOURCE_FLUSH to copy the guest-side contents to the framebuffer and > > pageflip > > > > With implicit udmabuf, it may be possible to do this: > > > > 1) update guest side resource > > 2) RESOURCE_FLUSH to page-flip > > > > > So I think crosvm should be fixed to not consider transfer commands for > > > VIRTGPU_BLOB_MEM_GUEST resources an error. > > > > It's a simple change to make and we can definitely do it, if TRANSFER_2D is > > helpful for the QEMU case. I haven't looked at the QEMU side patches. > > Well, we have two different cases: > > (1) No udmabuf available. qemu will have a host-side shadow then and > the workflow will be largely identical to the non-blob resource > workflow. > > I think this is the key difference. With BLOB_MEM_GUEST, crosvm can only > have a guest side iovecs and no host-side shadow memory. With > BLOB_MEM_GUEST_HOST3D, host-side shadow memory will exist. > > I guess it boils down the Pixman dependency. crosvm sits right on top of > display APIs (X, wayland) rather than having intermediary layers. Adding a > new Pixman API takes time too. > > There's a bunch of options: > > 1) Don't use BLOB_MEM_GUEST dumb buffers in 3D mode. > 2) virglrenderer or crosvm modified to implicitly ignore > TRANSFER_TO_HOST_2D for BLOB_MEM_GUEST when in 3D mode. > 3) It's probably possible to create an implicit udmabuf > for RESOURCE_CREATE_2D resources and ignore the transfer there too. The > benefit of this is TRANSFER_TO_HOST_2D makes a ton of sense for non-blob > resources. No kernel side change needed here, just QEMU. > 4) modify QEMU display integration > > I would choose (1) since it solves the log spam problem and it advances blob > support in QEMU. Though I leave the decision to QEMU devs. > > > (2) With udmabuf support. qemu can create udmabufs for the resources, > mmap() the dmabuf to get a linear mapping, create a pixman buffer > backed by that dmabuf (no copying needed then). Depending on > capabilities pass either the pixman image (gl=off) or the dmabuf > handle (gl=on) to the UI code to actually show the guest display. > > The guest doesn't need to know any of this, it'll just send transfer and > flush commands. In case (1) qemu must process the transfer commands and > for case (2) qemu can simply ignore them. > > > For the PCI-passthrough + guest blob case, the end goal is to share it with > > the host compositor. If there is no guarantee the guest memory can be > > converted to an OS-handle (to share with the host compositor), then I think > > the guest user space should fallback to another technique involving > > memcpy() to share the memory. > > This is what happens today (using non-blob resources). > > > So essentially, thinking for two new protocol additions: > > > > F_CREATE_GUEST_HANDLE (or F_HANDLE_FROM_GUEST) --> means an OS- > specific > > udmabuf-like mechanism exists on the host. > > > > BLOB_FLAG_CREATE_GUEST_HANDLE (or > BLOB_FLAG_HANDLE_FROM_GUEST)--> tells > > host userspace "you must create a udmabuf" [or OS-specific equivalent] > upon > > success > > Again: Why do we actually need that? Is there any benefit other than > the guest knowing it doesn't need to send transfer commands? > I see the whole udmabuf thing as a host-side performance optimization > and I think this should be fully transparent to the guest as the host > can easily just ignore the transfer commands. > > So the use case I'm most interested in (and Vivek/Tina?) is tiled/compressed > udmabufs, so they may be eventually shared with the host compositor via the > DRM modifier API. Hi, Yes, eventually, we would like to have the tiled/compressed framebuffers shared by the udmabufs mechanism. BR, Tina > > Transfers to linear udmabufs make sense. Maybe transfers to > tiled/compressed udmabufs shouldn't even be attempted. > > It's a complicated case with many ambiguities, especially with PCI > passthrough involved. Explicit tiled/compressed udmabufs are just an idea, > will have to think more about it / have some proof of concept [with virgl and > PCI passthrough], before making any concrete proposals. Will keep your idea > of just ignoring transfers on the host in mind. > > Given we batch commands > the extra commands don't lead to extra context switches, so there > shouldn't be much overhead. > > If we really want make the guest aware of the hosts udmabuf state I > think this should be designed the other way around: Add some way for > the host to tell the guest transfer commands are not needed for a > specific BLOB_MEM_GUEST resource. > > take care, > Gerd
diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c index 8502400b2f9c..5f49fb1cce65 100644 --- a/drivers/gpu/drm/virtio/virtgpu_gem.c +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c @@ -64,6 +64,7 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv, { struct drm_gem_object *gobj; struct virtio_gpu_object_params params = { 0 }; + struct virtio_gpu_device *vgdev = dev->dev_private; int ret; uint32_t pitch; @@ -79,6 +80,13 @@ int virtio_gpu_mode_dumb_create(struct drm_file *file_priv, params.height = args->height; params.size = args->size; params.dumb = true; + + if (vgdev->has_resource_blob) { + params.blob_mem = VIRTGPU_BLOB_MEM_GUEST; + params.blob_flags = VIRTGPU_BLOB_FLAG_USE_SHAREABLE; + params.blob = true; + } + ret = virtio_gpu_gem_create(file_priv, dev, ¶ms, &gobj, &args->handle); if (ret) diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index 4ff1ec28e630..f648b0e24447 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -254,6 +254,9 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, } if (params->blob) { + if (params->blob_mem == VIRTGPU_BLOB_MEM_GUEST) + bo->guest_blob = true; + virtio_gpu_cmd_resource_create_blob(vgdev, bo, params, ents, nents); } else if (params->virgl) {
If support for Blob resources is available, then dumb BOs created by the driver can be considered as guest Blobs. v2: Don't skip transfer and flush commands as part of plane update as the device may have created a shared mapping. (Gerd) Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com> --- drivers/gpu/drm/virtio/virtgpu_gem.c | 8 ++++++++ drivers/gpu/drm/virtio/virtgpu_object.c | 3 +++ 2 files changed, 11 insertions(+)