[PATCH 0/5] qemu: IDE/ATAPI emulation reliability fixes

[PATCH 0/5] qemu: IDE/ATAPI emulation reliability fixes

[Carlo Marcelo Arenas Belon]

The following patch series implements fixes to the IDE/ATAPI emulation
in qemu which are already committed upstream or proposed for inclusion; 
including 2 serious regressions that result in availability and data loss
problems when using OpenSolaris or FreeBSD guests (at least) by the first 2:

  Patch 1/5 : fixes GET_CONFIGURATION to allow OpenSolaris cdrom access 
  Patch 2/5 : fixes IDE sector write operations resulting in data lost
  Patch 3/5 : enables ACPI interrupts
  Patch 4/5 : uses DVD-ROM as model name for INQUIRY and IDENTIFY DEVICE
  Patch 5/5 : removes obsolete triton chip support

They had been tested in Gentoo Linux 2007.0 amd64 using an Intel Core 2
6320 host and several different versions of FreeBSD, OpenBSD, NetBSD, Linux,
OpenSolaris and Windows 2000 guests

Carlo

-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4

[PATCH 1/5] qemu: GET_CONFIGURATION fixes for MMC-6 DVD-ROM implementation

[Carlo Marcelo Arenas Belon]

The following patch complements "Partial IDE DVD emulation", added in ide.c
revision 1.66 and that was generating the following timeouts for OpenSolaris
guests when trying to access the ATAPI cdrom (during installation for example):

  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

The changes required to the "GET CONFIGURATION" implementation are :

* recognize and honor "Allocation Length" command parameter
* only set "current profile" for the response if a profile is current (either
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)

Signed-off-by: Carlo Marcelo Arenas Belon <carenas-kLeDWSohozoJb6fo7hG9ng@public.gmane.org>
---
 qemu/hw/ide.c |   27 +++++++++++++++++++--------
 1 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/qemu/hw/ide.c b/qemu/hw/ide.c
