[Qemu-devel] [PATCH 1.3 0/2] ide: PRDT error handling fixes

[Qemu-devel] [PATCH 1.3 0/2] ide: PRDT error handling fixes

[Kevin Wolf]

qtest cases for this exist, but they rely on prototype infrastructure that
isn't mergable. I'll submit them as soon as libqos is there and I've changed
the test to use it.

Kevin Wolf (2):
  ide: Fix crash with too long PRD
  ide: Fix status register after short PRDs

 hw/ide/core.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

-- 
1.7.6.5

[Qemu-devel] [PATCH 1/2] ide: Fix crash with too long PRD

[Kevin Wolf]

Without this, s->nsector can become negative and badness happens (trying
to malloc huge amount of memory and glib calls abort())

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/core.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 7d6b0fa..c2ab787 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -579,6 +579,7 @@ void ide_dma_cb(void *opaque, int ret)
     IDEState *s = opaque;
     int n;
     int64_t sector_num;
+    bool stay_active = false;
 
     if (ret < 0) {
         int op = BM_STATUS_DMA_RETRY;
@@ -594,6 +595,14 @@ void ide_dma_cb(void *opaque, int ret)
     }
 
     n = s->io_buffer_size >> 9;
+    if (n > s->nsector) {
+        /* The PRDs were longer than needed for this request. Shorten them so
+         * we don't get a negative remainder. The Active bit must remain set
+         * after the request completes. */
+        n = s->nsector;
+        stay_active = true;
+    }
+
     sector_num = ide_get_sector(s);
     if (n > 0) {
         dma_buf_commit(s);
@@ -646,6 +655,9 @@ eot:
         bdrv_acct_done(s->bs, &s->acct);
     }
     ide_set_inactive(s);
+    if (stay_active) {
+        s->bus->dma->ops->add_status(s->bus->dma, BM_STATUS_DMAING);
+    }
 }
 
 static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
-- 
1.7.6.5

[Qemu-devel] [PATCH 2/2] ide: Fix status register after short PRDs

[Kevin Wolf]

When failing a request because the length of the regions described by
the PRDT was too short for the requested number of sectors, the IDE
emulation forgot to update the status register, so that the device would
keep the BSY flag set indefinitely.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/ide/core.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index c2ab787..8da894f 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -625,6 +625,7 @@ void ide_dma_cb(void *opaque, int ret)
     if (s->bus->dma->ops->prepare_buf(s->bus->dma, ide_cmd_is_read(s)) == 0) {
         /* The PRDs were too short. Reset the Active bit, but don't raise an
          * interrupt. */
+        s->status = READY_STAT | SEEK_STAT;
         goto eot;
     }
 
-- 
1.7.6.5

Re: [Qemu-devel] [PATCH 1.3 0/2] ide: PRDT error handling fixes

[Stefan Hajnoczi]

On Tue, Nov 20, 2012 at 05:27:42PM +0100, Kevin Wolf wrote:
> qtest cases for this exist, but they rely on prototype infrastructure that
> isn't mergable. I'll submit them as soon as libqos is there and I've changed
> the test to use it.
> 
> Kevin Wolf (2):
>   ide: Fix crash with too long PRD
>   ide: Fix status register after short PRDs
> 
>  hw/ide/core.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> -- 
> 1.7.6.5
> 
> 

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

Stefan