qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] Bug in Sparc64/IDE Code
@ 2009-12-11 20:16 Nick Couchman
  2009-12-12 10:12 ` Blue Swirl
  0 siblings, 1 reply; 8+ messages in thread
From: Nick Couchman @ 2009-12-11 20:16 UTC (permalink / raw)
  To: qemu-devel

In working to try to get Sparc64 system emulation developed, we seem to have run into an issue with the IDE code in Qemu.  The OpenBIOS folks have been working quite a few issues with the OpenBIOS code that need to be resolved in order to boot 64-bit Solaris kernels correctly, but the most recent issue indicates that the IDE code for the Sparc64 emulator is reading from and writing to the wrong memory locations.  The end result is the following output when trying to boot off an ISO image in Qemu:

0 > boot cdrom 
[sparc64] Booting file 'cdrom' with parameters ''
Not a bootable ELF image
Not a Linux kernel image
Not a bootable a.out image
Loading FCode image...
Loaded 7084 bytes
entry point is 0x4000
Evaluating FCode...
qemu: unsupported keyboard cmd=0x57
sSegmentation fault

This is using the "-nographic" option - the results reported with graphics tend to be more interesting, with lots of garbage being written to the VGA device.  Turning on IDE debugging in Qemu and then trying to boot yields a lot of output - the last few lines look like this:

ide: write control addr=0x682 val=02
IDE: write addr=0x601 val=0x00
IDE: write addr=0x602 val=0x00
IDE: write addr=0x603 val=0x00
IDE: write addr=0x604 val=0x08
IDE: write addr=0x605 val=0x00
IDE: write addr=0x606 val=0x00
IDE: write addr=0x607 val=0xa0
ide: CMD=a0
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read addr=0x607 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read status addr=0x682 val=58
ide: read addr=0x607 val=58
ide: read addr=0x602 val=02
ide: read addr=0x604 val=08
ide: read addr=0x605 val=00
ide: read status addr=0x682 val=50
ide: read status addr=0x682 val=50
ide: read status addr=0x682 val=50
ide: read status addr=0x682 val=50
ide: read status addr=0x682 val=50
ide: read status addr=0x682 val=50
ide: read status addr=0x682 val=50
ide: read status addr=0x682 val=50
ide: read addr=0x607 val=50
ide: read status addsIDE: write addr=0x502 val=0x9a
IDE: write addr=0x503 val=0xa9
IDE: write addr=0x504 val=0x06
IDE: write addr=0x505 val=0xed
IDE: write addr=0x506 val=0x14
IDE: write addr=0x507 val=0xcd
ide: CMD=cd
ide: write control addr=0x582 val=97
IDE: write addr=0x602 val=0xba
IDE: write addr=0x603 val=0xae
IDE: write addr=0x604 val=0xce
IDE: write addr=0x605 val=0xb6
IDE: write addr=0x606 val=0x58
IDE: write addr=0x607 val=0x57
ide: CMD=57
ide: write control addr=0x682 val=70
bmdma_cmd_writeb: 0x00000054
bmdma: writeb 0x701 : 0xd7
bmdma: writeb 0x702 : 0x79
bmdma: writeb 0x703 : 0xfe
bmdma_addr_writew: 0x0000ddef
bmdma_addr_writew: 0x0000b12b
bmdma_cmd_writeb: 0x000000da
bmdma: writeb 0x709 : 0x95
Segmentation fault

On the OpenBIOS list it was pointed out that the addresses for the IDE control seem to be in the 0x6nn range (0x602 to 0x607 with the register at 0x682), but then toward the end the addresses switch over to the 0x5nn range, with the IDE code trying to write to locations 0x502 through 0x507 and the register at 0x582.  This seems to be what causes the segmentation fault and the garbage written to the screen, including the error about an invalid keyboard command.  Any help in tracking down the error would be greatly appreciated, and I'm happy to do any debugging/testing necessary!

Thanks!
-Nick


--------
This e-mail may contain confidential and privileged material for the sole use of the intended recipient.  If this email is not intended for you, or you are not responsible for the delivery of this message to the intended recipient, please note that this message may contain SEAKR Engineering (SEAKR) Privileged/Proprietary Information.  In such a case, you are strictly prohibited from downloading, photocopying, distributing or otherwise using this message, its contents or attachments in any way.  If you have received this message in error, please notify us immediately by replying to this e-mail and delete the message from your mailbox.  Information contained in this message that does not relate to the business of SEAKR is neither endorsed by nor attributable to SEAKR.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] Bug in Sparc64/IDE Code
  2009-12-11 20:16 [Qemu-devel] Bug in Sparc64/IDE Code Nick Couchman
@ 2009-12-12 10:12 ` Blue Swirl
  2009-12-12 12:18   ` Igor Kovalenko
  0 siblings, 1 reply; 8+ messages in thread
From: Blue Swirl @ 2009-12-12 10:12 UTC (permalink / raw)
  To: Nick Couchman; +Cc: qemu-devel

On Fri, Dec 11, 2009 at 10:16 PM, Nick Couchman <Nick.Couchman@seakr.com> wrote:
> In working to try to get Sparc64 system emulation developed, we seem to have run into an issue with the IDE code in Qemu.  The OpenBIOS folks have been working quite a few issues with the OpenBIOS code that need to be resolved in order to boot 64-bit Solaris kernels correctly, but the most recent issue indicates that the IDE code for the Sparc64 emulator is reading from and writing to the wrong memory locations.  The end result is the following output when trying to boot off an ISO image in Qemu:

> bmdma_cmd_writeb: 0x00000054
> bmdma: writeb 0x701 : 0xd7
> bmdma: writeb 0x702 : 0x79
> bmdma: writeb 0x703 : 0xfe
> bmdma_addr_writew: 0x0000ddef
> bmdma_addr_writew: 0x0000b12b
> bmdma_cmd_writeb: 0x000000da
> bmdma: writeb 0x709 : 0x95
> Segmentation fault

I can't reproduce this with milaX 0.3.1, QEMU git HEAD and OpenBIOS
svn r644. The bug could be that the BMDMA address may need BE to LE
conversion, or OpenBIOS could just clobber BMDMA registers with
garbage (the DMA address candidates 0xddefb12b and 0xb12bddef do not
look valid).

Another possibility is that the PCI host bridge should have an IOMMU
which is not implemented yet, but I doubt we are at that stage.

Could you run QEMU in a GDB session and send the backtrace from the segfault?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] Bug in Sparc64/IDE Code
  2009-12-12 10:12 ` Blue Swirl
