date:20240504


On 5/4/24 08:39, Richard Henderson wrote:

The following changes since commit 97c872276d147c882296f5da245bd8432f1582f6:

   Merge tag 'accel-sh4-ui-20240503' ofhttps://github.com/philmd/qemu  into 
staging (2024-05-03 14:42:50 -0700)

are available in the Git repository at:

   https://gitlab.com/rth7680/qemu.git  tags/pull-axp-20240504

for you to fetch changes up to 23bb086350c0de390f77dd034d775742314cabd7:

   target/alpha: Implement CF_PCREL (2024-05-04 08:05:51 -0700)


target/alpha: Implement CF_PCREL


Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/9.1 as 
appropriate.


r~

Re: [PATCH] Fixes: Indentation using TABs and improve formatting

2024-05-04 Thread Michael Tokarev


04.05.2024 21:58, Tanmay wrote:

Hi,

I have attached a patch file that fixes indentation and formatting for some files as listed in https://gitlab.com/qemu-project/qemu/-/issues/373 
.


it is sort of good you posted this patch to stable@.  It has absolutely nothing 
to do
with stable, but it serves as a an example of things which should - in my 
opinion -
not be done at all.  We had another similar change, 55339361276a "sh4: Coding 
style:
Remove tabs", which makes all further changes (fixes) in this area basically
non-back-portable to previous stable series.

FWIW,

/mjt
--
GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24.
New key: rsa4096/61AD3D98ECDF2C8E  9D8B E14E 3F2A 9DD7 9199  28F1 61AD 3D98 
ECDF 2C8E
Old key: rsa2048/457CE0A0804465C5  6EE1 95D1 886E 8FFB 810D  4324 457C E0A0 
8044 65C5
Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt

[PATCH v10 06/10] virtio-gpu: Support blob scanout using dmabuf fd

From: Robert Beckett 

Support displaying blob resources by handling SET_SCANOUT_BLOB
command.

Signed-by: Antonio Caggiano 
Signed-off-by: Robert Beckett 
Signed-off-by: Huang Rui 
Reviewed-by: Antonio Caggiano 
Signed-off-by: Dmitry Osipenko 
---
 hw/display/virtio-gpu-virgl.c  | 109 +
 hw/display/virtio-gpu.c|  12 ++--
 include/hw/virtio/virtio-gpu.h |   7 +++
 meson.build|   1 +
 4 files changed, 123 insertions(+), 6 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index a040324f5024..2fedccb1fc8d 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -17,6 +17,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-gpu-pixman.h"
 
 #include "ui/egl-helpers.h"
 
@@ -78,6 +80,7 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
 res->base.height = c2d.height;
 res->base.format = c2d.format;
 res->base.resource_id = c2d.resource_id;
+res->base.dmabuf_fd = -1;
 QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
 
 args.handle = c2d.resource_id;
@@ -125,6 +128,7 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
 res->base.height = c3d.height;
 res->base.format = c3d.format;
 res->base.resource_id = c3d.resource_id;
+res->base.dmabuf_fd = -1;
 QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
 
 args.handle = c3d.resource_id;
@@ -507,6 +511,106 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
 g_free(resp);
 }
 
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
+   struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_framebuffer fb = { 0 };
+struct virgl_renderer_resource_info info;
+struct virtio_gpu_virgl_resource *res;
+struct virtio_gpu_set_scanout_blob ss;
+uint64_t fbend;
+
+VIRTIO_GPU_FILL_CMD(ss);
+virtio_gpu_scanout_blob_bswap(&ss);
+trace_virtio_gpu_cmd_set_scanout_blob(ss.scanout_id, ss.resource_id,
+  ss.r.width, ss.r.height, ss.r.x,
+  ss.r.y);
+
+if (ss.scanout_id >= g->parent_obj.conf.max_outputs) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout id specified %d",
+  __func__, ss.scanout_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
+return;
+}
+
+if (ss.resource_id == 0) {
+virtio_gpu_disable_scanout(g, ss.scanout_id);
+return;
+}
+
+if (ss.width < 16 ||
+ss.height < 16 ||
+ss.r.x + ss.r.width > ss.width ||
+ss.r.y + ss.r.height > ss.height) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout %d bounds for"
+  " resource %d, rect (%d,%d)+%d,%d, fb %d %d\n",
+  __func__, ss.scanout_id, ss.resource_id,
+  ss.r.x, ss.r.y, ss.r.width, ss.r.height,
+  ss.width, ss.height);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+return;
+}
+
+res = virtio_gpu_virgl_find_resource(g, ss.resource_id);
+if (!res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+  __func__, ss.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+if (virgl_renderer_resource_get_info(ss.resource_id, &info)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not have info %d\n",
+  __func__, ss.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+if (res->base.dmabuf_fd < 0) {
+res->base.dmabuf_fd = info.fd;
+}
+if (res->base.dmabuf_fd < 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource not backed by dmabuf 
%d\n",
+  __func__, ss.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+fb.format = virtio_gpu_get_pixman_format(ss.format);
+if (!fb.format) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: pixel format not supported %d\n",
+  __func__, ss.format);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+return;
+}
+
+fb.bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb.format), 8);
+fb.width = ss.width;
+fb.height = ss.height;
+fb.stride = ss.strides[0];
+fb.offset = ss.offsets[0] + ss.r.x * fb.bytes_pp + ss.r.y * fb.stride;
+
+fbend = fb.offset;
+fbend += fb.stride * (ss.r.height - 1);
+fbend += fb.bytes_pp * ss.r.width;
+if (fbend > res->base.blob_size) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: fb end out of range\n",
+  __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+return;
+}
+
+

[PATCH v10 02/10] virtio-gpu: Use pkgconfig version to decide which virgl features are available

New virglrerenderer features were stabilized with release of v1.0.0.
Presence of symbols in virglrenderer.h doesn't guarantee ABI compatibility
with pre-release development versions of libvirglerender. Use virglrenderer
version to decide reliably which virgl features are available.

Signed-off-by: Dmitry Osipenko 
---
 meson.build | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/meson.build b/meson.build
index 5db2dbc12ec7..f4a4d71c1978 100644
--- a/meson.build
+++ b/meson.build
@@ -2286,11 +2286,8 @@ config_host_data.set('CONFIG_PNG', png.found())
 config_host_data.set('CONFIG_VNC', vnc.found())
 config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
 config_host_data.set('CONFIG_VNC_SASL', sasl.found())
-if virgl.found()
-  config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT',
-   cc.has_member('struct 
virgl_renderer_resource_info_ext', 'd3d_tex2d',
- prefix: '#include ',
- dependencies: virgl))
+if virgl.version().version_compare('>=1.0.0')
+  config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT', 1)
 endif
 config_host_data.set('CONFIG_VIRTFS', have_virtfs)
 config_host_data.set('CONFIG_VTE', vte.found())
-- 
2.44.0

[PATCH v10 04/10] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled

The udmabuf usage is mandatory when virgl is disabled and blobs feature
enabled in the Qemu machine configuration. If virgl and blobs are enabled,
then udmabuf requirement is optional. Since udmabuf isn't widely supported
by a popular Linux distros today, let's relax the udmabuf requirement for
blobs=on,virgl=on. Now, a full-featured virtio-gpu acceleration is
available to Qemu users without a need to have udmabuf available in the
system.

Reviewed-by: Antonio Caggiano 
Signed-off-by: Huang Rui 
Reviewed-by: Antonio Caggiano 
Reviewed-by: Marc-André Lureau 
Signed-off-by: Dmitry Osipenko 
---
 hw/display/virtio-gpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index ae831b6b3e3e..dac272ecadb1 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1472,6 +1472,7 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error 
**errp)
 
 if (virtio_gpu_blob_enabled(g->parent_obj.conf)) {
 if (!virtio_gpu_rutabaga_enabled(g->parent_obj.conf) &&
+!virtio_gpu_virgl_enabled(g->parent_obj.conf) &&
 !virtio_gpu_have_udmabuf()) {
 error_setg(errp, "need rutabaga or udmabuf for blob resources");
 return;
-- 
2.44.0

[PATCH v10 08/10] virtio-gpu: Handle resource blob commands

From: Antonio Caggiano 

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 
Signed-off-by: Xenia Ragiadakou 
Signed-off-by: Huang Rui 
Signed-off-by: Dmitry Osipenko 
---
 hw/display/virtio-gpu-virgl.c  | 266 +
 hw/display/virtio-gpu.c|   4 +-
 include/hw/virtio/virtio-gpu.h |   2 +
 3 files changed, 271 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index dc2b5a496630..d92c58b77865 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -26,6 +26,7 @@
 
 struct virtio_gpu_virgl_resource {
 struct virtio_gpu_simple_resource base;
+MemoryRegion *mr;
 };
 
 static struct virtio_gpu_virgl_resource *
@@ -49,6 +50,114 @@ virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
 }
 #endif
 
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+struct virtio_gpu_virgl_hostmem_region {
+MemoryRegion mr;
+struct VirtIOGPU *g;
+struct virtio_gpu_virgl_resource *res;
+};
+
+static void virtio_gpu_virgl_resume_cmdq_bh(void *opaque)
+{
+VirtIOGPU *g = opaque;
+
+virtio_gpu_process_cmdq(g);
+}
+
+static void virtio_gpu_virgl_hostmem_region_free(void *obj)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+struct virtio_gpu_virgl_hostmem_region *vmr;
+VirtIOGPUBase *b;
+
+vmr = container_of(mr, struct virtio_gpu_virgl_hostmem_region, mr);
+vmr->res->mr = NULL;
+
+b = VIRTIO_GPU_BASE(vmr->g);
+b->renderer_blocked--;
+
+/*
+ * memory_region_unref() is executed from RCU thread context, while
+ * virglrenderer works only on the main-loop thread that's holding GL
+ * context.
+ */
+qemu_bh_schedule(vmr->g->cmdq_resume_bh);
+g_free(vmr);
+}
+
+static int
+virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
+   struct virtio_gpu_virgl_resource *res,
+   uint64_t offset)
+{
+struct virtio_gpu_virgl_hostmem_region *vmr;
+VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+MemoryRegion *mr;
+uint64_t size;
+void *data;
+int ret;
+
+if (!virtio_gpu_hostmem_enabled(b->conf)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: hostmem disabled\n", __func__);
+return -EOPNOTSUPP;
+}
+
+ret = virgl_renderer_resource_map(res->base.resource_id, &data, &size);
+if (ret) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map virgl resource: 
%s\n",
+  __func__, strerror(-ret));
+return ret;
+}
+
+vmr = g_new0(struct virtio_gpu_virgl_hostmem_region, 1);
+vmr->res = res;
+vmr->g = g;
+
+mr = &vmr->mr;
+memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data);
+memory_region_add_subregion(&b->hostmem, offset, mr);
+memory_region_set_enabled(mr, true);
+
+/*
+ * Potentially, MR could outlive the resource if MR's reference is held
+ * outside of virtio-gpu. In order to prevent unmapping resource while
+ * MR is alive, and thus, making the data pointer invalid, we will block
+ * virtio-gpu command processing until MR is fully unreferenced and
+ * released.
+ */
+OBJECT(mr)->free = virtio_gpu_virgl_hostmem_region_free;
+
+res->mr = mr;
+
+return 0;
+}
+
+static void
+virtio_gpu_virgl_async_unmap_resource_blob(VirtIOGPU *g,
+   struct virtio_gpu_virgl_resource 
*res,
+   bool *cmd_suspended)
+{
+VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+MemoryRegion *mr = res->mr;
+
+if (mr && *cmd_suspended == false) {
+/* render will be unblocked when MR is freed */
+b->renderer_blocked++;
+
+/* memory region owns self res->mr object and frees it by itself */
+memory_region_set_enabled(mr, false);
+memory_region_del_subregion(&b->hostmem, mr);
+object_unparent(OBJECT(mr));
+
+*cmd_suspended = true;
+} else if (!mr && *cmd_suspended) {
+virgl_renderer_resource_unmap(res->base.resource_id);
+
+*cmd_suspended = false;
+}
+}
+#endif /* HAVE_VIRGL_RESOURCE_BLOB */
+
 static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
  struct virtio_gpu_ctrl_command *cmd)
 {
@@ -162,6 +271,13 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
 return;
 }
 
