* [PATCH v3 1/9] virtio: Add shared memory capability
2022-09-26 14:24 [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver Antonio Caggiano
@ 2022-09-26 14:24 ` Antonio Caggiano
2023-01-30 12:51 ` Alex Bennée
2022-09-26 14:24 ` [PATCH v3 2/9] virtio-gpu: hostmem Antonio Caggiano
` (8 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Antonio Caggiano @ 2022-09-26 14:24 UTC (permalink / raw)
To: qemu-devel
Cc: gert.wollny, dmitry.osipenko, Dr. David Alan Gilbert, Michael S. Tsirkin
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Define a new capability type 'VIRTIO_PCI_CAP_SHARED_MEMORY_CFG' to allow
defining shared memory regions with sizes and offsets of 2^32 and more.
Multiple instances of the capability are allowed and distinguished
by a device-specific 'id'.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
---
v3:
- Remove virtio_pci_shm_cap as virtio_pci_cap64 is used instead.
- No need for mask32 as cpu_to_le32 truncates the value.
hw/virtio/virtio-pci.c | 18 ++++++++++++++++++
include/hw/virtio/virtio-pci.h | 4 ++++
2 files changed, 22 insertions(+)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a50c5a57d7..377bb06fec 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1169,6 +1169,24 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
return offset;
}
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+ uint8_t bar, uint64_t offset, uint64_t length,
+ uint8_t id)
+{
+ struct virtio_pci_cap64 cap = {
+ .cap.cap_len = sizeof cap,
+ .cap.cfg_type = VIRTIO_PCI_CAP_SHARED_MEMORY_CFG,
+ };
+
+ cap.cap.bar = bar;
+ cap.cap.length = cpu_to_le32(length);
+ cap.length_hi = cpu_to_le32(length >> 32);
+ cap.cap.offset = cpu_to_le32(offset);
+ cap.offset_hi = cpu_to_le32(offset >> 32);
+ cap.cap.id = id;
+ return virtio_pci_add_mem_cap(proxy, &cap.cap);
+}
+
static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
unsigned size)
{
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 2446dcd9ae..5e5c4a4c6d 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -252,4 +252,8 @@ void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t);
*/
unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
+int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
+ uint8_t bar, uint64_t offset, uint64_t length,
+ uint8_t id);
+
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v3 1/9] virtio: Add shared memory capability
2022-09-26 14:24 ` [PATCH v3 1/9] virtio: Add shared memory capability Antonio Caggiano
@ 2023-01-30 12:51 ` Alex Bennée
0 siblings, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2023-01-30 12:51 UTC (permalink / raw)
To: Antonio Caggiano
Cc: gert.wollny, dmitry.osipenko, Dr. David Alan Gilbert,
Michael S. Tsirkin, qemu-devel
Antonio Caggiano <antonio.caggiano@collabora.com> writes:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Define a new capability type 'VIRTIO_PCI_CAP_SHARED_MEMORY_CFG' to allow
> defining shared memory regions with sizes and offsets of 2^32 and more.
> Multiple instances of the capability are allowed and distinguished
> by a device-specific 'id'.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
> ---
> v3:
> - Remove virtio_pci_shm_cap as virtio_pci_cap64 is used instead.
> - No need for mask32 as cpu_to_le32 truncates the value.
>
> hw/virtio/virtio-pci.c | 18 ++++++++++++++++++
> include/hw/virtio/virtio-pci.h | 4 ++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index a50c5a57d7..377bb06fec 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1169,6 +1169,24 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
> return offset;
> }
>
> +int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
> + uint8_t bar, uint64_t offset, uint64_t length,
> + uint8_t id)
> +{
> + struct virtio_pci_cap64 cap = {
> + .cap.cap_len = sizeof cap,
> + .cap.cfg_type = VIRTIO_PCI_CAP_SHARED_MEMORY_CFG,
> + };
> +
> + cap.cap.bar = bar;
> + cap.cap.length = cpu_to_le32(length);
> + cap.length_hi = cpu_to_le32(length >> 32);
> + cap.cap.offset = cpu_to_le32(offset);
> + cap.offset_hi = cpu_to_le32(offset >> 32);
> + cap.cap.id = id;
> + return virtio_pci_add_mem_cap(proxy, &cap.cap);
> +}
> +
> static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
> unsigned size)
> {
> diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
> index 2446dcd9ae..5e5c4a4c6d 100644
> --- a/include/hw/virtio/virtio-pci.h
> +++ b/include/hw/virtio/virtio-pci.h
> @@ -252,4 +252,8 @@ void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t);
> */
> unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
As this is adding to the internal API it would benefit from a kerneldoc
style comment block in the header.
> +int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy,
> + uint8_t bar, uint64_t offset, uint64_t length,
> + uint8_t id);
> +
> #endif
Otherwise:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 2/9] virtio-gpu: hostmem
2022-09-26 14:24 [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver Antonio Caggiano
2022-09-26 14:24 ` [PATCH v3 1/9] virtio: Add shared memory capability Antonio Caggiano
@ 2022-09-26 14:24 ` Antonio Caggiano
2023-01-30 16:22 ` Alex Bennée
2022-09-26 14:24 ` [PATCH v3 3/9] virtio-gpu: Handle resource blob commands Antonio Caggiano
` (7 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Antonio Caggiano @ 2022-09-26 14:24 UTC (permalink / raw)
To: qemu-devel
Cc: gert.wollny, dmitry.osipenko, Gerd Hoffmann, Michael S . Tsirkin
From: Gerd Hoffmann <kraxel@redhat.com>
Use VIRTIO_GPU_SHM_ID_HOST_VISIBLE as id for virtio-gpu.
Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
v3: Formatting fixes
hw/display/virtio-gpu-pci.c | 15 +++++++++++++++
hw/display/virtio-gpu.c | 1 +
hw/display/virtio-vga.c | 33 ++++++++++++++++++++++++---------
include/hw/virtio/virtio-gpu.h | 5 +++++
4 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index 93f214ff58..2cbbacd7fe 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -33,6 +33,21 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
DeviceState *vdev = DEVICE(g);
int i;
+ if (virtio_gpu_hostmem_enabled(g->conf)) {
+ vpci_dev->msix_bar_idx = 1;
+ vpci_dev->modern_mem_bar_idx = 2;
+ memory_region_init(&g->hostmem, OBJECT(g), "virtio-gpu-hostmem",
+ g->conf.hostmem);
+ pci_register_bar(&vpci_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ &g->hostmem);
+ virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+ VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+ }
+
+ qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus), errp);
virtio_pci_force_virtio_1(vpci_dev);
if (!qdev_realize(vdev, BUS(&vpci_dev->bus), errp)) {
return;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 20cc703dcc..506b3b8eef 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1424,6 +1424,7 @@ static Property virtio_gpu_properties[] = {
256 * MiB),
DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
+ DEFINE_PROP_SIZE("hostmem", VirtIOGPU, parent_obj.conf.hostmem, 0),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
index 4dcb34c4a7..aa8d1ab993 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -115,17 +115,32 @@ static void virtio_vga_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
pci_register_bar(&vpci_dev->pci_dev, 0,
PCI_BASE_ADDRESS_MEM_PREFETCH, &vga->vram);
- /*
- * Configure virtio bar and regions
- *
- * We use bar #2 for the mmio regions, to be compatible with stdvga.
- * virtio regions are moved to the end of bar #2, to make room for
- * the stdvga mmio registers at the start of bar #2.
- */
- vpci_dev->modern_mem_bar_idx = 2;
- vpci_dev->msix_bar_idx = 4;
vpci_dev->modern_io_bar_idx = 5;
+ if (!virtio_gpu_hostmem_enabled(g->conf)) {
+ /*
+ * Configure virtio bar and regions
+ *
+ * We use bar #2 for the mmio regions, to be compatible with stdvga.
+ * virtio regions are moved to the end of bar #2, to make room for
+ * the stdvga mmio registers at the start of bar #2.
+ */
+ vpci_dev->modern_mem_bar_idx = 2;
+ vpci_dev->msix_bar_idx = 4;
+ } else {
+ vpci_dev->msix_bar_idx = 1;
+ vpci_dev->modern_mem_bar_idx = 2;
+ memory_region_init(&g->hostmem, OBJECT(g), "virtio-gpu-hostmem",
+ g->conf.hostmem);
+ pci_register_bar(&vpci_dev->pci_dev, 4,
+ PCI_BASE_ADDRESS_SPACE_MEMORY |
+ PCI_BASE_ADDRESS_MEM_PREFETCH |
+ PCI_BASE_ADDRESS_MEM_TYPE_64,
+ &g->hostmem);
+ virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
+ VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
+ }
+
if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
/*
* with page-per-vq=off there is no padding space we can use
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 2e28507efe..eafce75b04 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -102,12 +102,15 @@ enum virtio_gpu_base_conf_flags {
(_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED))
#define virtio_gpu_blob_enabled(_cfg) \
(_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
+#define virtio_gpu_hostmem_enabled(_cfg) \
+ (_cfg.hostmem > 0)
struct virtio_gpu_base_conf {
uint32_t max_outputs;
uint32_t flags;
uint32_t xres;
uint32_t yres;
+ uint64_t hostmem;
};
struct virtio_gpu_ctrl_command {
@@ -131,6 +134,8 @@ struct VirtIOGPUBase {
int renderer_blocked;
int enable;
+ MemoryRegion hostmem;
+
struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUTS];
int enabled_output_bitmask;
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v3 2/9] virtio-gpu: hostmem
2022-09-26 14:24 ` [PATCH v3 2/9] virtio-gpu: hostmem Antonio Caggiano
@ 2023-01-30 16:22 ` Alex Bennée
0 siblings, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2023-01-30 16:22 UTC (permalink / raw)
To: Antonio Caggiano
Cc: gert.wollny, dmitry.osipenko, Gerd Hoffmann, Michael S . Tsirkin,
qemu-devel
Antonio Caggiano <antonio.caggiano@collabora.com> writes:
> From: Gerd Hoffmann <kraxel@redhat.com>
>
> Use VIRTIO_GPU_SHM_ID_HOST_VISIBLE as id for virtio-gpu.
>
> Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> v3: Formatting fixes
>
> hw/display/virtio-gpu-pci.c | 15 +++++++++++++++
> hw/display/virtio-gpu.c | 1 +
> hw/display/virtio-vga.c | 33 ++++++++++++++++++++++++---------
> include/hw/virtio/virtio-gpu.h | 5 +++++
> 4 files changed, 45 insertions(+), 9 deletions(-)
>
> diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
> index 93f214ff58..2cbbacd7fe 100644
> --- a/hw/display/virtio-gpu-pci.c
> +++ b/hw/display/virtio-gpu-pci.c
> @@ -33,6 +33,21 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> DeviceState *vdev = DEVICE(g);
> int i;
>
> + if (virtio_gpu_hostmem_enabled(g->conf)) {
> + vpci_dev->msix_bar_idx = 1;
> + vpci_dev->modern_mem_bar_idx = 2;
> + memory_region_init(&g->hostmem, OBJECT(g), "virtio-gpu-hostmem",
> + g->conf.hostmem);
> + pci_register_bar(&vpci_dev->pci_dev, 4,
> + PCI_BASE_ADDRESS_SPACE_MEMORY |
> + PCI_BASE_ADDRESS_MEM_PREFETCH |
> + PCI_BASE_ADDRESS_MEM_TYPE_64,
> + &g->hostmem);
> + virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
> + VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
> + }
> +
> + qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus), errp);
> virtio_pci_force_virtio_1(vpci_dev);
> if (!qdev_realize(vdev, BUS(&vpci_dev->bus), errp)) {
> return;
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 20cc703dcc..506b3b8eef 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1424,6 +1424,7 @@ static Property virtio_gpu_properties[] = {
> 256 * MiB),
> DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
> VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
> + DEFINE_PROP_SIZE("hostmem", VirtIOGPU, parent_obj.conf.hostmem,
> 0),
I note we don't have a definition for -device virtio-gpu or
virtio-gpu-pci in the invocation section of the manual (missing from
qemu-options.hx). Given the growing complexity of the device perhaps we
need those options and perhaps a new section under:
https://qemu.readthedocs.io/en/latest/system/device-emulation.html#emulated-devices
to outline how virtio-gpu works and what these control knobs are for and
why they would be used.
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
> index 4dcb34c4a7..aa8d1ab993 100644
> --- a/hw/display/virtio-vga.c
> +++ b/hw/display/virtio-vga.c
> @@ -115,17 +115,32 @@ static void virtio_vga_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> pci_register_bar(&vpci_dev->pci_dev, 0,
> PCI_BASE_ADDRESS_MEM_PREFETCH, &vga->vram);
>
> - /*
> - * Configure virtio bar and regions
> - *
> - * We use bar #2 for the mmio regions, to be compatible with stdvga.
> - * virtio regions are moved to the end of bar #2, to make room for
> - * the stdvga mmio registers at the start of bar #2.
> - */
> - vpci_dev->modern_mem_bar_idx = 2;
> - vpci_dev->msix_bar_idx = 4;
> vpci_dev->modern_io_bar_idx = 5;
>
> + if (!virtio_gpu_hostmem_enabled(g->conf)) {
> + /*
> + * Configure virtio bar and regions
> + *
> + * We use bar #2 for the mmio regions, to be compatible with stdvga.
> + * virtio regions are moved to the end of bar #2, to make room for
> + * the stdvga mmio registers at the start of bar #2.
> + */
> + vpci_dev->modern_mem_bar_idx = 2;
> + vpci_dev->msix_bar_idx = 4;
> + } else {
> + vpci_dev->msix_bar_idx = 1;
> + vpci_dev->modern_mem_bar_idx = 2;
modern_mem_bar_idx is the same for both legs so why move its setting and
comment out of the common path? There is no comment for why msix_bar_idx
moves between the two legs.
> + memory_region_init(&g->hostmem, OBJECT(g), "virtio-gpu-hostmem",
> + g->conf.hostmem);
> + pci_register_bar(&vpci_dev->pci_dev, 4,
> + PCI_BASE_ADDRESS_SPACE_MEMORY |
> + PCI_BASE_ADDRESS_MEM_PREFETCH |
> + PCI_BASE_ADDRESS_MEM_TYPE_64,
> + &g->hostmem);
> + virtio_pci_add_shm_cap(vpci_dev, 4, 0, g->conf.hostmem,
> + VIRTIO_GPU_SHM_ID_HOST_VISIBLE);
> + }
> +
> if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
> /*
> * with page-per-vq=off there is no padding space we can use
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 2e28507efe..eafce75b04 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -102,12 +102,15 @@ enum virtio_gpu_base_conf_flags {
> (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED))
> #define virtio_gpu_blob_enabled(_cfg) \
> (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
> +#define virtio_gpu_hostmem_enabled(_cfg) \
> + (_cfg.hostmem > 0)
>
> struct virtio_gpu_base_conf {
> uint32_t max_outputs;
> uint32_t flags;
> uint32_t xres;
> uint32_t yres;
> + uint64_t hostmem;
> };
>
> struct virtio_gpu_ctrl_command {
> @@ -131,6 +134,8 @@ struct VirtIOGPUBase {
> int renderer_blocked;
> int enable;
>
> + MemoryRegion hostmem;
> +
> struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUTS];
>
> int enabled_output_bitmask;
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 3/9] virtio-gpu: Handle resource blob commands
2022-09-26 14:24 [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver Antonio Caggiano
2022-09-26 14:24 ` [PATCH v3 1/9] virtio: Add shared memory capability Antonio Caggiano
2022-09-26 14:24 ` [PATCH v3 2/9] virtio-gpu: hostmem Antonio Caggiano
@ 2022-09-26 14:24 ` Antonio Caggiano
2023-01-30 16:29 ` Alex Bennée
2022-09-26 14:24 ` [PATCH v3 4/9] virtio-gpu: CONTEXT_INIT feature Antonio Caggiano
` (6 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Antonio Caggiano @ 2022-09-26 14:24 UTC (permalink / raw)
To: qemu-devel
Cc: gert.wollny, dmitry.osipenko, Gerd Hoffmann, Michael S. Tsirkin
Support BLOB resources creation, mapping and unmapping by calling the
new stable virglrenderer 0.10 interface. Only enabled when available and
via the blob config. E.g. -device virtio-vga-gl,blob=true
Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
v3: Fix memory leaks and unmap resource on destroy.
hw/display/virtio-gpu-virgl.c | 171 +++++++++++++++++++++++++++
hw/display/virtio-gpu.c | 12 +-
include/hw/virtio/virtio-gpu-bswap.h | 18 +++
include/hw/virtio/virtio-gpu.h | 8 ++
meson.build | 5 +
5 files changed, 210 insertions(+), 4 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 73cb92c8d5..17f00b3fb0 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -16,6 +16,8 @@
#include "trace.h"
#include "hw/virtio/virtio.h"
#include "hw/virtio/virtio-gpu.h"
+#include "hw/virtio/virtio-gpu-bswap.h"
+#include "hw/virtio/virtio-iommu.h"
#include <virglrenderer.h>
@@ -398,6 +400,164 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
g_free(resp);
}
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+
+static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd)
+{
+ struct virtio_gpu_simple_resource *res;
+ struct virtio_gpu_resource_create_blob cblob;
+ int ret;
+
+ VIRTIO_GPU_FILL_CMD(cblob);
+ virtio_gpu_create_blob_bswap(&cblob);
+ trace_virtio_gpu_cmd_res_create_blob(cblob.resource_id, cblob.size);
+
+ if (cblob.resource_id == 0) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+ __func__);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ res = virtio_gpu_find_resource(g, cblob.resource_id);
+ if (res) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+ __func__, cblob.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ res = g_new0(struct virtio_gpu_simple_resource, 1);
+ QTAILQ_INSERT_HEAD(&g->reslist, res, next);
+
+ res->resource_id = cblob.resource_id;
+ res->blob_size = cblob.size;
+
+ if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) {
+ ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
+ cmd, &res->addrs, &res->iov,
+ &res->iov_cnt);
+ if (ret != 0) {
+ cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+ return;
+ }
+ }
+
+ if (cblob.blob_mem == VIRTIO_GPU_BLOB_MEM_GUEST) {
+ virtio_gpu_init_udmabuf(res);
+ }
+ const struct virgl_renderer_resource_create_blob_args virgl_args = {
+ .res_handle = cblob.resource_id,
+ .ctx_id = cblob.hdr.ctx_id,
+ .blob_mem = cblob.blob_mem,
+ .blob_id = cblob.blob_id,
+ .blob_flags = cblob.blob_flags,
+ .size = cblob.size,
+ .iovecs = res->iov,
+ .num_iovs = res->iov_cnt,
+ };
+ ret = virgl_renderer_resource_create_blob(&virgl_args);
+ if (ret) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: virgl blob create error: %s\n",
+ __func__, strerror(-ret));
+ cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+ }
+}
+
+static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd)
+{
+ struct virtio_gpu_simple_resource *res;
+ struct virtio_gpu_resource_map_blob mblob;
+ int ret;
+ void *data;
+ uint64_t size;
+ struct virtio_gpu_resp_map_info resp;
+
+ VIRTIO_GPU_FILL_CMD(mblob);
+ virtio_gpu_map_blob_bswap(&mblob);
+
+ if (mblob.resource_id == 0) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+ __func__);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ res = virtio_gpu_find_resource(g, mblob.resource_id);
+ if (!res) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+ __func__, mblob.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ ret = virgl_renderer_resource_map(res->resource_id, &data, &size);
+ if (ret) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource map error: %s\n",
+ __func__, strerror(-ret));
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ memory_region_init_ram_device_ptr(&res->region, OBJECT(g), NULL, size, data);
+ memory_region_add_subregion(&g->parent_obj.hostmem, mblob.offset, &res->region);
+ memory_region_set_enabled(&res->region, true);
+
+ memset(&resp, 0, sizeof(resp));
+ resp.hdr.type = VIRTIO_GPU_RESP_OK_MAP_INFO;
+ virgl_renderer_resource_get_map_info(mblob.resource_id, &resp.map_info);
+ virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
+
+ res->mapped = true;
+}
+
+int virtio_gpu_virgl_resource_unmap(VirtIOGPU *g,
+ struct virtio_gpu_simple_resource *res)
+{
+ if (!res->mapped) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already unmapped %d\n",
+ __func__, res->resource_id);
+ return VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ }
+
+ memory_region_set_enabled(&res->region, false);
+ memory_region_del_subregion(&g->parent_obj.hostmem, &res->region);
+ object_unparent(OBJECT(&res->region));
+
+ res->mapped = false;
+ return virgl_renderer_resource_unmap(res->resource_id);
+}
+
+static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd)
+{
+ struct virtio_gpu_simple_resource *res;
+ struct virtio_gpu_resource_unmap_blob ublob;
+ VIRTIO_GPU_FILL_CMD(ublob);
+ virtio_gpu_unmap_blob_bswap(&ublob);
+
+ if (ublob.resource_id == 0) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+ __func__);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ res = virtio_gpu_find_resource(g, ublob.resource_id);
+ if (!res) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+ __func__, ublob.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ virtio_gpu_virgl_resource_unmap(g, res);
+}
+
+#endif /* HAVE_VIRGL_RESOURCE_BLOB */
+
void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
{
@@ -464,6 +624,17 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
case VIRTIO_GPU_CMD_GET_EDID:
virtio_gpu_get_edid(g, cmd);
break;
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+ case VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB:
+ virgl_cmd_resource_create_blob(g, cmd);
+ break;
+ case VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB:
+ virgl_cmd_resource_map_blob(g, cmd);
+ break;
+ case VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB:
+ virgl_cmd_resource_unmap_blob(g, cmd);
+ break;
+#endif /* HAVE_VIRGL_RESOURCE_BLOB */
default:
cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
break;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 506b3b8eef..f79693d44d 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -33,8 +33,6 @@
#define VIRTIO_GPU_VM_VERSION 1
-static struct virtio_gpu_simple_resource*
-virtio_gpu_find_resource(VirtIOGPU *g, uint32_t resource_id);
static struct virtio_gpu_simple_resource *
virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
bool require_backing,
@@ -115,7 +113,7 @@ static void update_cursor(VirtIOGPU *g, struct virtio_gpu_update_cursor *cursor)
cursor->resource_id ? 1 : 0);
}
-static struct virtio_gpu_simple_resource *
+struct virtio_gpu_simple_resource *
virtio_gpu_find_resource(VirtIOGPU *g, uint32_t resource_id)
{
struct virtio_gpu_simple_resource *res;
@@ -874,6 +872,10 @@ void virtio_gpu_cleanup_mapping_iov(VirtIOGPU *g,
static void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
struct virtio_gpu_simple_resource *res)
{
+ if (res->mapped) {
+ virtio_gpu_virgl_resource_unmap(g, res);
+ }
+
virtio_gpu_cleanup_mapping_iov(g, res->iov, res->iov_cnt);
res->iov = NULL;
res->iov_cnt = 0;
@@ -1323,10 +1325,12 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
return;
}
+#ifndef HAVE_VIRGL_RESOURCE_BLOB
if (virtio_gpu_virgl_enabled(g->parent_obj.conf)) {
- error_setg(errp, "blobs and virgl are not compatible (yet)");
+ error_setg(errp, "Linked virglrenderer does not support blob resources");
return;
}
+#endif
}
if (!virtio_gpu_base_device_realize(qdev,
diff --git a/include/hw/virtio/virtio-gpu-bswap.h b/include/hw/virtio/virtio-gpu-bswap.h
index 9124108485..dd1975e2d4 100644
--- a/include/hw/virtio/virtio-gpu-bswap.h
+++ b/include/hw/virtio/virtio-gpu-bswap.h
@@ -63,10 +63,28 @@ virtio_gpu_create_blob_bswap(struct virtio_gpu_resource_create_blob *cblob)
{
virtio_gpu_ctrl_hdr_bswap(&cblob->hdr);
le32_to_cpus(&cblob->resource_id);
+ le32_to_cpus(&cblob->blob_mem);
le32_to_cpus(&cblob->blob_flags);
+ le32_to_cpus(&cblob->nr_entries);
+ le64_to_cpus(&cblob->blob_id);
le64_to_cpus(&cblob->size);
}
+static inline void
+virtio_gpu_map_blob_bswap(struct virtio_gpu_resource_map_blob *mblob)
+{
+ virtio_gpu_ctrl_hdr_bswap(&mblob->hdr);
+ le32_to_cpus(&mblob->resource_id);
+ le64_to_cpus(&mblob->offset);
+}
+
+static inline void
+virtio_gpu_unmap_blob_bswap(struct virtio_gpu_resource_unmap_blob *ublob)
+{
+ virtio_gpu_ctrl_hdr_bswap(&ublob->hdr);
+ le32_to_cpus(&ublob->resource_id);
+}
+
static inline void
virtio_gpu_scanout_blob_bswap(struct virtio_gpu_set_scanout_blob *ssb)
{
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index eafce75b04..708cf1bb9c 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -55,6 +55,9 @@ struct virtio_gpu_simple_resource {
int dmabuf_fd;
uint8_t *remapped;
+ MemoryRegion region;
+ bool mapped;
+
QTAILQ_ENTRY(virtio_gpu_simple_resource) next;
};
@@ -245,6 +248,9 @@ void virtio_gpu_base_fill_display_info(VirtIOGPUBase *g,
struct virtio_gpu_resp_display_info *dpy_info);
/* virtio-gpu.c */
+struct virtio_gpu_simple_resource *
+virtio_gpu_find_resource(VirtIOGPU *g, uint32_t resource_id);
+
void virtio_gpu_ctrl_response(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd,
struct virtio_gpu_ctrl_hdr *resp,
@@ -289,5 +295,7 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
void virtio_gpu_virgl_reset(VirtIOGPU *g);
int virtio_gpu_virgl_init(VirtIOGPU *g);
int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
+int virtio_gpu_virgl_resource_unmap(VirtIOGPU *g,
+ struct virtio_gpu_simple_resource *res);
#endif
diff --git a/meson.build b/meson.build
index 3885fc1076..c4e801b4f5 100644
--- a/meson.build
+++ b/meson.build
@@ -718,6 +718,11 @@ if not get_option('virglrenderer').auto() or have_system or have_vhost_user_gpu
method: 'pkg-config',
required: get_option('virglrenderer'),
kwargs: static_kwargs)
+
+ config_host_data.set('HAVE_VIRGL_RESOURCE_BLOB',
+ cc.has_function('virgl_renderer_resource_create_blob',
+ prefix: '#include <virglrenderer.h>',
+ dependencies: virgl))
endif
curl = not_found
if not get_option('curl').auto() or have_block
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v3 3/9] virtio-gpu: Handle resource blob commands
2022-09-26 14:24 ` [PATCH v3 3/9] virtio-gpu: Handle resource blob commands Antonio Caggiano
@ 2023-01-30 16:29 ` Alex Bennée
0 siblings, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2023-01-30 16:29 UTC (permalink / raw)
To: Antonio Caggiano
Cc: gert.wollny, dmitry.osipenko, Gerd Hoffmann, Michael S. Tsirkin,
qemu-devel
Antonio Caggiano <antonio.caggiano@collabora.com> writes:
> Support BLOB resources creation, mapping and unmapping by calling the
> new stable virglrenderer 0.10 interface. Only enabled when available and
> via the blob config. E.g. -device virtio-vga-gl,blob=true
>
> Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
> v3: Fix memory leaks and unmap resource on destroy.
>
> hw/display/virtio-gpu-virgl.c | 171 +++++++++++++++++++++++++++
> hw/display/virtio-gpu.c | 12 +-
> include/hw/virtio/virtio-gpu-bswap.h | 18 +++
> include/hw/virtio/virtio-gpu.h | 8 ++
> meson.build | 5 +
> 5 files changed, 210 insertions(+), 4 deletions(-)
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 73cb92c8d5..17f00b3fb0 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -16,6 +16,8 @@
> #include "trace.h"
> #include "hw/virtio/virtio.h"
> #include "hw/virtio/virtio-gpu.h"
> +#include "hw/virtio/virtio-gpu-bswap.h"
> +#include "hw/virtio/virtio-iommu.h"
>
> #include <virglrenderer.h>
>
> @@ -398,6 +400,164 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
> g_free(resp);
> }
>
> +#ifdef HAVE_VIRGL_RESOURCE_BLOB
> +
> +static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd)
> +{
> + struct virtio_gpu_simple_resource *res;
> + struct virtio_gpu_resource_create_blob cblob;
> + int ret;
> +
> + VIRTIO_GPU_FILL_CMD(cblob);
> + virtio_gpu_create_blob_bswap(&cblob);
> + trace_virtio_gpu_cmd_res_create_blob(cblob.resource_id, cblob.size);
> +
> + if (cblob.resource_id == 0) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
> + __func__);
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> + return;
> + }
> +
> + res = virtio_gpu_find_resource(g, cblob.resource_id);
> + if (res) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
> + __func__, cblob.resource_id);
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> + return;
> + }
> +
> + res = g_new0(struct virtio_gpu_simple_resource, 1);
> + QTAILQ_INSERT_HEAD(&g->reslist, res, next);
> +
> + res->resource_id = cblob.resource_id;
> + res->blob_size = cblob.size;
> +
> + if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) {
> + ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
> + cmd, &res->addrs, &res->iov,
> + &res->iov_cnt);
> + if (ret != 0) {
> + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> + return;
> + }
> + }
> +
> + if (cblob.blob_mem == VIRTIO_GPU_BLOB_MEM_GUEST) {
> + virtio_gpu_init_udmabuf(res);
> + }
> + const struct virgl_renderer_resource_create_blob_args virgl_args = {
> + .res_handle = cblob.resource_id,
> + .ctx_id = cblob.hdr.ctx_id,
> + .blob_mem = cblob.blob_mem,
> + .blob_id = cblob.blob_id,
> + .blob_flags = cblob.blob_flags,
> + .size = cblob.size,
> + .iovecs = res->iov,
> + .num_iovs = res->iov_cnt,
> + };
> + ret = virgl_renderer_resource_create_blob(&virgl_args);
> + if (ret) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: virgl blob create error: %s\n",
> + __func__, strerror(-ret));
> + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> + }
> +}
> +
> +static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd)
> +{
> + struct virtio_gpu_simple_resource *res;
> + struct virtio_gpu_resource_map_blob mblob;
> + int ret;
> + void *data;
> + uint64_t size;
> + struct virtio_gpu_resp_map_info resp;
> +
> + VIRTIO_GPU_FILL_CMD(mblob);
> + virtio_gpu_map_blob_bswap(&mblob);
> +
> + if (mblob.resource_id == 0) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
> + __func__);
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> + return;
> + }
> +
> + res = virtio_gpu_find_resource(g, mblob.resource_id);
> + if (!res) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
> + __func__, mblob.resource_id);
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> + return;
> + }
> +
> + ret = virgl_renderer_resource_map(res->resource_id, &data, &size);
> + if (ret) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource map error: %s\n",
> + __func__, strerror(-ret));
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> + return;
> + }
> +
> + memory_region_init_ram_device_ptr(&res->region, OBJECT(g), NULL, size, data);
> + memory_region_add_subregion(&g->parent_obj.hostmem, mblob.offset, &res->region);
> + memory_region_set_enabled(&res->region, true);
> +
> + memset(&resp, 0, sizeof(resp));
> + resp.hdr.type = VIRTIO_GPU_RESP_OK_MAP_INFO;
> + virgl_renderer_resource_get_map_info(mblob.resource_id, &resp.map_info);
> + virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
> +
> + res->mapped = true;
> +}
> +
> +int virtio_gpu_virgl_resource_unmap(VirtIOGPU *g,
> + struct virtio_gpu_simple_resource *res)
> +{
> + if (!res->mapped) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already unmapped %d\n",
> + __func__, res->resource_id);
> + return VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> + }
> +
> + memory_region_set_enabled(&res->region, false);
> + memory_region_del_subregion(&g->parent_obj.hostmem, &res->region);
> + object_unparent(OBJECT(&res->region));
> +
> + res->mapped = false;
> + return virgl_renderer_resource_unmap(res->resource_id);
> +}
> +
> +static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd)
> +{
> + struct virtio_gpu_simple_resource *res;
> + struct virtio_gpu_resource_unmap_blob ublob;
> + VIRTIO_GPU_FILL_CMD(ublob);
> + virtio_gpu_unmap_blob_bswap(&ublob);
> +
> + if (ublob.resource_id == 0) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
> + __func__);
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> + return;
> + }
> +
> + res = virtio_gpu_find_resource(g, ublob.resource_id);
> + if (!res) {
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
> + __func__, ublob.resource_id);
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> + return;
> + }
> +
> + virtio_gpu_virgl_resource_unmap(g, res);
> +}
> +
> +#endif /* HAVE_VIRGL_RESOURCE_BLOB */
> +
> void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> struct virtio_gpu_ctrl_command *cmd)
> {
> @@ -464,6 +624,17 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> case VIRTIO_GPU_CMD_GET_EDID:
> virtio_gpu_get_edid(g, cmd);
> break;
> +#ifdef HAVE_VIRGL_RESOURCE_BLOB
> + case VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB:
> + virgl_cmd_resource_create_blob(g, cmd);
> + break;
> + case VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB:
> + virgl_cmd_resource_map_blob(g, cmd);
> + break;
> + case VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB:
> + virgl_cmd_resource_unmap_blob(g, cmd);
> + break;
> +#endif /* HAVE_VIRGL_RESOURCE_BLOB */
> default:
> cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> break;
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 506b3b8eef..f79693d44d 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -33,8 +33,6 @@
>
> #define VIRTIO_GPU_VM_VERSION 1
>
> -static struct virtio_gpu_simple_resource*
> -virtio_gpu_find_resource(VirtIOGPU *g, uint32_t resource_id);
> static struct virtio_gpu_simple_resource *
> virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
> bool require_backing,
> @@ -115,7 +113,7 @@ static void update_cursor(VirtIOGPU *g, struct virtio_gpu_update_cursor *cursor)
> cursor->resource_id ? 1 : 0);
> }
>
> -static struct virtio_gpu_simple_resource *
> +struct virtio_gpu_simple_resource *
> virtio_gpu_find_resource(VirtIOGPU *g, uint32_t resource_id)
> {
> struct virtio_gpu_simple_resource *res;
> @@ -874,6 +872,10 @@ void virtio_gpu_cleanup_mapping_iov(VirtIOGPU *g,
> static void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
> struct virtio_gpu_simple_resource *res)
> {
> + if (res->mapped) {
> + virtio_gpu_virgl_resource_unmap(g, res);
> + }
> +
This fails to build if an older VirGL is used:
/usr/bin/ld: libcommon.fa.p/hw_display_virtio-gpu.c.o: in function `virtio_gpu_cleanup_mapping':
/home/alex/lsrc/qemu.git/builds/arm.all/../../hw/display/virtio-gpu.c:877: undefined reference to `virtio_gpu_virgl_resource_unmap'
collect2: error: ld returned 1 exit status
so I think needs an ifdef protecting it. Can ->mapped be set without
blob support?
> virtio_gpu_cleanup_mapping_iov(g, res->iov, res->iov_cnt);
> res->iov = NULL;
> res->iov_cnt = 0;
> @@ -1323,10 +1325,12 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
> return;
> }
>
> +#ifndef HAVE_VIRGL_RESOURCE_BLOB
> if (virtio_gpu_virgl_enabled(g->parent_obj.conf)) {
> - error_setg(errp, "blobs and virgl are not compatible (yet)");
> + error_setg(errp, "Linked virglrenderer does not support blob resources");
> return;
> }
> +#endif
> }
>
> if (!virtio_gpu_base_device_realize(qdev,
> diff --git a/include/hw/virtio/virtio-gpu-bswap.h b/include/hw/virtio/virtio-gpu-bswap.h
> index 9124108485..dd1975e2d4 100644
> --- a/include/hw/virtio/virtio-gpu-bswap.h
> +++ b/include/hw/virtio/virtio-gpu-bswap.h
> @@ -63,10 +63,28 @@ virtio_gpu_create_blob_bswap(struct virtio_gpu_resource_create_blob *cblob)
> {
> virtio_gpu_ctrl_hdr_bswap(&cblob->hdr);
> le32_to_cpus(&cblob->resource_id);
> + le32_to_cpus(&cblob->blob_mem);
> le32_to_cpus(&cblob->blob_flags);
> + le32_to_cpus(&cblob->nr_entries);
> + le64_to_cpus(&cblob->blob_id);
> le64_to_cpus(&cblob->size);
> }
>
> +static inline void
> +virtio_gpu_map_blob_bswap(struct virtio_gpu_resource_map_blob *mblob)
> +{
> + virtio_gpu_ctrl_hdr_bswap(&mblob->hdr);
> + le32_to_cpus(&mblob->resource_id);
> + le64_to_cpus(&mblob->offset);
> +}
> +
> +static inline void
> +virtio_gpu_unmap_blob_bswap(struct virtio_gpu_resource_unmap_blob *ublob)
> +{
> + virtio_gpu_ctrl_hdr_bswap(&ublob->hdr);
> + le32_to_cpus(&ublob->resource_id);
> +}
> +
> static inline void
> virtio_gpu_scanout_blob_bswap(struct virtio_gpu_set_scanout_blob *ssb)
> {
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index eafce75b04..708cf1bb9c 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -55,6 +55,9 @@ struct virtio_gpu_simple_resource {
> int dmabuf_fd;
> uint8_t *remapped;
>
> + MemoryRegion region;
> + bool mapped;
> +
These probably need to be conditionally #ifdef'ed
> QTAILQ_ENTRY(virtio_gpu_simple_resource) next;
> };
>
> @@ -245,6 +248,9 @@ void virtio_gpu_base_fill_display_info(VirtIOGPUBase *g,
> struct virtio_gpu_resp_display_info *dpy_info);
>
> /* virtio-gpu.c */
> +struct virtio_gpu_simple_resource *
> +virtio_gpu_find_resource(VirtIOGPU *g, uint32_t resource_id);
> +
> void virtio_gpu_ctrl_response(VirtIOGPU *g,
> struct virtio_gpu_ctrl_command *cmd,
> struct virtio_gpu_ctrl_hdr *resp,
> @@ -289,5 +295,7 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
> void virtio_gpu_virgl_reset(VirtIOGPU *g);
> int virtio_gpu_virgl_init(VirtIOGPU *g);
> int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
> +int virtio_gpu_virgl_resource_unmap(VirtIOGPU *g,
> + struct virtio_gpu_simple_resource *res);
>
It would be nice for these API functions to have some kerneldoc comments.
> #endif
> diff --git a/meson.build b/meson.build
> index 3885fc1076..c4e801b4f5 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -718,6 +718,11 @@ if not get_option('virglrenderer').auto() or have_system or have_vhost_user_gpu
> method: 'pkg-config',
> required: get_option('virglrenderer'),
> kwargs: static_kwargs)
> +
> + config_host_data.set('HAVE_VIRGL_RESOURCE_BLOB',
> + cc.has_function('virgl_renderer_resource_create_blob',
> + prefix: '#include <virglrenderer.h>',
> + dependencies: virgl))
> endif
> curl = not_found
> if not get_option('curl').auto() or have_block
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 4/9] virtio-gpu: CONTEXT_INIT feature
2022-09-26 14:24 [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver Antonio Caggiano
` (2 preceding siblings ...)
2022-09-26 14:24 ` [PATCH v3 3/9] virtio-gpu: Handle resource blob commands Antonio Caggiano
@ 2022-09-26 14:24 ` Antonio Caggiano
2022-09-26 14:24 ` [PATCH v3 5/9] virtio-gpu: Unrealize Antonio Caggiano
` (5 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Antonio Caggiano @ 2022-09-26 14:24 UTC (permalink / raw)
To: qemu-devel
Cc: gert.wollny, dmitry.osipenko, Marc-André Lureau,
Gerd Hoffmann, Michael S. Tsirkin
Create virgl renderer context with flags using context_id when valid.
The feature can be enabled via the context_init config option.
A warning message will be emitted and the feature will not be used
when linking with virglrenderer versions without context_init support.
Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
v3:
- The feature can be enabled via the context_init config option.
- A warning message will be emitted and the feature will not be used
when linking with virglrenderer versions without context_init support.
- Define HAVE_VIRGL_CONTEXT_INIT in config_host_data.
hw/display/virtio-gpu-base.c | 3 +++
hw/display/virtio-gpu-virgl.c | 16 ++++++++++++++--
hw/display/virtio-gpu.c | 2 ++
include/hw/virtio/virtio-gpu.h | 3 +++
meson.build | 4 ++++
5 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index a29f191aa8..6c5f1f327f 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -215,6 +215,9 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features,
if (virtio_gpu_blob_enabled(g->conf)) {
features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB);
}
+ if (virtio_gpu_context_init_enabled(g->conf)) {
+ 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 17f00b3fb0..1bff8c66ce 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -99,8 +99,20 @@ 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) {
+#ifdef HAVE_VIRGL_CONTEXT_INIT
+ virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
+ cc.context_init,
+ cc.nlen,
+ cc.debug_name);
+ return;
+#else
+ qemu_log_mask(LOG_UNIMP,
+ "Linked virglrenderer does not support context-init\n");
+#endif
+ }
+
+ virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name);
}
static void virgl_cmd_context_destroy(VirtIOGPU *g,
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index f79693d44d..92cd96582e 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1429,6 +1429,8 @@ static Property virtio_gpu_properties[] = {
DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
DEFINE_PROP_SIZE("hostmem", VirtIOGPU, parent_obj.conf.hostmem, 0),
+ DEFINE_PROP_BIT("context_init", VirtIOGPU, parent_obj.conf.flags,
+ VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, false),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 708cf1bb9c..a23efb9568 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -93,6 +93,7 @@ enum virtio_gpu_base_conf_flags {
VIRTIO_GPU_FLAG_EDID_ENABLED,
VIRTIO_GPU_FLAG_DMABUF_ENABLED,
VIRTIO_GPU_FLAG_BLOB_ENABLED,
+ VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
};
#define virtio_gpu_virgl_enabled(_cfg) \
@@ -107,6 +108,8 @@ enum virtio_gpu_base_conf_flags {
(_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
#define virtio_gpu_hostmem_enabled(_cfg) \
(_cfg.hostmem > 0)
+#define virtio_gpu_context_init_enabled(_cfg) \
+ (_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED))
struct virtio_gpu_base_conf {
uint32_t max_outputs;
diff --git a/meson.build b/meson.build
index c4e801b4f5..6d4b844ffb 100644
--- a/meson.build
+++ b/meson.build
@@ -723,6 +723,10 @@ if not get_option('virglrenderer').auto() or have_system or have_vhost_user_gpu
cc.has_function('virgl_renderer_resource_create_blob',
prefix: '#include <virglrenderer.h>',
dependencies: virgl))
+ config_host_data.set('HAVE_VIRGL_CONTEXT_INIT',
+ cc.has_function('virgl_renderer_context_create_with_flags',
+ prefix: '#include <virglrenderer.h>',
+ dependencies: virgl))
endif
curl = not_found
if not get_option('curl').auto() or have_block
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v3 5/9] virtio-gpu: Unrealize
2022-09-26 14:24 [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver Antonio Caggiano
` (3 preceding siblings ...)
2022-09-26 14:24 ` [PATCH v3 4/9] virtio-gpu: CONTEXT_INIT feature Antonio Caggiano
@ 2022-09-26 14:24 ` Antonio Caggiano
2023-01-30 18:36 ` Alex Bennée
2022-09-26 14:24 ` [PATCH v3 6/9] virtio-gpu: Resource UUID Antonio Caggiano
` (4 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Antonio Caggiano @ 2022-09-26 14:24 UTC (permalink / raw)
To: qemu-devel
Cc: gert.wollny, dmitry.osipenko, Gerd Hoffmann, Michael S. Tsirkin
Implement an unrealize function for virtio gpu device.
Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
---
v3: Call virtio_gpu_base_device_unrealize from virtio_gpu_device_unrealize
hw/display/virtio-gpu-base.c | 2 +-
hw/display/virtio-gpu.c | 11 +++++++++++
include/hw/virtio/virtio-gpu.h | 1 +
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 6c5f1f327f..5cb71e71ad 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -230,7 +230,7 @@ virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features)
trace_virtio_gpu_features(((features & virgl) == virgl));
}
-static void
+void
virtio_gpu_base_device_unrealize(DeviceState *qdev)
{
VirtIOGPUBase *g = VIRTIO_GPU_BASE(qdev);
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 92cd96582e..f1772a15bb 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1349,6 +1349,16 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
QTAILQ_INIT(&g->fenceq);
}
+static void virtio_gpu_device_unrealize(DeviceState *qdev)
+{
+ VirtIOGPU *g = VIRTIO_GPU(qdev);
+
+ qemu_bh_delete(g->cursor_bh);
+ qemu_bh_delete(g->ctrl_bh);
+
+ virtio_gpu_base_device_unrealize(qdev);
+}
+
void virtio_gpu_reset(VirtIODevice *vdev)
{
VirtIOGPU *g = VIRTIO_GPU(vdev);
@@ -1447,6 +1457,7 @@ static void virtio_gpu_class_init(ObjectClass *klass, void *data)
vgbc->gl_flushed = virtio_gpu_handle_gl_flushed;
vdc->realize = virtio_gpu_device_realize;
+ vdc->unrealize = virtio_gpu_device_unrealize;
vdc->reset = virtio_gpu_reset;
vdc->get_config = virtio_gpu_get_config;
vdc->set_config = virtio_gpu_set_config;
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index a23efb9568..e9281c75f3 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -246,6 +246,7 @@ bool virtio_gpu_base_device_realize(DeviceState *qdev,
VirtIOHandleOutput ctrl_cb,
VirtIOHandleOutput cursor_cb,
Error **errp);
+void virtio_gpu_base_device_unrealize(DeviceState *qdev);
void virtio_gpu_base_reset(VirtIOGPUBase *g);
void virtio_gpu_base_fill_display_info(VirtIOGPUBase *g,
struct virtio_gpu_resp_display_info *dpy_info);
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v3 6/9] virtio-gpu: Resource UUID
2022-09-26 14:24 [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver Antonio Caggiano
` (4 preceding siblings ...)
2022-09-26 14:24 ` [PATCH v3 5/9] virtio-gpu: Unrealize Antonio Caggiano
@ 2022-09-26 14:24 ` Antonio Caggiano
2022-09-26 14:24 ` [PATCH v3 7/9] virtio-gpu: Support Venus capset Antonio Caggiano
` (3 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Antonio Caggiano @ 2022-09-26 14:24 UTC (permalink / raw)
To: qemu-devel
Cc: gert.wollny, dmitry.osipenko, Michael S. Tsirkin, Gerd Hoffmann
Enable resource UUID feature and implement command resource assign UUID.
This is done by introducing a hash table to map resource IDs to their
UUIDs.
Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
---
v3: Destroy the hash table in the unrealize function
hw/display/trace-events | 1 +
hw/display/virtio-gpu-base.c | 2 ++
hw/display/virtio-gpu-virgl.c | 11 +++++++++
hw/display/virtio-gpu.c | 41 ++++++++++++++++++++++++++++++++++
include/hw/virtio/virtio-gpu.h | 4 ++++
5 files changed, 59 insertions(+)
diff --git a/hw/display/trace-events b/hw/display/trace-events
index 0c0ffcbe42..6632344322 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -41,6 +41,7 @@ virtio_gpu_cmd_res_create_blob(uint32_t res, uint64_t size) "res 0x%x, size %" P
virtio_gpu_cmd_res_unref(uint32_t res) "res 0x%x"
virtio_gpu_cmd_res_back_attach(uint32_t res) "res 0x%x"
virtio_gpu_cmd_res_back_detach(uint32_t res) "res 0x%x"
+virtio_gpu_cmd_res_assign_uuid(uint32_t res) "res 0x%x"
virtio_gpu_cmd_res_xfer_toh_2d(uint32_t res) "res 0x%x"
virtio_gpu_cmd_res_xfer_toh_3d(uint32_t res) "res 0x%x"
virtio_gpu_cmd_res_xfer_fromh_3d(uint32_t res) "res 0x%x"
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 5cb71e71ad..54792aa501 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -219,6 +219,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features,
features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT);
}
+ features |= (1 << VIRTIO_GPU_F_RESOURCE_UUID);
+
return features;
}
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 1bff8c66ce..f9d8ccfdf8 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -45,6 +45,10 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
args.nr_samples = 0;
args.flags = VIRTIO_GPU_RESOURCE_FLAG_Y_0_TOP;
virgl_renderer_resource_create(&args, NULL, 0);
+
+ struct virtio_gpu_simple_resource *res = g_new0(struct virtio_gpu_simple_resource, 1);
+ res->resource_id = c2d.resource_id;
+ QTAILQ_INSERT_HEAD(&g->reslist, res, next);
}
static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
@@ -69,6 +73,10 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
args.nr_samples = c3d.nr_samples;
args.flags = c3d.flags;
virgl_renderer_resource_create(&args, NULL, 0);
+
+ struct virtio_gpu_simple_resource *res = g_new0(struct virtio_gpu_simple_resource, 1);
+ res->resource_id = c3d.resource_id;
+ QTAILQ_INSERT_HEAD(&g->reslist, res, next);
}
static void virgl_cmd_resource_unref(VirtIOGPU *g,
@@ -624,6 +632,9 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
/* TODO add security */
virgl_cmd_ctx_detach_resource(g, cmd);
break;
+ case VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID:
+ virtio_gpu_resource_assign_uuid(g, cmd);
+ break;
case VIRTIO_GPU_CMD_GET_CAPSET_INFO:
virgl_cmd_get_capset_info(g, cmd);
break;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index f1772a15bb..09a92b6e76 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -939,6 +939,37 @@ virtio_gpu_resource_detach_backing(VirtIOGPU *g,
virtio_gpu_cleanup_mapping(g, res);
}
+void virtio_gpu_resource_assign_uuid(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd)
+{
+ struct virtio_gpu_simple_resource *res;
+ struct virtio_gpu_resource_assign_uuid assign;
+ struct virtio_gpu_resp_resource_uuid resp;
+ QemuUUID *uuid = NULL;
+
+ VIRTIO_GPU_FILL_CMD(assign);
+ virtio_gpu_bswap_32(&assign, sizeof(assign));
+ trace_virtio_gpu_cmd_res_assign_uuid(assign.resource_id);
+
+ res = virtio_gpu_find_check_resource(g, assign.resource_id, false, __func__, &cmd->error);
+ if (!res) {
+ return;
+ }
+
+ memset(&resp, 0, sizeof(resp));
+ resp.hdr.type = VIRTIO_GPU_RESP_OK_RESOURCE_UUID;
+
+ uuid = g_hash_table_lookup(g->resource_uuids, GUINT_TO_POINTER(assign.resource_id));
+ if (!uuid) {
+ uuid = g_new(QemuUUID, 1);
+ qemu_uuid_generate(uuid);
+ g_hash_table_insert(g->resource_uuids, GUINT_TO_POINTER(assign.resource_id), uuid);
+ }
+
+ memcpy(resp.uuid, uuid, sizeof(QemuUUID));
+ virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
+}
+
void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
{
@@ -987,6 +1018,9 @@ void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
case VIRTIO_GPU_CMD_RESOURCE_DETACH_BACKING:
virtio_gpu_resource_detach_backing(g, cmd);
break;
+ case VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID:
+ virtio_gpu_resource_assign_uuid(g, cmd);
+ break;
default:
cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
break;
@@ -1347,12 +1381,15 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
QTAILQ_INIT(&g->reslist);
QTAILQ_INIT(&g->cmdq);
QTAILQ_INIT(&g->fenceq);
+
+ g->resource_uuids = g_hash_table_new_full(NULL, NULL, NULL, g_free);
}
static void virtio_gpu_device_unrealize(DeviceState *qdev)
{
VirtIOGPU *g = VIRTIO_GPU(qdev);
+ g_hash_table_destroy(g->resource_uuids);
qemu_bh_delete(g->cursor_bh);
qemu_bh_delete(g->ctrl_bh);
@@ -1382,6 +1419,10 @@ void virtio_gpu_reset(VirtIODevice *vdev)
g_free(cmd);
}
+ if (g->resource_uuids) {
+ g_hash_table_remove_all(g->resource_uuids);
+ }
+
virtio_gpu_base_reset(VIRTIO_GPU_BASE(vdev));
}
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index e9281c75f3..7d455b0a03 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -200,6 +200,8 @@ struct VirtIOGPU {
QTAILQ_HEAD(, VGPUDMABuf) bufs;
VGPUDMABuf *primary[VIRTIO_GPU_MAX_SCANOUTS];
} dmabuf;
+
+ GHashTable *resource_uuids;
};
struct VirtIOGPUClass {
@@ -273,6 +275,8 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
uint32_t *niov);
void virtio_gpu_cleanup_mapping_iov(VirtIOGPU *g,
struct iovec *iov, uint32_t count);
+void virtio_gpu_resource_assign_uuid(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd);
void virtio_gpu_process_cmdq(VirtIOGPU *g);
void virtio_gpu_device_realize(DeviceState *qdev, Error **errp);
void virtio_gpu_reset(VirtIODevice *vdev);
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v3 7/9] virtio-gpu: Support Venus capset
2022-09-26 14:24 [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver Antonio Caggiano
` (5 preceding siblings ...)
2022-09-26 14:24 ` [PATCH v3 6/9] virtio-gpu: Resource UUID Antonio Caggiano
@ 2022-09-26 14:24 ` Antonio Caggiano
2022-09-26 14:24 ` [PATCH v3 8/9] virtio-gpu: Initialize Venus Antonio Caggiano
` (2 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Antonio Caggiano @ 2022-09-26 14:24 UTC (permalink / raw)
To: qemu-devel
Cc: gert.wollny, dmitry.osipenko, Gerd Hoffmann, Michael S. Tsirkin
Add support for the Venus capset, which enables Vulkan support through
the Venus Vulkan driver for virtio-gpu.
Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
---
v3: Improve commit message
hw/display/virtio-gpu-virgl.c | 21 +++++++++++++++++----
include/standard-headers/linux/virtio_gpu.h | 2 ++
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index f9d8ccfdf8..16f600adbb 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -388,6 +388,11 @@ static void virgl_cmd_get_capset_info(VirtIOGPU *g,
virgl_renderer_get_cap_set(resp.capset_id,
&resp.capset_max_version,
&resp.capset_max_size);
+ } else if (info.capset_index == 2) {
+ resp.capset_id = VIRTIO_GPU_CAPSET_VENUS;
+ virgl_renderer_get_cap_set(resp.capset_id,
+ &resp.capset_max_version,
+ &resp.capset_max_size);
} else {
resp.capset_max_version = 0;
resp.capset_max_size = 0;
@@ -820,10 +825,18 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
{
- uint32_t capset2_max_ver, capset2_max_size;
+ uint32_t capset2_max_ver, capset2_max_size, num_capsets;
+ num_capsets = 1;
+
virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
- &capset2_max_ver,
- &capset2_max_size);
+ &capset2_max_ver,
+ &capset2_max_size);
+ num_capsets += capset2_max_ver ? 1 : 0;
+
+ virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VENUS,
+ &capset2_max_ver,
+ &capset2_max_size);
+ num_capsets += capset2_max_size ? 1 : 0;
- return capset2_max_ver ? 2 : 1;
+ return num_capsets;
}
diff --git a/include/standard-headers/linux/virtio_gpu.h b/include/standard-headers/linux/virtio_gpu.h
index 2da48d3d4c..2db643ed8f 100644
--- a/include/standard-headers/linux/virtio_gpu.h
+++ b/include/standard-headers/linux/virtio_gpu.h
@@ -309,6 +309,8 @@ struct virtio_gpu_cmd_submit {
#define VIRTIO_GPU_CAPSET_VIRGL 1
#define VIRTIO_GPU_CAPSET_VIRGL2 2
+/* 3 is reserved for gfxstream */
+#define VIRTIO_GPU_CAPSET_VENUS 4
/* VIRTIO_GPU_CMD_GET_CAPSET_INFO */
struct virtio_gpu_get_capset_info {
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v3 8/9] virtio-gpu: Initialize Venus
2022-09-26 14:24 [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver Antonio Caggiano
` (6 preceding siblings ...)
2022-09-26 14:24 ` [PATCH v3 7/9] virtio-gpu: Support Venus capset Antonio Caggiano
@ 2022-09-26 14:24 ` Antonio Caggiano
2023-01-30 15:55 ` Alex Bennée
2022-09-26 14:24 ` [PATCH v3 9/9] virtio-gpu: Get EGL Display callback Antonio Caggiano
2023-01-30 17:00 ` [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver Alex Bennée
9 siblings, 1 reply; 26+ messages in thread
From: Antonio Caggiano @ 2022-09-26 14:24 UTC (permalink / raw)
To: qemu-devel
Cc: gert.wollny, dmitry.osipenko, Michael S. Tsirkin, Gerd Hoffmann
Request Venus when initializing VirGL.
Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
---
hw/display/virtio-gpu-virgl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 16f600adbb..0f17bdddd0 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -806,7 +806,7 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
{
int ret;
- ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs);
+ ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs);
if (ret != 0) {
error_report("virgl could not be initialized: %d", ret);
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v3 8/9] virtio-gpu: Initialize Venus
2022-09-26 14:24 ` [PATCH v3 8/9] virtio-gpu: Initialize Venus Antonio Caggiano
@ 2023-01-30 15:55 ` Alex Bennée
0 siblings, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2023-01-30 15:55 UTC (permalink / raw)
To: Antonio Caggiano
Cc: gert.wollny, dmitry.osipenko, Michael S. Tsirkin, Gerd Hoffmann,
qemu-devel
Antonio Caggiano <antonio.caggiano@collabora.com> writes:
> Request Venus when initializing VirGL.
>
> Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
> ---
> hw/display/virtio-gpu-virgl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 16f600adbb..0f17bdddd0 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -806,7 +806,7 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
> {
> int ret;
>
> - ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs);
> + ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs);
> if (ret != 0) {
> error_report("virgl could not be initialized: %d", ret);
> return ret;
We need to probe for Venus support in virgl in configure:
irtio-gpu-virgl.c.o -c ../../hw/display/virtio-gpu-virgl.c
../../hw/display/virtio-gpu-virgl.c: In function ‘virtio_gpu_virgl_init’:
../../hw/display/virtio-gpu-virgl.c:820:34: error: ‘VIRGL_RENDERER_VENUS’ undeclared (first use in this function); did you mean ‘VIRGL_RENDERER_USE_EGL’?
820 | ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs);
| ^~~~~~~~~~~~~~~~~~~~
| VIRGL_RENDERER_USE_EGL
../../hw/display/virtio-gpu-virgl.c:820:34: note: each undeclared identifier is reported only once for each function it appears in
While I fixed the callback with:
modified hw/display/virtio-gpu-virgl.c
@@ -744,10 +744,12 @@ static int virgl_make_context_current(void *opaque, int scanout_idx,
qctx);
}
+#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
static void *virgl_get_egl_display(void *opaque)
{
return eglGetCurrentDisplay();
}
+#endif
static struct virgl_renderer_callbacks virtio_gpu_3d_cbs = {
.version = 4,
@@ -755,7 +757,9 @@ static struct virgl_renderer_callbacks virtio_gpu_3d_cbs = {
.create_gl_context = virgl_create_context,
.destroy_gl_context = virgl_destroy_context,
.make_current = virgl_make_context_current,
+#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
.get_egl_display = virgl_get_egl_display,
+#endif
};
static void virtio_gpu_print_stats(void *opaque)
I suspect we shouldn't unconditionally enable VENUS here. This sounds
like it needs a configuration knob on the device, i.e.:
-device virtio-gpu-pci,renderer=venus
where the default is EGL.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 9/9] virtio-gpu: Get EGL Display callback
2022-09-26 14:24 [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver Antonio Caggiano
` (7 preceding siblings ...)
2022-09-26 14:24 ` [PATCH v3 8/9] virtio-gpu: Initialize Venus Antonio Caggiano
@ 2022-09-26 14:24 ` Antonio Caggiano
2023-01-30 15:49 ` Alex Bennée
2023-01-30 17:00 ` [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver Alex Bennée
9 siblings, 1 reply; 26+ messages in thread
From: Antonio Caggiano @ 2022-09-26 14:24 UTC (permalink / raw)
To: qemu-devel
Cc: gert.wollny, dmitry.osipenko, Gerd Hoffmann, Michael S. Tsirkin
Implement get_egl_display callback for virglrenderer.
Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
---
hw/display/virtio-gpu-virgl.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 0f17bdddd0..0fd9ad8a3d 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -18,6 +18,7 @@
#include "hw/virtio/virtio-gpu.h"
#include "hw/virtio/virtio-gpu-bswap.h"
#include "hw/virtio/virtio-iommu.h"
+#include <epoxy/egl.h>
#include <virglrenderer.h>
@@ -743,12 +744,18 @@ static int virgl_make_context_current(void *opaque, int scanout_idx,
qctx);
}
+static void *virgl_get_egl_display(void *opaque)
+{
+ return eglGetCurrentDisplay();
+}
+
static struct virgl_renderer_callbacks virtio_gpu_3d_cbs = {
- .version = 1,
+ .version = 4,
.write_fence = virgl_write_fence,
.create_gl_context = virgl_create_context,
.destroy_gl_context = virgl_destroy_context,
.make_current = virgl_make_context_current,
+ .get_egl_display = virgl_get_egl_display,
};
static void virtio_gpu_print_stats(void *opaque)
--
2.34.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v3 9/9] virtio-gpu: Get EGL Display callback
2022-09-26 14:24 ` [PATCH v3 9/9] virtio-gpu: Get EGL Display callback Antonio Caggiano
@ 2023-01-30 15:49 ` Alex Bennée
0 siblings, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2023-01-30 15:49 UTC (permalink / raw)
To: Antonio Caggiano
Cc: gert.wollny, dmitry.osipenko, Gerd Hoffmann, Michael S. Tsirkin,
qemu-devel
Antonio Caggiano <antonio.caggiano@collabora.com> writes:
> Implement get_egl_display callback for virglrenderer.
>
> Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
> ---
> hw/display/virtio-gpu-virgl.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 0f17bdddd0..0fd9ad8a3d 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -18,6 +18,7 @@
> #include "hw/virtio/virtio-gpu.h"
> #include "hw/virtio/virtio-gpu-bswap.h"
> #include "hw/virtio/virtio-iommu.h"
> +#include <epoxy/egl.h>
>
> #include <virglrenderer.h>
>
> @@ -743,12 +744,18 @@ static int virgl_make_context_current(void *opaque, int scanout_idx,
> qctx);
> }
>
> +static void *virgl_get_egl_display(void *opaque)
> +{
> + return eglGetCurrentDisplay();
> +}
> +
> static struct virgl_renderer_callbacks virtio_gpu_3d_cbs = {
> - .version = 1,
> + .version = 4,
> .write_fence = virgl_write_fence,
> .create_gl_context = virgl_create_context,
> .destroy_gl_context = virgl_destroy_context,
> .make_current = virgl_make_context_current,
> + .get_egl_display = virgl_get_egl_display,
This fails for me:
FAILED: libcommon.fa.p/hw_display_virtio-gpu-virgl.c.o
cc -m64 -mcx16 -Ilibcommon.fa.p -I../../common-user/host/x86_64 -I../../linux-user/include/host/x86_64 -I../../linux-user/include -Idtc/libfdt -I../../dtc/libfdt -I/usr/include/capstone -I/usr/include/pixman-1 -I/usr/include/libpng16 -I/usr/include/spice-server -I/usr/include/spice-1 -I/usr/include/p11-kit-1 -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/gio-unix-2.0 -I/usr/include/slirp -I/usr/include/gtk-3.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0 -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/fribidi -I/usr/include/harfbuzz -I/usr/include/atk-1.0 -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/vte-2.91 -I/usr/include/virgl -I/usr/include/cacard -I/usr/include/nss -I/usr/include/nspr -I/usr/include/PCSC -I/usr/include/libusb-1.0 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /home/alex/lsrc/qemu.git/linux-headers -isystem linux-headers -iquote . -iquote /home/alex/lsrc/qemu.git -iquote /home/alex/lsrc/qemu.git/include -iquote /home/alex/lsrc/qemu.git/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -D_REENTRANT -D_DEFAULT_SOURCE -D_XOPEN_SOURCE=600 -DNCURSES_WIDECHAR=1 -DSTRUCT_IOVEC_DEFINED -MD -MQ libcommon.fa.p/hw_display_virtio-gpu-virgl.c.o -MF libcommon.fa.p/hw_display_virtio-gpu-virgl.c.o.d -o libcommon.fa.p/hw_display_virtio-gpu-virgl.c.o -c ../../hw/display/virtio-gpu-virgl.c
../../hw/display/virtio-gpu-virgl.c:758:6: error: ‘struct virgl_renderer_callbacks’ has no member named ‘get_egl_display’
758 | .get_egl_display = virgl_get_egl_display,
| ^~~~~~~~~~~~~~~
../../hw/display/virtio-gpu-virgl.c:758:28: error: initialization of ‘int (*)(void *)’ from incompatible pointer type ‘void * (*)(void *)’ [-Werror=incompatible-pointer-types]
758 | .get_egl_display = virgl_get_egl_display,
| ^~~~~~~~~~~~~~~~~~~~~
../../hw/display/virtio-gpu-virgl.c:758:28: note: (near initialization for ‘virtio_gpu_3d_cbs.get_drm_fd’)
../../hw/display/virtio-gpu-virgl.c: In function ‘virtio_gpu_virgl_init’:
../../hw/display/virtio-gpu-virgl.c:816:34: error: ‘VIRGL_RENDERER_VENUS’ undeclared (first use in this function); did you mean ‘VIRGL_RENDERER_USE_EGL’?
816 | ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs);
| ^~~~~~~~~~~~~~~~~~~~
| VIRGL_RENDERER_USE_EGL
../../hw/display/virtio-gpu-virgl.c:816:34: note: each undeclared identifier is reported only once for each function it appears in
cc1: all warnings being treated as errors
I assume because I'm either missing a library or my distro version is
too old. Either way this needs to be caught at configure time and
#ifdef'd.
> };
>
> static void virtio_gpu_print_stats(void *opaque)
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver
2022-09-26 14:24 [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver Antonio Caggiano
` (8 preceding siblings ...)
2022-09-26 14:24 ` [PATCH v3 9/9] virtio-gpu: Get EGL Display callback Antonio Caggiano
@ 2023-01-30 17:00 ` Alex Bennée
2023-01-31 23:14 ` Dmitry Osipenko
9 siblings, 1 reply; 26+ messages in thread
From: Alex Bennée @ 2023-01-30 17:00 UTC (permalink / raw)
To: Antonio Caggiano; +Cc: gert.wollny, dmitry.osipenko, qemu-devel
Antonio Caggiano <antonio.caggiano@collabora.com> writes:
> This series of patches enables support for the Venus VirtIO-GPU Vulkan
> driver by adding some features required by the driver:
>
> - CONTEXT_INIT
> - HOSTMEM
> - RESOURCE_UUID
> - BLOB_RESOURCES
>
> In addition to these features, Venus capset support was required
> together with the implementation for Virgl blob resource commands.
I managed to apply to current master but I needed a bunch of patches to
get it to compile with my old virgl:
--8<---------------cut here---------------start------------->8---
modified hw/display/virtio-gpu-virgl.c
@@ -744,10 +744,12 @@ static int virgl_make_context_current(void *opaque, int scanout_idx,
qctx);
}
+#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
static void *virgl_get_egl_display(void *opaque)
{
return eglGetCurrentDisplay();
}
+#endif
static struct virgl_renderer_callbacks virtio_gpu_3d_cbs = {
.version = 4,
@@ -755,7 +757,9 @@ static struct virgl_renderer_callbacks virtio_gpu_3d_cbs = {
.create_gl_context = virgl_create_context,
.destroy_gl_context = virgl_destroy_context,
.make_current = virgl_make_context_current,
+#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
.get_egl_display = virgl_get_egl_display,
+#endif
};
static void virtio_gpu_print_stats(void *opaque)
@@ -813,7 +817,7 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
{
int ret;
- ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS, &virtio_gpu_3d_cbs);
+ ret = virgl_renderer_init(g, 0 /* VIRGL_RENDERER_VENUS */, &virtio_gpu_3d_cbs);
if (ret != 0) {
error_report("virgl could not be initialized: %d", ret);
return ret;
modified hw/display/virtio-gpu.c
@@ -873,9 +873,12 @@ void virtio_gpu_cleanup_mapping_iov(VirtIOGPU *g,
static void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
struct virtio_gpu_simple_resource *res)
{
+
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
if (res->mapped) {
virtio_gpu_virgl_resource_unmap(g, res);
}
+#endif
virtio_gpu_cleanup_mapping_iov(g, res->iov, res->iov_cnt);
res->iov = NULL;
--8<---------------cut here---------------end--------------->8---
However when I run it with:
gdb --args ./qemu-system-aarch64 \
-cpu max,pauth-impdef=on \
-machine type=virt,virtualization=on,gic-version=3 \
-serial mon:stdio \
-netdev user,id=unet,hostfwd=tcp::2222-:22 \
-device virtio-net-pci,netdev=unet,id=virt-net,disable-legacy=on \
-device virtio-scsi-pci,id=virt-scsi,disable-legacy=on \
-blockdev driver=raw,node-name=hd,discard=unmap,file.driver=host_device,file.filename=/dev/zen-disk/debian-bullseye-arm64 \
-device scsi-hd,drive=hd,id=virt-scsi-hd \
-kernel $HOME/lsrc/linux.git/builds/arm64/arch/arm64/boot/Image \
-append "root=/dev/sda2" \
-smp 4 -m 4096 \
-object memory-backend-memfd,id=mem,size=4G,share=on \
-numa node,memdev=mem \
-device qemu-xhci \
-device usb-tablet \
-device usb-kbd -global virtio-mmio.force-legacy=false \
-display gtk,gl=on -device virtio-gpu-pci
something must be broken because it asserts:
qemu-system-aarch64: ../../hw/core/qdev.c:282: qdev_realize: Assertion `!dev->realized && !dev->parent_bus' failed.
Thread 1 "qemu-system-aar" received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x00007ffff5309537 in __GI_abort () at abort.c:79
#2 0x00007ffff530940f in __assert_fail_base (fmt=0x7ffff54816a8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x5555562da640 "!dev->realized && !dev->parent_bus",
file=0x5555562da6a7 "../../hw/core/qdev.c", line=282, function=<optimized out>) at assert.c:92
#3 0x00007ffff5318662 in __GI___assert_fail (assertion=assertion@entry=0x5555562da640 "!dev->realized && !dev->parent_bus", file=file@entry=0x5555562da6a7 "../../hw/core/qd
ev.c", line=line@entry=282, function=function@entry=0x5555562da868 <__PRETTY_FUNCTION__.14> "qdev_realize") at assert.c:101
#4 0x0000555555f64b6f in qdev_realize (dev=dev@entry=0x555558251370, bus=<optimized out>, errp=errp@entry=0x7fffffffd670) at ../../hw/core/qdev.c:282
#5 0x0000555555bbecaa in virtio_gpu_pci_base_realize (vpci_dev=0x555558248fa0, errp=0x7fffffffd670) at ../../hw/display/virtio-gpu-pci.c:52
#6 0x0000555555a6048d in pci_qdev_realize (qdev=0x555558248fa0, errp=<optimized out>) at ../../hw/pci/pci.c:2043
#7 0x0000555555f6416e in device_set_realized (obj=<optimized out>, value=<optimized out>, errp=0x7fffffffd880) at ../../hw/core/qdev.c:510
#8 0x0000555555f67ea6 in property_set_bool (obj=0x555558248fa0, v=<optimized out>, name=<optimized out>, opaque=0x555556c35ab0, errp=0x7fffffffd880) at ../../qom/object.c:2
285
#9 0x0000555555f6aee4 in object_property_set (obj=obj@entry=0x555558248fa0, name=name@entry=0x555556231289 "realized", v=v@entry=0x5555582545a0, errp=errp@entry=0x7fffffffd
880) at ../../qom/object.c:1420
#10 0x0000555555f6e290 in object_property_set_qobject (obj=obj@entry=0x555558248fa0, name=name@entry=0x555556231289 "realized", value=value@entry=0x555558253390, errp=errp@e
ntry=0x7fffffffd880) at ../../qom/qom-qobject.c:28
#11 0x0000555555f6b505 in object_property_set_bool (obj=0x555558248fa0, name=name@entry=0x555556231289 "realized", value=value@entry=true, errp=errp@entry=0x7fffffffd880) at
../../qom/object.c:1489
#12 0x0000555555f64aee in qdev_realize (dev=<optimized out>, bus=bus@entry=0x555557696f70, errp=errp@entry=0x7fffffffd880) at ../../hw/core/qdev.c:292
#13 0x0000555555b36d26 in qdev_device_add_from_qdict (opts=opts@entry=0x555557d52d40, from_json=from_json@entry=false, errp=0x7fffffffd880, errp@entry=0x555556b02790 <error_
fatal>) at ../../softmmu/qdev-monitor.c:714
#14 0x0000555555b36e42 in qdev_device_add (opts=0x555556c31d20, errp=errp@entry=0x555556b02790 <error_fatal>) at ../../softmmu/qdev-monitor.c:733
#15 0x0000555555b38c4f in device_init_func (opaque=<optimized out>, opts=<optimized out>, errp=0x555556b02790 <error_fatal>) at ../../softmmu/vl.c:1143
#16 0x00005555560e6382 in qemu_opts_foreach (list=<optimized out>, func=func@entry=0x555555b38c40 <device_init_func>, opaque=opaque@entry=0x0, errp=0x555556b02790 <error_fat
al>) at ../../util/qemu-option.c:1135
#17 0x0000555555b3b4ae in qemu_create_cli_devices () at ../../softmmu/vl.c:2539
#18 qmp_x_exit_preconfig (errp=<optimized out>) at ../../softmmu/vl.c:2607
#19 0x0000555555b3ee5d in qmp_x_exit_preconfig (errp=<optimized out>) at ../../softmmu/vl.c:2601
#20 qemu_init (argc=<optimized out>, argv=<optimized out>) at ../../softmmu/vl.c:3613
#21 0x00005555558b3fa9 in main (argc=<optimized out>, argv=<optimized out>) at
(gdb) p dev
$1 = (DeviceState *) 0x555558251370
(gdb) p *$
$2 = {parent_obj = {class = 0x555556e36b10, free = 0x0, properties = 0x555558204c60, ref = 2, parent = 0x555558248fa0}, id = 0x0, canonical_path = 0x0, realized = false, pending_deleted_event = false, pending_deleted_expires_ms = 0, opts = 0x0, hotplugged = 0, allow_unplug_during_migration = false, parent_bus = 0x5555582512e0, gpios = {lh_first = 0x0}, clocks = {lh_first = 0x0}, child_bus = {lh_first = 0x0}, num_child_bus = 0, instance_id_alias = -1, alias_required_for_version = 0, reset = {count = 0, hold_phase_pending = false, exit_phase_in_progress = false}, unplug_blockers = 0x0}
(gdb) p dev->realized
$3 = false
(gdb) p dev->parent_bus
$4 = (BusState *) 0x5555582512e0
(gdb) p *$
$5 = {obj = {class = 0x555556e192e0, free = 0x0, properties = 0x555558204aa0 = {[0x555558259d90 "hotplug-handler"] = 0x555558259ec0, [0x55555825a1e0 "child[0]"] = 0x55555825a180, [0x555558259d70 "realized"] = 0x555558259fe0}, ref = 2, parent = 0x555558248fa0}, parent = 0x555558248fa0, name = 0x55555825a040 "virtio-bus", hotplug_handler = 0x0, max_index = 1, realized = false, full = false, num_children = 1, children = {tqh_first = 0x55555825a120, tqh_circ = {tql_next = 0x55555825a120, tql_prev = 0x55555825a140}}, sibling = {le_next = 0x0, le_prev = 0x555558249010}, reset = {count = 0, hold_phase_pending = false, exit_phase_in_progress = false}}
(gdb)
>
> Antonio Caggiano (7):
> virtio-gpu: Handle resource blob commands
> virtio-gpu: CONTEXT_INIT feature
> virtio-gpu: Unrealize
> virtio-gpu: Resource UUID
> virtio-gpu: Support Venus capset
> virtio-gpu: Initialize Venus
> virtio-gpu: Get EGL Display callback
>
> Dr. David Alan Gilbert (1):
> virtio: Add shared memory capability
>
> Gerd Hoffmann (1):
> virtio-gpu: hostmem
>
> hw/display/trace-events | 1 +
> hw/display/virtio-gpu-base.c | 7 +-
> hw/display/virtio-gpu-pci.c | 15 ++
> hw/display/virtio-gpu-virgl.c | 230 +++++++++++++++++++-
> hw/display/virtio-gpu.c | 67 +++++-
> hw/display/virtio-vga.c | 33 ++-
> hw/virtio/virtio-pci.c | 18 ++
> include/hw/virtio/virtio-gpu-bswap.h | 18 ++
> include/hw/virtio/virtio-gpu.h | 21 ++
> include/hw/virtio/virtio-pci.h | 4 +
> include/standard-headers/linux/virtio_gpu.h | 2 +
> meson.build | 9 +
> 12 files changed, 403 insertions(+), 22 deletions(-)
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver
2023-01-30 17:00 ` [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver Alex Bennée
@ 2023-01-31 23:14 ` Dmitry Osipenko
2023-03-06 22:41 ` Gurchetan Singh
0 siblings, 1 reply; 26+ messages in thread
From: Dmitry Osipenko @ 2023-01-31 23:14 UTC (permalink / raw)
To: Alex Bennée; +Cc: gert.wollny, qemu-devel
Hello,
On 1/30/23 20:00, Alex Bennée wrote:
>
> Antonio Caggiano <antonio.caggiano@collabora.com> writes:
>
>> This series of patches enables support for the Venus VirtIO-GPU Vulkan
>> driver by adding some features required by the driver:
>>
>> - CONTEXT_INIT
>> - HOSTMEM
>> - RESOURCE_UUID
>> - BLOB_RESOURCES
>>
>> In addition to these features, Venus capset support was required
>> together with the implementation for Virgl blob resource commands.
>
> I managed to apply to current master but I needed a bunch of patches to
> get it to compile with my old virgl:
Thank you for reviewing and testing the patches! Antonio isn't working
on Venus anymore, I'm going to continue this effort. Last year we
stabilized some of the virglrenderer Venus APIs, this year Venus may
transition to supporting per-context fences only and require to init a
renderserver, which will result in a more changes to Qemu. I'm going to
wait a bit for Venus to settle down and then make a v4.
In the end we will either need to add more #ifdefs if we will want to
keep supporting older virglrenderer versions in Qemu, or bump the min
required virglrenderer version.
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver
2023-01-31 23:14 ` Dmitry Osipenko
@ 2023-03-06 22:41 ` Gurchetan Singh
2023-03-13 12:58 ` Marc-André Lureau
0 siblings, 1 reply; 26+ messages in thread
From: Gurchetan Singh @ 2023-03-06 22:41 UTC (permalink / raw)
To: Dmitry Osipenko; +Cc: Alex Bennée, gert.wollny, qemu-devel
On Tue, Jan 31, 2023 at 3:15 PM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
>
> Hello,
>
> On 1/30/23 20:00, Alex Bennée wrote:
> >
> > Antonio Caggiano <antonio.caggiano@collabora.com> writes:
> >
> >> This series of patches enables support for the Venus VirtIO-GPU Vulkan
> >> driver by adding some features required by the driver:
> >>
> >> - CONTEXT_INIT
> >> - HOSTMEM
> >> - RESOURCE_UUID
> >> - BLOB_RESOURCES
> >>
> >> In addition to these features, Venus capset support was required
> >> together with the implementation for Virgl blob resource commands.
> >
> > I managed to apply to current master but I needed a bunch of patches to
> > get it to compile with my old virgl:
>
> Thank you for reviewing and testing the patches! Antonio isn't working
> on Venus anymore, I'm going to continue this effort. Last year we
> stabilized some of the virglrenderer Venus APIs, this year Venus may
> transition to supporting per-context fences only and require to init a
> renderserver, which will result in a more changes to Qemu. I'm going to
> wait a bit for Venus to settle down and then make a v4.
>
> In the end we will either need to add more #ifdefs if we will want to
> keep supporting older virglrenderer versions in Qemu, or bump the min
> required virglrenderer version.
Hi Dmitry,
Thanks for working on this, it's great to see QEMU graphics moving
forward. I noticed a few things from your patchset:
1) Older versions of virglrenderer -- supported or not?
As you alluded to, there have been significant changes to
virglrenderer since the last QEMU graphics update. For example, the
asynchronous callback introduces an entirely different and
incompatible way to signal fence completion.
Notionally, QEMU must support older versions of virglrenderer, though
in practice I'm not sure how much that is true. If we want to keep up
the notion that older versions must be supported, you'll need:
a) virtio-gpu-virgl.c
b) virtio-gpu-virgl2.c (or an equivalent)
Similarly for the vhost-user paths (if you want to support that). If
older versions of virglrenderer don't need to be supported, then that
would simplify the amount of additional paths/#ifdefs.
2) Additional context type: gfxstream [i]?
One of the major motivations for adding context types in the
virtio-gpu spec was supporting gfxstream. gfxstream is used in the
Android Studio emulator (a variant of QEMU) [ii], among other places.
That would move the Android emulator closer to the goal of using
upstream QEMU for everything.
If (1) is resolved, I don't think it's actually too bad to add
gfxstream support. We just need an additional layer of dispatch
between virglrenderer and gfxstream (thus, virtio-gpu-virgl2.c would
be renamed virtio-gpu-context-types.c or something similar). The QEMU
command line will have to be modified to pass in the enabled context
type (--context={virgl, venus, gfxstream}). crosvm has been using the
same trick.
If (1) is resolved in v4, I would estimate adding gfxstream support
would at max take 1-2 months for a single engineer. I'm not saying
gfxstream need necessarily be a part of a v5 patch-stack, but given
this patch-stack has been around for 1 year plus, it certainly could
be. We can certainly design things in such a way that adding
gfxstream is easy subsequently.
The hardest part is actually package management (Debian) for
gfxstream, but those can be resolved.
I'm not sure exactly how QEMU accelerated graphics are utilized in
user-facing actual products currently, so not sure what the standard
is.
What do QEMU maintainers and users think about these issues,
particularly about the potential gfxstream addition in QEMU as a
context type? We are most interested in Android guests.
[i] https://android.googlesource.com/device/generic/vulkan-cereal/
[ii] https://developer.android.com/studio/run/emulator
>
> --
> Best regards,
> Dmitry
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver
2023-03-06 22:41 ` Gurchetan Singh
@ 2023-03-13 12:58 ` Marc-André Lureau
2023-03-13 13:27 ` Dmitry Osipenko
2023-03-13 18:44 ` Gurchetan Singh
0 siblings, 2 replies; 26+ messages in thread
From: Marc-André Lureau @ 2023-03-13 12:58 UTC (permalink / raw)
To: Gurchetan Singh
Cc: Dmitry Osipenko, Alex Bennée, gert.wollny, qemu-devel
Hi Gurchetan
On Tue, Mar 7, 2023 at 2:41 AM Gurchetan Singh
<gurchetansingh@chromium.org> wrote:
>
> On Tue, Jan 31, 2023 at 3:15 PM Dmitry Osipenko
> <dmitry.osipenko@collabora.com> wrote:
> >
> > Hello,
> >
> > On 1/30/23 20:00, Alex Bennée wrote:
> > >
> > > Antonio Caggiano <antonio.caggiano@collabora.com> writes:
> > >
> > >> This series of patches enables support for the Venus VirtIO-GPU Vulkan
> > >> driver by adding some features required by the driver:
> > >>
> > >> - CONTEXT_INIT
> > >> - HOSTMEM
> > >> - RESOURCE_UUID
> > >> - BLOB_RESOURCES
> > >>
> > >> In addition to these features, Venus capset support was required
> > >> together with the implementation for Virgl blob resource commands.
> > >
> > > I managed to apply to current master but I needed a bunch of patches to
> > > get it to compile with my old virgl:
> >
> > Thank you for reviewing and testing the patches! Antonio isn't working
> > on Venus anymore, I'm going to continue this effort. Last year we
> > stabilized some of the virglrenderer Venus APIs, this year Venus may
> > transition to supporting per-context fences only and require to init a
> > renderserver, which will result in a more changes to Qemu. I'm going to
> > wait a bit for Venus to settle down and then make a v4.
> >
> > In the end we will either need to add more #ifdefs if we will want to
> > keep supporting older virglrenderer versions in Qemu, or bump the min
> > required virglrenderer version.
>
> Hi Dmitry,
>
> Thanks for working on this, it's great to see QEMU graphics moving
> forward. I noticed a few things from your patchset:
>
> 1) Older versions of virglrenderer -- supported or not?
>
> As you alluded to, there have been significant changes to
> virglrenderer since the last QEMU graphics update. For example, the
> asynchronous callback introduces an entirely different and
> incompatible way to signal fence completion.
>
> Notionally, QEMU must support older versions of virglrenderer, though
> in practice I'm not sure how much that is true. If we want to keep up
> the notion that older versions must be supported, you'll need:
>
> a) virtio-gpu-virgl.c
> b) virtio-gpu-virgl2.c (or an equivalent)
>
> Similarly for the vhost-user paths (if you want to support that). If
> older versions of virglrenderer don't need to be supported, then that
> would simplify the amount of additional paths/#ifdefs.
We should support old versions of virgl (as described in
https://www.qemu.org/docs/master/about/build-platforms.html#linux-os-macos-freebsd-netbsd-openbsd).
Whether a new virtio-gpu-virgl2. (or equivalent) is necessary, we
can't really tell without seeing the changes involved.
>
> 2) Additional context type: gfxstream [i]?
>
> One of the major motivations for adding context types in the
> virtio-gpu spec was supporting gfxstream. gfxstream is used in the
> Android Studio emulator (a variant of QEMU) [ii], among other places.
> That would move the Android emulator closer to the goal of using
> upstream QEMU for everything.
What is the advantage of using gfxstream over virgl? or zink+venus?
Only AOSP can run with virgl perhaps? I am not familiar with Android
development.. I guess it doesn't make use of Mesa, and thus no virgl
at all?
>
> If (1) is resolved, I don't think it's actually too bad to add
> gfxstream support. We just need an additional layer of dispatch
> between virglrenderer and gfxstream (thus, virtio-gpu-virgl2.c would
> be renamed virtio-gpu-context-types.c or something similar). The QEMU
> command line will have to be modified to pass in the enabled context
> type (--context={virgl, venus, gfxstream}). crosvm has been using the
> same trick.
>
> If (1) is resolved in v4, I would estimate adding gfxstream support
> would at max take 1-2 months for a single engineer. I'm not saying
> gfxstream need necessarily be a part of a v5 patch-stack, but given
> this patch-stack has been around for 1 year plus, it certainly could
> be. We can certainly design things in such a way that adding
> gfxstream is easy subsequently.
>
> The hardest part is actually package management (Debian) for
> gfxstream, but those can be resolved.
>
It looks like gfxstream is actually offering an API similar to
virlgrenderer (with "pipe_" prefix). I suppose the guest needs to be
configured in a special way then (how? without mesa?).
> I'm not sure exactly how QEMU accelerated graphics are utilized in
> user-facing actual products currently, so not sure what the standard
> is.
>
> What do QEMU maintainers and users think about these issues,
> particularly about the potential gfxstream addition in QEMU as a
> context type? We are most interested in Android guests.
It would be great if the Android emulator was more aligned with
upstream QEMU development!
thanks
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver
2023-03-13 12:58 ` Marc-André Lureau
@ 2023-03-13 13:27 ` Dmitry Osipenko
2023-03-13 14:51 ` Alex Bennée
2023-03-13 18:44 ` Gurchetan Singh
1 sibling, 1 reply; 26+ messages in thread
From: Dmitry Osipenko @ 2023-03-13 13:27 UTC (permalink / raw)
To: Marc-André Lureau, Gurchetan Singh
Cc: Alex Bennée, gert.wollny, qemu-devel
On 3/13/23 15:58, Marc-André Lureau wrote:
...
>> 2) Additional context type: gfxstream [i]?
>>
>> One of the major motivations for adding context types in the
>> virtio-gpu spec was supporting gfxstream. gfxstream is used in the
>> Android Studio emulator (a variant of QEMU) [ii], among other places.
>> That would move the Android emulator closer to the goal of using
>> upstream QEMU for everything.
>
> What is the advantage of using gfxstream over virgl? or zink+venus?
>
> Only AOSP can run with virgl perhaps? I am not familiar with Android
> development.. I guess it doesn't make use of Mesa, and thus no virgl
> at all?
+1 I'm also very interested in getting an overview of gfxstream
advantages over virgl and why Android emulator can't move to use
virgl+venus (shouldn't it just work out-of-the-box already?). Thanks!
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver
2023-03-13 13:27 ` Dmitry Osipenko
@ 2023-03-13 14:51 ` Alex Bennée
2023-03-13 15:03 ` Dmitry Osipenko
0 siblings, 1 reply; 26+ messages in thread
From: Alex Bennée @ 2023-03-13 14:51 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Marc-André Lureau, Gurchetan Singh, gert.wollny, QEMU Developers
[-- Attachment #1: Type: text/plain, Size: 1161 bytes --]
If gfxstream is the android pipe based transport I think it's a legacy from
before the switch to pure VirtIO for the new Cuttlefish models.
On Mon, 13 Mar 2023, 13:27 Dmitry Osipenko, <dmitry.osipenko@collabora.com>
wrote:
> On 3/13/23 15:58, Marc-André Lureau wrote:
> ...
> >> 2) Additional context type: gfxstream [i]?
> >>
> >> One of the major motivations for adding context types in the
> >> virtio-gpu spec was supporting gfxstream. gfxstream is used in the
> >> Android Studio emulator (a variant of QEMU) [ii], among other places.
> >> That would move the Android emulator closer to the goal of using
> >> upstream QEMU for everything.
> >
> > What is the advantage of using gfxstream over virgl? or zink+venus?
> >
> > Only AOSP can run with virgl perhaps? I am not familiar with Android
> > development.. I guess it doesn't make use of Mesa, and thus no virgl
> > at all?
>
> +1 I'm also very interested in getting an overview of gfxstream
> advantages over virgl and why Android emulator can't move to use
> virgl+venus (shouldn't it just work out-of-the-box already?). Thanks!
>
> --
> Best regards,
> Dmitry
>
>
[-- Attachment #2: Type: text/html, Size: 1560 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver
2023-03-13 14:51 ` Alex Bennée
@ 2023-03-13 15:03 ` Dmitry Osipenko
0 siblings, 0 replies; 26+ messages in thread
From: Dmitry Osipenko @ 2023-03-13 15:03 UTC (permalink / raw)
To: Alex Bennée
Cc: Marc-André Lureau, Gurchetan Singh, gert.wollny, QEMU Developers
On 3/13/23 17:51, Alex Bennée wrote:
> If gfxstream is the android pipe based transport I think it's a legacy from
> before the switch to pure VirtIO for the new Cuttlefish models.
Right, so older Android versions will only work using gfxstream. Good
point, thanks.
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver
2023-03-13 12:58 ` Marc-André Lureau
2023-03-13 13:27 ` Dmitry Osipenko
@ 2023-03-13 18:44 ` Gurchetan Singh
2023-03-17 21:40 ` Gurchetan Singh
1 sibling, 1 reply; 26+ messages in thread
From: Gurchetan Singh @ 2023-03-13 18:44 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Dmitry Osipenko, Alex Bennée, gert.wollny, qemu-devel
On Mon, Mar 13, 2023 at 5:58 AM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi Gurchetan
>
> On Tue, Mar 7, 2023 at 2:41 AM Gurchetan Singh
> <gurchetansingh@chromium.org> wrote:
> >
> > On Tue, Jan 31, 2023 at 3:15 PM Dmitry Osipenko
> > <dmitry.osipenko@collabora.com> wrote:
> > >
> > > Hello,
> > >
> > > On 1/30/23 20:00, Alex Bennée wrote:
> > > >
> > > > Antonio Caggiano <antonio.caggiano@collabora.com> writes:
> > > >
> > > >> This series of patches enables support for the Venus VirtIO-GPU Vulkan
> > > >> driver by adding some features required by the driver:
> > > >>
> > > >> - CONTEXT_INIT
> > > >> - HOSTMEM
> > > >> - RESOURCE_UUID
> > > >> - BLOB_RESOURCES
> > > >>
> > > >> In addition to these features, Venus capset support was required
> > > >> together with the implementation for Virgl blob resource commands.
> > > >
> > > > I managed to apply to current master but I needed a bunch of patches to
> > > > get it to compile with my old virgl:
> > >
> > > Thank you for reviewing and testing the patches! Antonio isn't working
> > > on Venus anymore, I'm going to continue this effort. Last year we
> > > stabilized some of the virglrenderer Venus APIs, this year Venus may
> > > transition to supporting per-context fences only and require to init a
> > > renderserver, which will result in a more changes to Qemu. I'm going to
> > > wait a bit for Venus to settle down and then make a v4.
> > >
> > > In the end we will either need to add more #ifdefs if we will want to
> > > keep supporting older virglrenderer versions in Qemu, or bump the min
> > > required virglrenderer version.
> >
> > Hi Dmitry,
> >
> > Thanks for working on this, it's great to see QEMU graphics moving
> > forward. I noticed a few things from your patchset:
> >
> > 1) Older versions of virglrenderer -- supported or not?
> >
> > As you alluded to, there have been significant changes to
> > virglrenderer since the last QEMU graphics update. For example, the
> > asynchronous callback introduces an entirely different and
> > incompatible way to signal fence completion.
> >
> > Notionally, QEMU must support older versions of virglrenderer, though
> > in practice I'm not sure how much that is true. If we want to keep up
> > the notion that older versions must be supported, you'll need:
> >
> > a) virtio-gpu-virgl.c
> > b) virtio-gpu-virgl2.c (or an equivalent)
> >
> > Similarly for the vhost-user paths (if you want to support that). If
> > older versions of virglrenderer don't need to be supported, then that
> > would simplify the amount of additional paths/#ifdefs.
>
> We should support old versions of virgl (as described in
> https://www.qemu.org/docs/master/about/build-platforms.html#linux-os-macos-freebsd-netbsd-openbsd).
>
> Whether a new virtio-gpu-virgl2. (or equivalent) is necessary, we
> can't really tell without seeing the changes involved.
Ack. Something to keep in mind as Dmitry refactors.
>
> >
> > 2) Additional context type: gfxstream [i]?
> >
> > One of the major motivations for adding context types in the
> > virtio-gpu spec was supporting gfxstream. gfxstream is used in the
> > Android Studio emulator (a variant of QEMU) [ii], among other places.
> > That would move the Android emulator closer to the goal of using
> > upstream QEMU for everything.
>
> What is the advantage of using gfxstream over virgl? or zink+venus?
History/backstory:
gfxstream development has its roots in the development of the Android
Emulator (circa 2010). In those days, both DRM and Android were
relatively new and the communities didn't know much about each other.
A method was devised to auto-generate GLES calls (that's all Android
needed) and stream it over an interface very similar to pipe(..).
Host generated IDs were used to track shareable buffers.
That same method used to auto-generate GLES was expanded to Vulkan and
support for coherent memory was added. In 2018 the Android Emulator
was the first to ship CTS-compliant virtualized Vulkan via downstream
kernel interfaces, before work on venus began.
As virtio-gpu continued to mature, gfxstream was actually the first to
ship both blob resources [1] and context types [2] in production via
crosvm to form a completely upstreamable solution (I consider AOSP to
be an "upstream" as well).
[1] https://patchwork.kernel.org/project/dri-devel/cover/20200814024000.2485-1-gurchetansingh@chromium.org/
[2] https://lists.oasis-open.org/archives/virtio-dev/202108/msg00141.html
With this history out of the way, here are some advantages of
gfxstream GLES over virgl:
- gfxstream GLES actually has much less rendering artifacts than virgl
since it's autogenerated and not hand-written. Using an Gallium
command stream is lossy (partly since the GLES spec is ambiguous and
drivers are buggy), and we always had better dEQP runs on gfxstream
GLES than on virgl (especially on closed source drivers).
- Better memory management: virgl makes heavy use of
RESOURCE_CREATE_3D, which creates shadow buffers for every GL
texture/buffer. gfxstream just uses a single guest memory buffer per
DRM instance for buffer/texture upload. For example, gfxstream
doesn't need the virtio-gpu shrinker as much [3] since it doesn't use
as much guest memory. Though I know there have been recent fixes for
this in virgl, but talking from a design POV.
- Performance: In 2020, a vendor ran the GPU emulation stress test
comparing virgl and gfxstream GLES. Here are some results:
CVD: drm_virgl - 7.01 fps
CVD: gfxstream - 43.68 fps
I've seen similarly dramatic results with gfxbench/3D Mark on some
automotive platforms.
- Multi-threaded by design: gfxstream GLES is multi-threaded by
design. Each guest GL thread get's its own host thread to decod
commands. virgl is single threaded (before the asynchronous callback,
which hasn't landed in QEMU yet)
- Cross-platform: Windows, MacOS, and Linux support (though with
downstream QEMU patches unfortunately). virgl is more a Linux thing.
- Snapshots: Not possible with virgl. We don't intend to upstream
live migration snapshot support in the initial CL, but that's
something to note that users like.
gfxstream is the "native" solution for Android and is thus better
optimized, just like virgl is the native solution for Linux guests.
Re: Zink/ANGLE/venus versus ANGLE/gfxstream VK
venus in many ways has similar design characteristics as gfxstream VK
(auto-generated, multi-threaded). gfxstream VK has better
cross-platform support, with it shipping on via the Android emulator
and Google Play Games [4] on PC. venus is designed with open-source
Linux platforms in mind, with Chromebook gaming as the initial use
case [5].
That leads to different design decisions, mostly centered around
resource sharing/state-tracking. Snapshots are also a goal for
gfxstream VK, though not ready yet.
Both venus and gfxstream are Google-sponsored. There were meetings
between Android and ChromeOS bigwigs about gfxstream VK/venus in 2019,
and the outcome seemed to be "we'll share work where it makes sense,
but there might not be a one-size fits all solution".
Layering which passes CTS is expected to take quite a while,
especially for a cross-platform target such as the emulator. It would
be great to have gfxstream GLES support alone in the interim.
[3] https://lore.kernel.org/lkml/20230305221011.1404672-1-dmitry.osipenko@collabora.com/
[4] https://play.google.com/googleplaygames
[5] https://www.xda-developers.com/how-to-run-steam-chromebook/
>
> Only AOSP can run with virgl perhaps? I am not familiar with Android
> development.. I guess it doesn't make use of Mesa, and thus no virgl
> at all?
Some AOSP targets (Cuttlefish) can use virgl along with gfxstream,
just for testing sake. It's not hard to support both via crosvm, so
we do it.
https://source.android.com/docs/setup/create/cuttlefish-ref-gpu
The Android Emulator (the most relevant use case here) does ship
gfxstream when a developer uses Android Studio though, and plans to do
so in the future.
>
> >
> > If (1) is resolved, I don't think it's actually too bad to add
> > gfxstream support. We just need an additional layer of dispatch
> > between virglrenderer and gfxstream (thus, virtio-gpu-virgl2.c would
> > be renamed virtio-gpu-context-types.c or something similar). The QEMU
> > command line will have to be modified to pass in the enabled context
> > type (--context={virgl, venus, gfxstream}). crosvm has been using the
> > same trick.
> >
> > If (1) is resolved in v4, I would estimate adding gfxstream support
> > would at max take 1-2 months for a single engineer. I'm not saying
> > gfxstream need necessarily be a part of a v5 patch-stack, but given
> > this patch-stack has been around for 1 year plus, it certainly could
> > be. We can certainly design things in such a way that adding
> > gfxstream is easy subsequently.
> >
> > The hardest part is actually package management (Debian) for
> > gfxstream, but those can be resolved.
> >
>
> It looks like gfxstream is actually offering an API similar to
> virlgrenderer (with "pipe_" prefix).
For gfxstream, my ideal solution would not use that "pipe_" API
directly from QEMU (though vulkan-cereal will be packaged properly).
Instead, I intend to package the "rutabaga_gfx_ffi.h" proxy library
over gfxstream [6]:
https://chromium.googlesource.com/chromiumos/platform/crosvm/+/refs/heads/main/rutabaga_gfx/ffi/src/include/rutabaga_gfx_ffi.h
The advantage with this approach is one gets Wayland passthrough [7]
with Linux guests which is written in Rust , along with gfxstream.
The main issues are around Debian Rust packaging.
Rough sketch, here's what I think we might need:
a) virtio-gpu-virgl-legacy.c for older versions of virglrenderer
b) virtio-gpu-virgl2.c
c) virtio-gpu-rutabaga.c or virtio-gpu-gfxstream.c (depending on rust
packaging investigations)
Though Wayland passthrough is a "nice to have", upstreaming gfxstream
for Android Emulator is the most important product goal. So if Rust
Debian packaging becomes too onerous (virtio-gpu-rutabaga.c), we can
backtrack to a simpler solution (virtio-gpu-gfxstream.c).
[6] it can also proxy virglrenderer calls too, but I'll that decision
to virglrenderer maintainers
[7] try out the feature here: https://crosvm.dev/book/devices/wayland.html
> I suppose the guest needs to be
> configured in a special way then (how? without mesa?).
For AOSP, androidboot.hardware.vulkan and androidboot.hardware.egl
allow toggling of GLES and Vulkan impls. QEMU
won't have to do anything special given the way the launchers are
designed (there's an equivalent of a "virtmanager").
There needs to be logic around context selection for Linux guests.
QEMU needs a "--ctx_type={virgl, venus, drm, gfxstream}" argument.
See crosvm for an example:
https://chromium.googlesource.com/chromiumos/platform/crosvm/+/refs/heads/main/rutabaga_gfx/src/rutabaga_core.rs#910
This argument is important for Linux upcoming "DRM native" context
types [8] as well.
[8] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21658
>
> > I'm not sure exactly how QEMU accelerated graphics are utilized in
> > user-facing actual products currently, so not sure what the standard
> > is.
> >
> > What do QEMU maintainers and users think about these issues,
> > particularly about the potential gfxstream addition in QEMU as a
> > context type? We are most interested in Android guests.
>
> It would be great if the Android emulator was more aligned with
> upstream QEMU development!
Awesome! I envisage the initial gfxstream integration as just a first
step. With the graphics solution upstreamed, subsequent MacOS/Windows
specific patches will start to make more sense.
>
> thanks
>
> --
> Marc-André Lureau
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver
2023-03-13 18:44 ` Gurchetan Singh
@ 2023-03-17 21:40 ` Gurchetan Singh
2023-03-22 8:00 ` Michael Tokarev
0 siblings, 1 reply; 26+ messages in thread
From: Gurchetan Singh @ 2023-03-17 21:40 UTC (permalink / raw)
To: Marc-André Lureau
Cc: Dmitry Osipenko, Alex Bennée, gert.wollny, qemu-devel, mjt,
pkg-qemu-devel
[-- Attachment #1: Type: text/plain, Size: 14513 bytes --]
On Mon, Mar 13, 2023 at 11:44 AM Gurchetan Singh <
gurchetansingh@chromium.org> wrote:
> On Mon, Mar 13, 2023 at 5:58 AM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > Hi Gurchetan
> >
> > On Tue, Mar 7, 2023 at 2:41 AM Gurchetan Singh
> > <gurchetansingh@chromium.org> wrote:
> > >
> > > On Tue, Jan 31, 2023 at 3:15 PM Dmitry Osipenko
> > > <dmitry.osipenko@collabora.com> wrote:
> > > >
> > > > Hello,
> > > >
> > > > On 1/30/23 20:00, Alex Bennée wrote:
> > > > >
> > > > > Antonio Caggiano <antonio.caggiano@collabora.com> writes:
> > > > >
> > > > >> This series of patches enables support for the Venus VirtIO-GPU
> Vulkan
> > > > >> driver by adding some features required by the driver:
> > > > >>
> > > > >> - CONTEXT_INIT
> > > > >> - HOSTMEM
> > > > >> - RESOURCE_UUID
> > > > >> - BLOB_RESOURCES
> > > > >>
> > > > >> In addition to these features, Venus capset support was required
> > > > >> together with the implementation for Virgl blob resource commands.
> > > > >
> > > > > I managed to apply to current master but I needed a bunch of
> patches to
> > > > > get it to compile with my old virgl:
> > > >
> > > > Thank you for reviewing and testing the patches! Antonio isn't
> working
> > > > on Venus anymore, I'm going to continue this effort. Last year we
> > > > stabilized some of the virglrenderer Venus APIs, this year Venus may
> > > > transition to supporting per-context fences only and require to init
> a
> > > > renderserver, which will result in a more changes to Qemu. I'm going
> to
> > > > wait a bit for Venus to settle down and then make a v4.
> > > >
> > > > In the end we will either need to add more #ifdefs if we will want to
> > > > keep supporting older virglrenderer versions in Qemu, or bump the min
> > > > required virglrenderer version.
> > >
> > > Hi Dmitry,
> > >
> > > Thanks for working on this, it's great to see QEMU graphics moving
> > > forward. I noticed a few things from your patchset:
> > >
> > > 1) Older versions of virglrenderer -- supported or not?
> > >
> > > As you alluded to, there have been significant changes to
> > > virglrenderer since the last QEMU graphics update. For example, the
> > > asynchronous callback introduces an entirely different and
> > > incompatible way to signal fence completion.
> > >
> > > Notionally, QEMU must support older versions of virglrenderer, though
> > > in practice I'm not sure how much that is true. If we want to keep up
> > > the notion that older versions must be supported, you'll need:
> > >
> > > a) virtio-gpu-virgl.c
> > > b) virtio-gpu-virgl2.c (or an equivalent)
> > >
> > > Similarly for the vhost-user paths (if you want to support that). If
> > > older versions of virglrenderer don't need to be supported, then that
> > > would simplify the amount of additional paths/#ifdefs.
> >
> > We should support old versions of virgl (as described in
> >
> https://www.qemu.org/docs/master/about/build-platforms.html#linux-os-macos-freebsd-netbsd-openbsd
> ).
> >
> > Whether a new virtio-gpu-virgl2. (or equivalent) is necessary, we
> > can't really tell without seeing the changes involved.
>
> Ack. Something to keep in mind as Dmitry refactors.
>
> >
> > >
> > > 2) Additional context type: gfxstream [i]?
> > >
> > > One of the major motivations for adding context types in the
> > > virtio-gpu spec was supporting gfxstream. gfxstream is used in the
> > > Android Studio emulator (a variant of QEMU) [ii], among other places.
> > > That would move the Android emulator closer to the goal of using
> > > upstream QEMU for everything.
> >
> > What is the advantage of using gfxstream over virgl? or zink+venus?
>
> History/backstory:
>
> gfxstream development has its roots in the development of the Android
> Emulator (circa 2010). In those days, both DRM and Android were
> relatively new and the communities didn't know much about each other.
>
> A method was devised to auto-generate GLES calls (that's all Android
> needed) and stream it over an interface very similar to pipe(..).
> Host generated IDs were used to track shareable buffers.
>
> That same method used to auto-generate GLES was expanded to Vulkan and
> support for coherent memory was added. In 2018 the Android Emulator
> was the first to ship CTS-compliant virtualized Vulkan via downstream
> kernel interfaces, before work on venus began.
>
> As virtio-gpu continued to mature, gfxstream was actually the first to
> ship both blob resources [1] and context types [2] in production via
> crosvm to form a completely upstreamable solution (I consider AOSP to
> be an "upstream" as well).
>
> [1]
> https://patchwork.kernel.org/project/dri-devel/cover/20200814024000.2485-1-gurchetansingh@chromium.org/
> [2] https://lists.oasis-open.org/archives/virtio-dev/202108/msg00141.html
>
> With this history out of the way, here are some advantages of
> gfxstream GLES over virgl:
>
> - gfxstream GLES actually has much less rendering artifacts than virgl
> since it's autogenerated and not hand-written. Using an Gallium
> command stream is lossy (partly since the GLES spec is ambiguous and
> drivers are buggy), and we always had better dEQP runs on gfxstream
> GLES than on virgl (especially on closed source drivers).
>
> - Better memory management: virgl makes heavy use of
> RESOURCE_CREATE_3D, which creates shadow buffers for every GL
> texture/buffer. gfxstream just uses a single guest memory buffer per
> DRM instance for buffer/texture upload. For example, gfxstream
> doesn't need the virtio-gpu shrinker as much [3] since it doesn't use
> as much guest memory. Though I know there have been recent fixes for
> this in virgl, but talking from a design POV.
>
> - Performance: In 2020, a vendor ran the GPU emulation stress test
> comparing virgl and gfxstream GLES. Here are some results:
>
> CVD: drm_virgl - 7.01 fps
> CVD: gfxstream - 43.68 fps
>
> I've seen similarly dramatic results with gfxbench/3D Mark on some
> automotive platforms.
>
> - Multi-threaded by design: gfxstream GLES is multi-threaded by
> design. Each guest GL thread get's its own host thread to decod
> commands. virgl is single threaded (before the asynchronous callback,
> which hasn't landed in QEMU yet)
>
> - Cross-platform: Windows, MacOS, and Linux support (though with
> downstream QEMU patches unfortunately). virgl is more a Linux thing.
>
> - Snapshots: Not possible with virgl. We don't intend to upstream
> live migration snapshot support in the initial CL, but that's
> something to note that users like.
>
> gfxstream is the "native" solution for Android and is thus better
> optimized, just like virgl is the native solution for Linux guests.
>
> Re: Zink/ANGLE/venus versus ANGLE/gfxstream VK
>
> venus in many ways has similar design characteristics as gfxstream VK
> (auto-generated, multi-threaded). gfxstream VK has better
> cross-platform support, with it shipping on via the Android emulator
> and Google Play Games [4] on PC. venus is designed with open-source
> Linux platforms in mind, with Chromebook gaming as the initial use
> case [5].
>
> That leads to different design decisions, mostly centered around
> resource sharing/state-tracking. Snapshots are also a goal for
> gfxstream VK, though not ready yet.
>
> Both venus and gfxstream are Google-sponsored. There were meetings
> between Android and ChromeOS bigwigs about gfxstream VK/venus in 2019,
> and the outcome seemed to be "we'll share work where it makes sense,
> but there might not be a one-size fits all solution".
>
> Layering which passes CTS is expected to take quite a while,
> especially for a cross-platform target such as the emulator. It would
> be great to have gfxstream GLES support alone in the interim.
>
> [3]
> https://lore.kernel.org/lkml/20230305221011.1404672-1-dmitry.osipenko@collabora.com/
> [4] https://play.google.com/googleplaygames
> [5] https://www.xda-developers.com/how-to-run-steam-chromebook/
>
> >
> > Only AOSP can run with virgl perhaps? I am not familiar with Android
> > development.. I guess it doesn't make use of Mesa, and thus no virgl
> > at all?
>
> Some AOSP targets (Cuttlefish) can use virgl along with gfxstream,
> just for testing sake. It's not hard to support both via crosvm, so
> we do it.
>
> https://source.android.com/docs/setup/create/cuttlefish-ref-gpu
>
> The Android Emulator (the most relevant use case here) does ship
> gfxstream when a developer uses Android Studio though, and plans to do
> so in the future.
>
> >
> > >
> > > If (1) is resolved, I don't think it's actually too bad to add
> > > gfxstream support. We just need an additional layer of dispatch
> > > between virglrenderer and gfxstream (thus, virtio-gpu-virgl2.c would
> > > be renamed virtio-gpu-context-types.c or something similar). The QEMU
> > > command line will have to be modified to pass in the enabled context
> > > type (--context={virgl, venus, gfxstream}). crosvm has been using the
> > > same trick.
> > >
> > > If (1) is resolved in v4, I would estimate adding gfxstream support
> > > would at max take 1-2 months for a single engineer. I'm not saying
> > > gfxstream need necessarily be a part of a v5 patch-stack, but given
> > > this patch-stack has been around for 1 year plus, it certainly could
> > > be. We can certainly design things in such a way that adding
> > > gfxstream is easy subsequently.
> > >
> > > The hardest part is actually package management (Debian) for
> > > gfxstream, but those can be resolved.
> > >
> >
> > It looks like gfxstream is actually offering an API similar to
> > virlgrenderer (with "pipe_" prefix).
>
> For gfxstream, my ideal solution would not use that "pipe_" API
> directly from QEMU (though vulkan-cereal will be packaged properly).
> Instead, I intend to package the "rutabaga_gfx_ffi.h" proxy library
> over gfxstream [6]:
>
>
> https://chromium.googlesource.com/chromiumos/platform/crosvm/+/refs/heads/main/rutabaga_gfx/ffi/src/include/rutabaga_gfx_ffi.h
>
> The advantage with this approach is one gets Wayland passthrough [7]
> with Linux guests which is written in Rust , along with gfxstream.
> The main issues are around Debian Rust packaging.
>
> Rough sketch, here's what I think we might need:
>
> a) virtio-gpu-virgl-legacy.c for older versions of virglrenderer
> b) virtio-gpu-virgl2.c
> c) virtio-gpu-rutabaga.c or virtio-gpu-gfxstream.c (depending on rust
> packaging investigations)
>
> Though Wayland passthrough is a "nice to have", upstreaming gfxstream
> for Android Emulator is the most important product goal. So if Rust
> Debian packaging becomes too onerous (virtio-gpu-rutabaga.c), we can
> backtrack to a simpler solution (virtio-gpu-gfxstream.c).
>
> [6] it can also proxy virglrenderer calls too, but I'll that decision
> to virglrenderer maintainers
> [7] try out the feature here:
> https://crosvm.dev/book/devices/wayland.html
>
> > I suppose the guest needs to be
> > configured in a special way then (how? without mesa?).
>
> For AOSP, androidboot.hardware.vulkan and androidboot.hardware.egl
> allow toggling of GLES and Vulkan impls. QEMU
> won't have to do anything special given the way the launchers are
> designed (there's an equivalent of a "virtmanager").
>
> There needs to be logic around context selection for Linux guests.
> QEMU needs a "--ctx_type={virgl, venus, drm, gfxstream}" argument.
> See crosvm for an example:
>
>
> https://chromium.googlesource.com/chromiumos/platform/crosvm/+/refs/heads/main/rutabaga_gfx/src/rutabaga_core.rs#910
>
> This argument is important for Linux upcoming "DRM native" context
> types [8] as well.
>
> [8] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21658
>
> >
> > > I'm not sure exactly how QEMU accelerated graphics are utilized in
> > > user-facing actual products currently, so not sure what the standard
> > > is.
> > >
> > > What do QEMU maintainers and users think about these issues,
> > > particularly about the potential gfxstream addition in QEMU as a
> > > context type? We are most interested in Android guests.
> >
> > It would be great if the Android emulator was more aligned with
> > upstream QEMU development!
>
> Awesome! I envisage the initial gfxstream integration as just a first
> step. With the graphics solution upstreamed, subsequent MacOS/Windows
> specific patches will start to make more sense.
>
Okay, I think the next steps would actually be code so you can see our
vision. I have few questions that will help with my RFC:
1) Packaging -- before or after?
gfxstream does not have a package in upstream Portage or Debian (though
there are downstream implementations). Is it sufficient to have a
versioned release (i.e, Git tag) without the package before the change can
be merged into QEMU?
Is packaging required before merging into QEMU?
2) Optional Rust dependencies
To achieve seamless Wayland windowing with the same implementation as
crosvm, we'll need optional Rust dependencies. There actually has been
interest in making Rust a non-optional dependency:
https://wiki.qemu.org/RustInQemu
https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg04589.html
I actually only want Rust as an optional dependency on Linux, Windows, and
MacOS -- where Rust support is quite good. Is there any problem with using
Rust library with a C API from QEMU?
3) Rust "Build-Depends" in Debian
This is mostly a question to Debian packagers (CC: mjt@)
Any Rust package would likely depend on 10-30 additional packages (that's
just the way Rust works), but they are all in Debian stable right now.
https://packages.debian.org/stable/rust/
I noticed when enabling virgl, there were complaints about a ton of UI
libraries being pulled in.
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=813658
That necessitated the creation of the `qemu-system-gui` package for people
who don't need a UI. I want to make gfxstream a Suggested Package in
qemu-system-gui, but that would potentially pull in 10-30 additional Rust
build dependencies I mentioned.
Would the 10-30 Rust Build dependencies be problematic? I think QEMU
already has hundreds right now.
Thanks!
> >
> > thanks
> >
> > --
> > Marc-André Lureau
>
[-- Attachment #2: Type: text/html, Size: 18956 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v3 0/9] virtio-gpu: Support Venus Vulkan driver
2023-03-17 21:40 ` Gurchetan Singh
@ 2023-03-22 8:00 ` Michael Tokarev
0 siblings, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2023-03-22 8:00 UTC (permalink / raw)
To: Gurchetan Singh, Marc-André Lureau
Cc: Dmitry Osipenko, Alex Bennée, gert.wollny, qemu-devel,
pkg-qemu-devel
18.03.2023 00:40, Gurchetan Singh пишет:
[[big snip]]
> Okay, I think the next steps would actually be code so you can see our vision. I have few questions that will help with my RFC:
>
> 1) Packaging -- before or after?
>
> gfxstream does not have a package in upstream Portage or Debian (though there are downstream implementations). Is it sufficient to have a versioned
> release (i.e, Git tag) without the package before the change can be merged into QEMU?
>
> Is packaging required before merging into QEMU?
>
> 2) Optional Rust dependencies
>
> To achieve seamless Wayland windowing with the same implementation as crosvm, we'll need optional Rust dependencies. There actually has been interest
> in making Rust a non-optional dependency:
>
> https://wiki.qemu.org/RustInQemu <https://wiki.qemu.org/RustInQemu>
> https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg04589.html <https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg04589.html>
>
> I actually only want Rust as an optional dependency on Linux, Windows, and MacOS -- where Rust support is quite good. Is there any problem with using
> Rust library with a C API from QEMU?
>
> 3) Rust "Build-Depends" in Debian
>
> This is mostly a question to Debian packagers (CC: mjt@)
>
> Any Rust package would likely depend on 10-30 additional packages (that's just the way Rust works), but they are all in Debian stable right now.
>
> https://packages.debian.org/stable/rust/ <https://packages.debian.org/stable/rust/>
>
> I noticed when enabling virgl, there were complaints about a ton of UI libraries being pulled in.
>
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=813658 <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=813658>
>
> That necessitated the creation of the `qemu-system-gui` package for people who don't need a UI. I want to make gfxstream a Suggested Package in
> qemu-system-gui, but that would potentially pull in 10-30 additional Rust build dependencies I mentioned.
>
> Would the 10-30 Rust Build dependencies be problematic? I think QEMU already has hundreds right now.
There's no reason to worry about *build*-time dependencies.
There's also no big reason to worry about runtime dependencies, and to worry about
Suggests vs Recommends, etc, - this all can be adjusted when needed, packages can
be spilt further, etc, - it is normal practice.
/mjt
^ permalink raw reply [flat|nested] 26+ messages in thread