@ 2009-12-12 12:18   ` Igor Kovalenko
  2009-12-12 13:22     ` Igor Kovalenko
  0 siblings, 1 reply; 8+ messages in thread
From: Igor Kovalenko @ 2009-12-12 12:18 UTC (permalink / raw)
  To: Blue Swirl; +Cc: qemu-devel, Nick Couchman

On Sat, Dec 12, 2009 at 1:12 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Fri, Dec 11, 2009 at 10:16 PM, Nick Couchman <Nick.Couchman@seakr.com> wrote:
>> In working to try to get Sparc64 system emulation developed, we seem to have run into an issue with the IDE code in Qemu.  The OpenBIOS folks have been working quite a few issues with the OpenBIOS code that need to be resolved in order to boot 64-bit Solaris kernels correctly, but the most recent issue indicates that the IDE code for the Sparc64 emulator is reading from and writing to the wrong memory locations.  The end result is the following output when trying to boot off an ISO image in Qemu:
>
>> bmdma_cmd_writeb: 0x00000054
>> bmdma: writeb 0x701 : 0xd7
>> bmdma: writeb 0x702 : 0x79
>> bmdma: writeb 0x703 : 0xfe
>> bmdma_addr_writew: 0x0000ddef
>> bmdma_addr_writew: 0x0000b12b
>> bmdma_cmd_writeb: 0x000000da
>> bmdma: writeb 0x709 : 0x95
>> Segmentation fault
>
> I can't reproduce this with milaX 0.3.1, QEMU git HEAD and OpenBIOS
> svn r644. The bug could be that the BMDMA address may need BE to LE
> conversion, or OpenBIOS could just clobber BMDMA registers with
> garbage (the DMA address candidates 0xddefb12b and 0xb12bddef do not
> look valid).
>
> Another possibility is that the PCI host bridge should have an IOMMU
> which is not implemented yet, but I doubt we are at that stage.
>
> Could you run QEMU in a GDB session and send the backtrace from the segfault?
>

