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

[Aurelien Jarno]

On Mon, Oct 29, 2012 at 05:17:05PM +0200, Avi Kivity wrote:
> 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 <address@hidden>
> 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 <address@hidden>
> 
> 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;
> 

Thanks a lot for the patch, it fixes the problem and I have been able to
boot a MIPS guest up to the login prompt.

Tested-by: Aurelien Jarno <address@hidden>

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net