[Qemu-devel] [PULL 0/6] More patches for QEMU 2.8-rc2

[Qemu-devel] [PULL 0/6] More patches for QEMU 2.8-rc2

[Paolo Bonzini]

The following changes since commit 00227fefd2059464cd2f59aed29944874c630e2f:

  Update version for v2.8.0-rc1 release (2016-11-22 22:29:08 +0000)

are available in the git repository at:

  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to c96f0ee6a67ca6277366e78ce5d84d5c20dd596f:

  rules.mak: Use -r instead of -Wl, -r to fix building when PIE is default (2016-11-28 16:45:12 +0100)

----------------------------------------------------------------
Small fixes for rc2.

----------------------------------------------------------------
Adrian Bunk (1):
      rules.mak: Use -r instead of -Wl, -r to fix building when PIE is default

Dr. David Alan Gilbert (2):
      migration/pcspk: Add a property to state if pcspk is migrated
      migration/pcspk: Turn migration of pcspk off for 2.7 and older

Paolo Bonzini (2):
      megasas: do not call pci_dma_unmap after having freed the frame once
      megasas: clean up and fix request completion/cancellation

Peter Xu (1):
      pci-assign: sync MSI/MSI-X cap and table with PCIDevice

 hw/audio/pcspk.c         | 10 +++++++++
 hw/i386/kvm/pci-assign.c |  4 ++++
 hw/scsi/megasas.c        | 58 ++++++++++++++++++++++++++----------------------
 include/hw/i386/pc.h     |  5 +++++
 rules.mak                |  2 +-
 5 files changed, 52 insertions(+), 27 deletions(-)
-- 
1.8.3.1

[Qemu-devel] [PULL 1/6] megasas: do not call pci_dma_unmap after having freed the frame once

[Paolo Bonzini]

Commit 8cc4678 ("megasas: remove useless check for cmd->frame", 2016-07-17) was
wrong because I trusted Coverity too much.  It turns out that there _is_ a
path through which cmd->frame can become NULL.  After megasas_handle_frame's
switch (md->frame->header.frame_cmd), megasas_init_firmware can be called.
>From there, megasas_reset_frames will call megasas_unmap_frame which resets
cmd->frame = NULL.

However, there is another bug to fix in there, because megasas_unmap_frame
is called again after setting the command status.  In this case QEMU should
not do anything, instead it calls pci_dma_unmap again.  Harmless, but
better fix it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/megasas.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 52a4123..ca62952 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -461,9 +461,12 @@ static void megasas_unmap_frame(MegasasState *s, MegasasCmd *cmd)
 {
     PCIDevice *p = PCI_DEVICE(s);
 
-    pci_dma_unmap(p, cmd->frame, cmd->pa_size, 0, 0);
+    if (cmd->pa_size) {
+        pci_dma_unmap(p, cmd->frame, cmd->pa_size, 0, 0);
+    }
     cmd->frame = NULL;
     cmd->pa = 0;
+    cmd->pa_size = 0;
     clear_bit(cmd->index, s->frame_map);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 2/6] megasas: clean up and fix request completion/cancellation

[Paolo Bonzini]

megasas_command_cancel is a callback; it should report the abort in
the frame, not try another abort!  Compare for instance with
mptsas_request_cancelled.

So extract the common bits for request completion in a new function
megasas_complete_command, call it from both the .complete and .cancel
callbacks, and remove duplicate pieces from the DCMD path.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20161110152751.4267-2-pbonzini@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi/megasas.c | 53 ++++++++++++++++++++++++++++-------------------------
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index ca62952..67fc1e7 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -300,12 +300,6 @@ unmap:
     return iov_count - i;
 }
 
-static void megasas_unmap_sgl(MegasasCmd *cmd)
-{
-    qemu_sglist_destroy(&cmd->qsg);
-    cmd->iov_offset = 0;
-}
-
 /*
  * passthrough sense and io sense are at the same offset
  */
@@ -580,6 +574,20 @@ static void megasas_complete_frame(MegasasState *s, uint64_t context)
     }
 }
 