There seems to be an issue with pci_from_bm cast: bm->unit is not
assigned anywhere
in the code so it is zero for second unit, and pci_from_bm returns
wrong address.
Crash happens writing to address mapped for second unit.

-- 
Kind regards,
Igor V. Kovalenko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] Bug in Sparc64/IDE Code
  2009-12-12 12:18   ` Igor Kovalenko
@ 2009-12-12 13:22     ` Igor Kovalenko
  2009-12-13 19:06       ` [Qemu-devel] " Juan Quintela
  0 siblings, 1 reply; 8+ messages in thread
From: Igor Kovalenko @ 2009-12-12 13:22 UTC (permalink / raw)
  To: Blue Swirl; +Cc: Juan Quintela, qemu-devel, Nick Couchman

On Sat, Dec 12, 2009 at 3:18 PM, Igor Kovalenko
<igor.v.kovalenko@gmail.com> wrote:
> On Sat, Dec 12, 2009 at 1:12 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Fri, Dec 11, 2009 at 10:16 PM, Nick Couchman <Nick.Couchman@seakr.com> wrote:
>>> In working to try to get Sparc64 system emulation developed, we seem to have run into an issue with the IDE code in Qemu.  The OpenBIOS folks have been working quite a few issues with the OpenBIOS code that need to be resolved in order to boot 64-bit Solaris kernels correctly, but the most recent issue indicates that the IDE code for the Sparc64 emulator is reading from and writing to the wrong memory locations.  The end result is the following output when trying to boot off an ISO image in Qemu:
>>
>>> bmdma_cmd_writeb: 0x00000054
>>> bmdma: writeb 0x701 : 0xd7
>>> bmdma: writeb 0x702 : 0x79
>>> bmdma: writeb 0x703 : 0xfe
>>> bmdma_addr_writew: 0x0000ddef
>>> bmdma_addr_writew: 0x0000b12b
>>> bmdma_cmd_writeb: 0x000000da
>>> bmdma: writeb 0x709 : 0x95
>>> Segmentation fault
>>
>> I can't reproduce this with milaX 0.3.1, QEMU git HEAD and OpenBIOS
>> svn r644. The bug could be that the BMDMA address may need BE to LE
>> conversion, or OpenBIOS could just clobber BMDMA registers with
>> garbage (the DMA address candidates 0xddefb12b and 0xb12bddef do not
>> look valid).
>>
>> Another possibility is that the PCI host bridge should have an IOMMU
>> which is not implemented yet, but I doubt we are at that stage.
>>
>> Could you run QEMU in a GDB session and send the backtrace from the segfault?
>>
>
> There seems to be an issue with pci_from_bm cast: bm->unit is not
> assigned anywhere
> in the code so it is zero for second unit, and pci_from_bm returns
> wrong address.
> Crash happens writing to address mapped for second unit.

This appears to be a regression in cmd646. After removal of pci_dev from
BMDMAState structure we cannot do much to work around this issue.

The problem here is that we cannot rely on bm->unit value since it is getting
changed while dma operations are in progress, f.e. it is set to -1 on
dma cancel.
Thus we cannot get to pci_dev from BMDMAState passed to i/o read/write
callbacks.

Juan, can you please take a look at this issue?

-- 
Kind regards,
Igor V. Kovalenko

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] Re: Bug in Sparc64/IDE Code
  2009-12-12 13:22     ` Igor Kovalenko