+if (res->mr || cmd->suspended) {
+virtio_gpu_virgl_async_unmap_resource_blob(g, res, &cmd->suspended);
+if (cmd->suspended) {
+return;
+}
+}
+
 virgl_renderer_resource_detach_iov(unref.resource_id,
&res_iovs,
&num_iovs);
@@ -512,6 +628,138 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
 }
 
 #ifdef HAVE_VIRGL_RESOURCE_BLOB
+st

[PATCH v10 01/10] virtio-gpu: Unrealize GL device

Even though GL GPU doesn't support hotplugging today, free virgl
resources when GL device is unrealized. For consistency.

Signed-off-by: Dmitry Osipenko 
---
 hw/display/virtio-gpu-gl.c | 11 +++
 hw/display/virtio-gpu-virgl.c  |  9 +
 include/hw/virtio/virtio-gpu.h |  1 +
 3 files changed, 21 insertions(+)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index e06be60dfbfc..0c0a8d136954 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -136,6 +136,16 @@ static Property virtio_gpu_gl_properties[] = {
 DEFINE_PROP_END_OF_LIST(),
 };
 
+static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
+{
+VirtIOGPU *g = VIRTIO_GPU(qdev);
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
+
+if (gl->renderer_inited) {
+virtio_gpu_virgl_deinit(g);
+}
+}
+
 static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
@@ -149,6 +159,7 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, 
void *data)
 vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data;
 
 vdc->realize = virtio_gpu_gl_device_realize;
+vdc->unrealize = virtio_gpu_gl_device_unrealize;
 vdc->reset = virtio_gpu_gl_reset;
 device_class_set_props(dc, virtio_gpu_gl_properties);
 }
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 9f34d0e6619c..b0500eccf8e0 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -665,3 +665,12 @@ int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
 
 return capset2_max_ver ? 2 : 1;
 }
+
+void virtio_gpu_virgl_deinit(VirtIOGPU *g)
+{
+if (g->fence_poll) {
+timer_free(g->fence_poll);
+}
+
+virgl_renderer_cleanup(NULL);
+}
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index ed44cdad6b34..b657187159d9 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -336,6 +336,7 @@ void virtio_gpu_virgl_fence_poll(VirtIOGPU *g);
 void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
 void virtio_gpu_virgl_reset(VirtIOGPU *g);
 int virtio_gpu_virgl_init(VirtIOGPU *g);
+void virtio_gpu_virgl_deinit(VirtIOGPU *g);
 int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
 
 #endif
-- 
2.44.0

[PATCH v10 07/10] virtio-gpu: Support suspension of commands processing

Add new "suspended" flag to virtio_gpu_ctrl_command telling cmd
processor that it should stop processing commands and retry again
next time until flag is unset.

Signed-off-by: Dmitry Osipenko 
---
 hw/display/virtio-gpu-gl.c   | 1 +
 hw/display/virtio-gpu-rutabaga.c | 1 +
 hw/display/virtio-gpu-virgl.c| 3 +++
 hw/display/virtio-gpu.c  | 5 +
 include/hw/virtio/virtio-gpu.h   | 1 +
 5 files changed, 11 insertions(+)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 95806999189e..2a9e549ad2e9 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -79,6 +79,7 @@ static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 cmd->vq = vq;
 cmd->error = 0;
 cmd->finished = false;
+cmd->suspended = false;
 QTAILQ_INSERT_TAIL(&g->cmdq, cmd, next);
 cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
 }
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 17bf701a2163..b6e84d436fb2 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -1061,6 +1061,7 @@ static void virtio_gpu_rutabaga_handle_ctrl(VirtIODevice 
*vdev, VirtQueue *vq)
 cmd->vq = vq;
 cmd->error = 0;
 cmd->finished = false;
+cmd->suspended = false;
 QTAILQ_INSERT_TAIL(&g->cmdq, cmd, next);
 cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
 }
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 2fedccb1fc8d..dc2b5a496630 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -687,6 +687,9 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
 break;
 }
 
+if (cmd->suspended) {
+return;
+}
 if (cmd->finished) {
 return;
 }
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 1e57a53d346c..a1bd4d6914c4 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1054,6 +1054,10 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
 /* process command */
 vgc->process_cmd(g, cmd);
 
+if (cmd->suspended) {
+break;
+}
+
 QTAILQ_REMOVE(&g->cmdq, cmd, next);
 if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
 g->stats.requests++;
@@ -1113,6 +1117,7 @@ static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 cmd->vq = vq;
 cmd->error = 0;
 cmd->finished = false;
+cmd->suspended = false;
 QTAILQ_INSERT_TAIL(&g->cmdq, cmd, next);
 cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command));
 }
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index ba36497c477f..0c00c303f41b 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -132,6 +132,7 @@ struct virtio_gpu_ctrl_command {
 struct virtio_gpu_ctrl_hdr cmd_hdr;
 uint32_t error;
 bool finished;
+bool suspended;
 QTAILQ_ENTRY(virtio_gpu_ctrl_command) next;
 };
 
-- 
2.44.0

[PATCH v10 09/10] virtio-gpu: Register capsets dynamically

From: Pierre-Eric Pelloux-Prayer 

virtio_gpu_virgl_get_num_capsets will return "num_capsets", but we can't
assume that capset_index 1 is always VIRGL2 once we'll support more capsets,
like Venus and DRM capsets. Register capsets dynamically to avoid that problem.

Signed-off-by: Pierre-Eric Pelloux-Prayer 
Signed-off-by: Dmitry Osipenko 
---
 hw/display/virtio-gpu-gl.c |  6 --
 hw/display/virtio-gpu-virgl.c  | 33 +
 include/hw/virtio/virtio-gpu.h |  4 +++-
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 2a9e549ad2e9..cd39e0650862 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -125,8 +125,8 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, 
Error **errp)
 }
 
 g->parent_obj.conf.flags |= (1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED);
-VIRTIO_GPU_BASE(g)->virtio_config.num_capsets =
-virtio_gpu_virgl_get_num_capsets(g);
+g->capset_ids = virtio_gpu_virgl_get_capsets(g);
+VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = g->capset_ids->len;
 
 #ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS
 g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED;
@@ -149,6 +149,8 @@ static void virtio_gpu_gl_device_unrealize(DeviceState 
*qdev)
 if (gl->renderer_inited) {
 virtio_gpu_virgl_deinit(g);
 }
+
+g_array_unref(g->capset_ids);
 }
 
 static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index d92c58b77865..1babda4efad5 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -585,19 +585,13 @@ static void virgl_cmd_get_capset_info(VirtIOGPU *g,
 VIRTIO_GPU_FILL_CMD(info);
 
 memset(&resp, 0, sizeof(resp));
-if (info.capset_index == 0) {
-resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL;
-virgl_renderer_get_cap_set(resp.capset_id,
-   &resp.capset_max_version,
-   &resp.capset_max_size);
-} else if (info.capset_index == 1) {
-resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL2;
+
+if (info.capset_index < g->capset_ids->len) {
+resp.capset_id = g_array_index(g->capset_ids, uint32_t,
+   info.capset_index);
 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;
 }
 resp.hdr.type = VIRTIO_GPU_RESP_OK_CAPSET_INFO;
 virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
@@ -1120,14 +1114,29 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
 return 0;
 }
 
-int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
+static void virtio_gpu_virgl_add_capset(GArray *capset_ids, uint32_t capset_id)
+{
+g_array_append_val(capset_ids, capset_id);
+}
+
+GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
 {
 uint32_t capset2_max_ver, capset2_max_size;
+GArray *capset_ids;
+
+capset_ids = g_array_new(false, false, sizeof(uint32_t));
+
+/* VIRGL is always supported. */
+virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL);
+
 virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
   &capset2_max_ver,
   &capset2_max_size);
+if (capset2_max_ver) {
+virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2);
+}
 
-return capset2_max_ver ? 2 : 1;
+return capset_ids;
 }
 
 void virtio_gpu_virgl_deinit(VirtIOGPU *g)
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index a98847b88087..105308a36865 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -213,6 +213,8 @@ struct VirtIOGPU {
 } dmabuf;
 
 QEMUBH *cmdq_resume_bh;
+
+GArray *capset_ids;
 };
 
 struct VirtIOGPUClass {
@@ -347,6 +349,6 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
 void virtio_gpu_virgl_reset(VirtIOGPU *g);
 int virtio_gpu_virgl_init(VirtIOGPU *g);
 void virtio_gpu_virgl_deinit(VirtIOGPU *g);
-int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
+GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g);
 
 #endif
-- 
2.44.0

[PATCH v10 03/10] virtio-gpu: Support context-init feature with virglrenderer

From: Huang Rui 

Patch "virtio-gpu: CONTEXT_INIT feature" has added the context_init
feature flags. Expose this feature and support creating virglrenderer
context with flags using context_id if libvirglrenderer is new enough.

Originally-by: Antonio Caggiano 
Signed-off-by: Huang Rui 
Reviewed-by: Antonio Caggiano 
Signed-off-by: Dmitry Osipenko 
---
 hw/display/virtio-gpu-gl.c|  4 
 hw/display/virtio-gpu-virgl.c | 20 ++--
 meson.build   |  1 +
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 0c0a8d136954..95806999189e 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -127,6 +127,10 @@ static void virtio_gpu_gl_device_realize(DeviceState 
*qdev, Error **errp)
 VIRTIO_GPU_BASE(g)->virtio_config.num_capsets =
 virtio_gpu_virgl_get_num_capsets(g);
 
+#ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS
+g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED;
+#endif
+
 virtio_gpu_device_realize(qdev, errp);
 }
 
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index b0500eccf8e0..8306961ad502 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -106,8 +106,24 @@ 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) {
+if (!virtio_gpu_context_init_enabled(g->parent_obj.conf)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: context_init disabled",
+  __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+return;
+}
+
+#ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS
+virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
+ cc.context_init,
+ cc.nlen,
+ cc.debug_name);
+return;
+#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/meson.build b/meson.build
index f4a4d71c1978..513cb2ea6d03 100644
--- a/meson.build
+++ b/meson.build
@@ -2288,6 +2288,7 @@ config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
 config_host_data.set('CONFIG_VNC_SASL', sasl.found())
 if virgl.version().version_compare('>=1.0.0')
   config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT', 1)
+  config_host_data.set('HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS', 1)
 endif
 config_host_data.set('CONFIG_VIRTFS', have_virtfs)
 config_host_data.set('CONFIG_VTE', vte.found())
-- 
2.44.0

[PATCH v10 05/10] virtio-gpu: Add virgl resource management

From: Huang Rui 

In a preparation to adding host blobs support to virtio-gpu, add virgl
resource management that allows to retrieve resource based on its ID
and virgl resource wrapper on top of simple resource that will be contain
fields specific to virgl.

Signed-off-by: Huang Rui 
Reviewed-by: Antonio Caggiano 
Signed-off-by: Dmitry Osipenko 
---
 hw/display/virtio-gpu-virgl.c | 74 +++
 1 file changed, 74 insertions(+)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 8306961ad502..a040324f5024 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -22,6 +22,23 @@
 
 #include 
 
+struct virtio_gpu_virgl_resource {
+struct virtio_gpu_simple_resource base;
+};
+
+static struct virtio_gpu_virgl_resource *
+virtio_gpu_virgl_find_resource(VirtIOGPU *g, uint32_t resource_id)
+{
+struct virtio_gpu_simple_resource *res;
+
+res = virtio_gpu_find_resource(g, resource_id);
+if (!res) {
+return NULL;
+}
+
+return container_of(res, struct virtio_gpu_virgl_resource, base);
+}
+
 #if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
 static void *
 virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
