Re: [virtio-dev] [PATCH 2/2] virtio-gpu/2d: add docs/specs/virtio-gpu.txt
On Sa, 2014-09-13 at 07:14 +1000, Dave Airlie wrote: Can the host refuse due to lack of resources? Yes. virtgpu_ctrl_hdr.type in the response will be set to VIRTGPU_RESP_ERR_* then. Current implementation does that only on malloc() failure, there is no accounting (yet) to limit the amout of memory the guest is allowed to allocate. We do probably need to work out some sort of accounting system, it can probably reliably only be a total value of resources, since we've no idea if the host driver will store them in VRAM or main memory. Quite how to fail gracefully is a question, probably need to report to the guest what context did the allocation and see if we can destroy it. Best would be if virgilrenderer.so just fails virgl_renderer_resource_create() calls. Not reason I can remember, I think I was thinking of having separate inval and detach at one point, but it didn't really make any sense, so renaming to detach is fine with me. Done. cheers, Gerd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [virtio-dev] [PATCH 2/2] virtio-gpu/2d: add docs/specs/virtio-gpu.txt
On Thu, Sep 11, 2014 at 05:09:33PM +0200, Gerd Hoffmann wrote: diff --git a/docs/specs/virtio-gpu.txt b/docs/specs/virtio-gpu.txt new file mode 100644 index 000..9455383 --- /dev/null +++ b/docs/specs/virtio-gpu.txt @@ -0,0 +1,165 @@ +virtio-gpu specification + This document refers to the implementation for structs and does not fully document the semantics of the virtqueue commands. Mixing the implementation and specification is risky since implementation changes cannot be checked against the specification. In order to make this document self-contained you need to define the struct layouts. Error conditions and corner cases are not documented for the virtqueue commands. I've asked about a few of them below, but there more are required to make this specification complete enough so someone else could write a compatible implementation. +drive virtio-gpu in 2D mode +--- + +The virtio-gpu is based around the concept of resources private to the +host, the guest must DMA transfer into these resources. This is a +design requirement in order to interface with future 3D rendering. In +the unaccelerated there is no support for DMA transfers from the unaccelerated case? +VIRTGPU_CMD_RESOURCE_CREATE_2D: + Command: struct virtgpu_resource_create_2d + + Create a 2D resource on the host. + + This creates a 2D resource on the host with the specified width, + height and format. Only a small subset of formats are support. The + resource ids are generated by the guest. Can the host refuse due to lack of resources? +VIRTGPU_CMD_SET_SCANOUT: + Command: struct virtgpu_set_scanout + + Set the scanout parameters for a single output. + + This sets the scanout parameters for a single scanout. The + resource_id is the resource to be scanned out from, along with a + rectangle specified by x, y, width and height. What if x, y, width, and height are out-of-range for the given resource? What if width and height exceed the scanout width and height? Is it possible to unset the scanout for a resource? Can a resource be set on multiple scanouts? Does VIRTGPU_CMD_SET_SCANOUT need to be called between every VIRTGPU_CMD_RESOURCE_FLUSH or is does the assignment persist? +VIRTGPU_CMD_RESOURCE_ATTACH_BACKING: + Command: struct virtgpu_resource_attach_backing + + Assign backing pages to a resource. + + This assign an array of guest pages (struct virtgpu_mem_entry) as assigns + the backing store for a resource. These pages are then used for the + transfer operations for that resource from that point on. + +VIRTGPU_CMD_RESOURCE_INVAL_BACKING: + Command: struct virtgpu_resource_inval_backing Why is it called INVAL_BACKING instead of DETACH_BACKING? Detach is logical since there is also an attach command. pgpPouB7th7TW.pgp Description: PGP signature ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [virtio-dev] [PATCH 2/2] virtio-gpu/2d: add docs/specs/virtio-gpu.txt
On Fr, 2014-09-12 at 10:10 +0100, Stefan Hajnoczi wrote: On Thu, Sep 11, 2014 at 05:09:33PM +0200, Gerd Hoffmann wrote: diff --git a/docs/specs/virtio-gpu.txt b/docs/specs/virtio-gpu.txt new file mode 100644 index 000..9455383 --- /dev/null +++ b/docs/specs/virtio-gpu.txt @@ -0,0 +1,165 @@ +virtio-gpu specification + This document refers to the implementation for structs and does not fully document the semantics of the virtqueue commands. Mixing the implementation and specification is risky since implementation changes cannot be checked against the specification. In order to make this document self-contained you need to define the struct layouts. Error conditions and corner cases are not documented for the virtqueue commands. I've asked about a few of them below, but there more are required to make this specification complete enough so someone else could write a compatible implementation. Ok. The short-term goal for this text is to help reviewing the code by documenting how the device is supposed to work. Being good enough for an independent implementation is the next level. I'll keep it on the radar though. +VIRTGPU_CMD_RESOURCE_CREATE_2D: + Command: struct virtgpu_resource_create_2d + + Create a 2D resource on the host. + + This creates a 2D resource on the host with the specified width, + height and format. Only a small subset of formats are support. The + resource ids are generated by the guest. Can the host refuse due to lack of resources? Yes. virtgpu_ctrl_hdr.type in the response will be set to VIRTGPU_RESP_ERR_* then. Current implementation does that only on malloc() failure, there is no accounting (yet) to limit the amout of memory the guest is allowed to allocate. /me notes to write a section on error handling. +VIRTGPU_CMD_SET_SCANOUT: + Command: struct virtgpu_set_scanout + + Set the scanout parameters for a single output. + + This sets the scanout parameters for a single scanout. The + resource_id is the resource to be scanned out from, along with a + rectangle specified by x, y, width and height. What if x, y, width, and height are out-of-range for the given resource? You'll get an error. What if width and height exceed the scanout width and height? Using only a subrectangle of the resource for the scanout is legal. Is it possible to unset the scanout for a resource? Yes, use resource_id 0. Can a resource be set on multiple scanouts? Yes. Does VIRTGPU_CMD_SET_SCANOUT need to be called between every VIRTGPU_CMD_RESOURCE_FLUSH or is does the assignment persist? Assignment is persistent. + the backing store for a resource. These pages are then used for the + transfer operations for that resource from that point on. + +VIRTGPU_CMD_RESOURCE_INVAL_BACKING: + Command: struct virtgpu_resource_inval_backing Why is it called INVAL_BACKING instead of DETACH_BACKING? Detach is logical since there is also an attach command. No particular reason I think. Dave? cheers, Gerd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [virtio-dev] [PATCH 2/2] virtio-gpu/2d: add docs/specs/virtio-gpu.txt
Can the host refuse due to lack of resources? Yes. virtgpu_ctrl_hdr.type in the response will be set to VIRTGPU_RESP_ERR_* then. Current implementation does that only on malloc() failure, there is no accounting (yet) to limit the amout of memory the guest is allowed to allocate. We do probably need to work out some sort of accounting system, it can probably reliably only be a total value of resources, since we've no idea if the host driver will store them in VRAM or main memory. Quite how to fail gracefully is a question, probably need to report to the guest what context did the allocation and see if we can destroy it. + +VIRTGPU_CMD_RESOURCE_INVAL_BACKING: + Command: struct virtgpu_resource_inval_backing Why is it called INVAL_BACKING instead of DETACH_BACKING? Detach is logical since there is also an attach command. No particular reason I think. Dave? Not reason I can remember, I think I was thinking of having separate inval and detach at one point, but it didn't really make any sense, so renaming to detach is fine with me. Dave. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
