Re: [Qemu-devel] [memory] abort with head a8170e5

[Avi Kivity <avi@redhat.com>]

On 10/29/2012 09:54 AM, Aurelien Jarno wrote:
> On Thu, Oct 25, 2012 at 06:12:06PM +0200, Avi Kivity wrote:
>> On 10/25/2012 04:39 PM, Aurelien Jarno wrote:
>> > On Thu, Oct 25, 2012 at 03:47:34PM +0200, Avi Kivity wrote:
>> >> On 10/24/2012 04:00 PM, Aurelien Jarno wrote:
>> >> > 
>> >> > mips is also broken but by commit 1c380f9460522f32c8dd2577b2a53d518ec91c6d:
>> >> > 
>> >> > | [    0.436000] PCI: Enabling device 0000:00:0a.1 (0000 -> 0001)
>> >> > | Segmentation fault (core dumped)
>> >> > 
>> >> 
>> >> How do you reproduce it?
>> > 
>> > You can use the mips kernel version 2.6.32 from:
>> >   http://people.debian.org/~aurel32/qemu/mips/
>> > 
>> > Then just run it with the following command:
>> >   qemu-system-mips -M malta -kernel vmlinux-2.6.32-5-4kc-malta -append "console=tty0"
>> > 
>> > (You can also get the README command line if you don't care about
>> > downloading the disk image).
>> 
>> Doesn't reproduce here with this command line (upstream + the bridge patch).
>> 
>> [    0.568000] PCI: Enabling device 0000:00:12.0 (0000 -> 0002)
>> [    0.572000] cirrusfb 0000:00:12.0: Cirrus Logic chipset on PCI bus,
>> RAM (4096 kB) at 0x10000000
>> 
>> ...
>> 
>> [    1.172000] PCI: Enabling device 0000:00:0a.1 (0000 -> 0001)
>> [    1.188000] scsi0 : ata_piix
>> 
>> (with console=ttyS0)
> 
> Ok, looks like I didn't provide the right command line. I am only able
> to reproduce it when using -nographic, and only with -vga cirrus (yes it
> starts to be quite strange). In that case it's better to pass 
> console=ttyS0, even if you can reproduce it with console=tty0.
> 
> In short it seems heavily related to the cirrus VGA card.

I was able to reproduce it, and in fact it's unrelated to VGA, it's deep in the memory core.

Please try this patch:

From: Avi Kivity <avi@redhat.com>
Date: Mon, 29 Oct 2012 17:07:09 +0200
Subject: [PATCH] memory: fix rendering of a region obscured by another

The memory core drops regions that are hidden by another region (for example,
during BAR sizing), but it doesn't do so correctly if the lower address of the
existing range is below the lower address of the new range.

Example (qemu-system-mips -M malta -kernel vmlinux-2.6.32-5-4kc-malta
         -append "console=ttyS0"  -nographic -vga cirrus):

Existing range: 10000000-107fffff
New range:      100a0000-100bffff

Correct behaviour: drop new range
Incorrect behaviour: add new range

Fix by taking this case into account (previously we only considered
equal lower boundaries).

Signed-off-by: Avi Kivity <avi@redhat.com>

diff --git a/memory.c b/memory.c
index 36bb9a5..243cb23 100644
--- a/memory.c
+++ b/memory.c
@@ -539,12 +539,12 @@ static void render_memory_region(FlatView *view,
             offset_in_region += int128_get64(now);
             int128_subfrom(&remain, now);
         }
-        if (int128_eq(base, view->ranges[i].addr.start)) {
-            now = int128_min(remain, view->ranges[i].addr.size);
-            int128_addto(&base, now);
-            offset_in_region += int128_get64(now);
-            int128_subfrom(&remain, now);
-        }
+        now = int128_sub(int128_min(int128_add(base, remain),
+                                    addrrange_end(view->ranges[i].addr)),
+                         base);
+        int128_addto(&base, now);
+        offset_in_region += int128_get64(now);
+        int128_subfrom(&remain, now);
     }
     if (int128_nz(remain)) {
         fr.mr = mr;



-- 
error compiling committee.c: too many arguments to function