Patchwork Re: [PATCH 00/23] VGA cleanup

login
register
mail settings
Submitter Juan Quintela
Date Sept. 16, 2009, 1:45 p.m.
Message ID <m3tyz3m29a.fsf@neno.mitica>
Download mbox | patch
Permalink /patch/33711/
State Superseded
Headers show

Comments

Juan Quintela - Sept. 16, 2009, 1:45 p.m.
Pierre Riteau <Pierre.Riteau@irisa.fr> wrote:
>>> This patchset introduced a bug. I noticed today that I can't boot an
>>> installation disk of ubuntu-9.04-server-i386.
>>> After entering the install program ("Install Ubuntu Server"), I just
>>> get a black background with gray vertical lines.
>>>
>>> More specifically, from oldest to newest commit:
>>> 86948bb104f419db9af6b621b85703e8f0d3234c works
>>> f705db9df04c6491f242a5a4585dfe72b708f197 throws a warning about an
>>> uninitialized variable (had to disable werror) but works
>>
>> could you post the error message?  I didn't have that problem.
>
> /mnt/qemu/hw/cirrus_vga.c: In function 'cirrus_vga_ioport_read':
> /mnt/qemu/hw/cirrus_vga.c:2656: warning: 'val' may be used
> uninitialized in this function

This is fixed on a later, patch, sorry :(

> I'm using ./configure --target-list=i386-softmmu --disable-werror, gcc
> is version 4.3.2 (from Debian Lenny).
>
>>
>>> 22286bc6468adac10b2eb7e603f1a8ba524bfb03 throws a warning about an
>>> uninitialized variable (had to disable werror) and doesn't work
>>> b863d51490b7c6e339c9565eda786cadc1218d48 compiles without warning and
>>> doesn't work
>>
>> Weird, it did worked for me.  Could you post what command line did you
>> use?  I tested with fedora + win XP.
>
> qemu -cdrom ~/ubuntu-9.04-server-i386.iso -boot d
>
>>
>>> So 22286bc6468adac10b2eb7e603f1a8ba524bfb03 must be the problematic
>>> commit.
>>
>> Thanks, will take a look.

could you check this patch?
(still not sure why it worked for fedora + xp for me)

From 97869744f37ccd2846e3edcc1ad9f582d9bea1c6 Mon Sep 17 00:00:00 2001
From: Juan Quintela <quintela@redhat.com>
Date: Wed, 16 Sep 2009 15:44:49 +0200
Subject: [PATCH] cirrus_vga: also assign gr0/1 when writting shadow_gr0/1


Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/cirrus_vga.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Pierre Riteau - Sept. 16, 2009, 2:21 p.m.
On 16 sept. 2009, at 15:45, Juan Quintela wrote:

> Pierre Riteau <Pierre.Riteau@irisa.fr> wrote:
>>>> This patchset introduced a bug. I noticed today that I can't boot  
>>>> an
>>>> installation disk of ubuntu-9.04-server-i386.
>>>> After entering the install program ("Install Ubuntu Server"), I  
>>>> just
>>>> get a black background with gray vertical lines.
>>>>
>>>> More specifically, from oldest to newest commit:
>>>> 86948bb104f419db9af6b621b85703e8f0d3234c works
>>>> f705db9df04c6491f242a5a4585dfe72b708f197 throws a warning about an
>>>> uninitialized variable (had to disable werror) but works
>>>
>>> could you post the error message?  I didn't have that problem.
>>
>> /mnt/qemu/hw/cirrus_vga.c: In function 'cirrus_vga_ioport_read':
>> /mnt/qemu/hw/cirrus_vga.c:2656: warning: 'val' may be used
>> uninitialized in this function
>
> This is fixed on a later, patch, sorry :(
>
>> I'm using ./configure --target-list=i386-softmmu --disable-werror,  
>> gcc
>> is version 4.3.2 (from Debian Lenny).
>>
>>>
>>>> 22286bc6468adac10b2eb7e603f1a8ba524bfb03 throws a warning about an
>>>> uninitialized variable (had to disable werror) and doesn't work
>>>> b863d51490b7c6e339c9565eda786cadc1218d48 compiles without warning  
>>>> and
>>>> doesn't work
>>>
>>> Weird, it did worked for me.  Could you post what command line did  
>>> you
>>> use?  I tested with fedora + win XP.
>>
>> qemu -cdrom ~/ubuntu-9.04-server-i386.iso -boot d
>>
>>>
>>>> So 22286bc6468adac10b2eb7e603f1a8ba524bfb03 must be the problematic
>>>> commit.
>>>
>>> Thanks, will take a look.
>
> could you check this patch?
> (still not sure why it worked for fedora + xp for me)
>
> From 97869744f37ccd2846e3edcc1ad9f582d9bea1c6 Mon Sep 17 00:00:00 2001
> From: Juan Quintela <quintela@redhat.com>
> Date: Wed, 16 Sep 2009 15:44:49 +0200
> Subject: [PATCH] cirrus_vga: also assign gr0/1 when writting  
> shadow_gr0/1
>
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> hw/cirrus_vga.c |    2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
> index 004ae7d..9dfe76a 100644
> --- a/hw/cirrus_vga.c
> +++ b/hw/cirrus_vga.c
> @@ -1490,9 +1490,11 @@ cirrus_vga_write_gr(CirrusVGAState * s,  
> unsigned reg_index, int reg_value)
> #endif
>     switch (reg_index) {
>     case 0x00:			// Standard VGA, BGCOLOR 0x000000ff
> +	s->vga.gr[reg_index] = reg_value & gr_mask[reg_index];
> 	s->cirrus_shadow_gr0 = reg_value;
> 	break;
>     case 0x01:			// Standard VGA, FGCOLOR 0x000000ff
> +	s->vga.gr[reg_index] = reg_value & gr_mask[reg_index];
> 	s->cirrus_shadow_gr1 = reg_value;
> 	break;
>     case 0x02:			// Standard VGA
> -- 
> 1.6.2.5
>
>
>


It works now! Thanks.

Patch

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 004ae7d..9dfe76a 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -1490,9 +1490,11 @@  cirrus_vga_write_gr(CirrusVGAState * s, unsigned reg_index, int reg_value)
 #endif
     switch (reg_index) {
     case 0x00:			// Standard VGA, BGCOLOR 0x000000ff
+	s->vga.gr[reg_index] = reg_value & gr_mask[reg_index];
 	s->cirrus_shadow_gr0 = reg_value;
 	break;
     case 0x01:			// Standard VGA, FGCOLOR 0x000000ff
+	s->vga.gr[reg_index] = reg_value & gr_mask[reg_index];
 	s->cirrus_shadow_gr1 = reg_value;
 	break;
     case 0x02:			// Standard VGA