From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53980) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZxDBK-0002QP-NQ for qemu-devel@nongnu.org; Fri, 13 Nov 2015 07:10:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZxDBE-0007LZ-HU for qemu-devel@nongnu.org; Fri, 13 Nov 2015 07:10:18 -0500 Message-ID: <5645D31E.6050205@reactos.org> Date: Fri, 13 Nov 2015 13:10:06 +0100 From: =?UTF-8?B?SGVydsOpIFBvdXNzaW5lYXU=?= MIME-Version: 1.0 References: <11643EA3-BD07-4DD1-8599-1DD91D1CDE4D@gmail.com> <5645B132.2070404@reactos.org> <5645BE00.9070101@redhat.com> In-Reply-To: <5645BE00.9070101@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-ppc] [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: Thomas Huth , Programmingkid , qemu-devel qemu-devel Cc: "qemu-ppc@nongnu.org list:PowerPC" Le 13/11/2015 11:40, Thomas Huth a =C3=A9crit : > On 13/11/15 10:45, Herv=C3=A9 Poussineau wrote: >> Le 13/11/2015 05:09, Programmingkid a =C3=A9crit : >>> >>> On Nov 12, 2015, at 11:04 PM, qemu-ppc-request@nongnu.org wrote: >>> >>>> Message: 3 >>>> Date: Thu, 12 Nov 2015 22:24:08 +0100 >>>> From: Herv? Poussineau >>>> To: qemu-devel@nongnu.org >>>> Cc: "open list:Old World" , Herv? Poussineau >>>> >>>> Subject: [Qemu-ppc] [PATCH for-2.5] mac_dbdma: always initialize >>>> channel field in DBDMA_channel >>>> Message-ID: <1447363448-20405-1-git-send-email-hpoussin@reactos.org> >>>> Content-Type: text/plain; charset=3DUTF-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 >>>> --- >>>> 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 =3D irq; >>>> - ch->channel =3D nchan; >>>> ch->rw =3D rw; >>>> ch->flush =3D flush; >>>> ch->io.opaque =3D opaque; >>>> @@ -753,6 +752,7 @@ void* DBDMA_init (MemoryRegion **dbdma_mem) >>>> for (i =3D 0; i < DBDMA_CHANNELS; i++) { >>>> DBDMA_io *io =3D &s->channels[i].io; >>>> qemu_iovec_init(&io->iov, 1); >>>> + s->channels[i].channel =3D 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 ID= E >> 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 chann= el) >> { >> s->dbdma =3D 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. > > Where does it crash? Could you provide a backtrace? ... sounds like the > function where this goes wrong should do some more checking for valid > channels? The structure is: typedef struct { MemoryRegion mem; DBDMA_channel channels[DBDMA_CHANNELS]; QEMUBH *bh; } DBDMAState; static DBDMAState *dbdma_from_ch(DBDMA_channel *ch) { return container_of(ch, DBDMAState, channels[ch->channel]); } Guest can deal with whatever DMA channel it wants (< DBDMA_CHANNELS). Som= e work will then be done with s->channels["guest_provided_channel"]. Note= that DMA channel can be registered to some device, or=20 not. Later, dbdma_from_ch(DBDMA_channel) method is called to get back the DBDM= AState structure from the channel. This method uses the ch->channel field= . If this field is not initialized (ie is 0), a wrong=20 DBDMAState pointer is returned, and memory corruption starts. QEMU crashe= s later, without any useful hint. I just tried to register a wrong DMA channel for IDE and start MacOS 9. W= ithout my patch, QEMU crashes. With my patch, MacOS 9 freezes and waits f= or the DMA transfer to complete, which never happens. As you want a stack trace, here it is: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fffdaf7a700 (LWP 32484)] qemu_bh_schedule (bh=3D0x0) at async.c:130 130 ctx =3D bh->ctx; (gdb) bt #0 qemu_bh_schedule (bh=3D0x0) at async.c:130 #1 0x00005555557211b5 in memory_region_write_accessor (mr=3D0x555556df3f= 10, addr=3D3328, value=3D, size=3D4, shift=3D, mask=3D, attrs=3D...) at memory.c:450 #2 0x0000555555720635 in access_with_adjusted_size (addr=3D0, addr@entry= =3D3328, value=3D0x0, value@entry=3D0x7fffdaf79498, size=3D4, access_size= _min=3D0, access_size_max=3D0, access=3D0x555555721100=20 , mr=3D0x555556df3f10, attrs=3D...) at memory.c:506 #3 0x00005555557225eb in memory_region_dispatch_write (mr=3D0x555556df3f= 10, addr=3D3328, data=3D2415955968, size=3D4, attrs=3D...) at memory.c:11= 72 #4 0x00007fffde735ab9 in code_gen_buffer () #5 0x00005555556e8510 in cpu_tb_exec (tb_ptr=3D0x7fffde735a30 "A\213n\360\205\355\017\205\246", cpu=3D0x555556ae6bc0) a= t cpu-exec.c:157 #6 cpu_ppc_exec (cpu=3Dcpu@entry=3D0x555556ae6bc0) at cpu-exec.c:520 #7 0x0000555555711d98 in tcg_cpu_exec (cpu=3D0x555556ae6bc0) at cpus.c:1= 470 #8 tcg_exec_all () at cpus.c:1503 #9 qemu_tcg_cpu_thread_fn (arg=3D) at cpus.c:1138 Herv=C3=A9