Re: [Qemu-devel] [RESEND] [PATCH] ide: fix GET_CONFIGURATION DVD-ROM support

[Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>]

Can someone please comment on the mergability of this patch? or in what needs
to be done to it so that it can be committed?

The patch is still current and the bug it fixes would otherwise prevent
OpenSolaris guests to be installed in qemu.  the MMC-6 command it fixes (GET
CONFIGURATION) has been verified to behave as per the SPECs and compared to
behave just like real hardware does.

Carlo

On Wed, Dec 26, 2007 at 01:36:15AM -0600, Carlo Marcelo Arenas Belon wrote:
> The following patch implements fixes to the CD-ROM IDE/ATAPI emulation
> since ide.c revision 1.66 and that prevents installation of OpenSolaris
> guests because of timeouts like :
> 
>   WARNING: /pci@0,0/pci-ide@1,1/ide@1 (ata1);
>           timeout: abort request, target=0 lun=0
>   WARNING: /pci@0,0/pci-ide@1,1/ide@1 (ata1):
>           timeout: abort device, target=0 lun=0
>   WARNING: /pci@0,0/pci-ide@1,1/ide@1 (ata1):
>           timeout: reset target, target=0 lun=0
>   WARNING: /pci@0,0/pci-ide@1,1/ide@1 (ata1):
>           timeout: reset bus, target=0 lun=0
> 
> It has been tested in Gentoo Linux 2007.0 amd64 (64bit) and x86 (32bit) hosts
> using several different versions of Linux, FreeBSD, OpenBSD, NetBSD,
> DragonFlyBSD, OpenSolaris and Windows 2000 guests
> 
> The patch implements changes to the following IDE commands as described by :
> 
> * change the model name (used by "INQUIRY" and "IDENTIFY DEVICE") to DVD
> * recognize and honor "Allocation Length" parameter in "GET CONFIGURATION"
> * only set "current profile" for the "GET CONFIGURATION" response if a profile
> is current (CD or DVD loaded)
> * calculate "data length" including all headers
> * refactor code and add comments to help document references to all
> implemented standards (ATAPI-4, SPC-3 and MMC-6)
> 
> Carlo
> ---
> Index: hw/ide.c
> ===================================================================
> RCS file: /sources/qemu/qemu/hw/ide.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 ide.c
> --- hw/ide.c	24 Dec 2007 14:33:24 -0000	1.79
> +++ hw/ide.c	26 Dec 2007 07:11:01 -0000
> @@ -1,5 +1,5 @@
>  /*
> - * QEMU IDE disk and CD-ROM Emulator
> + * QEMU IDE disk and CD/DVD-ROM Emulator
>   *
>   * Copyright (c) 2003 Fabrice Bellard
>   * Copyright (c) 2006 Openedhand Ltd.
> @@ -540,7 +540,7 @@ static void ide_atapi_identify(IDEState 
>      put_le16(p + 21, 512); /* cache size in sectors */
>      put_le16(p + 22, 4); /* ecc bytes */
>      padstr((char *)(p + 23), QEMU_VERSION, 8); /* firmware version */
> -    padstr((char *)(p + 27), "QEMU CD-ROM", 40); /* model */
> +    padstr((char *)(p + 27), "QEMU DVD-ROM", 40); /* model */
>      put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */
>  #ifdef USE_DMA_CDROM
>      put_le16(p + 49, 1 << 9 | 1 << 8); /* DMA and LBA supported */
> @@ -1634,12 +1634,13 @@ static void ide_atapi_cmd(IDEState *s)
>          buf[6] = 0; /* reserved */
>          buf[7] = 0; /* reserved */
>          padstr8(buf + 8, 8, "QEMU");
> -        padstr8(buf + 16, 16, "QEMU CD-ROM");
> +        padstr8(buf + 16, 16, "QEMU DVD-ROM");
>          padstr8(buf + 32, 4, QEMU_VERSION);
>          ide_atapi_cmd_reply(s, 36, max_len);
>          break;
>      case GPCMD_GET_CONFIGURATION:
>          {
> +            uint32_t len;
>              uint64_t total_sectors;
>  
>              /* only feature 0 is supported */
> @@ -1648,17 +1649,27 @@ static void ide_atapi_cmd(IDEState *s)
>                                      ASC_INV_FIELD_IN_CMD_PACKET);
>                  break;
>              }
> -            memset(buf, 0, 32);
> +            max_len = ube16_to_cpu(packet + 7);
>              bdrv_get_geometry(s->bs, &total_sectors);
> -            buf[3] = 16;
> -            buf[7] = total_sectors <= 1433600 ? 0x08 : 0x10; /* current profile */
> -            buf[10] = 0x10 | 0x1;
> -            buf[11] = 0x08; /* size of profile list */
> +            memset(buf, 0, 32);
> +            if (total_sectors) {
> +                if (total_sectors > 1433600) {
> +                    buf[7] = 0x10; /* DVD-ROM */
> +                } else {
> +                    buf[7] = 0x08; /* CD-ROM */
> +                }
> +            } else {
> +                buf[7] = 0x00; /* no current profile */
> +            }
> +            buf[10] = 0x02 | 0x01; /* persistent and current */
> +            buf[11] = 0x08; /* size of profile list = 4 bytes per profile */
>              buf[13] = 0x10; /* DVD-ROM profile */
> -            buf[14] = buf[7] == 0x10; /* (in)active */
> +            buf[14] = buf[13] == buf[7]; /* (in)active */
>              buf[17] = 0x08; /* CD-ROM profile */
> -            buf[18] = buf[7] == 0x08; /* (in)active */
> -            ide_atapi_cmd_reply(s, 32, 32);
> +            buf[18] = buf[17] == buf[7]; /* (in)active */
> +            len = 8 + 4 + buf[11]; /* headers + size of profile list */
> +            cpu_to_ube32(buf, len - 4); /* data length */
> +            ide_atapi_cmd_reply(s, len, max_len);
>              break;
>          }
>      default:
> 
>