[PATCH v2] hw/scsi/vmw_pvscsi: Remove assertion for kick after reset

Liran Alon posted 1 patch 4 years, 1 month ago
Test asan passed
Test docker-mingw@fedora passed
Test checkpatch passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200316121428.48723-1-liran.alon@oracle.com
Maintainers: Fam Zheng <fam@euphon.net>, Paolo Bonzini <pbonzini@redhat.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>
hw/scsi/vmw_pvscsi.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
[PATCH v2] hw/scsi/vmw_pvscsi: Remove assertion for kick after reset
Posted by Liran Alon 4 years, 1 month ago
From: Elazar Leibovich <elazar.leibovich@oracle.com>

When running Ubuntu 3.13.0-65-generic guest, QEMU sometimes crashes
during guest ACPI reset. It crashes on assert(s->rings_info_valid)
in pvscsi_process_io().

Analyzing the crash revealed that it happens when userspace issues
a sync during a reboot syscall.

Below are backtraces we gathered from the guests.

Guest backtrace when issuing PVSCSI_CMD_ADAPTER_RESET:
    pci_device_shutdown
    device_shutdown
    init_pid_ns
    init_pid_ns
    kernel_power_off
    SYSC_reboot

Guest backtrace when issuing PVSCSI_REG_OFFSET_KICK_RW_IO:
    scsi_done
    scsi_dispatch_cmd
    blk_add_timer
    scsi_request_fn
    elv_rb_add
    __blk_run_queue
    queue_unplugged
    blk_flush_plug_list
    blk_finish_plug
    ext4_writepages
    set_next_entity
    do_writepages
    __filemap_fdatawrite_range
    filemap_write_and_wait_range
    ext4_sync_file
    ext4_sync_file
    do_fsync
    sys_fsync

Since QEMU pvscsi should imitate VMware pvscsi device emulation,
we decided to imitate VMware's behavior in this case.

To check VMware behavior, we wrote a kernel module that issues
a reset to the pvscsi device and then issues a kick. We ran it on
VMware ESXi 6.5 and it seems that it simply ignores the kick.
Hence, we decided to ignore the kick as well.

Signed-off-by: Elazar Leibovich <elazar.leibovich@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 hw/scsi/vmw_pvscsi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index c91352cf46de..2491f204ddd7 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -29,6 +29,7 @@
 #include "qapi/error.h"
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
+#include "qemu/log.h"
 #include "hw/scsi/scsi.h"
 #include "migration/vmstate.h"
 #include "scsi/constants.h"
@@ -719,7 +720,12 @@ pvscsi_process_io(PVSCSIState *s)
     PVSCSIRingReqDesc descr;
     hwaddr next_descr_pa;
 
-    assert(s->rings_info_valid);
+    if (!s->rings_info_valid) {
+        qemu_log("WARNING: PVSCSI: Cannot process I/O when "
+                 "rings are not valid.\n");
+        return;
+    }
+
     while ((next_descr_pa = pvscsi_ring_pop_req_descr(&s->rings)) != 0) {
 
         /* Only read after production index verification */
-- 
2.20.1