* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.