@ 2009-12-13 19:06       ` Juan Quintela
  2009-12-13 21:14         ` Igor Kovalenko
  0 siblings, 1 reply; 8+ messages in thread
From: Juan Quintela @ 2009-12-13 19:06 UTC (permalink / raw)
  To: Igor Kovalenko; +Cc: Blue Swirl, qemu-devel, Nick Couchman

Igor Kovalenko <igor.v.kovalenko@gmail.com> wrote:
> On Sat, Dec 12, 2009 at 3:18 PM, Igor Kovalenko
> <igor.v.kovalenko@gmail.com> wrote:
>> On Sat, Dec 12, 2009 at 1:12 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
>>> On Fri, Dec 11, 2009 at 10:16 PM, Nick Couchman <Nick.Couchman@seakr.com> wrote:
>>>> In working to try to get Sparc64 system emulation developed, we seem to have run into an issue with the IDE code in Qemu.  The OpenBIOS folks have been working quite a few issues with the OpenBIOS code that need to be resolved in order to boot 64-bit Solaris kernels correctly, but the most recent issue indicates that the IDE code for the Sparc64 emulator is reading from and writing to the wrong memory locations.  The end result is the following output when trying to boot off an ISO image in Qemu:
>>>
>>>> bmdma_cmd_writeb: 0x00000054
>>>> bmdma: writeb 0x701 : 0xd7
>>>> bmdma: writeb 0x702 : 0x79
>>>> bmdma: writeb 0x703 : 0xfe
>>>> bmdma_addr_writew: 0x0000ddef
>>>> bmdma_addr_writew: 0x0000b12b
>>>> bmdma_cmd_writeb: 0x000000da
>>>> bmdma: writeb 0x709 : 0x95
>>>> Segmentation fault
>>>
>>> I can't reproduce this with milaX 0.3.1, QEMU git HEAD and OpenBIOS
>>> svn r644. The bug could be that the BMDMA address may need BE to LE
>>> conversion, or OpenBIOS could just clobber BMDMA registers with
>>> garbage (the DMA address candidates 0xddefb12b and 0xb12bddef do not
>>> look valid).
>>>
>>> Another possibility is that the PCI host bridge should have an IOMMU
>>> which is not implemented yet, but I doubt we are at that stage.
>>>
>>> Could you run QEMU in a GDB session and send the backtrace from the segfault?
>>>
>>
>> There seems to be an issue with pci_from_bm cast: bm->unit is not
>> assigned anywhere
>> in the code so it is zero for second unit, and pci_from_bm returns
>> wrong address.
>> Crash happens writing to address mapped for second unit.
>
> This appears to be a regression in cmd646. After removal of pci_dev from
> BMDMAState structure we cannot do much to work around this issue.
>
> The problem here is that we cannot rely on bm->unit value since it is getting
> changed while dma operations are in progress, f.e. it is set to -1 on
> dma cancel.
> Thus we cannot get to pci_dev from BMDMAState passed to i/o read/write
> callbacks.
>
> Juan, can you please take a look at this issue?

   I don't have a sparc setup, but could you try this patch?  It also fixes
   the test for bm.

>From 6d2dfb38804fdf869b59f75768c8376951f81bb5 Mon Sep 17 00:00:00 2001
From: Juan Quintela <quintela@redhat.com>
Date: Sun, 13 Dec 2009 20:03:30 +0100
Subject: [PATCH] cmd646: pass pci_dev as it needs it

Instead of doing tricks to get the pci_dev, just pass it in the 1st
place.  Patch is a bit longer that reverting the pci_dev field, but it
states more clearly (IMHO) what we are doing.

It also fixes the bm test, now that you told me that ->unit is not
always valid.

Could you test and tell me the result?  Thanks in advance.

Later, Juan.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ide/cmd646.c |   64 ++++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 45 insertions(+), 19 deletions(-)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 3b7c606..f5edc14 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -68,19 +68,9 @@ static void ide_map(PCIDevice *pci_dev, int region_num,
     }
 }

