qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Li Qiang <liq3ea@163.com>
To: mst@redhat.com, kraxel@redhat.com, dmitry.fleytman@gmail.com,
	jasowang@redhat.com, alxndr@bu.edu, peter.maydell@linaro.org,
	pbonzini@redhat.com
Cc: Li Qiang <liq3ea@163.com>, liq3ea@gmail.com, qemu-devel@nongnu.org
Subject: [RFC 3/3] virtio-gpu: make the IO handler reentrant
Date: Wed,  2 Sep 2020 09:22:06 -0700	[thread overview]
Message-ID: <20200902162206.101872-4-liq3ea@163.com> (raw)
In-Reply-To: <20200902162206.101872-1-liq3ea@163.com>

The guest can program the virtio desc table. When the used ring
is programmed by virtio-gpu's MMIO it may cause an reentrant issue.

Following is the reproducer:

cat << EOF | ./i386-softmmu/qemu-system-i386 -nographic -M pc -nodefaults -m 512M -device virtio-vga -qtest stdio
outl 0xcf8 0x80001018
outl 0xcfc 0xe0800000
outl 0xcf8 0x80001020
outl 0xcf8 0x80001004
outw 0xcfc 0x7
writeq 0xe0801024 0x10646c00776c6cff
writeq 0xe080102d 0xe0801000320000
writeq 0xe0801015 0x12b2901ba000000
write 0x10646c02 0x1 0x2c
write 0x999 0x1 0x25
write 0x8 0x1 0x78
write 0x2c7 0x1 0x32
write 0x2cb 0x1 0xff
write 0x2cc 0x1 0x7e
writeq 0xe0803000 0xf2b8f0540ff83
EOF

This patch avoid this by adding a 'in_io' in VirtIOGPU to indicate it is in IO processing.
Notice this also address the race condition between 'virtio_gpu_process_cmdq' and
'virtio_gpu_reset' as the 'virtio_gpu_process_cmdq' is run in a BH which in the main thread
and 'virtio_gpu_reset' is run in the vcpu thread and both of them access the 'g->cmdq'.

Buglink: https://bugs.launchpad.net/qemu/+bug/1888606

Reported-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Li Qiang <liq3ea@163.com>
---
 hw/display/virtio-gpu.c        | 10 ++++++++++
 include/hw/virtio/virtio-gpu.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 5f0dd7c150..404b7dc174 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -809,6 +809,10 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
 {
     struct virtio_gpu_ctrl_command *cmd;
 
+    if (atomic_read(&g->in_io)) {
+        return;
+    }
+    atomic_set(&g->in_io, 1);
     while (!QTAILQ_EMPTY(&g->cmdq)) {
         cmd = QTAILQ_FIRST(&g->cmdq);
 
@@ -838,6 +842,7 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
             g_free(cmd);
         }
     }
+    atomic_set(&g->in_io, 0);
 }
 
 static void virtio_gpu_gl_unblock(VirtIOGPUBase *b)
@@ -1144,6 +1149,10 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
     struct virtio_gpu_simple_resource *res, *tmp;
     struct virtio_gpu_ctrl_command *cmd;
 
+    if (atomic_read(&g->in_io)) {
+        return;
+    }
+    atomic_set(&g->in_io, 1);
 #ifdef CONFIG_VIRGL
     if (g->parent_obj.use_virgl_renderer) {
         virtio_gpu_virgl_reset(g);
@@ -1179,6 +1188,7 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
 #endif
 
     virtio_gpu_base_reset(VIRTIO_GPU_BASE(vdev));
+    atomic_set(&g->in_io, 0);
 }
 
 static void
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 7517438e10..aadcf0e332 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -150,6 +150,7 @@ typedef struct VirtIOGPU {
 
     bool renderer_inited;
     bool renderer_reset;
+    bool in_io;
     QEMUTimer *fence_poll;
     QEMUTimer *print_stats;
 
-- 
2.17.1



  parent reply	other threads:[~2020-09-02 16:23 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-02 16:22 [RFC 0/3] try to solve the DMA to MMIO issue Li Qiang
2020-09-02 16:22 ` [RFC 1/3] e1000e: make the IO handler reentrant Li Qiang
2020-09-02 16:22 ` [RFC 2/3] xhci: " Li Qiang
2020-09-02 16:22 ` Li Qiang [this message]
2020-09-03  5:12   ` [RFC 3/3] virtio-gpu: " Michael Tokarev
2020-09-03 10:32     ` Li Qiang
2020-09-03  3:54 ` [RFC 0/3] try to solve the DMA to MMIO issue Jason Wang
2020-09-03  4:06   ` Alexander Bulekov
2020-09-03  4:24     ` Jason Wang
2020-09-03  4:50       ` Li Qiang
2020-09-03  6:16         ` Jason Wang
2020-09-03  6:28           ` Li Qiang
2020-09-03 10:53   ` Peter Maydell
2020-09-03 11:11     ` Li Qiang
2020-09-03 11:19       ` Peter Maydell
2020-09-03 11:23         ` Li Qiang
2020-09-03 11:28           ` Peter Maydell
2020-09-03 13:35             ` Philippe Mathieu-Daudé
2020-09-03 13:41               ` Peter Maydell
2020-09-04  2:45         ` Jason Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200902162206.101872-4-liq3ea@163.com \
    --to=liq3ea@163.com \
    --cc=alxndr@bu.edu \
    --cc=dmitry.fleytman@gmail.com \
    --cc=jasowang@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=liq3ea@gmail.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).