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

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

[John Snow]

The following changes since commit 4b6eda626fdb8bf90472c6868d502a2ac09abeeb:

  Merge remote-tracking branch 'remotes/lalrae/tags/mips-20151124' into staging (2015-11-24 17:05:06 +0000)

are available in the git repository at:

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

for you to fetch changes up to 9c73517ca56d6611371376bd298b4b20f3ad6140:

  ide-test: fix timeouts (2015-11-25 11:37:34 -0500)

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

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

Alberto Garcia (2):
  atapi: Account for failed and invalid operations in cd_read_sector()
  atapi: Fix code indentation

John Snow (2):
  ide-test: cdrom_pio_impl fixup
  ide-test: fix timeouts

 hw/ide/atapi.c   |  8 +++++---
 tests/ide-test.c | 32 +++++++++++++++++++++++---------
 2 files changed, 28 insertions(+), 12 deletions(-)

-- 
2.4.3

[Qemu-devel] [PULL 1/4] ide-test: cdrom_pio_impl fixup

[John Snow]

Final tidying: move the interrupt wait into the loop,
document that the status read clears the IRQ, and move
the final interrupt check outside of the loop.

This should be functionally equivalent to how it works
currently, but a little less ambiguous and slightly more
explicit about the state transitions.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-id: 1448060035-31973-3-git-send-email-jsnow@redhat.com
---
 tests/ide-test.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index fc1ce52..46763db 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -709,9 +709,6 @@ static void cdrom_pio_impl(int nblocks)
     /* SCSI CDB (READ10) -- read n*2048 bytes from block 0 */
     send_scsi_cdb_read10(0, nblocks);
 
-    /* HP3: INTRQ_Wait */
-    ide_wait_intr(IDE_PRIMARY_IRQ);
-
     /* Read data back: occurs in bursts of 'BYTE_COUNT_LIMIT' bytes.
      * If BYTE_COUNT_LIMIT is odd, we transfer BYTE_COUNT_LIMIT - 1 bytes.
      * We allow an odd limit only when the remaining transfer size is
@@ -723,16 +720,25 @@ static void cdrom_pio_impl(int nblocks)
     for (i = 0; i < DIV_ROUND_UP(rxsize, limit); i++) {
         size_t offset = i * (limit / 2);
         size_t rem = (rxsize / 2) - offset;
-        /* HP2: Check_Status_B */
+
+        /* HP3: INTRQ_Wait */
+        ide_wait_intr(IDE_PRIMARY_IRQ);
+
+        /* HP2: Check_Status_B (and clear IRQ) */
         data = ide_wait_clear(BSY);
         assert_bit_set(data, DRQ | DRDY);
         assert_bit_clear(data, ERR | DF | BSY);
+
         /* HP4: Transfer_Data */
         for (j = 0; j < MIN((limit / 2), rem); j++) {
             rx[offset + j] = le16_to_cpu(inw(IDE_BASE + reg_data));
         }
-        ide_wait_intr(IDE_PRIMARY_IRQ);
     }
+
+    /* Check for final completion IRQ */
+    ide_wait_intr(IDE_PRIMARY_IRQ);
+
+    /* Sanity check final state */
     data = ide_wait_clear(DRQ);
     assert_bit_set(data, DRDY);
     assert_bit_clear(data, DRQ | ERR | DF | BSY);
-- 
2.4.3

[Qemu-devel] [PULL 2/4] atapi: Account for failed and invalid operations in cd_read_sector()

[John Snow]

From: Alberto Garcia <berto@igalia.com>