+static void megasas_complete_command(MegasasCmd *cmd)
+{
+    qemu_sglist_destroy(&cmd->qsg);
+    cmd->iov_size = 0;
+    cmd->iov_offset = 0;
+
+    cmd->req->hba_private = NULL;
+    scsi_req_unref(cmd->req);
+    cmd->req = NULL;
+
+    megasas_unmap_frame(cmd->state, cmd);
+    megasas_complete_frame(cmd->state, cmd->context);
+}
+
 static void megasas_reset_frames(MegasasState *s)
 {
     int i;
@@ -596,9 +604,9 @@ static void megasas_reset_frames(MegasasState *s)
 
 static void megasas_abort_command(MegasasCmd *cmd)
 {
-    if (cmd->req) {
+    /* Never abort internal commands.  */
+    if (cmd->req != NULL) {
         scsi_req_cancel(cmd->req);
-        cmd->req = NULL;
     }
 }
 
@@ -689,9 +697,6 @@ static void megasas_finish_dcmd(MegasasCmd *cmd, uint32_t iov_size)
 {
     trace_megasas_finish_dcmd(cmd->index, iov_size);
 
-    if (cmd->frame->header.sge_count) {
-        qemu_sglist_destroy(&cmd->qsg);
-    }
     if (iov_size > cmd->iov_size) {
         if (megasas_frame_is_ieee_sgl(cmd)) {
             cmd->frame->dcmd.sgl.sg_skinny->len = cpu_to_le32(iov_size);
@@ -701,7 +706,6 @@ static void megasas_finish_dcmd(MegasasCmd *cmd, uint32_t iov_size)
             cmd->frame->dcmd.sgl.sg32->len = cpu_to_le32(iov_size);
         }
     }
-    cmd->iov_size = 0;
 }
 
 static int megasas_ctrl_get_info(MegasasState *s, MegasasCmd *cmd)
@@ -1589,7 +1593,6 @@ static int megasas_finish_internal_dcmd(MegasasCmd *cmd,
     int lun = req->lun;
 
     opcode = le32_to_cpu(cmd->frame->dcmd.opcode);
-    scsi_req_unref(req);
     trace_megasas_dcmd_internal_finish(cmd->index, opcode, lun);
     switch (opcode) {
     case MFI_DCMD_PD_GET_INFO:
@@ -1860,7 +1863,11 @@ static void megasas_command_complete(SCSIRequest *req, uint32_t status,
 
     trace_megasas_command_complete(cmd->index, status, resid);
 
-    if (cmd->req != req) {
+    if (req->io_canceled) {
+        return;
+    }
+
+    if (cmd->req == NULL) {
         /*
          * Internal command complete
          */
@@ -1879,25 +1886,21 @@ static void megasas_command_complete(SCSIRequest *req, uint32_t status,
             megasas_copy_sense(cmd);
         }
 
-        megasas_unmap_sgl(cmd);
         cmd->frame->header.scsi_status = req->status;
-        scsi_req_unref(cmd->req);
-        cmd->req = NULL;
     }
     cmd->frame->header.cmd_status = cmd_status;
-    megasas_unmap_frame(cmd->state, cmd);
-    megasas_complete_frame(cmd->state, cmd->context);
+    megasas_complete_command(cmd);
 }
 
-static void megasas_command_cancel(SCSIRequest *req)
+static void megasas_command_cancelled(SCSIRequest *req)
 {
     MegasasCmd *cmd = req->hba_private;
 
-    if (cmd) {
-        megasas_abort_command(cmd);
-    } else {
-        scsi_req_unref(req);
+    if (!cmd) {
+        return;
     }
+    cmd->frame->header.cmd_status = MFI_STAT_SCSI_IO_FAILED;
+    megasas_complete_command(cmd);
 }
 
 static int megasas_handle_abort(MegasasState *s, MegasasCmd *cmd)
@@ -2316,7 +2319,7 @@ static const struct SCSIBusInfo megasas_scsi_info = {
     .transfer_data = megasas_xfer_complete,
     .get_sg_list = megasas_get_sg_list,
     .complete = megasas_command_complete,
-    .cancel = megasas_command_cancel,
+    .cancel = megasas_command_cancelled,
 };
 
 static void megasas_scsi_realize(PCIDevice *dev, Error **errp)
-- 
1.8.3.1

[Qemu-devel] [PULL 3/6] pci-assign: sync MSI/MSI-X cap and table with PCIDevice

[Paolo Bonzini]

From: Peter Xu <peterx@redhat.com>

Since commit e1d4fb2d ("kvm-irqchip: x86: add msi route notify fn"),
kvm_irqchip_add_msi_route() starts to use pci_get_msi_message() to fetch
MSI info. This requires that we setup MSI related fields in PCIDevice.
For most devices, that won't be a problem, as long as we are using
general interfaces like msi_init()/msix_init().

However, for pci-assign devices, MSI/MSI-X is treated differently - PCI
assign devices are maintaining its own MSI table and cap information in
AssignedDevice struct. however that's not synced up with PCIDevice's
fields. That will leads to pci_get_msi_message() failed to find correct
MSI capability, even with an NULL msix_table.

A quick fix is to sync up the two places: both the capability bits and
table address for MSI/MSI-X.

Reported-by: Changlimin <changlimin@h3c.com>
Tested-by: Changlimin <changlimin@h3c.com>
Cc: qemu-stable@nongnu.org
Fixes: e1d4fb2d ("kvm-irqchip: x86: add msi route notify fn")
Signed-off-by: Peter Xu <peterx@redhat.com>

Message-Id: <1480042522-16551-1-git-send-email-peterx@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/kvm/pci-assign.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 8238fbc..87dcbdd 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1251,6 +1251,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
             error_propagate(errp, local_err);
             return -ENOTSUP;
         }
+        dev->dev.cap_present |= QEMU_PCI_CAP_MSI;
         dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
         /* Only 32-bit/no-mask currently supported */
         ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSI, pos, 10,
@@ -1285,6 +1286,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
             error_propagate(errp, local_err);
             return -ENOTSUP;
         }
+        dev->dev.cap_present |= QEMU_PCI_CAP_MSIX;
         dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX;
         ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSIX, pos, 12,
                                   &local_err);
@@ -1648,6 +1650,7 @@ static void assigned_dev_register_msix_mmio(AssignedDevice *dev, Error **errp)
         dev->msix_table = NULL;
         return;
     }
+    dev->dev.msix_table = (uint8_t *)dev->msix_table;
 
     assigned_dev_msix_reset(dev);
 
@@ -1665,6 +1668,7 @@ static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev)
         error_report("error unmapping msix_table! %s", strerror(errno));
     }
     dev->msix_table = NULL;
