Re: [Qemu-ppc] [PATCH for-2.5] mac_dbdma: always initialize channel field in DBDMA_channel

[Hervé Poussineau]

Le 13/11/2015 05:09, Programmingkid a écrit :
> On Nov 12, 2015, at 11:04 PM, address@hidden wrote:
>
> > Message: 3
> > Date: Thu, 12 Nov 2015 22:24:08 +0100
> > From: Herv? Poussineau <address@hidden>
> > To: address@hidden
> > Cc: "open list:Old World" <address@hidden>, Herv? Poussineau
> >         <address@hidden>
> > Subject: [Qemu-ppc] [PATCH for-2.5] mac_dbdma: always initialize
> >         channel field in DBDMA_channel
> > Message-ID: <address@hidden>
> > Content-Type: text/plain; charset=UTF-8
> >
> > dbdma_from_ch() uses channel field to return the right DBDMA object.
> > Previous code was working if guest OS was only using registered DMA channels.
> > However, it lead to QEMU crashes if guest OS was using unregistered DMA 
> > channels.
> >
> > Signed-off-by: Herv? Poussineau <address@hidden>
> > ---
> > hw/misc/macio/mac_dbdma.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/misc/macio/mac_dbdma.c b/hw/misc/macio/mac_dbdma.c
> > index 779683c..5ee8f02 100644
> > --- a/hw/misc/macio/mac_dbdma.c
> > +++ b/hw/misc/macio/mac_dbdma.c
> > @@ -557,7 +557,6 @@ void DBDMA_register_channel(void *dbdma, int nchan, 
> > qemu_irq irq,
> >      DBDMA_DPRINTF("DBDMA_register_channel 0x%x\n", nchan);
> >
> >      ch->irq = irq;
> > -    ch->channel = nchan;
> >      ch->rw = rw;
> >      ch->flush = flush;
> >      ch->io.opaque = opaque;
> > @@ -753,6 +752,7 @@ void* DBDMA_init (MemoryRegion **dbdma_mem)
> >      for (i = 0; i < DBDMA_CHANNELS; i++) {
> >          DBDMA_io *io = &s->channels[i].io;
> >          qemu_iovec_init(&io->iov, 1);
> > +        s->channels[i].channel = i;
> >      }
> >
> >      memory_region_init_io(&s->mem, NULL, &dbdma_ops, s, "dbdma", 0x1000);
> > --
> > 2.1.4
>
> What operating system(s) did you use to test this patch out?

It was during some custom tests with OpenBIOS, where i miswrote the IDE DMA 
channel.

However, you can see the problem by using this "patch":
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 3ee962f..73dfec0 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -629,7 +629,7 @@ void macio_ide_init_drives(MACIOIDEState *s, DriveInfo 
**hd_table)
 void macio_ide_register_dma(MACIOIDEState *s, void *dbdma, int channel)
 {
     s->dbdma = dbdma;
-    DBDMA_register_channel(dbdma, channel, s->dma_irq,
+    DBDMA_register_channel(dbdma, channel + 1, s->dma_irq,
                            pmac_ide_transfer, pmac_ide_flush, s);
 }

And starting whatever operating system. As soon as DMA is used to read the 
disk/cdrom, QEMU will crash.

Regards,

Hervé