@@ -35,11 +52,34 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
 {
 struct virtio_gpu_resource_create_2d c2d;
 struct virgl_renderer_resource_create_args args;
+struct virtio_gpu_virgl_resource *res;
 
 VIRTIO_GPU_FILL_CMD(c2d);
 trace_virtio_gpu_cmd_res_create_2d(c2d.resource_id, c2d.format,
c2d.width, c2d.height);
 
+if (c2d.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_virgl_find_resource(g, c2d.resource_id);
+if (res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+  __func__, c2d.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = g_new0(struct virtio_gpu_virgl_resource, 1);
+res->base.width = c2d.width;
+res->base.height = c2d.height;
+res->base.format = c2d.format;
+res->base.resource_id = c2d.resource_id;
+QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
+
 args.handle = c2d.resource_id;
 args.target = 2;
 args.format = c2d.format;
@@ -59,11 +99,34 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
 {
 struct virtio_gpu_resource_create_3d c3d;
 struct virgl_renderer_resource_create_args args;
+struct virtio_gpu_virgl_resource *res;
 
 VIRTIO_GPU_FILL_CMD(c3d);
 trace_virtio_gpu_cmd_res_create_3d(c3d.resource_id, c3d.format,
c3d.width, c3d.height, c3d.depth);
 
+if (c3d.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_virgl_find_resource(g, c3d.resource_id);
+if (res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+  __func__, c3d.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = g_new0(struct virtio_gpu_virgl_resource, 1);
+res->base.width = c3d.width;
+res->base.height = c3d.height;
+res->base.format = c3d.format;
+res->base.resource_id = c3d.resource_id;
+QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
+
 args.handle = c3d.resource_id;
 args.target = c3d.target;
 args.format = c3d.format;
@@ -82,12 +145,19 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
  struct virtio_gpu_ctrl_command *cmd)
 {
 struct virtio_gpu_resource_unref unref;
+struct virtio_gpu_virgl_resource *res;
 struct iovec *res_iovs = NULL;
 int num_iovs = 0;
 
 VIRTIO_GPU_FILL_CMD(unref);
 trace_virtio_gpu_cmd_res_unref(unref.resource_id);
 
+res = virtio_gpu_virgl_find_resource(g, unref.resource_id);
+if (!res) {
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
 virgl_renderer_resource_detach_iov(unref.resource_id,
&res_iovs,
&num_iovs);
@@ -95,6 +165,10 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
 virtio_gpu_cleanup_mapping_iov(g, res_iovs, num_iovs);
 }
 virgl_renderer_resource_unref(unref.resource_id);
+
+QTAILQ_REMOVE(&g->reslist, &res->base, next);
+
+g_free(res);
 }
 
 static void virgl_cmd_context_create(VirtIOGPU *g,
-- 
2.44.0

[PATCH v10 10/10] virtio-gpu: Support Venus context

From: Antonio Caggiano 

Request Venus when initializing VirGL and if venus=true flag is set for
virtio-gpu-gl device.

Signed-off-by: Antonio Caggiano 
Signed-off-by: Huang Rui 
Signed-off-by: Dmitry Osipenko 
---
 hw/display/virtio-gpu-gl.c |  2 ++
 hw/display/virtio-gpu-virgl.c  | 22 ++
 hw/display/virtio-gpu.c| 13 +
 include/hw/virtio/virtio-gpu.h |  3 +++
 meson.build|  1 +
 5 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index cd39e0650862..f2e4da56d8c4 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -138,6 +138,8 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, 
Error **errp)
 static Property virtio_gpu_gl_properties[] = {
 DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags,
 VIRTIO_GPU_FLAG_STATS_ENABLED, false),
+DEFINE_PROP_BIT("venus", VirtIOGPU, parent_obj.conf.flags,
+VIRTIO_GPU_FLAG_VENUS_ENABLED, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 1babda4efad5..a0a5e5b7a3de 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -1091,6 +1091,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
 flags |= VIRGL_RENDERER_D3D11_SHARE_TEXTURE;
 }
 #endif
+#ifdef VIRGL_RENDERER_VENUS
+if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
+flags |= VIRGL_RENDERER_VENUS | VIRGL_RENDERER_RENDER_SERVER;
+}
+#endif
 
 ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs);
 if (ret != 0) {
@@ -1121,7 +1126,7 @@ static void virtio_gpu_virgl_add_capset(GArray 
*capset_ids, uint32_t capset_id)
 
 GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
 {
-uint32_t capset2_max_ver, capset2_max_size;
+uint32_t capset_max_ver, capset_max_size;
 GArray *capset_ids;
 
 capset_ids = g_array_new(false, false, sizeof(uint32_t));
@@ -1130,12 +1135,21 @@ GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
 virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL);
 
 virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
-  &capset2_max_ver,
-  &capset2_max_size);
-if (capset2_max_ver) {
+   &capset_max_ver,
+   &capset_max_size);
+if (capset_max_ver) {
 virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2);
 }
 
+if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
+virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VENUS,
+   &capset_max_ver,
+   &capset_max_size);
+if (capset_max_size) {
+virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VENUS);
+}
+}
+
 return capset_ids;
 }
 
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 45c1f2006712..e86326b25a72 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1491,6 +1491,19 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error 
**errp)
 #endif
 }
 