-static PCIIDEState *pci_from_bm(BMDMAState *bm)
+static uint32_t bmdma_readb_common(PCIIDEState *pci_dev, BMDMAState *bm,
+                                   uint32_t addr)
 {
-    if (bm->unit == 0) {
-        return container_of(bm, PCIIDEState, bmdma[0]);
-    } else {
-        return container_of(bm, PCIIDEState, bmdma[1]);
-    }
-}
-
-static uint32_t bmdma_readb(void *opaque, uint32_t addr)
-{
-    BMDMAState *bm = opaque;
-    PCIIDEState *pci_dev = pci_from_bm(bm);
     uint32_t val;

     switch(addr & 3) {
@@ -94,7 +84,7 @@ static uint32_t bmdma_readb(void *opaque, uint32_t addr)
         val = bm->status;
         break;
     case 3:
-        if (bm->unit == 0) {
+        if (bm == &pci_dev->bmdma[0]) {
             val = pci_dev->dev.config[UDIDETCR0];
         } else {
             val = pci_dev->dev.config[UDIDETCR1];
@@ -110,10 +100,25 @@ static uint32_t bmdma_readb(void *opaque, uint32_t addr)
     return val;
 }

-static void bmdma_writeb(void *opaque, uint32_t addr, uint32_t val)
+static uint32_t bmdma_readb_0(void *opaque, uint32_t addr)
+{
+    PCIIDEState *pci_dev = opaque;
+    BMDMAState *bm = &pci_dev->bmdma[0];
+
+    return bmdma_readb_common(pci_dev, bm, addr);
+}
+
+static uint32_t bmdma_readb_1(void *opaque, uint32_t addr)
+{
+    PCIIDEState *pci_dev = opaque;
+    BMDMAState *bm = &pci_dev->bmdma[1];
+
+    return bmdma_readb_common(pci_dev, bm, addr);
+}
+
+static void bmdma_writeb_common(PCIIDEState *pci_dev, BMDMAState *bm,
+                                uint32_t addr, uint32_t val)
 {
-    BMDMAState *bm = opaque;
-    PCIIDEState *pci_dev = pci_from_bm(bm);
 #ifdef DEBUG_IDE
     printf("bmdma: writeb 0x%02x : 0x%02x\n", addr, val);
 #endif
@@ -127,7 +132,7 @@ static void bmdma_writeb(void *opaque, uint32_t addr, uint32_t val)
         bm->status = (val & 0x60) | (bm->status & 1) | (bm->status & ~val & 0x06);
         break;
     case 3:
-        if (bm->unit == 0)
+        if (bm == &pci_dev->bmdma[0])
             pci_dev->dev.config[UDIDETCR0] = val;
         else
             pci_dev->dev.config[UDIDETCR1] = val;
@@ -135,6 +140,22 @@ static void bmdma_writeb(void *opaque, uint32_t addr, uint32_t val)
     }
 }

+static void bmdma_writeb_0(void *opaque, uint32_t addr, uint32_t val)
+{
+    PCIIDEState *pci_dev = opaque;
+    BMDMAState *bm = &pci_dev->bmdma[0];
+
+    bmdma_writeb_common(pci_dev, bm, addr, val);
+}
+
+static void bmdma_writeb_1(void *opaque, uint32_t addr, uint32_t val)
+{
+    PCIIDEState *pci_dev = opaque;
+    BMDMAState *bm = &pci_dev->bmdma[1];
+
+    bmdma_writeb_common(pci_dev, bm, addr, val);
+}
+
 static void bmdma_map(PCIDevice *pci_dev, int region_num,
                     pcibus_t addr, pcibus_t size, int type)
 {
@@ -149,8 +170,13 @@ static void bmdma_map(PCIDevice *pci_dev, int region_num,

         register_ioport_write(addr, 1, 1, bmdma_cmd_writeb, bm);

-        register_ioport_write(addr + 1, 3, 1, bmdma_writeb, bm);
-        register_ioport_read(addr, 4, 1, bmdma_readb, bm);
+        if (i == 0) {
+            register_ioport_write(addr + 1, 3, 1, bmdma_writeb_0, d);
+            register_ioport_read(addr, 4, 1, bmdma_readb_0, d);
+        } else {
+            register_ioport_write(addr + 1, 3, 1, bmdma_writeb_1, d);
+            register_ioport_read(addr, 4, 1, bmdma_readb_1, d);
+        }

         register_ioport_write(addr + 4, 4, 1, bmdma_addr_writeb, bm);
         register_ioport_read(addr + 4, 4, 1, bmdma_addr_readb, bm);
-- 
1.6.5.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] Re: Bug in Sparc64/IDE Code
  2009-12-13 19:06       ` [Qemu-devel] " Juan Quintela
@ 2009-12-13 21:14         ` Igor Kovalenko
  2009-12-13 22:41           ` Juan Quintela
  0 siblings, 1 reply; 8+ messages in thread
From: Igor Kovalenko @ 2009-12-13 21:14 UTC (permalink / raw)
  To: Juan Quintela; +Cc: Blue Swirl, qemu-devel, Nick Couchman

On Sun, Dec 13, 2009 at 10:06 PM, Juan Quintela <quintela@redhat.com> wrote:
> Igor Kovalenko <igor.v.kovalenko@gmail.com> wrote:
>> On Sat, Dec 12, 2009 at 3:18 PM, Igor Kovalenko
>> <igor.v.kovalenko@gmail.com> wrote:
>>> On Sat, Dec 12, 2009 at 1:12 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
>>>> On Fri, Dec 11, 2009 at 10:16 PM, Nick Couchman <Nick.Couchman@seakr.com> wrote:
>>>>> In working to try to get Sparc64 system emulation developed, we seem to have run into an issue with the IDE code in Qemu.  The OpenBIOS folks have been working quite a few issues with the OpenBIOS code that need to be resolved in order to boot 64-bit Solaris kernels correctly, but the most recent issue indicates that the IDE code for the Sparc64 emulator is reading from and writing to the wrong memory locations.  The end result is the following output when trying to boot off an ISO image in Qemu:
>>>>
>>>>> bmdma_cmd_writeb: 0x00000054
>>>>> bmdma: writeb 0x701 : 0xd7
>>>>> bmdma: writeb 0x702 : 0x79
>>>>> bmdma: writeb 0x703 : 0xfe
>>>>> bmdma_addr_writew: 0x0000ddef
>>>>> bmdma_addr_writew: 0x0000b12b
>>>>> bmdma_cmd_writeb: 0x000000da
>>>>> bmdma: writeb 0x709 : 0x95
>>>>> Segmentation fault
>>>>
>>>> I can't reproduce this with milaX 0.3.1, QEMU git HEAD and OpenBIOS
>>>> svn r644. The bug could be that the BMDMA address may need BE to LE
>>>> conversion, or OpenBIOS could just clobber BMDMA registers with
>>>> garbage (the DMA address candidates 0xddefb12b and 0xb12bddef do not
>>>> look valid).
>>>>
>>>> Another possibility is that the PCI host bridge should have an IOMMU
>>>> which is not implemented yet, but I doubt we are at that stage.
>>>>
>>>> Could you run QEMU in a GDB session and send the backtrace from the segfault?
>>>>
>>>
>>> There seems to be an issue with pci_from_bm cast: bm->unit is not
>>> assigned anywhere
>>> in the code so it is zero for second unit, and pci_from_bm returns
>>> wrong address.
>>> Crash happens writing to address mapped for second unit.
>>
>> This appears to be a regression in cmd646. After removal of pci_dev from
>> BMDMAState structure we cannot do much to work around this issue.
>>
>> The problem here is that we cannot rely on bm->unit value since it is getting
>> changed while dma operations are in progress, f.e. it is set to -1 on
>> dma cancel.
>> Thus we cannot get to pci_dev from BMDMAState passed to i/o read/write
>> callbacks.
>>
>> Juan, can you please take a look at this issue?
>
>   I don't have a sparc setup, but could you try this patch?  It also fixes
>   the test for bm.

Looks good, but runtime aborts in register_ioport_read.

You cannot install different opaque for read and write of the same i/o address.
Seems like every other device has the same driver for reading and writing,
but in cmd646 it calls out to ide/pci.c code for bmdma_cmd_writeb write
method, whereas it reads with own bmdma_readb_0 method.

Probably bmdma_writeb_* should call out to bmdma_cmd_writeb for address 0
and whole 4 byte is to be mapped to bmdma_writeb_*

I tested the following fix on top of yours patch with my previous workaround
reverted. Both my workaround and these two combined show the same
qemu.log trace.

commit 26c618af44c91a806d88044d468733b86028e352
Author: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
Date:   Mon Dec 14 00:05:10 2009 +0300

    cmd646 fix abort due to changed opaque pointer for ioport read

    Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 9d60590..07fcf4d 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -123,6 +123,9 @@ static void bmdma_writeb_common(PCIIDEState
*pci_dev, BMDMAState *bm,
     printf("bmdma: writeb 0x%02x : 0x%02x\n", addr, val);
 #endif
     switch(addr & 3) {
+    case 0:
+        bmdma_cmd_writeb(bm, addr, val);
+        break;
     case 1:
         pci_dev->dev.config[MRDMODE] =
             (pci_dev->dev.config[MRDMODE] & ~0x30) | (val & 0x30);
@@ -168,13 +171,11 @@ static void bmdma_map(PCIDevice *pci_dev, int region_num,
         bm->bus = d->bus+i;
         qemu_add_vm_change_state_handler(ide_dma_restart_cb, bm);

-        register_ioport_write(addr, 1, 1, bmdma_cmd_writeb, bm);
-
         if (i == 0) {
-            register_ioport_write(addr + 1, 3, 1, bmdma_writeb_0, d);
+            register_ioport_write(addr, 4, 1, bmdma_writeb_0, d);
             register_ioport_read(addr, 4, 1, bmdma_readb_0, d);
         } else {
-            register_ioport_write(addr + 1, 3, 1, bmdma_writeb_1, d);
+            register_ioport_write(addr, 4, 1, bmdma_writeb_1, d);
             register_ioport_read(addr, 4, 1, bmdma_readb_1, d);
         }



-- 
Kind regards,
Igor V. Kovalenko

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] Re: Bug in Sparc64/IDE Code
  2009-12-13 21:14         ` Igor Kovalenko
@ 2009-12-13 22:41           ` Juan Quintela
  2009-12-15 14:30             ` Nick Couchman
  0 siblings, 1 reply; 8+ messages in thread
From: Juan Quintela @ 2009-12-13 22:41 UTC (permalink / raw)
  To: Igor Kovalenko; +Cc: Blue Swirl, qemu-devel, Nick Couchman

Igor Kovalenko <igor.v.kovalenko@gmail.com> wrote:
> On Sun, Dec 13, 2009 at 10:06 PM, Juan Quintela <quintela@redhat.com> wrote:

> Looks good, but runtime aborts in register_ioport_read.
>
> You cannot install different opaque for read and write of the same i/o address.
> Seems like every other device has the same driver for reading and writing,
> but in cmd646 it calls out to ide/pci.c code for bmdma_cmd_writeb write
> method, whereas it reads with own bmdma_readb_0 method.

Grr, another _nice_ interface.  I guess that qemu should have a single
method for registering both read/write functions.

> Probably bmdma_writeb_* should call out to bmdma_cmd_writeb for address 0
> and whole 4 byte is to be mapped to bmdma_writeb_*
>
> I tested the following fix on top of yours patch with my previous workaround
> reverted. Both my workaround and these two combined show the same
> qemu.log trace.

Nice.  Blue can we got this two patches integrated instead of getting
back pci_dev?  I removed pci_dev from PCIIDEState because it is only
used by cmd646.  This two new patches  just register the right opaque,
that is what we need in the first place.

Igor, thanks for the testing.

Later, Juan.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] Re: Bug in Sparc64/IDE Code
  2009-12-13 22:41           ` Juan Quintela
@ 2009-12-15 14:30             ` Nick Couchman
  0 siblings, 0 replies; 8+ messages in thread
From: Nick Couchman @ 2009-12-15 14:30 UTC (permalink / raw)
  To: Igor Kovalenko, Juan Quintela; +Cc: Blue Swirl, qemu-devel

Did these patches ever get committed to the Qemu repo?  I tried applying
them to the latest git repo, and they don't apply.  Furthermore,
building qemu out of the latest version of the repository causes very
similar messages, just a few more of them.

OpenBIOS for Sparc64
Configuration device id QEMU version 1 machine id 0
CPUs: 1 x SUNW,UltraSPARC-II
UUID: 00000000-0000-0000-0000-000000000000
Welcome to OpenBIOS v1.0 built on Dec 14 2009 04:04
  Type 'help' for detailed information

[sparc64] Booting file 'cdrom' with parameters ''
Not a bootable ELF image
Not a Linux kernel image
Not a bootable a.out image
Loading FCode image...
Loaded 7084 bytes
entry point is 0x4000
Evaluating FCode...
qemu: unsupported keyboard cmd=0x57
sqemu: unsupported keyboard cmd=0xca
�qemu: unsupported keyboard cmd=0xdb
FLOPPY ERROR: fdctrl_unimplemented: unimplemented command 0xce

It looks like perhaps there are more instances where the IDE code
chooses to write to the wrong memory location.  I can provide detailed
debug output if needed...

-Nick

>>> On 2009/12/13 at 15:41, Juan Quintela <quintela@redhat.com> wrote:

> Igor Kovalenko <igor.v.kovalenko@gmail.com> wrote:
>> On Sun, Dec 13, 2009 at 10:06 PM, Juan Quintela
<quintela@redhat.com> wrote:
> 
>> Looks good, but runtime aborts in register_ioport_read.
>>
>> You cannot install different opaque for read and write of the same
i/o 
> address.
>> Seems like every other device has the same driver for reading and
writing,
>> but in cmd646 it calls out to ide/pci.c code for bmdma_cmd_writeb
write
>> method, whereas it reads with own bmdma_readb_0 method.
> 
> Grr, another _nice_ interface.  I guess that qemu should have a
single
> method for registering both read/write functions.
> 
>> Probably bmdma_writeb_* should call out to bmdma_cmd_writeb for
address 0
>> and whole 4 byte is to be mapped to bmdma_writeb_*
>>
>> I tested the following fix on top of yours patch with my previous
workaround
>> reverted. Both my workaround and these two combined show the same
>> qemu.log trace.
> 
> Nice.  Blue can we got this two patches integrated instead of
getting
> back pci_dev?  I removed pci_dev from PCIIDEState because it is only
> used by cmd646.  This two new patches  just register the right
opaque,
> that is what we need in the first place.
> 
> Igor, thanks for the testing.
> 
> Later, Juan.



--------
This e-mail may contain confidential and privileged material for the sole use of the intended recipient.  If this email is not intended for you, or you are not responsible for the delivery of this message to the intended recipient, please note that this message may contain SEAKR Engineering (SEAKR) Privileged/Proprietary Information.  In such a case, you are strictly prohibited from downloading, photocopying, distributing or otherwise using this message, its contents or attachments in any way.  If you have received this message in error, please notify us immediately by replying to this e-mail and delete the message from your mailbox.  Information contained in this message that does not relate to the business of SEAKR is neither endorsed by nor attributable to SEAKR.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2009-12-15 14:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-11 20:16 [Qemu-devel] Bug in Sparc64/IDE Code Nick Couchman
2009-12-12 10:12 ` Blue Swirl
2009-12-12 12:18   ` Igor Kovalenko
2009-12-12 13:22     ` Igor Kovalenko
2009-12-13 19:06       ` [Qemu-devel] " Juan Quintela
2009-12-13 21:14         ` Igor Kovalenko
2009-12-13 22:41           ` Juan Quintela
2009-12-15 14:30             ` Nick Couchman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).