Igor Kovalenko <igor.v.kovale...@gmail.com> wrote:
> On Sat, Dec 12, 2009 at 3:18 PM, Igor Kovalenko
> <igor.v.kovale...@gmail.com> wrote:
>> On Sat, Dec 12, 2009 at 1:12 PM, Blue Swirl <blauwir...@gmail.com> wrote:
>>> On Fri, Dec 11, 2009 at 10:16 PM, Nick Couchman <nick.couch...@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 <quint...@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 <quint...@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
