From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36577) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZxGrJ-0005vm-IM for qemu-devel@nongnu.org; Fri, 13 Nov 2015 11:05:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZxGrG-0002gL-8B for qemu-devel@nongnu.org; Fri, 13 Nov 2015 11:05:53 -0500 Message-ID: <56460A20.2040906@ilande.co.uk> Date: Fri, 13 Nov 2015 16:04:48 +0000 From: Mark Cave-Ayland MIME-Version: 1.0 References: <1447363448-20405-1-git-send-email-hpoussin@reactos.org> In-Reply-To: <1447363448-20405-1-git-send-email-hpoussin@reactos.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH for-2.5] mac_dbdma: always initialize channel field in DBDMA_channel List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?SGVydsOpIFBvdXNzaW5lYXU=?= , qemu-devel@nongnu.org Cc: Old World , David Gibson On 12/11/15 21:24, Hervé Poussineau wrote: > 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 > --- > 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); > Reviewed-by: Mark Cave-Ayland This definitely prevents a host crash here, although it is possible to argue whether or not an assert is more useful than a hang for developers. David, can you queue this for ppc-next? It's not urgent but it would be good not to have a potential host crash in 2.5. Incidentally the QEMU iov can disappear from the io struct now since it was only used for Alex's original alignment fixes for the macio code, and it is probably more correct to move head_remainder and tail_remainder into MACIOIDEState since DBDMA doesn't always involve disk devices - but that can wait for another day... ATB, Mark.