+    dev->dev.msix_table = NULL;
 }
 
 static const VMStateDescription vmstate_assigned_device = {
-- 
1.8.3.1

[Qemu-devel] [PULL 4/6] migration/pcspk: Add a property to state if pcspk is migrated

[Paolo Bonzini]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Allow us to turn migration of pcspk off for compatibility.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20161128133201.16104-2-dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/audio/pcspk.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index 984534b..7980022 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -54,6 +54,7 @@ typedef struct {
     unsigned int play_pos;
     uint8_t data_on;
     uint8_t dummy_refresh_clock;
+    bool migrate;
 } PCSpkState;
 
 static const char *s_spk = "pcspk";
@@ -187,11 +188,19 @@ static void pcspk_realizefn(DeviceState *dev, Error **errp)
     pcspk_state = s;
 }
 
+static bool migrate_needed(void *opaque)
+{
+    PCSpkState *s = opaque;
+
+    return s->migrate;
+}
+
 static const VMStateDescription vmstate_spk = {
     .name = "pcspk",
     .version_id = 1,
     .minimum_version_id = 1,
     .minimum_version_id_old = 1,
+    .needed = migrate_needed,
     .fields      = (VMStateField[]) {
         VMSTATE_UINT8(data_on, PCSpkState),
         VMSTATE_UINT8(dummy_refresh_clock, PCSpkState),
@@ -201,6 +210,7 @@ static const VMStateDescription vmstate_spk = {
 
 static Property pcspk_properties[] = {
     DEFINE_PROP_UINT32("iobase", PCSpkState, iobase,  -1),
+    DEFINE_PROP_BOOL("migrate", PCSpkState, migrate,  true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
1.8.3.1

[Qemu-devel] [PULL 5/6] migration/pcspk: Turn migration of pcspk off for 2.7 and older

[Paolo Bonzini]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

To keep backwards migration compatibility allow us to turn pcspk
migration off.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20161128133201.16104-3-dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/hw/i386/pc.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 67a1a9e..4b74130 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -395,6 +395,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
         .driver   = "Opteron_G3" "-" TYPE_X86_CPU,\
         .property = "stepping",\
         .value    = "1",\
+    },\
+    {\
+        .driver   = "isa-pcspk",\
+        .property = "migrate",\
+        .value    = "off",\
     },
 
 #define PC_COMPAT_2_6 \
-- 
1.8.3.1

[Qemu-devel] [PULL 6/6] rules.mak: Use -r instead of -Wl, -r to fix building when PIE is default

[Paolo Bonzini]

From: Adrian Bunk <bunk@stusta.de>

Building qemu fails in distributions where gcc enables PIE by default
(e.g. Debian unstable) with:

/usr/bin/ld: -r and -pie may not be used together

Use -r instead of -Wl,-r to avoid gcc passing -pie to the linker
when PIE is enabled and a relocatable object is passed.

Signed-off-by: Adrian Bunk <bunk@stusta.de>
Message-Id: <20161127162817.15144-1-bunk@stusta.de>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 rules.mak | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rules.mak b/rules.mak
index 0333ae3..545ebd9 100644
--- a/rules.mak
+++ b/rules.mak
@@ -93,7 +93,7 @@ module-common.o: CFLAGS += $(DSO_OBJ_CFLAGS)
 	$(if $(findstring /,$@),$(call quiet-command,cp $@ $(subst /,-,$@),"CP","$(subst /,-,$@)"))
 
 
-LD_REL := $(CC) -nostdlib -Wl,-r $(LD_REL_FLAGS)
+LD_REL := $(CC) -nostdlib -r $(LD_REL_FLAGS)
 
 %.mo:
 	$(call quiet-command,$(LD_REL) -o $@ $^,"LD","$(TARGET_DIR)$@")
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 0/6] More patches for QEMU 2.8-rc2

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1649 bytes --]

On Mon, Nov 28, 2016 at 11:40:31PM +0100, Paolo Bonzini wrote:
> The following changes since commit 00227fefd2059464cd2f59aed29944874c630e2f:
> 
>   Update version for v2.8.0-rc1 release (2016-11-22 22:29:08 +0000)
> 
> are available in the git repository at:
> 
>   git://github.com/bonzini/qemu.git tags/for-upstream
> 
> for you to fetch changes up to c96f0ee6a67ca6277366e78ce5d84d5c20dd596f:
> 
>   rules.mak: Use -r instead of -Wl, -r to fix building when PIE is default (2016-11-28 16:45:12 +0100)
> 
> ----------------------------------------------------------------
> Small fixes for rc2.
> 
> ----------------------------------------------------------------
> Adrian Bunk (1):
>       rules.mak: Use -r instead of -Wl, -r to fix building when PIE is default
> 
> Dr. David Alan Gilbert (2):
>       migration/pcspk: Add a property to state if pcspk is migrated
>       migration/pcspk: Turn migration of pcspk off for 2.7 and older
> 
> Paolo Bonzini (2):
>       megasas: do not call pci_dma_unmap after having freed the frame once
>       megasas: clean up and fix request completion/cancellation
> 
> Peter Xu (1):
>       pci-assign: sync MSI/MSI-X cap and table with PCIDevice
> 
>  hw/audio/pcspk.c         | 10 +++++++++
>  hw/i386/kvm/pci-assign.c |  4 ++++
>  hw/scsi/megasas.c        | 58 ++++++++++++++++++++++++++----------------------
>  include/hw/i386/pc.h     |  5 +++++
>  rules.mak                |  2 +-
>  5 files changed, 52 insertions(+), 27 deletions(-)
> -- 
> 1.8.3.1
> 
> 

Thanks, applied to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PULL 0/6] More patches for QEMU 2.8-rc2

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]

