* Re: [Qemu-devel] [Qemu-ppc] standard test image not booting with qemu-system-ppc [not found] ` <52EFFFC1.7040303@ilande.co.uk> @ 2014-02-03 21:13 ` Alexander Graf 2014-02-03 22:58 ` Alexander Graf 0 siblings, 1 reply; 8+ messages in thread From: Alexander Graf @ 2014-02-03 21:13 UTC (permalink / raw) To: Mark Cave-Ayland Cc: Stefano Stabellini, qemu-ppc@nongnu.org, qemu-devel, Nitin Srivastava On 03.02.2014, at 21:44, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: > On 03/02/14 20:02, Nitin Srivastava wrote: > >> Hi , >> I downloaded the latest qemu source from git and compiled it on my >> centos 6.5 machine. >> According to the following e-mail from this mailing list I tried the >> following >> _http://comments.gmane.org/gmane.comp.emulators.qemu/181171_ >> Test case: download the squeeze standard image from >> _http://people.debian.org/~aurel32/qemu/powerpc/_ >> <http://people.debian.org/%7Eaurel32/qemu/powerpc/> >> and run with >> qemu-system-ppc -hda debian_squeeze_powerpc_standard.qcow2 >> but I get following error >> VNC server running on `::1:5900' >>> > ============================================================= >>> > OpenBIOS 1.1 [Oct 2 2013 22:57] >>> > Configuration device id QEMU version 1 machine id 2 >>> > CPUs: 1 >>> > Memory: 128M >>> > UUID: 00000000-0000-0000-0000-000000000000 >>> > CPU type PowerPC,750 >>> > Not a bootable ELF image >> qemu: terminating on signal 2 >> nitins@nhost02%:~:117# >> also please note that my qemu-system-ppc is latest, as its built from >> source. >> nitins@nhost02%:~:117#qemu-system-ppc -version >> QEMU emulator version 1.7.50, Copyright (c) 2003-2008 Fabrice Bellard >> nitins@nhost02%:~:118# >> Please help. >> Regds. >> Nitin > > Hi Nitin, > > Having just updated to git master, I now see this issue too. A quick session with git bisect shows the culprit is this commit: > > > build@kentang:~/src/qemu/git/qemu$ git bisect bad > 360e607b88a23d378f6efaa769c76d26f538234d is the first bad commit > commit 360e607b88a23d378f6efaa769c76d26f538234d > Author: Stefano Stabellini <Stefano.Stabellini@eu.citrix.com> > Date: Thu Jan 30 12:46:05 2014 +0000 > > address_space_translate: do not cross page boundaries > > The following commit: > > commit 149f54b53b7666a3facd45e86eece60ce7d3b114 > Author: Paolo Bonzini <pbonzini@redhat.com> > Date: Fri May 24 12:59:37 2013 +0200 > > memory: add address_space_translate > > breaks Xen support in QEMU, in particular the Xen mapcache. The effect > is that one Windows XP installation out of ten would end up with BSOD. > > The reason is that after this commit l in address_space_rw can span a > page boundary, however qemu_get_ram_ptr still calls xen_map_cache asking > to map a single page (if block->offset == 0). > > Fix the issue by reverting to the previous behaviour: do not return a > length from address_space_translate_internal that can span a page > boundary. > > Also in address_space_translate do not ignore the length returned by > address_space_translate_internal. > > This patch should be backported to QEMU 1.6.x. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Signed-off-by: Anthony Perard <anthony.perard@citrix.com> > Tested-by: Paolo Bonzini <pbonzini@redhat.com> > Acked-by: Paolo Bonzini <pbonzini@redhat.com> > Cc: qemu-stable@nongnu.org > > > Stefano/Alex, is there any reason why this would break qemu-system-ppc? Ugh, sorry Nitin, I should have read the email to the end. The image does work for me with -nographic, so I'd assume it's something about the frame buffer map going wrong? We do successfully run the guest: agraf@boysenberry-1:/home/agraf/release/qemu> ./ppc-softmmu/qemu-system-ppc -vnc :8 -snapshot -hda /dev/shm/debian_squeeze_powerpc_standard.qcow2 -serial mon:stdio >> ============================================================= >> OpenBIOS 1.1 [Oct 2 2013 22:57] >> Configuration device id QEMU version 1 machine id 2 >> CPUs: 1 >> Memory: 128M >> UUID: 00000000-0000-0000-0000-000000000000 >> CPU type PowerPC,750 >> Not a bootable ELF image QEMU 1.7.50 monitor - type 'help' for more information (qemu) x /i $pc 0xfff0c2e0: b 0xfff0c2ec (qemu) x /i $pc 0xfff25dd4: lwz r0,4(r11) (qemu) x /i $pc 0xfff1552c: lhz r9,0(r4) (qemu) x /i $pc 0xfff0a868: lwz r0,20(r1) (qemu) x /i $pc 0xfff25d3c: stw r31,-4(r11) (qemu) x /i $pc 0xfff0aeb8: mr r31,r3 (qemu) x /i $pc 0xfff0b050: mr r31,r3 (qemu) x /i $pc 0xfff0a6cc: lis r9,-5 (qemu) x /i $pc 0xc0252a5c: mr r3,r31 (qemu) x /i $pc 0xc003e274: lwz r9,68(r30) so it's really only the VGA output that's broken. Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] standard test image not booting with qemu-system-ppc 2014-02-03 21:13 ` [Qemu-devel] [Qemu-ppc] standard test image not booting with qemu-system-ppc Alexander Graf @ 2014-02-03 22:58 ` Alexander Graf 2014-02-03 23:28 ` Alexander Graf 0 siblings, 1 reply; 8+ messages in thread From: Alexander Graf @ 2014-02-03 22:58 UTC (permalink / raw) To: Mark Cave-Ayland Cc: Nitin Srivastava, qemu-ppc@nongnu.org, qemu-devel, Stefano Stabellini On 03.02.2014, at 22:13, Alexander Graf <agraf@suse.de> wrote: > > On 03.02.2014, at 21:44, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: > >> On 03/02/14 20:02, Nitin Srivastava wrote: >> >>> Hi , >>> I downloaded the latest qemu source from git and compiled it on my >>> centos 6.5 machine. >>> According to the following e-mail from this mailing list I tried the >>> following >>> _http://comments.gmane.org/gmane.comp.emulators.qemu/181171_ >>> Test case: download the squeeze standard image from >>> _http://people.debian.org/~aurel32/qemu/powerpc/_ >>> <http://people.debian.org/%7Eaurel32/qemu/powerpc/> >>> and run with >>> qemu-system-ppc -hda debian_squeeze_powerpc_standard.qcow2 >>> but I get following error >>> VNC server running on `::1:5900' >>>>> ============================================================= >>>>> OpenBIOS 1.1 [Oct 2 2013 22:57] >>>>> Configuration device id QEMU version 1 machine id 2 >>>>> CPUs: 1 >>>>> Memory: 128M >>>>> UUID: 00000000-0000-0000-0000-000000000000 >>>>> CPU type PowerPC,750 >>>>> Not a bootable ELF image >>> qemu: terminating on signal 2 >>> nitins@nhost02%:~:117# >>> also please note that my qemu-system-ppc is latest, as its built from >>> source. >>> nitins@nhost02%:~:117#qemu-system-ppc -version >>> QEMU emulator version 1.7.50, Copyright (c) 2003-2008 Fabrice Bellard >>> nitins@nhost02%:~:118# >>> Please help. >>> Regds. >>> Nitin >> >> Hi Nitin, >> >> Having just updated to git master, I now see this issue too. A quick session with git bisect shows the culprit is this commit: >> >> >> build@kentang:~/src/qemu/git/qemu$ git bisect bad >> 360e607b88a23d378f6efaa769c76d26f538234d is the first bad commit >> commit 360e607b88a23d378f6efaa769c76d26f538234d >> Author: Stefano Stabellini <Stefano.Stabellini@eu.citrix.com> >> Date: Thu Jan 30 12:46:05 2014 +0000 >> >> address_space_translate: do not cross page boundaries >> >> The following commit: >> >> commit 149f54b53b7666a3facd45e86eece60ce7d3b114 >> Author: Paolo Bonzini <pbonzini@redhat.com> >> Date: Fri May 24 12:59:37 2013 +0200 >> >> memory: add address_space_translate >> >> breaks Xen support in QEMU, in particular the Xen mapcache. The effect >> is that one Windows XP installation out of ten would end up with BSOD. >> >> The reason is that after this commit l in address_space_rw can span a >> page boundary, however qemu_get_ram_ptr still calls xen_map_cache asking >> to map a single page (if block->offset == 0). >> >> Fix the issue by reverting to the previous behaviour: do not return a >> length from address_space_translate_internal that can span a page >> boundary. >> >> Also in address_space_translate do not ignore the length returned by >> address_space_translate_internal. >> >> This patch should be backported to QEMU 1.6.x. >> >> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >> Signed-off-by: Anthony Perard <anthony.perard@citrix.com> >> Tested-by: Paolo Bonzini <pbonzini@redhat.com> >> Acked-by: Paolo Bonzini <pbonzini@redhat.com> >> Cc: qemu-stable@nongnu.org >> >> >> Stefano/Alex, is there any reason why this would break qemu-system-ppc? > > Ugh, sorry Nitin, I should have read the email to the end. > > The image does work for me with -nographic, so I'd assume it's something about the frame buffer map going wrong? We do successfully run the guest: > > agraf@boysenberry-1:/home/agraf/release/qemu> ./ppc-softmmu/qemu-system-ppc -vnc :8 -snapshot -hda /dev/shm/debian_squeeze_powerpc_standard.qcow2 -serial mon:stdio > >>> ============================================================= >>> OpenBIOS 1.1 [Oct 2 2013 22:57] >>> Configuration device id QEMU version 1 machine id 2 >>> CPUs: 1 >>> Memory: 128M >>> UUID: 00000000-0000-0000-0000-000000000000 >>> CPU type PowerPC,750 >>> Not a bootable ELF image > QEMU 1.7.50 monitor - type 'help' for more information > (qemu) x /i $pc > 0xfff0c2e0: b 0xfff0c2ec > (qemu) x /i $pc > 0xfff25dd4: lwz r0,4(r11) > (qemu) x /i $pc > 0xfff1552c: lhz r9,0(r4) > (qemu) x /i $pc > 0xfff0a868: lwz r0,20(r1) > (qemu) x /i $pc > 0xfff25d3c: stw r31,-4(r11) > (qemu) x /i $pc > 0xfff0aeb8: mr r31,r3 > (qemu) x /i $pc > 0xfff0b050: mr r31,r3 > (qemu) x /i $pc > 0xfff0a6cc: lis r9,-5 > (qemu) x /i $pc > 0xc0252a5c: mr r3,r31 > (qemu) x /i $pc > 0xc003e274: lwz r9,68(r30) > > so it's really only the VGA output that's broken. A simple git revert of Stefanos patch makes VGA work again. The diff of "info mtree" of a mac99 system with and without his patch is the following: --- x 2014-02-03 23:57:20.000000000 +0100 +++ y 2014-02-03 23:56:14.000000000 +0100 @@ -30,6 +30,7 @@ aliases pci-mmio 0000000000000000-00000000ffffffff (prio 0, RW): pci-mmio + 00000000000a0000-00000000000affff (prio 2, RW): alias vga.chain4 @vga.vram 0000000000000000-000000000000ffff 00000000000a0000-00000000000bffff (prio 1, RW): vga-lowmem 0000000080000000-0000000080ffffff (prio 1, RW): vga.vram 0000000081000000-0000000081000fff (prio 1, RW): vga.mmio @@ -77,6 +78,8 @@ 00000000000003d4-00000000000003d5 (prio 0, RW): vga 00000000000003da-00000000000003da (prio 0, RW): vga 0000000000000400-00000000000004ff (prio 1, RW): ne2000 +vga.vram +0000000080000000-0000000080ffffff (prio 1, RW): vga.vram escc-bar 0000000000013000-000000000001303f (prio 0, RW): alias escc-bar @escc 0000000000000000-000000000000003f escc I'm still not quite sure what did cause the breakage. Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] standard test image not booting with qemu-system-ppc 2014-02-03 22:58 ` Alexander Graf @ 2014-02-03 23:28 ` Alexander Graf 2014-02-04 0:44 ` Peter Maydell 0 siblings, 1 reply; 8+ messages in thread From: Alexander Graf @ 2014-02-03 23:28 UTC (permalink / raw) To: Mark Cave-Ayland Cc: Stefano Stabellini, Nitin Srivastava, Michael Tokarev, qemu-devel, qemu-ppc@nongnu.org, anthony.perard, pbonzini On 03.02.2014, at 23:58, Alexander Graf <agraf@suse.de> wrote: > > On 03.02.2014, at 22:13, Alexander Graf <agraf@suse.de> wrote: > >> >> On 03.02.2014, at 21:44, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: >> >>> On 03/02/14 20:02, Nitin Srivastava wrote: >>> >>>> Hi , >>>> I downloaded the latest qemu source from git and compiled it on my >>>> centos 6.5 machine. >>>> According to the following e-mail from this mailing list I tried the >>>> following >>>> _http://comments.gmane.org/gmane.comp.emulators.qemu/181171_ >>>> Test case: download the squeeze standard image from >>>> _http://people.debian.org/~aurel32/qemu/powerpc/_ >>>> <http://people.debian.org/%7Eaurel32/qemu/powerpc/> >>>> and run with >>>> qemu-system-ppc -hda debian_squeeze_powerpc_standard.qcow2 >>>> but I get following error >>>> VNC server running on `::1:5900' >>>>>> ============================================================= >>>>>> OpenBIOS 1.1 [Oct 2 2013 22:57] >>>>>> Configuration device id QEMU version 1 machine id 2 >>>>>> CPUs: 1 >>>>>> Memory: 128M >>>>>> UUID: 00000000-0000-0000-0000-000000000000 >>>>>> CPU type PowerPC,750 >>>>>> Not a bootable ELF image >>>> qemu: terminating on signal 2 >>>> nitins@nhost02%:~:117# >>>> also please note that my qemu-system-ppc is latest, as its built from >>>> source. >>>> nitins@nhost02%:~:117#qemu-system-ppc -version >>>> QEMU emulator version 1.7.50, Copyright (c) 2003-2008 Fabrice Bellard >>>> nitins@nhost02%:~:118# >>>> Please help. >>>> Regds. >>>> Nitin >>> >>> Hi Nitin, >>> >>> Having just updated to git master, I now see this issue too. A quick session with git bisect shows the culprit is this commit: >>> >>> >>> build@kentang:~/src/qemu/git/qemu$ git bisect bad >>> 360e607b88a23d378f6efaa769c76d26f538234d is the first bad commit >>> commit 360e607b88a23d378f6efaa769c76d26f538234d >>> Author: Stefano Stabellini <Stefano.Stabellini@eu.citrix.com> >>> Date: Thu Jan 30 12:46:05 2014 +0000 >>> >>> address_space_translate: do not cross page boundaries >>> >>> The following commit: >>> >>> commit 149f54b53b7666a3facd45e86eece60ce7d3b114 >>> Author: Paolo Bonzini <pbonzini@redhat.com> >>> Date: Fri May 24 12:59:37 2013 +0200 >>> >>> memory: add address_space_translate >>> >>> breaks Xen support in QEMU, in particular the Xen mapcache. The effect >>> is that one Windows XP installation out of ten would end up with BSOD. >>> >>> The reason is that after this commit l in address_space_rw can span a >>> page boundary, however qemu_get_ram_ptr still calls xen_map_cache asking >>> to map a single page (if block->offset == 0). >>> >>> Fix the issue by reverting to the previous behaviour: do not return a >>> length from address_space_translate_internal that can span a page >>> boundary. >>> >>> Also in address_space_translate do not ignore the length returned by >>> address_space_translate_internal. >>> >>> This patch should be backported to QEMU 1.6.x. >>> >>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> >>> Signed-off-by: Anthony Perard <anthony.perard@citrix.com> >>> Tested-by: Paolo Bonzini <pbonzini@redhat.com> >>> Acked-by: Paolo Bonzini <pbonzini@redhat.com> >>> Cc: qemu-stable@nongnu.org >>> >>> >>> Stefano/Alex, is there any reason why this would break qemu-system-ppc? >> >> Ugh, sorry Nitin, I should have read the email to the end. >> >> The image does work for me with -nographic, so I'd assume it's something about the frame buffer map going wrong? We do successfully run the guest: >> >> agraf@boysenberry-1:/home/agraf/release/qemu> ./ppc-softmmu/qemu-system-ppc -vnc :8 -snapshot -hda /dev/shm/debian_squeeze_powerpc_standard.qcow2 -serial mon:stdio >> >>>> ============================================================= >>>> OpenBIOS 1.1 [Oct 2 2013 22:57] >>>> Configuration device id QEMU version 1 machine id 2 >>>> CPUs: 1 >>>> Memory: 128M >>>> UUID: 00000000-0000-0000-0000-000000000000 >>>> CPU type PowerPC,750 >>>> Not a bootable ELF image >> QEMU 1.7.50 monitor - type 'help' for more information >> (qemu) x /i $pc >> 0xfff0c2e0: b 0xfff0c2ec >> (qemu) x /i $pc >> 0xfff25dd4: lwz r0,4(r11) >> (qemu) x /i $pc >> 0xfff1552c: lhz r9,0(r4) >> (qemu) x /i $pc >> 0xfff0a868: lwz r0,20(r1) >> (qemu) x /i $pc >> 0xfff25d3c: stw r31,-4(r11) >> (qemu) x /i $pc >> 0xfff0aeb8: mr r31,r3 >> (qemu) x /i $pc >> 0xfff0b050: mr r31,r3 >> (qemu) x /i $pc >> 0xfff0a6cc: lis r9,-5 >> (qemu) x /i $pc >> 0xc0252a5c: mr r3,r31 >> (qemu) x /i $pc >> 0xc003e274: lwz r9,68(r30) >> >> so it's really only the VGA output that's broken. > > A simple git revert of Stefanos patch makes VGA work again. The diff of "info mtree" of a mac99 system with and without his patch is the following: > > --- x 2014-02-03 23:57:20.000000000 +0100 > +++ y 2014-02-03 23:56:14.000000000 +0100 > @@ -30,6 +30,7 @@ > aliases > pci-mmio > 0000000000000000-00000000ffffffff (prio 0, RW): pci-mmio > + 00000000000a0000-00000000000affff (prio 2, RW): alias vga.chain4 @vga.vram 0000000000000000-000000000000ffff > 00000000000a0000-00000000000bffff (prio 1, RW): vga-lowmem > 0000000080000000-0000000080ffffff (prio 1, RW): vga.vram > 0000000081000000-0000000081000fff (prio 1, RW): vga.mmio > @@ -77,6 +78,8 @@ > 00000000000003d4-00000000000003d5 (prio 0, RW): vga > 00000000000003da-00000000000003da (prio 0, RW): vga > 0000000000000400-00000000000004ff (prio 1, RW): ne2000 > +vga.vram > +0000000080000000-0000000080ffffff (prio 1, RW): vga.vram > escc-bar > 0000000000013000-000000000001303f (prio 0, RW): alias escc-bar @escc 0000000000000000-000000000000003f > escc > > I'm still not quite sure what did cause the breakage. Consider me still confused. The following patch makes it work again: diff --git a/exec.c b/exec.c index 9ad0a4b..dcc0fc1 100644 --- a/exec.c +++ b/exec.c @@ -351,7 +351,7 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, hwaddr len = *plen; for (;;) { - section = address_space_translate_internal(as->dispatch, addr, &addr, &len, true); + section = address_space_translate_internal(as->dispatch, addr, &addr, plen, true); mr = section->mr; if (!mr->iommu_ops) { The problem seems to be that But we do set *plen after the loop, so why would this change possibly change anything? This patch for example does _not_ make it work: diff --git a/exec.c b/exec.c index 9ad0a4b..a37c5f4 100644 --- a/exec.c +++ b/exec.c @@ -352,6 +352,7 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, for (;;) { section = address_space_translate_internal(as->dispatch, addr, &addr, &len, true); + *plen = len; mr = section->mr; if (!mr->iommu_ops) { That means that something relies on the incorrect behavior we did before to set *plen (end) = *plen (start) after the loop when !mr->iommu_ops. I wonder what that could be... Alex ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] standard test image not booting with qemu-system-ppc 2014-02-03 23:28 ` Alexander Graf @ 2014-02-04 0:44 ` Peter Maydell 2014-02-04 7:55 ` Alexander Graf 0 siblings, 1 reply; 8+ messages in thread From: Peter Maydell @ 2014-02-04 0:44 UTC (permalink / raw) To: Alexander Graf Cc: Nitin Srivastava, Mark Cave-Ayland, Stefano Stabellini, Michael Tokarev, qemu-devel, qemu-ppc@nongnu.org, Anthony PERARD, Paolo Bonzini On 3 February 2014 23:28, Alexander Graf <agraf@suse.de> wrote: > That means that something relies on the incorrect behavior we did > before to set *plen (end) = *plen (start) after the loop when > !mr->iommu_ops. I wonder what that could be... I bounced around in a debugger for a bit looking for cases where the *plen got updated. One of them is cpu_physical_memory_write_rom_internal(): we now rather less efficiently do memcpy of 4K at a time into the guest RAM rather than blasting the entire ROM image, but the code still works. This backtrace, on the other hand, looks rather more suspect: #0 portio_write (opaque=0x101686160, addr=0, data=4, size=1) at /Users/pm215/src/qemu/ioport.c:203 #1 0x000000010032f143 in memory_region_write_accessor (mr=0x101686160, addr=0, value=0x105fab508, size=1, shift=0, mask=255) at /Users/pm215/src/qemu/memory.c:441 #2 0x000000010032f062 in access_with_adjusted_size (addr=0, value=0x105fab508, size=1, access_size_min=1, access_size_max=4, access=0x10032f0a0 <memory_region_write_accessor>, mr=0x101686160) at /Users/pm215/src/qemu/memory.c:478 #3 0x000000010032e16f in memory_region_dispatch_write (mr=0x101686160, addr=0, data=4, size=1) at /Users/pm215/src/qemu/memory.c:985 #4 0x000000010032e079 in io_mem_write (mr=0x101686160, addr=0, val=4, size=1) at /Users/pm215/src/qemu/memory.c:1744 #5 0x00000001002c47c1 in address_space_rw (as=0x10067fd80, addr=4261413326, buf=0x105fab66c "\004", len=2, is_write=true) at /Users/pm215/src/qemu/exec.c:2011 #6 0x00000001002c511f in address_space_write (as=0x10067fd80, addr=4261413326, buf=0x105fab66c "\004", len=2) at /Users/pm215/src/qemu/exec.c:2068 #7 0x00000001002c3b13 in subpage_write (opaque=0x103058200, addr=462, value=1024, len=2) at /Users/pm215/src/qemu/exec.c:1662 #8 0x000000010032f143 in memory_region_write_accessor (mr=0x103058200, addr=462, value=0x105fab798, size=2, shift=0, mask=65535) at /Users/pm215/src/qemu/memory.c:441 #9 0x000000010032f00a in access_with_adjusted_size (addr=462, value=0x105fab798, size=2, access_size_min=1, access_size_max=4, access=0x10032f0a0 <memory_region_write_accessor>, mr=0x103058200) at /Users/pm215/src/qemu/memory.c:473 #10 0x000000010032e16f in memory_region_dispatch_write (mr=0x103058200, addr=462, data=1024, size=2) at /Users/pm215/src/qemu/memory.c:985 #11 0x000000010032e079 in io_mem_write (mr=0x103058200, addr=462, val=1024, size=2) at /Users/pm215/src/qemu/memory.c:1744 #12 0x000000010037da52 in io_writew (env=0x1019ae338, physaddr=462, val=1024, addr=4261413326, retaddr=4430414751) at softmmu_template.h:336 #13 0x000000010037dc37 in helper_be_stw_mmu (env=0x1019ae338, addr=4261413326, val=1024, mmu_idx=1, retaddr=4430414751) at softmmu_template.h:448 The address_space access/write functions from subpage_write have decided that len==2 isn't valid (ie we called address_space_translate with plen pointing at a value of 2, address_space_translate_internal updated that to 1 (prior to this commit it would have left it alone) and we are now going to do what started off as a 16 bit access as two single byte writes. I have a feeling this is also bypassing the endianness swapping that would otherwise happen for this 16 bit write, as well. This happens because the IO port we're trying to access: { 2, 1, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data }, is declared as having a length of 1 and a size of 2. That would be nonsensical for MMIO but is apparently entirely fine for IO ports (go x86 madness). With this change, the memory system is now refusing to allow an access of size 2 through, because it's greater than the region length. So it splits it into two 1 byte accesses, and find_portio() then says "no, no such port" because find_portio() insists on an exact match between size and mrp->size. I'll leave you all to figure out exactly how this is supposed to work; I'm going to bed :-) thanks -- PMM ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] standard test image not booting with qemu-system-ppc 2014-02-04 0:44 ` Peter Maydell @ 2014-02-04 7:55 ` Alexander Graf 2014-02-04 13:17 ` Paolo Bonzini 0 siblings, 1 reply; 8+ messages in thread From: Alexander Graf @ 2014-02-04 7:55 UTC (permalink / raw) To: Peter Maydell Cc: Nitin Srivastava, Mark Cave-Ayland, Stefano Stabellini, Michael Tokarev, qemu-devel, qemu-ppc@nongnu.org, Anthony PERARD, Paolo Bonzini On 04.02.2014, at 01:44, Peter Maydell <peter.maydell@linaro.org> wrote: > On 3 February 2014 23:28, Alexander Graf <agraf@suse.de> wrote: >> That means that something relies on the incorrect behavior we did >> before to set *plen (end) = *plen (start) after the loop when >> !mr->iommu_ops. I wonder what that could be... > > I bounced around in a debugger for a bit looking for cases where > the *plen got updated. One of them is > cpu_physical_memory_write_rom_internal(): we now rather less > efficiently do memcpy of 4K at a time into the guest RAM rather > than blasting the entire ROM image, but the code still works. I suppose the same happens on DMA access, rendering it less efficient, no? > > This backtrace, on the other hand, looks rather more suspect: > > #0 portio_write (opaque=0x101686160, addr=0, data=4, size=1) at > /Users/pm215/src/qemu/ioport.c:203 > #1 0x000000010032f143 in memory_region_write_accessor > (mr=0x101686160, addr=0, value=0x105fab508, size=1, shift=0, mask=255) > at /Users/pm215/src/qemu/memory.c:441 > #2 0x000000010032f062 in access_with_adjusted_size (addr=0, > value=0x105fab508, size=1, access_size_min=1, access_size_max=4, > access=0x10032f0a0 <memory_region_write_accessor>, mr=0x101686160) at > /Users/pm215/src/qemu/memory.c:478 > #3 0x000000010032e16f in memory_region_dispatch_write > (mr=0x101686160, addr=0, data=4, size=1) at > /Users/pm215/src/qemu/memory.c:985 > #4 0x000000010032e079 in io_mem_write (mr=0x101686160, addr=0, val=4, > size=1) at /Users/pm215/src/qemu/memory.c:1744 > #5 0x00000001002c47c1 in address_space_rw (as=0x10067fd80, > addr=4261413326, buf=0x105fab66c "\004", len=2, is_write=true) at > /Users/pm215/src/qemu/exec.c:2011 > #6 0x00000001002c511f in address_space_write (as=0x10067fd80, > addr=4261413326, buf=0x105fab66c "\004", len=2) at > /Users/pm215/src/qemu/exec.c:2068 > #7 0x00000001002c3b13 in subpage_write (opaque=0x103058200, addr=462, > value=1024, len=2) at /Users/pm215/src/qemu/exec.c:1662 > #8 0x000000010032f143 in memory_region_write_accessor > (mr=0x103058200, addr=462, value=0x105fab798, size=2, shift=0, > mask=65535) at /Users/pm215/src/qemu/memory.c:441 > #9 0x000000010032f00a in access_with_adjusted_size (addr=462, > value=0x105fab798, size=2, access_size_min=1, access_size_max=4, > access=0x10032f0a0 <memory_region_write_accessor>, mr=0x103058200) at > /Users/pm215/src/qemu/memory.c:473 > #10 0x000000010032e16f in memory_region_dispatch_write > (mr=0x103058200, addr=462, data=1024, size=2) at > /Users/pm215/src/qemu/memory.c:985 > #11 0x000000010032e079 in io_mem_write (mr=0x103058200, addr=462, > val=1024, size=2) at /Users/pm215/src/qemu/memory.c:1744 > #12 0x000000010037da52 in io_writew (env=0x1019ae338, physaddr=462, > val=1024, addr=4261413326, retaddr=4430414751) at > softmmu_template.h:336 > #13 0x000000010037dc37 in helper_be_stw_mmu (env=0x1019ae338, > addr=4261413326, val=1024, mmu_idx=1, retaddr=4430414751) at > softmmu_template.h:448 > > The address_space access/write functions from > subpage_write have decided that len==2 isn't valid > (ie we called address_space_translate with plen pointing > at a value of 2, address_space_translate_internal updated > that to 1 (prior to this commit it would have left it alone) and > we are now going to do what started off as a 16 bit access > as two single byte writes. I have a feeling this is also > bypassing the endianness swapping that would otherwise > happen for this 16 bit write, as well. > > This happens because the IO port we're trying to access: > { 2, 1, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data }, > > is declared as having a length of 1 and a size of 2. That > would be nonsensical for MMIO but is apparently entirely > fine for IO ports (go x86 madness). With this change, the > memory system is now refusing to allow an access of size > 2 through, because it's greater than the region length. So Ouch. Yes, for ioport reads/writes we definitely have to only cap the port range, not the length. Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] standard test image not booting with qemu-system-ppc 2014-02-04 7:55 ` Alexander Graf @ 2014-02-04 13:17 ` Paolo Bonzini 2014-02-04 14:37 ` Stefano Stabellini 2014-02-04 18:18 ` Anthony PERARD 0 siblings, 2 replies; 8+ messages in thread From: Paolo Bonzini @ 2014-02-04 13:17 UTC (permalink / raw) To: Alexander Graf, Peter Maydell Cc: Nitin Srivastava, Mark Cave-Ayland, Stefano Stabellini, Michael Tokarev, qemu-devel, qemu-ppc@nongnu.org, Anthony PERARD Il 04/02/2014 08:55, Alexander Graf ha scritto: >> With this change, the >> memory system is now refusing to allow an access of size >> 2 through, because it's greater than the region length. So > > Ouch. Yes, for ioport reads/writes we definitely have to only cap the port range, not the length. We can do it in general for MMIO. Something like this? diff --git a/exec.c b/exec.c index 9ad0a4b..9a1eef3 100644 --- a/exec.c +++ b/exec.c @@ -325,7 +325,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x hwaddr *plen, bool resolve_subpage) { MemoryRegionSection *section; - Int128 diff, diff_page; + Int128 diff; section = address_space_lookup_region(d, addr, resolve_subpage); /* Compute offset within MemoryRegionSection */ @@ -334,9 +334,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x /* Compute offset within MemoryRegion */ *xlat = addr + section->offset_within_region; - diff_page = int128_make64(((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr); diff = int128_sub(section->mr->size, int128_make64(addr)); - diff = int128_min(diff, diff_page); *plen = int128_get64(int128_min(diff, int128_make64(*plen))); return section; } @@ -370,6 +368,11 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, as = iotlb.target_as; } + if (memory_access_is_direct(mr, is_write)) { + hwaddr page = (addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE - addr; + len = MIN(page, len); + } + *plen = len; *xlat = addr; return mr; Stefano, Anthony, can you test it on Xen? I wouldn't mind sticking a "xen_enabled()" in there, and/or a comment to document why we do it. Paolo ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] standard test image not booting with qemu-system-ppc 2014-02-04 13:17 ` Paolo Bonzini @ 2014-02-04 14:37 ` Stefano Stabellini 2014-02-04 18:18 ` Anthony PERARD 1 sibling, 0 replies; 8+ messages in thread From: Stefano Stabellini @ 2014-02-04 14:37 UTC (permalink / raw) To: Paolo Bonzini Cc: Peter Maydell, Nitin Srivastava, Mark Cave-Ayland, Stefano Stabellini, Michael Tokarev, Alexander Graf, qemu-devel, qemu-ppc@nongnu.org, Anthony PERARD On Tue, 4 Feb 2014, Paolo Bonzini wrote: > Il 04/02/2014 08:55, Alexander Graf ha scritto: > >> With this change, the > >> memory system is now refusing to allow an access of size > >> 2 through, because it's greater than the region length. So > > > > Ouch. Yes, for ioport reads/writes we definitely have to only cap the port range, not the length. > > We can do it in general for MMIO. Something like this? > > diff --git a/exec.c b/exec.c > index 9ad0a4b..9a1eef3 100644 > --- a/exec.c > +++ b/exec.c > @@ -325,7 +325,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x > hwaddr *plen, bool resolve_subpage) > { > MemoryRegionSection *section; > - Int128 diff, diff_page; > + Int128 diff; > > section = address_space_lookup_region(d, addr, resolve_subpage); > /* Compute offset within MemoryRegionSection */ > @@ -334,9 +334,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x > /* Compute offset within MemoryRegion */ > *xlat = addr + section->offset_within_region; > > - diff_page = int128_make64(((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr); > diff = int128_sub(section->mr->size, int128_make64(addr)); > - diff = int128_min(diff, diff_page); > *plen = int128_get64(int128_min(diff, int128_make64(*plen))); > return section; > } > @@ -370,6 +368,11 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, > as = iotlb.target_as; > } > > + if (memory_access_is_direct(mr, is_write)) { > + hwaddr page = (addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE - addr; > + len = MIN(page, len); > + } > + > *plen = len; > *xlat = addr; > return mr; > > > Stefano, Anthony, can you test it on Xen? > > I wouldn't mind sticking a "xen_enabled()" in there, and/or a comment to document > why we do it. The patch looks OK as it is, let's see how Anthony's tests turn out. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] standard test image not booting with qemu-system-ppc 2014-02-04 13:17 ` Paolo Bonzini 2014-02-04 14:37 ` Stefano Stabellini @ 2014-02-04 18:18 ` Anthony PERARD 1 sibling, 0 replies; 8+ messages in thread From: Anthony PERARD @ 2014-02-04 18:18 UTC (permalink / raw) To: Paolo Bonzini Cc: Peter Maydell, Nitin Srivastava, Mark Cave-Ayland, Stefano Stabellini, Michael Tokarev, Alexander Graf, qemu-devel, qemu-ppc@nongnu.org On Tue, Feb 04, 2014 at 02:17:55PM +0100, Paolo Bonzini wrote: > Il 04/02/2014 08:55, Alexander Graf ha scritto: > >> With this change, the > >> memory system is now refusing to allow an access of size > >> 2 through, because it's greater than the region length. So > > > > Ouch. Yes, for ioport reads/writes we definitely have to only cap the port range, not the length. > > We can do it in general for MMIO. Something like this? > > diff --git a/exec.c b/exec.c > index 9ad0a4b..9a1eef3 100644 > --- a/exec.c > +++ b/exec.c > @@ -325,7 +325,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x > hwaddr *plen, bool resolve_subpage) > { > MemoryRegionSection *section; > - Int128 diff, diff_page; > + Int128 diff; > > section = address_space_lookup_region(d, addr, resolve_subpage); > /* Compute offset within MemoryRegionSection */ > @@ -334,9 +334,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x > /* Compute offset within MemoryRegion */ > *xlat = addr + section->offset_within_region; > > - diff_page = int128_make64(((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr); > diff = int128_sub(section->mr->size, int128_make64(addr)); > - diff = int128_min(diff, diff_page); > *plen = int128_get64(int128_min(diff, int128_make64(*plen))); > return section; > } > @@ -370,6 +368,11 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, > as = iotlb.target_as; > } > > + if (memory_access_is_direct(mr, is_write)) { > + hwaddr page = (addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE - addr; > + len = MIN(page, len); > + } > + > *plen = len; > *xlat = addr; > return mr; > > > Stefano, Anthony, can you test it on Xen? This patches works fine (after adding a prototype for memory_access_is_direct before the function). -- Anthony PERARD ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-02-04 18:18 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <246b6975027245a0bc428eb33808390d@CO1PR05MB490.namprd05.prod.outlook.com>
[not found] ` <52EFFFC1.7040303@ilande.co.uk>
2014-02-03 21:13 ` [Qemu-devel] [Qemu-ppc] standard test image not booting with qemu-system-ppc Alexander Graf
2014-02-03 22:58 ` Alexander Graf
2014-02-03 23:28 ` Alexander Graf
2014-02-04 0:44 ` Peter Maydell
2014-02-04 7:55 ` Alexander Graf
2014-02-04 13:17 ` Paolo Bonzini
2014-02-04 14:37 ` Stefano Stabellini
2014-02-04 18:18 ` Anthony PERARD
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).