+if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
+#ifdef HAVE_VIRGL_VENUS
+if (!virtio_gpu_blob_enabled(g->parent_obj.conf) ||
+!virtio_gpu_hostmem_enabled(g->parent_obj.conf)) {
+error_setg(errp, "venus requires enabled blob and hostmem 
options");
+return;
+}
+#else
+error_setg(errp, "old virglrenderer, venus unsupported");
+return;
+#endif
+}
+
 if (!virtio_gpu_base_device_realize(qdev,
 virtio_gpu_handle_ctrl_cb,
 virtio_gpu_handle_cursor_cb,
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 105308a36865..8b7bbf853fe8 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -99,6 +99,7 @@ enum virtio_gpu_base_conf_flags {
 VIRTIO_GPU_FLAG_BLOB_ENABLED,
 VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
 VIRTIO_GPU_FLAG_RUTABAGA_ENABLED,
+VIRTIO_GPU_FLAG_VENUS_ENABLED,
 };
 
 #define virtio_gpu_virgl_enabled(_cfg) \
@@ -117,6 +118,8 @@ enum virtio_gpu_base_conf_flags {
 (_cfg.flags & (1 << VIRTIO_GPU_FLAG_RUTABAGA_ENABLED))
 #define virtio_gpu_hostmem_enabled(_cfg) \
 (_cfg.hostmem > 0)
+#define virtio_gpu_venus_enabled(_cfg) \
+(_cfg.flags & (1 << VIRTIO_GPU_FLAG_VENUS_ENABLED))
 
 struct virtio_gpu_base_conf {
 uint32_t max_outputs;
diff --git a/meson.build b/meson.build
index 9a00e29a80cf..5e59f031b4b7 100644
--- a/meson.build
+++ b/meson.build
@@ -2290,6 +2290,7 @@ if virgl.version().version_compare('>=1.0.0')
   config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT', 1)
   config_host_data.set('HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS', 1)
   config_host_data.set('HAVE_VIRGL_RESOURCE_BLOB', 1)
+  config_host_d

[PATCH v10 00/10] Support blob memory and venus on qemu

Hello,

This series enables Vulkan Venus context support on virtio-gpu.

All virglrender and almost all Linux kernel prerequisite changes
needed by Venus are already in upstream. For kernel there is a pending
KVM patchset that fixes mapping of compound pages needed for DRM drivers
using TTM [1], othewrwise hostmem blob mapping will fail with a KVM error
from Qemu.

[1] https://lore.kernel.org/kvm/20240229025759.1187910-1-steve...@google.com/

You'll need to use recent Mesa version containing patch that removes
dependency on cross-device feature from Venus that isn't supported by
Qemu [2].

[2]
https://gitlab.freedesktop.org/mesa/mesa/-/commit/087e9a96d13155e26987befae78b6ccbb7ae242b

Example Qemu cmdline that enables Venus for virtio-gpu:

qemu-system-x86_64 -device virtio-vga-gl,hostmem=4G,blob=true,venus=true

Changes from V9 to V10

- Dropped 'async_unmap_in_progress' variable and switched to use
aio_bh_new() isntead of oneshot variant in the "blob commands" patch.

- Further improved error messages by printing error code when actual error
occurrs and using ERR_UNSPEC instead of ERR_ENOMEM when we don't really
know if it was ENOMEM for sure.

- Added vdc->unrealize for the virtio GL device and freed virgl data.

- Dropped UUID and doc/migration patches. UUID feature isn't needed
anymore, instead we changed Mesa Venus driver to not require UUID.

- Renamed virtio-gpu-gl "vulkan" property name back to "venus".

Changes from V8 to V9

- Added resuming of cmdq processing when hostmem MR is freed,
as was suggested by Akihiko Odaki.

- Added more error messages, suggested by Akihiko Odaki

- Dropped superfluous 'res->async_unmap_completed', suggested
by Akihiko Odaki.

- Kept using cmd->suspended flag. Akihiko Odaki suggested to make
virtio_gpu_virgl_process_cmd() return false if cmd processing is
suspended, but it's not easy to implement due to ubiquitous
VIRTIO_GPU_FILL_CMD() macros that returns void, requiring to change
all the virtio-gpu processing code.

- Added back virtio_gpu_virgl_resource as was requested by Akihiko Odaki,
though I'm not convinced it's really needed.

- Switched to use GArray, renamed capset2_max_ver/size vars and moved
"vulkan" property definition to the virtio-gpu-gl device in the Venus
patch, like was suggested by Akihiko Odaki.

- Moved UUID to virtio_gpu_virgl_resource and dropped UUID save/restore
since it will require bumping VM version and virgl device isn't miratable
anyways.

- Fixed exposing UUID feature with Rutabaga

- Dropped linux-headers update patch because headers were already updated
in Qemu/staging.

- Added patch that updates virtio migration doc with a note about virtio-gpu
migration specifics, suggested by Akihiko Odaki.

- Addressed coding style issue noticed by Akihiko Odaki

Changes from V7 to V8

- Supported suspension of virtio-gpu commands processing and made
unmapping of hostmem region asynchronous by blocking/suspending
cmd processing until region is unmapped. Suggested by Akihiko Odaki.

- Fixed arm64 building of x86 targets using updated linux-headers.
Corrected the update script. Thanks to Rob Clark for reporting
the issue.

- Added new patch that makes registration of virgl capsets dynamic.
Requested by Antonio Caggiano and Pierre-Eric Pelloux-Prayer.

- Venus capset now isn't advertised if Vulkan is disabled with vulkan=false

Changes from V6 to V7

- Used scripts/update-linux-headers.sh to update Qemu headers based
on Linux v6.8-rc3 that adds Venus capset definition to virtio-gpu
protocol, was requested by Peter Maydel

- Added r-bs that were given to v6 patches. Corrected missing s-o-bs

- Dropped context_init Qemu's virtio-gpu device configuration flag,
was suggested by Marc-André Lureau

- Added missing error condition checks spotted by Marc-André Lureau
and Akihiko Odaki, and few more

- Returned back res->mr referencing to memory_region_init_ram_ptr() like
was suggested by Akihiko Odaki. Incorporated fix suggested by Pierre-Eric
to specify the MR name

- Dropped the virgl_gpu_resource wrapper, cleaned up and simplified
patch that adds blob-cmd support

- Fixed improper blob resource removal from resource list on resource_unref
that was spotted by Akihiko Odaki

- Change order of the blob patches, was suggested by Akihiko Odaki.
The cmd_set_scanout_blob support is enabled first

- Factored out patch that adds resource management support to virtio-gpu-gl,
was requested by Marc-André Lureau

- Simplified and improved the UUID support patch, dropped the hash table
as we don't need it for now. Moved QemuUUID to virtio_gpu_simple_resource.
This all was suggested by Akihiko Odaki and Marc-André Lureau

- Dropped console_has_gl() check, suggested by Akihiko Odaki

- Reworked Meson cheking of libvirglrender features, made new features
available based on virglrender pkgconfig version instead of checking
symbols in header. This should fix build error using older virglrender
ve

[PULL 1/9] target/alpha: Use cpu_env in preference to ALPHA_CPU

ALPHA_CPU has a dynamic object type assert, which is
unnecessary considering that these are all class hooks.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20240503072014.24751-2-phi...@linaro.org>
---
 target/alpha/cpu.c| 15 ++-
 target/alpha/helper.c |  8 
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index 05f9ee41e9..f98d022671 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -28,25 +28,22 @@
 
 static void alpha_cpu_set_pc(CPUState *cs, vaddr value)
 {
-AlphaCPU *cpu = ALPHA_CPU(cs);
-
-cpu->env.pc = value;
+CPUAlphaState *env = cpu_env(cs);
+env->pc = value;
 }
 
 static vaddr alpha_cpu_get_pc(CPUState *cs)
 {
-AlphaCPU *cpu = ALPHA_CPU(cs);
-
-return cpu->env.pc;
+CPUAlphaState *env = cpu_env(cs);
+return env->pc;
 }
 
 static void alpha_restore_state_to_opc(CPUState *cs,
const TranslationBlock *tb,
const uint64_t *data)
 {
-AlphaCPU *cpu = ALPHA_CPU(cs);
-
-cpu->env.pc = data[0];
+CPUAlphaState *env = cpu_env(cs);
+env->pc = data[0];
 }
 
 static bool alpha_cpu_has_work(CPUState *cs)
diff --git a/target/alpha/helper.c b/target/alpha/helper.c
index d6d4353edd..c5e4958f8b 100644
--- a/target/alpha/helper.c
+++ b/target/alpha/helper.c
@@ -124,7 +124,7 @@ void alpha_cpu_record_sigsegv(CPUState *cs, vaddr address,
   MMUAccessType access_type,
   bool maperr, uintptr_t retaddr)
 {
-AlphaCPU *cpu = ALPHA_CPU(cs);
+CPUAlphaState *env = cpu_env(cs);
 target_ulong mmcsr, cause;
 
 /* Assuming !maperr, infer the missing protection. */
@@ -155,9 +155,9 @@ void alpha_cpu_record_sigsegv(CPUState *cs, vaddr address,
 }
 
 /* Record the arguments that PALcode would give to the kernel. */
-cpu->env.trap_arg0 = address;
-cpu->env.trap_arg1 = mmcsr;
-cpu->env.trap_arg2 = cause;
+env->trap_arg0 = address;
+env->trap_arg1 = mmcsr;
+env->trap_arg2 = cause;
 }
 #else
 /* Returns the OSF/1 entMM failure indication, or -1 on success.  */
-- 
2.34.1

[PULL 3/9] target/alpha: Use DISAS_NEXT definition instead of magic '0' value

Signed-off-by: Richard Henderson 
Message-Id: <20240424234436.995410-4-richard.hender...@linaro.org>
[PMD: Split bigger patch, part 1/5]
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20240503072014.24751-4-phi...@linaro.org>
---
 target/alpha/translate.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 52c2e6248b..9ad7bf6e5f 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -440,8 +440,10 @@ static DisasJumpType gen_bdirect(DisasContext *ctx, int 
ra, int32_t disp)
 
 /* Notice branch-to-next; used to initialize RA with the PC.  */
 if (disp == 0) {
-return 0;
-} else if (use_goto_tb(ctx, dest)) {
+return DISAS_NEXT;
+}
+
+if (use_goto_tb(ctx, dest)) {
 tcg_gen_goto_tb(0);
 tcg_gen_movi_i64(cpu_pc, dest);
 tcg_gen_exit_tb(ctx->base.tb, 0);
-- 
2.34.1

[PULL 9/9] target/alpha: Implement CF_PCREL

Signed-off-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20240503072014.24751-10-phi...@linaro.org>
---
 target/alpha/cpu.c   | 23 ++-
 target/alpha/translate.c | 29 +
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
index f98d022671..0e2fbcb397 100644
--- a/target/alpha/cpu.c
+++ b/target/alpha/cpu.c
@@ -38,12 +38,27 @@ static vaddr alpha_cpu_get_pc(CPUState *cs)
 return env->pc;
 }
 
+static void alpha_cpu_synchronize_from_tb(CPUState *cs,
+  const TranslationBlock *tb)
+{
+/* The program counter is always up to date with CF_PCREL. */
+if (!(tb_cflags(tb) & CF_PCREL)) {
+CPUAlphaState *env = cpu_env(cs);
+env->pc = tb->pc;
+}
+}
+
 static void alpha_restore_state_to_opc(CPUState *cs,
const TranslationBlock *tb,
const uint64_t *data)
 {
 CPUAlphaState *env = cpu_env(cs);
-env->pc = data[0];
+
+if (tb_cflags(tb) & CF_PCREL) {
+env->pc = (env->pc & TARGET_PAGE_MASK) | data[0];
+} else {
+env->pc = data[0];
+}
 }
 
 static bool alpha_cpu_has_work(CPUState *cs)
@@ -78,6 +93,11 @@ static void alpha_cpu_realizefn(DeviceState *dev, Error 
**errp)
 AlphaCPUClass *acc = ALPHA_CPU_GET_CLASS(dev);
 Error *local_err = NULL;
 
+#ifndef CONFIG_USER_ONLY
+/* Use pc-relative instructions in system-mode */
+cs->tcg_cflags |= CF_PCREL;
+#endif
+
 cpu_exec_realizefn(cs, &local_err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
@@ -190,6 +210,7 @@ static const struct SysemuCPUOps alpha_sysemu_ops = {
 
 static const TCGCPUOps alpha_tcg_ops = {
 .initialize = alpha_translate_init,
+.synchronize_from_tb = alpha_cpu_synchronize_from_tb,
 .restore_state_to_opc = alpha_restore_state_to_opc,
 
 #ifdef CONFIG_USER_ONLY
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 86402d96d5..db847e7a23 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -54,6 +54,9 @@ struct DisasContext {
 uint32_t tbflags;
 int mem_idx;
 
+/* True if generating pc-relative code.  */
+bool pcrel;
+
 /* implver and amask values for this CPU.  */
 int implver;
 int amask;
@@ -254,7 +257,12 @@ static void st_flag_byte(TCGv val, unsigned shift)
 
 static void gen_pc_disp(DisasContext *ctx, TCGv dest, int32_t disp)
 {
-tcg_gen_movi_i64(dest, ctx->base.pc_next + disp);
+uint64_t addr = ctx->base.pc_next + disp;
+if (ctx->pcrel) {
+tcg_gen_addi_i64(dest, cpu_pc, addr - ctx->base.pc_first);
+} else {
+tcg_gen_movi_i64(dest, addr);
+}
 }
 
 static void gen_excp_1(int exception, int error_code)
@@ -433,8 +441,14 @@ static DisasJumpType gen_store_conditional(DisasContext 
*ctx, int ra, int rb,
 static void gen_goto_tb(DisasContext *ctx, int idx, int32_t disp)
 {
 if (translator_use_goto_tb(&ctx->base, ctx->base.pc_next + disp)) {
-tcg_gen_goto_tb(idx);
-gen_pc_disp(ctx, cpu_pc, disp);
+/* With PCREL, PC must always be up-to-date. */
+if (ctx->pcrel) {
+gen_pc_disp(ctx, cpu_pc, disp);
+tcg_gen_goto_tb(idx);
+} else {
+tcg_gen_goto_tb(idx);
+gen_pc_disp(ctx, cpu_pc, disp);
+}
 tcg_gen_exit_tb(ctx->base.tb, idx);
 } else {
 gen_pc_disp(ctx, cpu_pc, disp);
@@ -2852,6 +2866,7 @@ static void alpha_tr_init_disas_context(DisasContextBase 
*dcbase, CPUState *cpu)
 
 ctx->tbflags = ctx->base.tb->flags;
 ctx->mem_idx = alpha_env_mmu_index(env);
+ctx->pcrel = ctx->base.tb->cflags & CF_PCREL;
 ctx->implver = env->implver;
 ctx->amask = env->amask;
 
@@ -2887,7 +2902,13 @@ static void alpha_tr_tb_start(DisasContextBase *db, 
CPUState *cpu)
 
 static void alpha_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
 {
-tcg_gen_insn_start(dcbase->pc_next);
+DisasContext *ctx = container_of(dcbase, DisasContext, base);
+
+if (ctx->pcrel) {
+tcg_gen_insn_start(dcbase->pc_next & ~TARGET_PAGE_MASK);
+} else {
+tcg_gen_insn_start(dcbase->pc_next);
+}
 }
 
 static void alpha_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
-- 
2.34.1

[PULL 0/9] target/alpha: Implement CF_PCREL

The following changes since commit 97c872276d147c882296f5da245bd8432f1582f6:

  Merge tag 'accel-sh4-ui-20240503' of https://github.com/philmd/qemu into 
staging (2024-05-03 14:42:50 -0700)

are available in the Git repository at:

  https://gitlab.com/rth7680/qemu.git tags/pull-axp-20240504

for you to fetch changes up to 23bb086350c0de390f77dd034d775742314cabd7:

  target/alpha: Implement CF_PCREL (2024-05-04 08:05:51 -0700)


target/alpha: Implement CF_PCREL


Philippe Mathieu-Daudé (1):
  target/alpha: Simplify gen_bcond_internal()

Richard Henderson (8):
  target/alpha: Use cpu_env in preference to ALPHA_CPU
  target/alpha: Hoist branch shift to initial decode
  target/alpha: Use DISAS_NEXT definition instead of magic '0' value
  target/alpha: Inline DISAS_PC_UPDATED and return DISAS_NORETURN
  target/alpha: Return DISAS_NORETURN once
  target/alpha: Split out gen_goto_tb
  target/alpha: Split out gen_pc_disp
  target/alpha: Implement CF_PCREL

 target/alpha/cpu.c   |  32 ++---
 target/alpha/helper.c|   8 ++--
 target/alpha/translate.c | 117 +--
 3 files changed, 91 insertions(+), 66 deletions(-)

[PULL 4/9] target/alpha: Inline DISAS_PC_UPDATED and return DISAS_NORETURN

Inline DISAS_PC_UPDATED switch case from alpha_tr_tb_stop():

switch (ctx->base.is_jmp) {
...
case DISAS_PC_UPDATED:
tcg_gen_lookup_and_goto_ptr();
break;

Signed-off-by: Richard Henderson 
Message-Id: <20240424234436.995410-4-richard.hender...@linaro.org>
[PMD: Split bigger patch, part 2/5]
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20240503072014.24751-5-phi...@linaro.org>
---
 target/alpha/translate.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 9ad7bf6e5f..01914e7b56 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -450,7 +450,8 @@ static DisasJumpType gen_bdirect(DisasContext *ctx, int ra, 
int32_t disp)
 return DISAS_NORETURN;
 } else {
 tcg_gen_movi_i64(cpu_pc, dest);
-return DISAS_PC_UPDATED;
+tcg_gen_lookup_and_goto_ptr();
+return DISAS_NORETURN;
 }
 }
 
@@ -479,7 +480,8 @@ static DisasJumpType gen_bcond_internal(DisasContext *ctx, 
TCGCond cond,
 TCGv_i64 p = tcg_constant_i64(ctx->base.pc_next);
 
 tcg_gen_movcond_i64(cond, cpu_pc, cmp, i, d, p);
-return DISAS_PC_UPDATED;
+tcg_gen_lookup_and_goto_ptr();
+return DISAS_NORETURN;
 }
 }
 
-- 
2.34.1

[PULL 8/9] target/alpha: Split out gen_pc_disp

Prepare for pcrel by not modifying cpu_pc before use,
in the case of JSR.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20240503072014.24751-9-phi...@linaro.org>
---
 target/alpha/translate.c | 41 ++--
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index c1a55e5153..86402d96d5 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -252,6 +252,11 @@ static void st_flag_byte(TCGv val, unsigned shift)
 tcg_gen_st8_i64(val, tcg_env, get_flag_ofs(shift));
 }
 
+static void gen_pc_disp(DisasContext *ctx, TCGv dest, int32_t disp)
+{
+tcg_gen_movi_i64(dest, ctx->base.pc_next + disp);
+}
+
 static void gen_excp_1(int exception, int error_code)
 {
 TCGv_i32 tmp1, tmp2;
@@ -263,7 +268,7 @@ static void gen_excp_1(int exception, int error_code)
 
 static DisasJumpType gen_excp(DisasContext *ctx, int exception, int error_code)
 {
-tcg_gen_movi_i64(cpu_pc, ctx->base.pc_next);
+gen_pc_disp(ctx, cpu_pc, 0);
 gen_excp_1(exception, error_code);
 return DISAS_NORETURN;
 }
@@ -427,14 +432,12 @@ static DisasJumpType gen_store_conditional(DisasContext 
*ctx, int ra, int rb,
 
 static void gen_goto_tb(DisasContext *ctx, int idx, int32_t disp)
 {
-uint64_t dest = ctx->base.pc_next + disp;
-
-if (translator_use_goto_tb(&ctx->base, dest)) {
+if (translator_use_goto_tb(&ctx->base, ctx->base.pc_next + disp)) {
 tcg_gen_goto_tb(idx);
-tcg_gen_movi_i64(cpu_pc, dest);
+gen_pc_disp(ctx, cpu_pc, disp);
 tcg_gen_exit_tb(ctx->base.tb, idx);
 } else {
-tcg_gen_movi_i64(cpu_pc, dest);
+gen_pc_disp(ctx, cpu_pc, disp);
 tcg_gen_lookup_and_goto_ptr();
 }
 }
@@ -442,7 +445,7 @@ static void gen_goto_tb(DisasContext *ctx, int idx, int32_t 
disp)
 static DisasJumpType gen_bdirect(DisasContext *ctx, int ra, int32_t disp)
 {
 if (ra != 31) {
-tcg_gen_movi_i64(ctx->ir[ra], ctx->base.pc_next);
+gen_pc_disp(ctx, ctx->ir[ra], 0);
 }
 
 /* Notice branch-to-next; used to initialize RA with the PC.  */
@@ -1091,7 +1094,7 @@ static DisasJumpType gen_call_pal(DisasContext *ctx, int 
palcode)
 }
 
 /* Allow interrupts to be recognized right away.  */
-tcg_gen_movi_i64(cpu_pc, ctx->base.pc_next);
+gen_pc_disp(ctx, cpu_pc, 0);
 return DISAS_PC_UPDATED_NOCHAIN;
 
 case 0x36:
@@ -1138,19 +1141,17 @@ static DisasJumpType gen_call_pal(DisasContext *ctx, 
int palcode)
 #else
 {
 TCGv tmp = tcg_temp_new();
-uint64_t exc_addr = ctx->base.pc_next;
-uint64_t entry = ctx->palbr;
+uint64_t entry;
 
+gen_pc_disp(ctx, tmp, 0);
 if (ctx->tbflags & ENV_FLAG_PAL_MODE) {
-exc_addr |= 1;
+tcg_gen_ori_i64(tmp, tmp, 1);
 } else {
-tcg_gen_movi_i64(tmp, 1);
-st_flag_byte(tmp, ENV_FLAG_PAL_SHIFT);
+st_flag_byte(tcg_constant_i64(1), ENV_FLAG_PAL_SHIFT);
 }
-
-tcg_gen_movi_i64(tmp, exc_addr);
 tcg_gen_st_i64(tmp, tcg_env, offsetof(CPUAlphaState, exc_addr));
 
+entry = ctx->palbr;
 entry += (palcode & 0x80
   ? 0x2000 + (palcode - 0x80) * 64
   : 0x1000 + palcode * 64);
@@ -2344,9 +2345,13 @@ static DisasJumpType translate_one(DisasContext *ctx, 
uint32_t insn)
 /* JMP, JSR, RET, JSR_COROUTINE.  These only differ by the branch
prediction stack action, which of course we don't implement.  */
 vb = load_gpr(ctx, rb);
-tcg_gen_andi_i64(cpu_pc, vb, ~3);
 if (ra != 31) {
-tcg_gen_movi_i64(ctx->ir[ra], ctx->base.pc_next);
+tmp = tcg_temp_new();
+tcg_gen_andi_i64(tmp, vb, ~3);
+gen_pc_disp(ctx, ctx->ir[ra], 0);
+tcg_gen_mov_i64(cpu_pc, tmp);
+} else {
+tcg_gen_andi_i64(cpu_pc, vb, ~3);
 }
 ret = DISAS_PC_UPDATED;
 break;
@@ -2908,7 +2913,7 @@ static void alpha_tr_tb_stop(DisasContextBase *dcbase, 
CPUState *cpu)
 gen_goto_tb(ctx, 0, 0);
 break;
 case DISAS_PC_STALE:
-tcg_gen_movi_i64(cpu_pc, ctx->base.pc_next);
+gen_pc_disp(ctx, cpu_pc, 0);
 /* FALLTHRU */
 case DISAS_PC_UPDATED:
 tcg_gen_lookup_and_goto_ptr();
-- 
2.34.1

[PULL 7/9] target/alpha: Split out gen_goto_tb

Signed-off-by: Richard Henderson 
Message-Id: <20240424234436.995410-4-richard.hender...@linaro.org>
[PMD: Split bigger patch, part 5/5]
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20240503072014.24751-8-phi...@linaro.org>
---
 target/alpha/translate.c | 53 
 1 file changed, 16 insertions(+), 37 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index b7b94cc378..c1a55e5153 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -425,15 +425,22 @@ static DisasJumpType gen_store_conditional(DisasContext 
*ctx, int ra, int rb,
 return DISAS_NEXT;
 }
 
-static bool use_goto_tb(DisasContext *ctx, uint64_t dest)
+static void gen_goto_tb(DisasContext *ctx, int idx, int32_t disp)
 {
-return translator_use_goto_tb(&ctx->base, dest);
+uint64_t dest = ctx->base.pc_next + disp;
+
+if (translator_use_goto_tb(&ctx->base, dest)) {
+tcg_gen_goto_tb(idx);
+tcg_gen_movi_i64(cpu_pc, dest);
+tcg_gen_exit_tb(ctx->base.tb, idx);
+} else {
+tcg_gen_movi_i64(cpu_pc, dest);
+tcg_gen_lookup_and_goto_ptr();
+}
 }
 
 static DisasJumpType gen_bdirect(DisasContext *ctx, int ra, int32_t disp)
 {
-uint64_t dest = ctx->base.pc_next + disp;
-
 if (ra != 31) {
 tcg_gen_movi_i64(ctx->ir[ra], ctx->base.pc_next);
 }
@@ -442,43 +449,19 @@ static DisasJumpType gen_bdirect(DisasContext *ctx, int 
ra, int32_t disp)
 if (disp == 0) {
 return DISAS_NEXT;
 }
-
-if (use_goto_tb(ctx, dest)) {
-tcg_gen_goto_tb(0);
-tcg_gen_movi_i64(cpu_pc, dest);
-tcg_gen_exit_tb(ctx->base.tb, 0);
-} else {
-tcg_gen_movi_i64(cpu_pc, dest);
-tcg_gen_lookup_and_goto_ptr();
-}
-
+gen_goto_tb(ctx, 0, disp);
 return DISAS_NORETURN;
 }
 
 static DisasJumpType gen_bcond_internal(DisasContext *ctx, TCGCond cond,
 TCGv cmp, uint64_t imm, int32_t disp)
 {
-uint64_t dest = ctx->base.pc_next + disp;
 TCGLabel *lab_true = gen_new_label();
 
 tcg_gen_brcondi_i64(cond, cmp, imm, lab_true);
-if (use_goto_tb(ctx, ctx->base.pc_next)) {
-tcg_gen_goto_tb(0);
-tcg_gen_movi_i64(cpu_pc, ctx->base.pc_next);
-tcg_gen_exit_tb(ctx->base.tb, 0);
-} else {
-tcg_gen_movi_i64(cpu_pc, ctx->base.pc_next);
-tcg_gen_lookup_and_goto_ptr();
-}
+gen_goto_tb(ctx, 0, 0);
 gen_set_label(lab_true);
-if (use_goto_tb(ctx, dest)) {
-tcg_gen_goto_tb(1);
-tcg_gen_movi_i64(cpu_pc, dest);
-tcg_gen_exit_tb(ctx->base.tb, 1);
-} else {
-tcg_gen_movi_i64(cpu_pc, dest);
-tcg_gen_lookup_and_goto_ptr();
-}
+gen_goto_tb(ctx, 1, disp);
 
 return DISAS_NORETURN;
 }
@@ -2922,12 +2905,8 @@ static void alpha_tr_tb_stop(DisasContextBase *dcbase, 
CPUState *cpu)
 case DISAS_NORETURN:
 break;
 case DISAS_TOO_MANY:
-if (use_goto_tb(ctx, ctx->base.pc_next)) {
-tcg_gen_goto_tb(0);
-tcg_gen_movi_i64(cpu_pc, ctx->base.pc_next);
-tcg_gen_exit_tb(ctx->base.tb, 0);
-}
-/* FALLTHRU */
+gen_goto_tb(ctx, 0, 0);
+break;
 case DISAS_PC_STALE:
 tcg_gen_movi_i64(cpu_pc, ctx->base.pc_next);
 /* FALLTHRU */
-- 
2.34.1

[PULL 5/9] target/alpha: Return DISAS_NORETURN once

Trivial change to make next commits easier to understand.

Signed-off-by: Richard Henderson 
Message-Id: <20240424234436.995410-4-richard.hender...@linaro.org>
[PMD: Split bigger patch, part 3/5]
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20240503072014.24751-6-phi...@linaro.org>
---
 target/alpha/translate.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 01914e7b56..41151f002e 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -447,12 +447,12 @@ static DisasJumpType gen_bdirect(DisasContext *ctx, int 
ra, int32_t disp)
 tcg_gen_goto_tb(0);
 tcg_gen_movi_i64(cpu_pc, dest);
 tcg_gen_exit_tb(ctx->base.tb, 0);
-return DISAS_NORETURN;
 } else {
 tcg_gen_movi_i64(cpu_pc, dest);
 tcg_gen_lookup_and_goto_ptr();
-return DISAS_NORETURN;
 }
+
+return DISAS_NORETURN;
 }
 
 static DisasJumpType gen_bcond_internal(DisasContext *ctx, TCGCond cond,
@@ -472,8 +472,6 @@ static DisasJumpType gen_bcond_internal(DisasContext *ctx, 
TCGCond cond,
 tcg_gen_goto_tb(1);
 tcg_gen_movi_i64(cpu_pc, dest);
 tcg_gen_exit_tb(ctx->base.tb, 1);
-
-return DISAS_NORETURN;
 } else {
 TCGv_i64 i = tcg_constant_i64(imm);
 TCGv_i64 d = tcg_constant_i64(dest);
@@ -481,8 +479,9 @@ static DisasJumpType gen_bcond_internal(DisasContext *ctx, 
TCGCond cond,
 
 tcg_gen_movcond_i64(cond, cpu_pc, cmp, i, d, p);
 tcg_gen_lookup_and_goto_ptr();
-return DISAS_NORETURN;
 }
+
+return DISAS_NORETURN;
 }
 
 static DisasJumpType gen_bcond(DisasContext *ctx, TCGCond cond, int ra,
-- 
2.34.1

[PULL 6/9] target/alpha: Simplify gen_bcond_internal()

From: Philippe Mathieu-Daudé 

Richard Henderson explained on IRC:

  bcond_internal() used to insist that both branch
  destination and branch fallthrough are use_goto_tb;
  if not, we'd use movcond to compute an indirect jump.
  But it's perfectly fine for e.g. the branch fallthrough
  to use_goto_tb, and the branch destination to use
  an indirect branch.

Signed-off-by: Richard Henderson 
Message-Id: <20240424234436.995410-4-richard.hender...@linaro.org>
[PMD: Split bigger patch, part 4/5]
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20240503072014.24751-7-phi...@linaro.org>
---
 target/alpha/translate.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 41151f002e..b7b94cc378 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -461,23 +461,22 @@ static DisasJumpType gen_bcond_internal(DisasContext 
*ctx, TCGCond cond,
 uint64_t dest = ctx->base.pc_next + disp;
 TCGLabel *lab_true = gen_new_label();
 
-if (use_goto_tb(ctx, dest)) {
-tcg_gen_brcondi_i64(cond, cmp, imm, lab_true);
-
+tcg_gen_brcondi_i64(cond, cmp, imm, lab_true);
+if (use_goto_tb(ctx, ctx->base.pc_next)) {
 tcg_gen_goto_tb(0);
 tcg_gen_movi_i64(cpu_pc, ctx->base.pc_next);
 tcg_gen_exit_tb(ctx->base.tb, 0);
-
-gen_set_label(lab_true);
+} else {
+tcg_gen_movi_i64(cpu_pc, ctx->base.pc_next);
+tcg_gen_lookup_and_goto_ptr();
+}
+gen_set_label(lab_true);
+if (use_goto_tb(ctx, dest)) {
 tcg_gen_goto_tb(1);
 tcg_gen_movi_i64(cpu_pc, dest);
 tcg_gen_exit_tb(ctx->base.tb, 1);
 } else {
-TCGv_i64 i = tcg_constant_i64(imm);
-TCGv_i64 d = tcg_constant_i64(dest);
-TCGv_i64 p = tcg_constant_i64(ctx->base.pc_next);
-
-tcg_gen_movcond_i64(cond, cpu_pc, cmp, i, d, p);
+tcg_gen_movi_i64(cpu_pc, dest);
 tcg_gen_lookup_and_goto_ptr();
 }
 
-- 
2.34.1

[PULL 2/9] target/alpha: Hoist branch shift to initial decode

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20240503072014.24751-3-phi...@linaro.org>
---
 target/alpha/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index a97cd54f0c..52c2e6248b 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -432,7 +432,7 @@ static bool use_goto_tb(DisasContext *ctx, uint64_t dest)
 
 static DisasJumpType gen_bdirect(DisasContext *ctx, int ra, int32_t disp)
 {
-uint64_t dest = ctx->base.pc_next + (disp << 2);
+uint64_t dest = ctx->base.pc_next + disp;
 
 if (ra != 31) {
 tcg_gen_movi_i64(ctx->ir[ra], ctx->base.pc_next);
@@ -455,7 +455,7 @@ static DisasJumpType gen_bdirect(DisasContext *ctx, int ra, 
int32_t disp)
 static DisasJumpType gen_bcond_internal(DisasContext *ctx, TCGCond cond,
 TCGv cmp, uint64_t imm, int32_t disp)
 {
-uint64_t dest = ctx->base.pc_next + (disp << 2);
+uint64_t dest = ctx->base.pc_next + disp;
 TCGLabel *lab_true = gen_new_label();
 
 if (use_goto_tb(ctx, dest)) {
@@ -1382,7 +1382,7 @@ static DisasJumpType translate_one(DisasContext *ctx, 
uint32_t insn)
 real_islit = islit = extract32(insn, 12, 1);
 lit = extract32(insn, 13, 8);
 
-disp21 = sextract32(insn, 0, 21);
+disp21 = sextract32(insn, 0, 21) * 4;
 disp16 = sextract32(insn, 0, 16);
 disp12 = sextract32(insn, 0, 12);
 
-- 
2.34.1

Re: [PATCH v2 6/9] target/alpha: Simplify gen_bcond_internal()


On 5/3/24 00:20, Philippe Mathieu-Daudé wrote:

Richard Henderson explained on IRC:

   bcond_internal() used to insist that both branch
   destination and branch fallthrough are use_goto_tb;
   if not, we'd use movcond to compute an indirect jump.
   But it's perfectly fine for e.g. the branch fallthrough
   to use_goto_tb, and the branch destination to use
   an indirect branch.

Signed-off-by: Richard Henderson 
Message-Id: <20240424234436.995410-4-richard.hender...@linaro.org>
[PMD: Split bigger patch, part 4/5]
Signed-off-by: Philippe Mathieu-Daudé 
---
  target/alpha/translate.c | 17 +++--
  1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 41151f002e..079bd5d3fd 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -461,23 +461,20 @@ static DisasJumpType gen_bcond_internal(DisasContext 
*ctx, TCGCond cond,
  uint64_t dest = ctx->base.pc_next + disp;
  TCGLabel *lab_true = gen_new_label();
  
-if (use_goto_tb(ctx, dest)) {

-tcg_gen_brcondi_i64(cond, cmp, imm, lab_true);
-
+tcg_gen_brcondi_i64(cond, cmp, imm, lab_true);
+if (use_goto_tb(ctx, ctx->base.pc_next)) {
  tcg_gen_goto_tb(0);
  tcg_gen_movi_i64(cpu_pc, ctx->base.pc_next);
  tcg_gen_exit_tb(ctx->base.tb, 0);
-
-gen_set_label(lab_true);
+}
+/* FALLTHRU */


fall through does not work.



+gen_set_label(lab_true);
+if (use_goto_tb(ctx, dest)) {
  tcg_gen_goto_tb(1);
  tcg_gen_movi_i64(cpu_pc, dest);
  tcg_gen_exit_tb(ctx->base.tb, 1);
  } else {
-TCGv_i64 i = tcg_constant_i64(imm);
-TCGv_i64 d = tcg_constant_i64(dest);
-TCGv_i64 p = tcg_constant_i64(ctx->base.pc_next);
-
-tcg_gen_movcond_i64(cond, cpu_pc, cmp, i, d, p);
+tcg_gen_movi_i64(cpu_pc, dest);
  tcg_gen_lookup_and_goto_ptr();
  }


Need the whole else replicated above.


r~

Re: [PATCH] hw/arm/npcm7xx: remove setting of mp-affinity


On 5/4/24 07:17, Dorjoy Chowdhury wrote:

The value of the mp-affinity property being set in npcm7xx_realize is
always the same as the default value it would have when arm_cpu_realizefn
is called if the property is not set here. So there is no need to set
the property value in npcm7xx_realize function.

Signed-off-by: Dorjoy Chowdhury
---
  hw/arm/npcm7xx.c | 3 ---
  1 file changed, 3 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH] target/arm: fix MPIDR value for ARM CPUs with SMT

2024-05-04 Thread Dorjoy Chowdhury

On Sat, May 4, 2024 at 7:31 PM Peter Maydell  wrote:
>
> On Fri, 3 May 2024 at 19:14, Dorjoy Chowdhury  wrote:
> >
> > On Fri, May 3, 2024 at 10:28 PM Peter Maydell  
> > wrote:
> > > In the meantime, there is one tiny bit of this that we can
> > > do now:
> > >
> > > > diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
> > > > index cc68b5d8f1..9d5dcf1a3f 100644
> > > > --- a/hw/arm/npcm7xx.c
> > > > +++ b/hw/arm/npcm7xx.c
> > > > @@ -487,7 +487,7 @@ static void npcm7xx_realize(DeviceState *dev, Error 
> > > > **errp)
> > > >  /* CPUs */
> > > >  for (i = 0; i < nc->num_cpus; i++) {
> > > >  object_property_set_int(OBJECT(&s->cpu[i]), "mp-affinity",
> > > > -arm_build_mp_affinity(i, 
> > > > NPCM7XX_MAX_NUM_CPUS),
> > > > +
> > > > arm_build_mp_affinity(ARM_CPU(&s->cpu[i]), i, NPCM7XX_MAX_NUM_CPUS),
> > > >  &error_abort);
> > > >  object_property_set_int(OBJECT(&s->cpu[i]), "reset-cbar",
> > > >  NPCM7XX_GIC_CPU_IF_ADDR, &error_abort);
> > >
> > > In this file, the value of the mp-affinity property that the
> > > board is setting is always the same as the default value it
> > > would have anyway. So we can delete the call to
> > > object_property_set_int() entirely, which gives us one fewer
> > > place we need to deal with when we do eventually figure out
> > > how the MPIDR values should work.
> > >
> >
> > Before I send the patch removing the "object_property_set_int" line
> > for "mp-affinity", just so that I understand, where else is it that
> > for npcm7xx the mp_affinity is being set? I can't follow the code
> > easily and I am not seeing where else it is being set to the same
> > value. It's a bit hard to follow the initialization codes in QEMU.
>
> The value that npcm7xx sets here is identical to the default value
> that the Arm CPU will use if we don't set the property at all.
> If the board doesn't set the property then the cpu mp_affinity field
> is left at its default of ARM64_AFFINITY_INVALID, which then causes
> arm_cpu_realizefn() to set it to the result of
>arm_build_mp_affinity(cs->cpu_index, ARM_DEFAULT_CPUS_PER_CLUSTER)
> Although ARM_DEFAULT_CPUS_PER_CLUSTER and NPCM7XX_MAX_NUM_CPUS are
> different, the number of CPUs on an npcm7xx is always exactly 2,
> so we never get to a CPU number high enough for that difference
> to cause the mp_affinity value to be different from the default.
> (The two CPUs get an mp_affinity of 0 and 1.)
>

Understood. Thanks! I sent a patch.

Regards,
Dorjoy

[PATCH] hw/arm/npcm7xx: remove setting of mp-affinity

2024-05-04 Thread Dorjoy Chowdhury

The value of the mp-affinity property being set in npcm7xx_realize is
always the same as the default value it would have when arm_cpu_realizefn
is called if the property is not set here. So there is no need to set
the property value in npcm7xx_realize function.

Signed-off-by: Dorjoy Chowdhury 
---
 hw/arm/npcm7xx.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index 9f2d96c733..cb7791301b 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -487,9 +487,6 @@ static void npcm7xx_realize(DeviceState *dev, Error **errp)
 
 /* CPUs */
 for (i = 0; i < nc->num_cpus; i++) {
-object_property_set_int(OBJECT(&s->cpu[i]), "mp-affinity",
-arm_build_mp_affinity(i, NPCM7XX_MAX_NUM_CPUS),
-&error_abort);
 object_property_set_int(OBJECT(&s->cpu[i]), "reset-cbar",
 NPCM7XX_GIC_CPU_IF_ADDR, &error_abort);
 object_property_set_bool(OBJECT(&s->cpu[i]), "reset-hivecs", true,
-- 
2.39.2

Re: [PATCH V8 6/8] physmem: Add helper function to destroy CPU AddressSpace

2024-05-04 Thread Peter Maydell

On Tue, 12 Mar 2024 at 02:02, Salil Mehta  wrote:
>
> Virtual CPU Hot-unplug leads to unrealization of a CPU object. This also
> involves destruction of the CPU AddressSpace. Add common function to help
> destroy the CPU AddressSpace.
>
> Signed-off-by: Salil Mehta 
> Tested-by: Vishnu Pajjuri 
> Reviewed-by: Gavin Shan 
> Tested-by: Xianglai Li 
> Tested-by: Miguel Luis 
> Reviewed-by: Shaoqin Huang 

> diff --git a/system/physmem.c b/system/physmem.c
> index 6e9ed97597..61b32ac4f2 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -761,6 +761,7 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
>
>  if (!cpu->cpu_ases) {
>  cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
> +cpu->cpu_ases_count = cpu->num_ases;
>  }
>
>  newas = &cpu->cpu_ases[asidx];
> @@ -774,6 +775,34 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
>  }
>  }
>
> +void cpu_address_space_destroy(CPUState *cpu, int asidx)
> +{
> +CPUAddressSpace *cpuas;
> +
> +assert(cpu->cpu_ases);
> +assert(asidx >= 0 && asidx < cpu->num_ases);
> +/* KVM cannot currently support multiple address spaces. */
> +assert(asidx == 0 || !kvm_enabled());
> +
> +cpuas = &cpu->cpu_ases[asidx];
> +if (tcg_enabled()) {
> +memory_listener_unregister(&cpuas->tcg_as_listener);
> +}
> +
> +address_space_destroy(cpuas->as);
> +g_free_rcu(cpuas->as, rcu);
> +
> +if (asidx == 0) {
> +/* reset the convenience alias for address space 0 */
> +cpu->as = NULL;
> +}
> +
> +if (--cpu->cpu_ases_count == 0) {
> +g_free(cpu->cpu_ases);
> +cpu->cpu_ases = NULL;
> +}
> +}

When do we need to destroy a single address space in this way
that means we need to keep a count of how many ASes the CPU
currently has? The commit message talks about the case when we
unrealize the whole CPU object, but in that situation you can
just throw away all the ASes at once (eg by calling some
cpu_destroy_address_spaces() function from cpu_common_unrealizefn()).

Also, if we're leaking stuff here by failing to destroy it,
is that a problem for existing CPU types like x86 that we
can already hotplug?

thanks
-- PMM

Re: [PATCH] target/arm: fix MPIDR value for ARM CPUs with SMT

2024-05-04 Thread Peter Maydell

On Fri, 3 May 2024 at 19:14, Dorjoy Chowdhury  wrote:
>
> On Fri, May 3, 2024 at 10:28 PM Peter Maydell  
> wrote:
> > In the meantime, there is one tiny bit of this that we can
> > do now:
> >
> > > diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
> > > index cc68b5d8f1..9d5dcf1a3f 100644
> > > --- a/hw/arm/npcm7xx.c
> > > +++ b/hw/arm/npcm7xx.c
> > > @@ -487,7 +487,7 @@ static void npcm7xx_realize(DeviceState *dev, Error 
> > > **errp)
> > >  /* CPUs */
> > >  for (i = 0; i < nc->num_cpus; i++) {
> > >  object_property_set_int(OBJECT(&s->cpu[i]), "mp-affinity",
> > > -arm_build_mp_affinity(i, 
> > > NPCM7XX_MAX_NUM_CPUS),
> > > +
> > > arm_build_mp_affinity(ARM_CPU(&s->cpu[i]), i, NPCM7XX_MAX_NUM_CPUS),
> > >  &error_abort);
> > >  object_property_set_int(OBJECT(&s->cpu[i]), "reset-cbar",
> > >  NPCM7XX_GIC_CPU_IF_ADDR, &error_abort);
> >
> > In this file, the value of the mp-affinity property that the
> > board is setting is always the same as the default value it
> > would have anyway. So we can delete the call to
> > object_property_set_int() entirely, which gives us one fewer
> > place we need to deal with when we do eventually figure out
> > how the MPIDR values should work.
> >
>
> Before I send the patch removing the "object_property_set_int" line
> for "mp-affinity", just so that I understand, where else is it that
> for npcm7xx the mp_affinity is being set? I can't follow the code
> easily and I am not seeing where else it is being set to the same
> value. It's a bit hard to follow the initialization codes in QEMU.

The value that npcm7xx sets here is identical to the default value
that the Arm CPU will use if we don't set the property at all.
If the board doesn't set the property then the cpu mp_affinity field
is left at its default of ARM64_AFFINITY_INVALID, which then causes
arm_cpu_realizefn() to set it to the result of
   arm_build_mp_affinity(cs->cpu_index, ARM_DEFAULT_CPUS_PER_CLUSTER)
Although ARM_DEFAULT_CPUS_PER_CLUSTER and NPCM7XX_MAX_NUM_CPUS are
different, the number of CPUs on an npcm7xx is always exactly 2,
so we never get to a CPU number high enough for that difference
to cause the mp_affinity value to be different from the default.
(The two CPUs get an mp_affinity of 0 and 1.)

thanks
-- PMM

Re: [PATCH v3] target/i386: Fix CPUID encoding of Fn8000001E_ECX

2024-05-04 Thread Paolo Bonzini

Queued, thanks.

Paolo

Re: [PATCH v2 0/5] vvfat: Fix write bugs for large files and add iotests

Explaination of the patch list:

These patches fix some bugs found when modifying files in vvfat.
First, there was a bug when writing to the cluster 2 or above of a file, it
will copy the cluster before it instead, so, when writing to cluster=2, the
content of cluster=1 will be copied into disk instead in its place.

Another issue was modifying the clusters of a file and adding new
clusters, this showed 2 issues:
- If the new cluster is not immediately after the last cluster, it will
cause issues when reading from this file in the future.
- Generally, the usage of info.file.offset was incorrect, and the
system would crash on abort() when the file is modified and a new
cluster was added.

Also, added some iotests for vvfat , covering the this fix and also
general behavior such as reading and writing to the first cluster which
would pass even before this patch.

On May 4 2024, at 4:44 pm, Amjad Alsharafi  wrote:
> v2:
> Added iotests for `vvfat` driver along with a simple `fat16` module to run 
> the tests.
>
> v1:
> https://patchew.org/QEMU/20240327201231.31046-1-amjadsharaf...@gmail.com/
> Fix the issue of writing to the middle of the file in vvfat
>
> Amjad Alsharafi (5):
> vvfat: Fix bug in writing to middle of file
> vvfat: Fix usage of `info.file.offset`
> vvfat: Fix reading files with non-continuous clusters
> iotests: Add `vvfat` tests
> iotests: Filter out `vvfat` fmt from failing tests
>
> .gitlab-ci.d/buildtest.yml | 1 +
> block/vvfat.c | 32 +-
> tests/qemu-iotests/001 | 1 +
> tests/qemu-iotests/002 | 1 +
> tests/qemu-iotests/003 | 1 +
> tests/qemu-iotests/005 | 1 +
> tests/qemu-iotests/008 | 1 +
> tests/qemu-iotests/009 | 1 +
> tests/qemu-iotests/010 | 1 +
> tests/qemu-iotests/011 | 1 +
> tests/qemu-iotests/012 | 1 +
> tests/qemu-iotests/021 | 1 +
> tests/qemu-iotests/032 | 1 +
> tests/qemu-iotests/033 | 1 +
> tests/qemu-iotests/052 | 1 +
> tests/qemu-iotests/094 | 1 +
> tests/qemu-iotests/120 | 2 +-
> tests/qemu-iotests/140 | 1 +
> tests/qemu-iotests/145 | 1 +
> tests/qemu-iotests/157 | 1 +
> tests/qemu-iotests/159 | 2 +-
> tests/qemu-iotests/170 | 2 +-
> tests/qemu-iotests/192 | 1 +
> tests/qemu-iotests/197 | 2 +-
> tests/qemu-iotests/208 | 2 +-
> tests/qemu-iotests/215 | 2 +-
> tests/qemu-iotests/236 | 2 +-
> tests/qemu-iotests/251 | 1 +
> tests/qemu-iotests/307 | 2 +-
> tests/qemu-iotests/308 | 2 +-
> tests/qemu-iotests/check | 2 +-
> tests/qemu-iotests/fat16.py | 507 ++
> tests/qemu-iotests/meson.build | 3 +-
> .../tests/export-incoming-iothread | 2 +-
> tests/qemu-iotests/tests/fuse-allow-other | 1 +
> .../tests/mirror-ready-cancel-error | 2 +-
> tests/qemu-iotests/tests/regression-vhdx-log | 1 +
> tests/qemu-iotests/tests/vvfat | 400 ++
> tests/qemu-iotests/tests/vvfat.out | 5 +
> 39 files changed, 967 insertions(+), 26 deletions(-)
> create mode 100644 tests/qemu-iotests/fat16.py
> create mode 100755 tests/qemu-iotests/tests/vvfat
> create mode 100755 tests/qemu-iotests/tests/vvfat.out
>
> --
> 2.44.0
>

[PATCH v2 4/5] iotests: Add `vvfat` tests

Added several tests to verify the implementation of the vvfat driver.

We needed a way to interact with it, so created a basic `fat16.py` driver that 
handled writing correct sectors for us.

Signed-off-by: Amjad Alsharafi 
---
 tests/qemu-iotests/check   |   2 +-
 tests/qemu-iotests/fat16.py| 507 +
 tests/qemu-iotests/tests/vvfat | 400 +++
 tests/qemu-iotests/tests/vvfat.out |   5 +
 4 files changed, 913 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemu-iotests/fat16.py
 create mode 100755 tests/qemu-iotests/tests/vvfat
 create mode 100755 tests/qemu-iotests/tests/vvfat.out

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 56d88ca423..545f9ec7bd 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -84,7 +84,7 @@ def make_argparser() -> argparse.ArgumentParser:
 p.set_defaults(imgfmt='raw', imgproto='file')
 
 format_list = ['raw', 'bochs', 'cloop', 'parallels', 'qcow', 'qcow2',
-   'qed', 'vdi', 'vpc', 'vhdx', 'vmdk', 'luks', 'dmg']
+   'qed', 'vdi', 'vpc', 'vhdx', 'vmdk', 'luks', 'dmg', 'vvfat']
 g_fmt = p.add_argument_group(
 '  image format options',
 'The following options set the IMGFMT environment variable. '
diff --git a/tests/qemu-iotests/fat16.py b/tests/qemu-iotests/fat16.py
new file mode 100644
index 00..6ac5508d8d
--- /dev/null
+++ b/tests/qemu-iotests/fat16.py
@@ -0,0 +1,507 @@
+# A simple FAT16 driver that is used to test the `vvfat` driver in QEMU.
+#
+# Copyright (C) 2024 Amjad Alsharafi 
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+from typing import List
+
+SECTOR_SIZE = 512
+DIRENTRY_SIZE = 32
+
+
+class MBR:
+def __init__(self, data: bytes):
+assert len(data) == 512
+self.partition_table = []
+for i in range(4):
+partition = data[446 + i * 16 : 446 + (i + 1) * 16]
+self.partition_table.append(
+{
+"status": partition[0],
+"start_head": partition[1],
+"start_sector": partition[2] & 0x3F,
+"start_cylinder": ((partition[2] & 0xC0) << 2) | 
partition[3],
+"type": partition[4],
+"end_head": partition[5],
+"end_sector": partition[6] & 0x3F,
+"end_cylinder": ((partition[6] & 0xC0) << 2) | 
partition[7],
+"start_lba": int.from_bytes(partition[8:12], "little"),
+"size": int.from_bytes(partition[12:16], "little"),
+}
+)
+
+def __str__(self):
+return "\n".join(
+[f"{i}: {partition}" for i, partition in 
enumerate(self.partition_table)]
+)
+
+
+class FatBootSector:
+def __init__(self, data: bytes):
+assert len(data) == 512
+self.bytes_per_sector = int.from_bytes(data[11:13], "little")
+self.sectors_per_cluster = data[13]
+self.reserved_sectors = int.from_bytes(data[14:16], "little")
+self.fat_count = data[16]
+self.root_entries = int.from_bytes(data[17:19], "little")
+self.media_descriptor = data[21]
+self.fat_size = int.from_bytes(data[22:24], "little")
+self.sectors_per_fat = int.from_bytes(data[22:24], "little")
+self.sectors_per_track = int.from_bytes(data[24:26], "little")
+self.heads = int.from_bytes(data[26:28], "little")
+self.hidden_sectors = int.from_bytes(data[28:32], "little")
+self.total_sectors = int.from_bytes(data[32:36], "little")
+self.drive_number = data[36]
+self.volume_id = int.from_bytes(data[39:43], "little")
+self.volume_label = data[43:54].decode("ascii").strip()
+self.fs_type = data[54:62].decode("ascii").strip()
+
+def root_dir_start(self):
+"""
+Calculate the start sector of the root directory.
+"""
+return self.reserved_sectors + self.fat_count * self.sectors_per_fat
+
+def root_dir_size(self):
+"""
+Calculate the size of the root directory in sectors.
+"""
+return (
+self.root_entries * DIRENTRY_SIZE + self.bytes_per_sector - 1
+) // self.bytes_per_sector
+
+def data_sector_start(self):
+"""
+Calculate the start sector o

[PATCH v2 5/5] iotests: Filter out `vvfat` fmt from failing tests

`vvfat` is a special format and not all tests (even generic) can run without 
crashing.
So, added `unsupported_fmt: vvfat` to all failling tests.

Also added `vvfat` format into `meson.build`, vvfaat tests can be run on the 
`block-thorough` suite.

Signed-off-by: Amjad Alsharafi 
---
 .gitlab-ci.d/buildtest.yml | 1 +
 tests/qemu-iotests/001 | 1 +
 tests/qemu-iotests/002 | 1 +
 tests/qemu-iotests/003 | 1 +
 tests/qemu-iotests/005 | 1 +
 tests/qemu-iotests/008 | 1 +
 tests/qemu-iotests/009 | 1 +
 tests/qemu-iotests/010 | 1 +
 tests/qemu-iotests/011 | 1 +
 tests/qemu-iotests/012 | 1 +
 tests/qemu-iotests/021 | 1 +
 tests/qemu-iotests/032 | 1 +
 tests/qemu-iotests/033 | 1 +
 tests/qemu-iotests/052 | 1 +
 tests/qemu-iotests/094 | 1 +
 tests/qemu-iotests/120 | 2 +-
 tests/qemu-iotests/140 | 1 +
 tests/qemu-iotests/145 | 1 +
 tests/qemu-iotests/157 | 1 +
 tests/qemu-iotests/159 | 2 +-
 tests/qemu-iotests/170 | 2 +-
 tests/qemu-iotests/192 | 1 +
 tests/qemu-iotests/197 | 2 +-
 tests/qemu-iotests/208 | 2 +-
 tests/qemu-iotests/215 | 2 +-
 tests/qemu-iotests/236 | 2 +-
 tests/qemu-iotests/251 | 1 +
 tests/qemu-iotests/307 | 2 +-
 tests/qemu-iotests/308 | 2 +-
 tests/qemu-iotests/meson.build | 3 ++-
 tests/qemu-iotests/tests/export-incoming-iothread  | 2 +-
 tests/qemu-iotests/tests/fuse-allow-other  | 1 +
 tests/qemu-iotests/tests/mirror-ready-cancel-error | 2 +-
 tests/qemu-iotests/tests/regression-vhdx-log   | 1 +
 34 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index cfdff175c3..a46c179a6b 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -347,6 +347,7 @@ build-tcg-disabled:
 124 132 139 142 144 145 151 152 155 157 165 194 196 200 202
 208 209 216 218 227 234 246 247 248 250 254 255 257 258
 260 261 262 263 264 270 272 273 277 279 image-fleecing
+- ./check -vvfat vvfat
 
 build-user:
   extends: .native_build_job_template
diff --git a/tests/qemu-iotests/001 b/tests/qemu-iotests/001
index 6f980fd34d..cf905b5d00 100755
--- a/tests/qemu-iotests/001
+++ b/tests/qemu-iotests/001
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 
 _supported_fmt generic
+_unsupported_fmt vvfat
 _supported_proto generic
 
 
diff --git a/tests/qemu-iotests/002 b/tests/qemu-iotests/002
index 5ce1647531..1e557fad8c 100755
--- a/tests/qemu-iotests/002
+++ b/tests/qemu-iotests/002
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 
 _supported_fmt generic
+_unsupported_fmt vvfat
 _supported_proto generic
 _unsupported_imgopts "subformat=streamOptimized"
 
diff --git a/tests/qemu-iotests/003 b/tests/qemu-iotests/003
index 03f902a83c..6e74f1faeb 100755
--- a/tests/qemu-iotests/003
+++ b/tests/qemu-iotests/003
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 
 _supported_fmt generic
+_unsupported_fmt vvfat
 _supported_proto generic
 _unsupported_imgopts "subformat=streamOptimized"
 
diff --git a/tests/qemu-iotests/005 b/tests/qemu-iotests/005
index ba377543b0..28ae66bfcd 100755
--- a/tests/qemu-iotests/005
+++ b/tests/qemu-iotests/005
@@ -41,6 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 
 _supported_fmt generic
+_unsupported_fmt vvfat
 _supported_proto generic
 _supported_os Linux
 _unsupported_imgopts "subformat=twoGbMaxExtentFlat" \
diff --git a/tests/qemu-iotests/008 b/tests/qemu-iotests/008
index fa4990b513..80850ecf12 100755
--- a/tests/qemu-iotests/008
+++ b/tests/qemu-iotests/008
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 
 _supported_fmt generic
+_unsupported_fmt vvfat
 _supported_proto generic
 
 
diff --git a/tests/qemu-iotests/009 b/tests/qemu-iotests/009
index efa852bad3..408617b0bc 100755
--- a/tests/qemu-iotests/009
+++ b/tests/qemu-iotests/009
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 
 _supported_fmt generic
+_unsupported_fmt vvfat
 _supported_proto generic
 _unsupported_imgopts "subformat=streamOptimized"
 
diff --git a/tests/qemu-iotests/010 b/tests/qemu-iotests/010
index 4ae9

[PATCH v2 2/5] vvfat: Fix usage of `info.file.offset`

The field is marked as "the offset in the file (in clusters)", but it
was being used like this
`cluster_size*(nums)+mapping->info.file.offset`, which is incorrect.

Additionally, removed the `abort` when `first_mapping_index` does not
match, as this matches the case when adding new clusters for files, and
its inevitable that we reach this condition when doing that if the
clusters are not after one another, so there is no reason to `abort`
here, execution continues and the new clusters are written to disk
correctly.

Signed-off-by: Amjad Alsharafi 
---
 block/vvfat.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index ab342f0743..cb3ab81e29 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1408,7 +1408,7 @@ read_cluster_directory:
 
 assert(s->current_fd);
 
-
offset=s->cluster_size*(cluster_num-s->current_mapping->begin)+s->current_mapping->info.file.offset;
+offset=s->cluster_size*((cluster_num - s->current_mapping->begin) + 
s->current_mapping->info.file.offset);
 if(lseek(s->current_fd, offset, SEEK_SET)!=offset)
 return -3;
 s->cluster=s->cluster_buffer;
@@ -1929,8 +1929,8 @@ get_cluster_count_for_direntry(BDRVVVFATState* s, 
direntry_t* direntry, const ch
 (mapping->mode & MODE_DIRECTORY) == 0) {
 
 /* was modified in qcow */
-if (offset != mapping->info.file.offset + s->cluster_size
-* (cluster_num - mapping->begin)) {
+if (offset != s->cluster_size
+* ((cluster_num - mapping->begin) + 
mapping->info.file.offset)) {
 /* offset of this cluster in file chain has changed */
 abort();
 copy_it = 1;
@@ -1944,7 +1944,6 @@ get_cluster_count_for_direntry(BDRVVVFATState* s, 
direntry_t* direntry, const ch
 
 if (mapping->first_mapping_index != first_mapping_index
 && mapping->info.file.offset > 0) {
-abort();
 copy_it = 1;
 }
 
@@ -2404,7 +2403,7 @@ static int commit_mappings(BDRVVVFATState* s,
 (mapping->end - mapping->begin);
 } else
 next_mapping->info.file.offset = mapping->info.file.offset +
-mapping->end - mapping->begin;
+(mapping->end - mapping->begin);
 
 mapping = next_mapping;
 }
-- 
2.44.0

[PATCH v2 3/5] vvfat: Fix reading files with non-continuous clusters

When reading with `read_cluster` we get the `mapping` with
`find_mapping_for_cluster` and then we call `open_file` for this
mapping.
The issue appear when its the same file, but a second cluster that is
not immediately after it, imagine clusters `500 -> 503`, this will give
us 2 mappings one has the range `500..501` and another `503..504`, both
point to the same file, but different offsets.

When we don't open the file since the path is the same, we won't assign
`s->current_mapping` and thus accessing way out of bound of the file.

>From our example above, after `open_file` (that didn't open anything) we
will get the offset into the file with
`s->cluster_size*(cluster_num-s->current_mapping->begin)`, which will
give us `0x2000 * (504-500)`, which is out of bound for this mapping and
will produce some issues.

Signed-off-by: Amjad Alsharafi 
---
 block/vvfat.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index cb3ab81e29..87165abc26 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1360,15 +1360,22 @@ static int open_file(BDRVVVFATState* s,mapping_t* 
mapping)
 {
 if(!mapping)
 return -1;
+int new_path = 1;
 if(!s->current_mapping ||
-strcmp(s->current_mapping->path,mapping->path)) {
-/* open file */
-int fd = qemu_open_old(mapping->path,
+
s->current_mapping->first_mapping_index!=mapping->first_mapping_index ||
+(new_path = strcmp(s->current_mapping->path,mapping->path))) {
+
+if (new_path) {
+/* open file */
+int fd = qemu_open_old(mapping->path,
O_RDONLY | O_BINARY | O_LARGEFILE);
-if(fd<0)
-return -1;
-vvfat_close_current_file(s);
-s->current_fd = fd;
+if(fd<0)
+return -1;
+vvfat_close_current_file(s);
+
+s->current_fd = fd;
+}
+assert(s->current_fd);
 s->current_mapping = mapping;
 }
 return 0;
-- 
2.44.0

[PATCH v2 0/5] vvfat: Fix write bugs for large files and add iotests

v2:
  Added iotests for `vvfat` driver along with a simple `fat16` module to run 
the tests.

v1:
  https://patchew.org/QEMU/20240327201231.31046-1-amjadsharaf...@gmail.com/
  Fix the issue of writing to the middle of the file in vvfat

Amjad Alsharafi (5):
  vvfat: Fix bug in writing to middle of file
  vvfat: Fix usage of `info.file.offset`
  vvfat: Fix reading files with non-continuous clusters
  iotests: Add `vvfat` tests
  iotests: Filter out `vvfat` fmt from failing tests

 .gitlab-ci.d/buildtest.yml|   1 +
 block/vvfat.c |  32 +-
 tests/qemu-iotests/001|   1 +
 tests/qemu-iotests/002|   1 +
 tests/qemu-iotests/003|   1 +
 tests/qemu-iotests/005|   1 +
 tests/qemu-iotests/008|   1 +
 tests/qemu-iotests/009|   1 +
 tests/qemu-iotests/010|   1 +
 tests/qemu-iotests/011|   1 +
 tests/qemu-iotests/012|   1 +
 tests/qemu-iotests/021|   1 +
 tests/qemu-iotests/032|   1 +
 tests/qemu-iotests/033|   1 +
 tests/qemu-iotests/052|   1 +
 tests/qemu-iotests/094|   1 +
 tests/qemu-iotests/120|   2 +-
 tests/qemu-iotests/140|   1 +
 tests/qemu-iotests/145|   1 +
 tests/qemu-iotests/157|   1 +
 tests/qemu-iotests/159|   2 +-
 tests/qemu-iotests/170|   2 +-
 tests/qemu-iotests/192|   1 +
 tests/qemu-iotests/197|   2 +-
 tests/qemu-iotests/208|   2 +-
 tests/qemu-iotests/215|   2 +-
 tests/qemu-iotests/236|   2 +-
 tests/qemu-iotests/251|   1 +
 tests/qemu-iotests/307|   2 +-
 tests/qemu-iotests/308|   2 +-
 tests/qemu-iotests/check  |   2 +-
 tests/qemu-iotests/fat16.py   | 507 ++
 tests/qemu-iotests/meson.build|   3 +-
 .../tests/export-incoming-iothread|   2 +-
 tests/qemu-iotests/tests/fuse-allow-other |   1 +
 .../tests/mirror-ready-cancel-error   |   2 +-
 tests/qemu-iotests/tests/regression-vhdx-log  |   1 +
 tests/qemu-iotests/tests/vvfat| 400 ++
 tests/qemu-iotests/tests/vvfat.out|   5 +
 39 files changed, 967 insertions(+), 26 deletions(-)
 create mode 100644 tests/qemu-iotests/fat16.py
 create mode 100755 tests/qemu-iotests/tests/vvfat
 create mode 100755 tests/qemu-iotests/tests/vvfat.out

-- 
2.44.0

[PATCH v2 1/5] vvfat: Fix bug in writing to middle of file