Commit 5f81724d made PIO read requests async but didn't add the
relevant block_acct_failed() and block_acct_invalid() calls.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 9b87e09d61019c128139b6c999ed0c07f0674170.1448367341.git.berto@igalia.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/atapi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 7b9f74c..5e3791c 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -148,17 +148,18 @@ static void cd_read_sector_cb(void *opaque, int ret)
 {
     IDEState *s = opaque;
 
-    block_acct_done(blk_get_stats(s->blk), &s->acct);
-
 #ifdef DEBUG_IDE_ATAPI
     printf("cd_read_sector_cb: lba=%d ret=%d\n", s->lba, ret);
 #endif
 
     if (ret < 0) {
+        block_acct_failed(blk_get_stats(s->blk), &s->acct);
         ide_atapi_io_error(s, ret);
         return;
     }
 
+    block_acct_done(blk_get_stats(s->blk), &s->acct);
+
     if (s->cd_sector_size == 2352) {
         cd_data_to_raw(s->io_buffer, s->lba);
     }
@@ -173,6 +174,7 @@ static void cd_read_sector_cb(void *opaque, int ret)
 static int cd_read_sector(IDEState *s)
 {
     if (s->cd_sector_size != 2048 && s->cd_sector_size != 2352) {
+        block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_READ);
         return -EINVAL;
     }
 
-- 
2.4.3

[Qemu-devel] [PULL 3/4] atapi: Fix code indentation

[John Snow]

From: Alberto Garcia <berto@igalia.com>

This was accidentally changed by commit 5f81724d

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 93fb43522e3b8dddb6c709d568919347d9a5ba3f.1448367341.git.berto@igalia.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/atapi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 5e3791c..65f8dd4 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -443,7 +443,7 @@ eot:
     if (ret < 0) {
         block_acct_failed(blk_get_stats(s->blk), &s->acct);
     } else {
-    block_acct_done(blk_get_stats(s->blk), &s->acct);
+        block_acct_done(blk_get_stats(s->blk), &s->acct);
     }
     ide_set_inactive(s, false);
 }
-- 
2.4.3

[Qemu-devel] [PULL 4/4] ide-test: fix timeouts

[John Snow]

Use explicit timeouts instead of trying to approximate it by counting
the cumulative duration of nsleep calls.

In practice, the timeout if inb() dwarfed the nsleep delays, and as a
result the real timeout value became a lot larger than 5 seconds.

So: change the semantics from "Not sooner than 5 seconds" to "no more
than 5 seconds" to ensure we don't hang the tester for very long.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-id: 1448393771-15483-2-git-send-email-jsnow@redhat.com
---
 tests/ide-test.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index 46763db..c3aacd2 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -642,15 +642,19 @@ static void nsleep(int64_t nsecs)
 
 static uint8_t ide_wait_clear(uint8_t flag)
 {
-    int i;
     uint8_t data;
+    time_t st;
 
     /* Wait with a 5 second timeout */
-    for (i = 0; i <= 12500000; i++) {
+    time(&st);
+    while (true) {
         data = inb(IDE_BASE + reg_status);
         if (!(data & flag)) {
             return data;
         }
+        if (difftime(time(NULL), st) > 5.0) {
+            break;
+        }
         nsleep(400);
     }
     g_assert_not_reached();
@@ -658,14 +662,18 @@ static uint8_t ide_wait_clear(uint8_t flag)
 
 static void ide_wait_intr(int irq)
 {
-    int i;
+    time_t st;
     bool intr;
 
-    for (i = 0; i <= 12500000; i++) {
+    time(&st);
+    while (true) {
         intr = get_irq(irq);
         if (intr) {
             return;
         }
+        if (difftime(time(NULL), st) > 5.0) {
+            break;
+        }
         nsleep(400);
     }
 
-- 
2.4.3

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

[Peter Maydell]

On 25 November 2015 at 20:25, John Snow <jsnow@redhat.com> wrote:
> The following changes since commit 4b6eda626fdb8bf90472c6868d502a2ac09abeeb:
>
>   Merge remote-tracking branch 'remotes/lalrae/tags/mips-20151124' into staging (2015-11-24 17:05:06 +0000)
>
> are available in the git repository at:
>
>   https://github.com/jnsnow/qemu.git tags/ide-pull-request
>
> for you to fetch changes up to 9c73517ca56d6611371376bd298b4b20f3ad6140:
>
>   ide-test: fix timeouts (2015-11-25 11:37:34 -0500)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------
>
> Alberto Garcia (2):
>   atapi: Account for failed and invalid operations in cd_read_sector()
>   atapi: Fix code indentation
>
> John Snow (2):
>   ide-test: cdrom_pio_impl fixup
>   ide-test: fix timeouts
>
>  hw/ide/atapi.c   |  8 +++++---
>  tests/ide-test.c | 32 +++++++++++++++++++++++---------
>  2 files changed, 28 insertions(+), 12 deletions(-)

Applied, thanks.

-- PMM