CD-ROM boot broken on current revision?

Hi,

I'm a new subscriber to seabios.org. Not much exp. with mailing lists nor C++, so feel
free to straighten me out as 
necessary. I found out an issue: Booting from CD-ROM doesn't work anymore, though
related make options have been set. I 
tried to debug it a little bit. To me it looks that order changes related to media
sensing/capacity request broke it up.

In 1.6.3,  cdb_read_capacity(line 204 in src/cdrom.c) takes place before
cdb_get_sense(line 209) resulting in a break.

In current revision, this has been changed, has it?
cdrom_boot(src/cdrom.cline 188) calls scsi_is_ready(line 197), which looks similar to
atapi_is_ready from 1.6.3/src/cdrom.c.

However: scsi_is_ready is missing

   struct cdbres_read_capacity info;
   ret = cdb_read_capacity(op, &info);
   if (!ret)
   // Success
   break;

contrary to 1.6.3. In line cdb_get_sense returns 0xC with a medium in the drive, but this
seems to result into an 
endless loop due to continue;

For me, I added above snippet to blockcmd.c right before ret = cdb_get_sense(..) and it
worked.
Later CD-ROM boots using boot managers(e.g. grub4dos or PloP) work fine without changes,
though.

Debugging was done in Qemu 1.0.1. Since I'm on Windows, I used this build:
http://lassauge.free.fr/qemu/release/Qemu-1.0.1-windows.zip

Please confirm or advise.

Kind Regards,
fellaw

[Paolo Bonzini]

Il 18/03/2012 22:47, fellaw(a)gmx.net ha scritto:
  Hi,
 
 I'm a new subscriber to seabios.org. Not much exp. with mailing lists
 nor C++, so feel free to straighten me out as necessary. I found out an
 issue: Booting from CD-ROM doesn't work anymore, though related make
 options have been set. I tried to debug it a little bit. To me it looks
 that order changes related to media sensing/capacity request broke it up.
 
 In 1.6.3,  cdb_read_capacity(line 204 in src/cdrom.c) takes place before
 cdb_get_sense(line 209) resulting in a break.
 
 In current revision, this has been changed, has it?
 cdrom_boot(src/cdrom.cline 188) calls scsi_is_ready(line 197), which
 looks similar to atapi_is_ready from 1.6.3/src/cdrom.c.
 
 However: scsi_is_ready is missing
 
   struct cdbres_read_capacity info;
   ret = cdb_read_capacity(op, &info);
   if (!ret)
   // Success
   break;
 
 contrary to 1.6.3. In line cdb_get_sense returns 0xC with a medium in
 the drive, but this seems to result into an endless loop due to continue;
 
 For me, I added above snippet to blockcmd.c right before ret =
 cdb_get_sense(..) and it worked.
 Later CD-ROM boots using boot managers(e.g. grub4dos or PloP) work fine
 without changes, though.
 
 Debugging was done in Qemu 1.0.1. Since I'm on Windows, I used this build:
 http://lassauge.free.fr/qemu/release/Qemu-1.0.1-windows.zip
 
 Please confirm or advise.
This works for me, but only after a time out on test_unit_ready which
more or less confirms your bug.  I'll reply to this message with a patch
that fixes it for me.  Can you please test it?

Paolo

[PATCH] ata: send TEST UNIT READY correctly

[Paolo Bonzini]

The ATAPI driver does not need to support writes, but it does needs to
avoid the PIO transfer and DRQ check when TEST UNIT READY is sent.
Since TEST UNIT READY has no payload, checking for not busy is enough.

This fixes a timeout when booting from CD/DVD, which fellaw(a)gmx.net
reported to cause boot failures.

Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com>
---
 src/ata.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/ata.c b/src/ata.c
index 76e4f20..c37691a 100644
--- a/src/ata.c
+++ b/src/ata.c
@@ -645,13 +645,15 @@ atapi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize)
         ret = -2;
         goto fail;
     }
-    if (!(status & ATA_CB_STAT_DRQ)) {
-        dprintf(6, "send_atapi_cmd : DRQ not set (status %02x)\n", status);
-        ret = -3;
-        goto fail;
-    }
+    if (blocksize) {
+        if (!(status & ATA_CB_STAT_DRQ)) {
+            dprintf(6, "send_atapi_cmd : DRQ not set (status %02x)\n",
status);
+            ret = -3;
+            goto fail;
+        }
 
-    ret = ata_pio_transfer(op, 0, blocksize);
+        ret = ata_pio_transfer(op, 0, blocksize);
+    }
 
 fail:
     // Enable interrupts
-- 
1.7.7.6

[PATCH] ata: send TEST UNIT READY correctly

Confirmed working.

Paolo Bonzini schrieb:
  The ATAPI driver does not need to support writes, but
it does needs to
 avoid the PIO transfer and DRQ check when TEST UNIT READY is sent.
 Since TEST UNIT READY has no payload, checking for not busy is enough.

 This fixes a timeout when booting from CD/DVD, which fellaw(a)gmx.net
 reported to cause boot failures.

 Signed-off-by: Paolo Bonzini<pbonzini(a)redhat.com>
 ---
   src/ata.c |   14 ++++++++------
   1 files changed, 8 insertions(+), 6 deletions(-)

 diff --git a/src/ata.c b/src/ata.c
 index 76e4f20..c37691a 100644
 --- a/src/ata.c
 +++ b/src/ata.c
 @@ -645,13 +645,15 @@ atapi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize)
           ret = -2;
           goto fail;
       }
 -    if (!(status&  ATA_CB_STAT_DRQ)) {
 -        dprintf(6, "send_atapi_cmd : DRQ not set (status %02x)\n", status);
 -        ret = -3;
 -        goto fail;
 -    }
 +    if (blocksize) {
 +        if (!(status&  ATA_CB_STAT_DRQ)) {
 +            dprintf(6, "send_atapi_cmd : DRQ not set (status %02x)\n",
status);
 +            ret = -3;
 +            goto fail;
 +        }

 -    ret = ata_pio_transfer(op, 0, blocksize);
 +        ret = ata_pio_transfer(op, 0, blocksize);
 +    }

   fail:
       // Enable interrupts

[PATCH] ata: send TEST UNIT READY correctly

[Kevin O'Connor]

On Mon, Mar 19, 2012 at 11:41:09AM +0100, Paolo Bonzini wrote:
  The ATAPI driver does not need to support writes, but
it does needs to
 avoid the PIO transfer and DRQ check when TEST UNIT READY is sent.
 Since TEST UNIT READY has no payload, checking for not busy is enough.
 
 This fixes a timeout when booting from CD/DVD, which fellaw(a)gmx.net
 reported to cause boot failures.
Thanks - I commited this change.

-Kevin