[Qemu-devel] [PULL 0/4] IDE patches

[Qemu-devel] [PULL 0/4] IDE patches

[John Snow]

The following changes since commit c640f2849ee8775fe1bbd7a2772610aa77816f9f:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2016-09-28 23:02:56 +0100)

are available in the git repository at:

  https://github.com/jnsnow/qemu.git tags/ide-pull-request

for you to fetch changes up to ca44141d5fb801dd5903102acefd0f2d8e8bb6a1:

  ide: Fix memory leak in ide_register_restart_cb() (2016-09-29 15:50:29 -0400)

----------------------------------------------------------------

----------------------------------------------------------------

Ashijeet Acharya (1):
  ide: Fix memory leak in ide_register_restart_cb()

John Snow (2):
  ide: fix DMA register transitions
  ahci: clear aiocb in ncq_cb

Thomas Huth (1):
  MAINTAINERS: Add some more headers to the IDE section

 MAINTAINERS               |  1 +
 hw/ide/ahci.c             |  1 +
 hw/ide/core.c             |  4 ++--
 hw/ide/qdev.c             | 11 +++++++++++
 include/hw/ide/internal.h |  1 +
 5 files changed, 16 insertions(+), 2 deletions(-)

-- 
2.7.4

[Qemu-devel] [PULL 1/4] ide: fix DMA register transitions

[John Snow]

ATA8-APT defines the state transitions for both a host controller and
for the hardware device during the lifecycle of a DMA transfer, in
section 9.7 "DMA command protocol."

One of the interesting tidbits here is that when a device transitions
from DDMA0 ("Prepare state") to DDMA1 ("Data_Transfer State"), it can
choose to set either BSY or DRQ to signal this transition, but not both.

as ide_sector_dma_start is the last point in our preparation process
before we begin the real data transfer process (for either AHCI or BMDMA),
this is the correct transition point for DDMA0 to DDMA1.

I have chosen !BSY && DRQ for QEMU to make the transition from DDMA0 the
most obvious.

Reported-by: Benjamin David Lunt <fys@fysnet.net>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Tested-by: Stefan Weil <sw@weilnetz.de>
Message-id: 1470175541-19344-1-git-send-email-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index b0e42a6..1bee18d 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -908,7 +908,7 @@ eot:
 
 static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
 {
-    s->status = READY_STAT | SEEK_STAT | DRQ_STAT | BUSY_STAT;
+    s->status = READY_STAT | SEEK_STAT | DRQ_STAT;
     s->io_buffer_size = 0;
     s->dma_cmd = dma_cmd;
 
-- 
2.7.4

[Qemu-devel] [PULL 2/4] ahci: clear aiocb in ncq_cb

[John Snow]

Similar to existing fixes for IDE (87ac25fd) and ATAPI (7f951b2d), the
AIOCB must be cleared in the callback. Otherwise, we may accidentally
try to reset a dangling pointer in bdrv_aio_cancel() from a port reset.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1474575040-32079-2-git-send-email-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/ahci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index f3438ad..63ead21 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -948,6 +948,7 @@ static void ncq_cb(void *opaque, int ret)
     NCQTransferState *ncq_tfs = (NCQTransferState *)opaque;
     IDEState *ide_state = &ncq_tfs->drive->port.ifs[0];
 
+    ncq_tfs->aiocb = NULL;
     if (ret == -ECANCELED) {
         return;
     }
-- 
2.7.4

[Qemu-devel] [PULL 3/4] MAINTAINERS: Add some more headers to the IDE section

[John Snow]

From: Thomas Huth <thuth@redhat.com>

The folder include/hw/ide/ belongs to the IDE section.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1474646996-30421-1-git-send-email-thuth@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f3c1f7f..9b7e846 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -772,6 +772,7 @@ M: John Snow <jsnow@redhat.com>
 L: qemu-block@nongnu.org
 S: Supported
 F: include/hw/ide.h
+F: include/hw/ide/
 F: hw/ide/
 F: hw/block/block.c
 F: hw/block/cdrom.c
-- 
2.7.4

[Qemu-devel] [PULL 4/4] ide: Fix memory leak in ide_register_restart_cb()

[John Snow]

From: Ashijeet Acharya <ashijeetacharya@gmail.com>

Fix a memory leak in ide_register_restart_cb() in hw/ide/core.c and add
idebus_unrealize() in hw/ide/qdev.c to have calls to
qemu_del_vm_change_state_handler() to deal with the dangling change
state handler during hot-unplugging ide devices which might lead to a
crash.

Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1474995212-10580-1-git-send-email-ashijeetacharya@gmail.com
[Minor whitespace fix --js]
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/core.c             |  2 +-
 hw/ide/qdev.c             | 11 +++++++++++
 include/hw/ide/internal.h |  1 +
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 1bee18d..7291677 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2582,7 +2582,7 @@ static void ide_restart_cb(void *opaque, int running, RunState state)
 void ide_register_restart_cb(IDEBus *bus)
 {
     if (bus->dma->ops->restart_dma) {
-        qemu_add_vm_change_state_handler(ide_restart_cb, bus);
+        bus->vmstate = qemu_add_vm_change_state_handler(ide_restart_cb, bus);
     }
 }
 
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 2eb055a..dbaa75c 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -31,6 +31,7 @@
 /* --------------------------------- */
 
 static char *idebus_get_fw_dev_path(DeviceState *dev);
+static void idebus_unrealize(DeviceState *qdev, Error **errp);
 
 static Property ide_props[] = {
     DEFINE_PROP_UINT32("unit", IDEDevice, unit, -1),
@@ -44,6 +45,15 @@ static void ide_bus_class_init(ObjectClass *klass, void *data)
     k->get_fw_dev_path = idebus_get_fw_dev_path;
 }
 
+static void idebus_unrealize(DeviceState *qdev, Error **errp)
+{
+    IDEBus *bus = DO_UPCAST(IDEBus, qbus, qdev->parent_bus);
+
+    if (bus->vmstate) {
+        qemu_del_vm_change_state_handler(bus->vmstate);
+    }
+}
+
 static const TypeInfo ide_bus_info = {
     .name = TYPE_IDE_BUS,
     .parent = TYPE_BUS,
@@ -355,6 +365,7 @@ static void ide_device_class_init(ObjectClass *klass, void *data)
     k->init = ide_qdev_init;
     set_bit(DEVICE_CATEGORY_STORAGE, k->categories);
     k->bus_type = TYPE_IDE_BUS;
+    k->unrealize = idebus_unrealize;
     k->props = ide_props;
 }
 
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index a6dd2c3..88dc118 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -482,6 +482,7 @@ struct IDEBus {
     uint32_t retry_nsector;
     PortioList portio_list;
     PortioList portio2_list;
+    VMChangeStateEntry *vmstate;
 };
 
 #define TYPE_IDE_DEVICE "ide-device"
-- 
2.7.4

Re: [Qemu-devel] [PULL 0/4] IDE patches

[Peter Maydell]

On 29 September 2016 at 13:15, John Snow <jsnow@redhat.com> wrote:
> The following changes since commit c640f2849ee8775fe1bbd7a2772610aa77816f9f:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2016-09-28 23:02:56 +0100)
>
> are available in the git repository at:
>
>   https://github.com/jnsnow/qemu.git tags/ide-pull-request
>
> for you to fetch changes up to ca44141d5fb801dd5903102acefd0f2d8e8bb6a1:
>
>   ide: Fix memory leak in ide_register_restart_cb() (2016-09-29 15:50:29 -0400)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------


Applied, thanks.

-- PMM