* [PATCH 0/1] virtio-gpu: CONTEXT_INIT feature @ 2021-09-27 14:48 Antonio Caggiano 2021-09-27 14:48 ` [PATCH 1/1] " Antonio Caggiano 0 siblings, 1 reply; 5+ messages in thread From: Antonio Caggiano @ 2021-09-27 14:48 UTC (permalink / raw) To: qemu-devel This is a different attempt at upstreaming the work I have been doing to enable support for the Venus Virtio-GPU Vulkan driver. I believe the previous one [0] was a bit too much stuff in one place, therefore with this I would like to try a more fine-grained approach. I will just start by the CONTEXT_INIT feature as it was the first commit of the series aforementioned and the virtio-spec has been updated recently on that regard [1]. Hopefully this would also answer Gerd's comment on the previous patch [2]. [0] https://www.mail-archive.com/qemu-devel@nongnu.org/msg826897.html [1] https://github.com/oasis-tcs/virtio-spec/commit/aad2b6f3620ec0c9d16aaf046db8c282c24cce3e [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg827304.html Antonio Caggiano (1): virtio-gpu: CONTEXT_INIT feature hw/display/virtio-gpu-base.c | 2 ++ hw/display/virtio-gpu-virgl.c | 10 ++++++++-- include/hw/virtio/virtio-gpu-bswap.h | 2 +- include/standard-headers/linux/virtio_gpu.h | 9 +++++++-- 4 files changed, 18 insertions(+), 5 deletions(-) -- 2.30.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] virtio-gpu: CONTEXT_INIT feature 2021-09-27 14:48 [PATCH 0/1] virtio-gpu: CONTEXT_INIT feature Antonio Caggiano @ 2021-09-27 14:48 ` Antonio Caggiano 2021-09-28 5:13 ` Gerd Hoffmann 0 siblings, 1 reply; 5+ messages in thread From: Antonio Caggiano @ 2021-09-27 14:48 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, Michael S. Tsirkin Create virgl renderer context with flags using context_id when valid. Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com> --- hw/display/virtio-gpu-base.c | 2 ++ hw/display/virtio-gpu-virgl.c | 10 ++++++++-- include/hw/virtio/virtio-gpu-bswap.h | 2 +- include/standard-headers/linux/virtio_gpu.h | 9 +++++++-- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c index c8da4806e0..619185a9d2 100644 --- a/hw/display/virtio-gpu-base.c +++ b/hw/display/virtio-gpu-base.c @@ -212,6 +212,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features, features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB); } + features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT); + return features; } diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index 18d054922f..5a184cf445 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -97,8 +97,14 @@ static void virgl_cmd_context_create(VirtIOGPU *g, trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id, cc.debug_name); - virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, - cc.debug_name); + if (cc.context_init) { + virgl_renderer_context_create_with_flags(cc.hdr.ctx_id, + cc.context_init, + cc.nlen, + cc.debug_name); + } else { + virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name); + } } static void virgl_cmd_context_destroy(VirtIOGPU *g, diff --git a/include/hw/virtio/virtio-gpu-bswap.h b/include/hw/virtio/virtio-gpu-bswap.h index e2bee8f595..6267cb57e5 100644 --- a/include/hw/virtio/virtio-gpu-bswap.h +++ b/include/hw/virtio/virtio-gpu-bswap.h @@ -24,7 +24,7 @@ virtio_gpu_ctrl_hdr_bswap(struct virtio_gpu_ctrl_hdr *hdr) le32_to_cpus(&hdr->flags); le64_to_cpus(&hdr->fence_id); le32_to_cpus(&hdr->ctx_id); - le32_to_cpus(&hdr->padding); + le32_to_cpus(&hdr->info); } static inline void diff --git a/include/standard-headers/linux/virtio_gpu.h b/include/standard-headers/linux/virtio_gpu.h index 1357e4774e..c9f9c24d6a 100644 --- a/include/standard-headers/linux/virtio_gpu.h +++ b/include/standard-headers/linux/virtio_gpu.h @@ -59,6 +59,11 @@ * VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB */ #define VIRTIO_GPU_F_RESOURCE_BLOB 3 +/* + * VIRTIO_GPU_CMD_CREATE_CONTEXT with + * context_init + */ +#define VIRTIO_GPU_F_CONTEXT_INIT 4 enum virtio_gpu_ctrl_type { VIRTIO_GPU_UNDEFINED = 0, @@ -129,7 +134,7 @@ struct virtio_gpu_ctrl_hdr { uint32_t flags; uint64_t fence_id; uint32_t ctx_id; - uint32_t padding; + uint32_t info; }; /* data passed in the cursor vq */ @@ -272,7 +277,7 @@ struct virtio_gpu_resource_create_3d { struct virtio_gpu_ctx_create { struct virtio_gpu_ctrl_hdr hdr; uint32_t nlen; - uint32_t padding; + uint32_t context_init; char debug_name[64]; }; -- 2.30.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] virtio-gpu: CONTEXT_INIT feature 2021-09-27 14:48 ` [PATCH 1/1] " Antonio Caggiano @ 2021-09-28 5:13 ` Gerd Hoffmann 2021-09-28 10:16 ` Antonio Caggiano 0 siblings, 1 reply; 5+ messages in thread From: Gerd Hoffmann @ 2021-09-28 5:13 UTC (permalink / raw) To: Antonio Caggiano; +Cc: qemu-devel, Michael S. Tsirkin > @@ -212,6 +212,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features, > features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB); > } > > + features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT); This needs a config option, simliar to the other features. It is a guest-visible change so we must be able to turn it off for live migration compatibility reasons. We also need a compat property to turn it off by default for 6.1 + older machine types. > + if (cc.context_init) { > + virgl_renderer_context_create_with_flags(cc.hdr.ctx_id, > + cc.context_init, > + cc.nlen, > + cc.debug_name); This requires a minimum virglrenderer version I guess? > --- a/include/standard-headers/linux/virtio_gpu.h > +++ b/include/standard-headers/linux/virtio_gpu.h Separate patch please. Also use scripts/update-linux-headers.sh for this. take care, Gerd ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] virtio-gpu: CONTEXT_INIT feature 2021-09-28 5:13 ` Gerd Hoffmann @ 2021-09-28 10:16 ` Antonio Caggiano 2021-09-28 11:56 ` Gerd Hoffmann 0 siblings, 1 reply; 5+ messages in thread From: Antonio Caggiano @ 2021-09-28 10:16 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel, Michael S. Tsirkin On 28/09/21 07:13, Gerd Hoffmann wrote: >> @@ -212,6 +212,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features, >> features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB); >> } >> >> + features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT); > > This needs a config option, simliar to the other features. It is a > guest-visible change so we must be able to turn it off for live > migration compatibility reasons. We also need a compat property to > turn it off by default for 6.1 + older machine types. Could you give me a hint on how to add this compat property? >> + if (cc.context_init) { >> + virgl_renderer_context_create_with_flags(cc.hdr.ctx_id, >> + cc.context_init, >> + cc.nlen, >> + cc.debug_name); > > This requires a minimum virglrenderer version I guess? Definitely, that is going to be >= 0.9.0 >> --- a/include/standard-headers/linux/virtio_gpu.h >> +++ b/include/standard-headers/linux/virtio_gpu.h > > Separate patch please. > Also use scripts/update-linux-headers.sh for this. Well, then I believe we will need to wait for this patch series: https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg367531.html Cheers, Antonio ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] virtio-gpu: CONTEXT_INIT feature 2021-09-28 10:16 ` Antonio Caggiano @ 2021-09-28 11:56 ` Gerd Hoffmann 0 siblings, 0 replies; 5+ messages in thread From: Gerd Hoffmann @ 2021-09-28 11:56 UTC (permalink / raw) To: Antonio Caggiano; +Cc: qemu-devel, Michael S. Tsirkin Hi, > > This needs a config option, simliar to the other features. It is a > > guest-visible change so we must be able to turn it off for live > > migration compatibility reasons. We also need a compat property to > > turn it off by default for 6.1 + older machine types. > > Could you give me a hint on how to add this compat property? No need to do that for now, see below. But "git log" or "git blame" should find the patches doing the same for the other features). > > > + if (cc.context_init) { > > > + virgl_renderer_context_create_with_flags(cc.hdr.ctx_id, > > > + cc.context_init, > > > + cc.nlen, > > > + cc.debug_name); > > > > This requires a minimum virglrenderer version I guess? > > Definitely, that is going to be >= 0.9.0 ... because we can hardly enable that by default if it isn't even released. We'll need #ifdefs so qemu continues to build with older virglrenderer versions for a while. It also must stay disabled by default so you don't get different qemu behavior depending on the version compiled against. Then, in 1-2 years, when distributions have picked up the new version, we can consider to raise the minimal required version to 0.9.0 and flip the default to enabled. > > > --- a/include/standard-headers/linux/virtio_gpu.h > > > +++ b/include/standard-headers/linux/virtio_gpu.h > > > > Separate patch please. > > Also use scripts/update-linux-headers.sh for this. > Well, then I believe we will need to wait for this patch series: > > https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg367531.html Ah, right. Too much stuff on my todo list :( take care, Gerd ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-28 11:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-27 14:48 [PATCH 0/1] virtio-gpu: CONTEXT_INIT feature Antonio Caggiano 2021-09-27 14:48 ` [PATCH 1/1] " Antonio Caggiano 2021-09-28 5:13 ` Gerd Hoffmann 2021-09-28 10:16 ` Antonio Caggiano 2021-09-28 11:56 ` Gerd Hoffmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).