On Mon, Nov 28, 2016 at 11:40:31PM +0100, Paolo Bonzini wrote:
> The following changes since commit 00227fefd2059464cd2f59aed29944874c630e2f:
> 
>   Update version for v2.8.0-rc1 release (2016-11-22 22:29:08 +0000)
> 
> are available in the git repository at:
> 
>   git://github.com/bonzini/qemu.git tags/for-upstream
> 
> for you to fetch changes up to c96f0ee6a67ca6277366e78ce5d84d5c20dd596f:
> 
>   rules.mak: Use -r instead of -Wl, -r to fix building when PIE is default (2016-11-28 16:45:12 +0100)
> 
> ----------------------------------------------------------------
> Small fixes for rc2.
> 
> ----------------------------------------------------------------
> Adrian Bunk (1):
>       rules.mak: Use -r instead of -Wl, -r to fix building when PIE is default
> 
> Dr. David Alan Gilbert (2):
>       migration/pcspk: Add a property to state if pcspk is migrated
>       migration/pcspk: Turn migration of pcspk off for 2.7 and older
> 
> Paolo Bonzini (2):
>       megasas: do not call pci_dma_unmap after having freed the frame once
>       megasas: clean up and fix request completion/cancellation
> 
> Peter Xu (1):
>       pci-assign: sync MSI/MSI-X cap and table with PCIDevice
> 
>  hw/audio/pcspk.c         | 10 +++++++++
>  hw/i386/kvm/pci-assign.c |  4 ++++
>  hw/scsi/megasas.c        | 58 ++++++++++++++++++++++++++----------------------
>  include/hw/i386/pc.h     |  5 +++++
>  rules.mak                |  2 +-
>  5 files changed, 52 insertions(+), 27 deletions(-)
> -- 
> 1.8.3.1
> 

Now on the correct branch... :D

Thanks, applied to my staging tree:
https://github.com/stefanha/qemu/commits/staging

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]