index 329d053..29fc7a9 100644
--- a/qemu/hw/ide.c
+++ b/qemu/hw/ide.c
@@ -1648,6 +1648,7 @@ static void ide_atapi_cmd(IDEState *s)
         break;
     case GPCMD_GET_CONFIGURATION:
         {
+            uint32_t len;
             int64_t total_sectors;
 
             /* only feature 0 is supported */
@@ -1656,17 +1657,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:
-- 
1.5.2.5


-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4

[PATCH 2/5] qemu: revert ide sector write operation not to use AIO

[Carlo Marcelo Arenas Belon]

This patch revert "Last AIO Patch" as requested by Fabrice, it is incomplete
and breaks other (non-Windows) systems.

originally from qemu CVS :

  cvs -q diff -r1.69 -r1.70 hw/ide.c

Data corruption resulting in kernel panics and unbootable images is experienced
frequently with FreeBSD and OpenSolaris guests unless committed

Signed-off-by: Carlo Marcelo Arenas Belon <carenas-kLeDWSohozoJb6fo7hG9ng@public.gmane.org>
---
 qemu/hw/ide.c |   56 ++++++++++++++++++--------------------------------------
 1 files changed, 18 insertions(+), 38 deletions(-)

diff --git a/qemu/hw/ide.c b/qemu/hw/ide.c
index 29fc7a9..a8d4339 100644
--- a/qemu/hw/ide.c
+++ b/qemu/hw/ide.c
@@ -358,7 +358,7 @@ typedef struct IDEState {
     uint8_t *data_ptr;
     uint8_t *data_end;
     uint8_t io_buffer[MAX_MULT_SECTORS*512 + 4];
-    QEMUTimer *sector_write_timer; /* only used for win2k instal hack */
+    QEMUTimer *sector_write_timer; /* only used for win2k install hack */
     uint32_t irq_count; /* counts IRQs when using win2k install hack */
     /* CF-ATA extended error */
     uint8_t ext_error;
@@ -865,44 +865,10 @@ static void ide_sector_write_timer_cb(void *opaque)
     ide_set_irq(s);
 }
 
-static void ide_sector_write_aio_cb(void *opaque, int ret)
-{
-    BMDMAState *bm = opaque;
-    IDEState *s = bm->ide_if;
-
-#ifdef TARGET_I386
-    if (win2k_install_hack && ((++s->irq_count % 16) == 0)) {
-	/* It seems there is a bug in the Windows 2000 installer HDD
-	   IDE driver which fills the disk with empty logs when the
-	   IDE write IRQ comes too early. This hack tries to correct
-	   that at the expense of slower write performances. Use this
-	   option _only_ to install Windows 2000. You must disable it
-	   for normal use. */
-	qemu_mod_timer(s->sector_write_timer,
-		       qemu_get_clock(vm_clock) + (ticks_per_sec / 1000));
-    } else
-#endif
-    {
-	ide_set_irq(s);
-    }
-    bm->aiocb = NULL;
-}
-
 static void ide_sector_write(IDEState *s)
 {
-    BMDMAState *bm;
     int64_t sector_num;
-    int n, n1;
-
-    s->io_buffer_index = 0;
-    s->io_buffer_size = 0;
-    bm = s->bmdma;
-    if(bm == NULL) {
-	bm = qemu_mallocz(sizeof(BMDMAState));
-	s->bmdma = bm;
-    }
-    bm->ide_if = s;
-    bm->dma_cb = ide_sector_write_aio_cb;
+    int ret, n, n1;
 
     s->status = READY_STAT | SEEK_STAT;
     sector_num = ide_get_sector(s);
@@ -912,6 +878,7 @@ static void ide_sector_write(IDEState *s)
     n = s->nsector;
     if (n > s->req_nb_sectors)
         n = s->req_nb_sectors;
+    ret = bdrv_write(s->bs, sector_num, s->io_buffer, n);
     s->nsector -= n;
     if (s->nsector == 0) {
         /* no more sectors to write */
@@ -924,8 +891,21 @@ static void ide_sector_write(IDEState *s)
     }
     ide_set_sector(s, sector_num + n);
 
-    bm->aiocb = bdrv_aio_write(s->bs, sector_num, s->io_buffer, n,
-			       ide_sector_write_aio_cb, bm);
+#ifdef TARGET_I386
+    if (win2k_install_hack && ((++s->irq_count % 16) == 0)) {
+        /* It seems there is a bug in the Windows 2000 installer HDD
+           IDE driver which fills the disk with empty logs when the
+           IDE write IRQ comes too early. This hack tries to correct
+           that at the expense of slower write performances. Use this
+           option _only_ to install Windows 2000. You must disable it
+           for normal use. */
+        qemu_mod_timer(s->sector_write_timer, 
+                       qemu_get_clock(vm_clock) + (ticks_per_sec / 1000));
+    } else 
+#endif
+    {
+        ide_set_irq(s);
+    }
 }
 
 /* XXX: handle errors */
-- 
1.5.2.5


-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4

[PATCH 3/5] qemu: piix_pci enable ACPI interrupts

[Carlo Marcelo Arenas Belon]

This patch enable ACPI interrupts, by Igor Lvovsky

Originally from qemu CVS :

  cvs -q diff -r1.11 -r1.12 hw/piix_pci.c

Signed-off-by: Carlo Marcelo Arenas Belon <carenas-kLeDWSohozoJb6fo7hG9ng@public.gmane.org>
---
 qemu/hw/piix_pci.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/qemu/hw/piix_pci.c b/qemu/hw/piix_pci.c
index 8c00f0d..3c04e3a 100644
--- a/qemu/hw/piix_pci.c
+++ b/qemu/hw/piix_pci.c
@@ -208,6 +208,7 @@ static void piix3_set_irq(qemu_irq *pic, int irq_num, int level)
 {
     int i, pic_irq, pic_level;
 
+    piix3_dev->config[0x60 + irq_num] &= ~0x80;   // enable bit
     pci_irq_levels[irq_num] = level;
 
     /* now we change the pic irq level according to the piix irq mappings */
-- 
1.5.2.5


-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4

[PATCH 4/5] qemu: ide INQUIRY and IDENTIFY DEVICE report DVD-ROM model

[Carlo Marcelo Arenas Belon]

This patch complements "Partial IDE DVD emulation" which was added in ide.c
revision 1.66 so that the CD-ROM identifies itself as a DVD-ROM
to "INQUIRY" and "IDENTIFY DEVICE" commands

Signed-off-by: Carlo Marcelo Arenas Belon <carenas-kLeDWSohozoJb6fo7hG9ng@public.gmane.org>
---
 qemu/hw/ide.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu/hw/ide.c b/qemu/hw/ide.c
index a8d4339..a97d519 100644
--- a/qemu/hw/ide.c
+++ b/qemu/hw/ide.c
@@ -533,7 +533,7 @@ static void ide_atapi_identify(IDEState *s)
     put_le16(p + 21, 512); /* cache size in sectors */
     put_le16(p + 22, 4); /* ecc bytes */
     padstr((uint8_t *)(p + 23), QEMU_VERSION, 8); /* firmware version */
-    padstr((uint8_t *)(p + 27), "QEMU CD-ROM", 40); /* model */
+    padstr((uint8_t *)(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 */
@@ -1622,7 +1622,7 @@ 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;
-- 
1.5.2.5


-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4

[PATCH 5/5] qemu: piix_pci: remove 82371FB support

[Carlo Marcelo Arenas Belon]

This patch removes support for 82371FB Step A1 (AKA triton) chips as they are
superseded by 82371SB and are currently dead code as all other
references were removed already from ide.c since revision 1.57

Signed-off-by: Carlo Marcelo Arenas Belon <carenas-kLeDWSohozoJb6fo7hG9ng@public.gmane.org>
---
 qemu/hw/piix_pci.c |   25 -------------------------
 1 files changed, 0 insertions(+), 25 deletions(-)

diff --git a/qemu/hw/piix_pci.c b/qemu/hw/piix_pci.c
index 3c04e3a..f40dad9 100644
--- a/qemu/hw/piix_pci.c
+++ b/qemu/hw/piix_pci.c
@@ -311,31 +311,6 @@ static int piix_load(QEMUFile* f, void *opaque, int version_id)
     return pci_device_load(d, f);
 }
 
-int piix_init(PCIBus *bus, int devfn)
-{
-    PCIDevice *d;
-    uint8_t *pci_conf;
-
-    d = pci_register_device(bus, "PIIX", sizeof(PCIDevice),
-                                    devfn, NULL, NULL);
-    register_savevm("PIIX", 0, 2, piix_save, piix_load, d);
-
-    piix3_dev = d;
-    pci_conf = d->config;
-
-    pci_conf[0x00] = 0x86; // Intel
-    pci_conf[0x01] = 0x80;
-    pci_conf[0x02] = 0x2E; // 82371FB PIIX PCI-to-ISA bridge
-    pci_conf[0x03] = 0x12;
-    pci_conf[0x08] = 0x02; // Step A1
-    pci_conf[0x0a] = 0x01; // class_sub = PCI_ISA
-    pci_conf[0x0b] = 0x06; // class_base = PCI_bridge
-    pci_conf[0x0e] = 0x80; // header_type = PCI_multifunction, generic
-
-    piix3_reset(d);
-    return d->devfn;
-}
-
 int piix3_init(PCIBus *bus, int devfn)
 {
     PCIDevice *d;
-- 
1.5.2.5


-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4

Re: [PATCH 0/5] qemu: IDE/ATAPI emulation reliability fixes

[Avi Kivity]

Carlo Marcelo Arenas Belon wrote:
> The following patch series implements fixes to the IDE/ATAPI emulation
> in qemu which are already committed upstream or proposed for inclusion; 
> including 2 serious regressions that result in availability and data loss
> problems when using OpenSolaris or FreeBSD guests (at least) by the first 2:
>
>   Patch 1/5 : fixes GET_CONFIGURATION to allow OpenSolaris cdrom access 
>   Patch 2/5 : fixes IDE sector write operations resulting in data lost
>   Patch 3/5 : enables ACPI interrupts
>   Patch 4/5 : uses DVD-ROM as model name for INQUIRY and IDENTIFY DEVICE
>   Patch 5/5 : removes obsolete triton chip support
>
> They had been tested in Gentoo Linux 2007.0 amd64 using an Intel Core 2
> 6320 host and several different versions of FreeBSD, OpenBSD, NetBSD, Linux,
> OpenSolaris and Windows 2000 guests
>
>   

Thanks; the last time I tried to merge qemu I had several regressions. 
I'll try again soon once these are merged (or merge them myself if
qemu-devel takes too long).


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4

Re: [PATCH 0/5] qemu: IDE/ATAPI emulation reliability fixes

[Carlo Marcelo Arenas Belon]

On Fri, Nov 30, 2007 at 09:15:55AM +0200, Avi Kivity wrote:
> Carlo Marcelo Arenas Belon wrote:
> > The following patch series implements fixes to the IDE/ATAPI emulation
> > in qemu which are already committed upstream or proposed for inclusion; 
> > including 2 serious regressions that result in availability and data loss
> > problems when using OpenSolaris or FreeBSD guests (at least) by the first 2:
> >
> >   Patch 1/5 : fixes GET_CONFIGURATION to allow OpenSolaris cdrom access 
> >   Patch 2/5 : fixes IDE sector write operations resulting in data lost
> >   Patch 3/5 : enables ACPI interrupts
> >   Patch 4/5 : uses DVD-ROM as model name for INQUIRY and IDENTIFY DEVICE
> >   Patch 5/5 : removes obsolete triton chip support
> >
> > They had been tested in Gentoo Linux 2007.0 amd64 using an Intel Core 2
> > 6320 host and several different versions of FreeBSD, OpenBSD, NetBSD, Linux,
> > OpenSolaris and Windows 2000 guests
> 
> Thanks; the last time I tried to merge qemu I had several regressions. 

some of the patches I cherry picked from qemu cvs are part of the last import
in the qemu-vendor-drops branch but since they had not been yet merged (as 
you mention) I though it was better to cherry pick them from now to expedite
this patch.

> I'll try again soon once these are merged (or merge them myself if
> qemu-devel takes too long).

for qemu is probably not that much of a priority as they already have the
patches, most of them committed and the bugs are only in their development
tree which they don't release anyway, for kvm it is IMHO different since the
bugs are on released code with patch 2 (which ate 3 of my FreeBSD guests with
unrecoverable images which wouldn't even have a valid partition table) being
around since the qemu import of Oct 1 and therefore part of kvm-45 and all
releases up to the date.

I understand though why you wouldn't like to divert from qemu, and I'll do my
best to try to get qemu to commit those patches so that any future conflicts
are resolved automatically, but as I said before, qemu just had different
priorities.

Carlo

-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4

Re: [PATCH 0/5] qemu: IDE/ATAPI emulation reliability fixes

[Avi Kivity]

Carlo Marcelo Arenas Belon wrote:
> for qemu is probably not that much of a priority as they already have the
> patches, most of them committed and the bugs are only in their development
> tree which they don't release anyway, for kvm it is IMHO different since the
> bugs are on released code with patch 2 (which ate 3 of my FreeBSD guests with
> unrecoverable images which wouldn't even have a valid partition table) being
> around since the qemu import of Oct 1 and therefore part of kvm-45 and all
> releases up to the date.
>
> I understand though why you wouldn't like to divert from qemu, and I'll do my
> best to try to get qemu to commit those patches so that any future conflicts
> are resolved automatically, but as I said before, qemu just had different
> priorities.
>
>   

You make some good points;  I'll respin the merge and this time bisect
the problems and revert the offending patches.  I'll add these patches
as well if qemu hasn't merged it by that time.


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell.  From the desktop to the data center, Linux is going
mainstream.  Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4