[PATCH] x86, setup (2.6.30-stable) fix 80x34 and 80x60 console modes

[PATCH] x86, setup (2.6.30-stable) fix 80x34 and 80x60 console modes

[Marc Aurele La France]

As coded, most INT10 calls in video-vga.c allow the compiler to assume EAX 
remains unchanged across them, which is not always the case.  This 
triggers an optimisation issue that causes vga_set_vertical_end() to be 
called with an incorrect number of scanlines.  Fix this by beefing up the 
asm constraints on these calls.

Reported-by: Marc Aurele La France <tsi@xfree86.org>
Signed-off-by: Marc Aurele La France <tsi@xfree86.org>
Acked-by: H. Peter Anvin <hpa@zytor.com>

diff -adNpru linux-2.6.30/arch/x86/boot/video-vga.c devel-2.6.30/arch/x86/boot/video-vga.c
--- linux-2.6.30/arch/x86/boot/video-vga.c	2009-06-10 12:33:53.000000000 -0600
+++ devel-2.6.30/arch/x86/boot/video-vga.c	2009-06-25 13:35:16.000000000 -0600
@@ -45,8 +45,10 @@ static u8 vga_set_basic_mode(void)

 #ifdef CONFIG_VIDEO_400_HACK
 	if (adapter >= ADAPTER_VGA) {
+		ax = 0x1202;
 		asm volatile(INT10
-			     : : "a" (0x1202), "b" (0x0030)
+			     : "+a" (ax)
+			     : "b" (0x0030)
 			     : "ecx", "edx", "esi", "edi");
 	}
 #endif
@@ -81,44 +83,59 @@ static u8 vga_set_basic_mode(void)

 static void vga_set_8font(void)
 {
+	u16 ax;
+
 	/* Set 8x8 font - 80x43 on EGA, 80x50 on VGA */

 	/* Set 8x8 font */
-	asm volatile(INT10 : : "a" (0x1112), "b" (0));
+	ax = 0x1112;
+	asm volatile(INT10 : "+a" (ax) : "b" (0));

 	/* Use alternate print screen */
-	asm volatile(INT10 : : "a" (0x1200), "b" (0x20));
+	ax = 0x1200;
+	asm volatile(INT10 : "+a" (ax) : "b" (0x20));

 	/* Turn off cursor emulation */
-	asm volatile(INT10 : : "a" (0x1201), "b" (0x34));
+	ax = 0x1201;
+	asm volatile(INT10 : "+a" (ax) : "b" (0x34));

 	/* Cursor is scan lines 6-7 */
-	asm volatile(INT10 : : "a" (0x0100), "c" (0x0607));
+	ax = 0x0100;
+	asm volatile(INT10 : "+a" (ax) : "c" (0x0607));
 }

 static void vga_set_14font(void)
 {
+	u16 ax;
+
 	/* Set 9x14 font - 80x28 on VGA */

 	/* Set 9x14 font */
-	asm volatile(INT10 : : "a" (0x1111), "b" (0));
+	ax = 0x1111;
+	asm volatile(INT10 : "+a" (ax) : "b" (0));

 	/* Turn off cursor emulation */
-	asm volatile(INT10 : : "a" (0x1201), "b" (0x34));
+	ax = 0x1201;
+	asm volatile(INT10 : "+a" (ax) : "b" (0x34));

 	/* Cursor is scan lines 11-12 */
-	asm volatile(INT10 : : "a" (0x0100), "c" (0x0b0c));
+	ax = 0x0100;
+	asm volatile(INT10 : "+a" (ax) : "c" (0x0b0c));
 }

 static void vga_set_80x43(void)
 {
+	u16 ax;
+
 	/* Set 80x43 mode on VGA (not EGA) */

 	/* Set 350 scans */
-	asm volatile(INT10 : : "a" (0x1201), "b" (0x30));
+	ax = 0x1201;
+	asm volatile(INT10 : "+a" (ax) : "b" (0x30));

 	/* Reset video mode */
-	asm volatile(INT10 : : "a" (0x0003));
+	ax = 0x0003;
+	asm volatile(INT10 : "+a" (ax));

 	vga_set_8font();
 }
@@ -225,7 +242,7 @@ static int vga_set_mode(struct mode_info
  */
 static int vga_probe(void)
 {
-	u16 ega_bx;
+	u16 ax, ega_bx;

 	static const char *card_name[] = {
 		"CGA/MDA/HGC", "EGA", "VGA"
@@ -242,9 +259,10 @@ static int vga_probe(void)
 	};
 	u8 vga_flag;

+	ax = 0x1200;
 	asm(INT10
-	    : "=b" (ega_bx)
-	    : "a" (0x1200), "b" (0x10) /* Check EGA/VGA */
+	    : "+a" (ax), "=b" (ega_bx)
+	    : "b" (0x10) /* Check EGA/VGA */
 	    : "ecx", "edx", "esi", "edi");

 #ifndef _WAKEUP

Marc.

+----------------------------------+----------------------------------+
|  Marc Aurele La France           |  work:   1-780-492-9310          |
|  Academic Information and        |  fax:    1-780-492-1729          |
|    Communications Technologies   |  email:  tsi@ualberta.ca         |
|  352 General Services Building   +----------------------------------+
|  University of Alberta           |                                  |
|  Edmonton, Alberta               |    Standard disclaimers apply    |
|  T6G 2H1                         |                                  |
|  CANADA                          |                                  |
+----------------------------------+----------------------------------+
XFree86 developer and VP.  ATI driver and X server internals.

Re: [PATCH] x86, setup (2.6.30-stable) fix 80x34 and 80x60 console modes

[H. Peter Anvin]

Marc Aurele La France wrote:
> As coded, most INT10 calls in video-vga.c allow the compiler to assume EAX 
> remains unchanged across them, which is not always the case.  This 
> triggers an optimisation issue that causes vga_set_vertical_end() to be 
> called with an incorrect number of scanlines.  Fix this by beefing up the 
> asm constraints on these calls.
> 
> Reported-by: Marc Aurele La France <tsi@xfree86.org>
> Signed-off-by: Marc Aurele La France <tsi@xfree86.org>
> Acked-by: H. Peter Anvin <hpa@zytor.com>

Note: this is not in upstream since upstream is not affected due to the
new "BIOS glovebox" subsystem.

	-hpa

Re: [stable] [PATCH] x86, setup (2.6.30-stable) fix 80x34 and 80x60 console modes

[Greg KH]

On Mon, Jun 29, 2009 at 06:27:40PM -0700, H. Peter Anvin wrote:
> Marc Aurele La France wrote:
> > As coded, most INT10 calls in video-vga.c allow the compiler to assume EAX 
> > remains unchanged across them, which is not always the case.  This 
> > triggers an optimisation issue that causes vga_set_vertical_end() to be 
> > called with an incorrect number of scanlines.  Fix this by beefing up the 
> > asm constraints on these calls.
> > 
> > Reported-by: Marc Aurele La France <tsi@xfree86.org>
> > Signed-off-by: Marc Aurele La France <tsi@xfree86.org>
> > Acked-by: H. Peter Anvin <hpa@zytor.com>
> 
> Note: this is not in upstream since upstream is not affected due to the
> new "BIOS glovebox" subsystem.

So it is a ".30 only" type patch?  Any older kernel versions affected?

thanks,

greg k-h

Re: [stable] [PATCH] x86, setup (2.6.30-stable) fix 80x34 and 80x60 console modes

[H. Peter Anvin]

Greg KH wrote:
> On Mon, Jun 29, 2009 at 06:27:40PM -0700, H. Peter Anvin wrote:
>> Marc Aurele La France wrote:
>>> As coded, most INT10 calls in video-vga.c allow the compiler to assume EAX 
>>> remains unchanged across them, which is not always the case.  This 
>>> triggers an optimisation issue that causes vga_set_vertical_end() to be 
>>> called with an incorrect number of scanlines.  Fix this by beefing up the 
>>> asm constraints on these calls.
>>>
>>> Reported-by: Marc Aurele La France <tsi@xfree86.org>
>>> Signed-off-by: Marc Aurele La France <tsi@xfree86.org>
>>> Acked-by: H. Peter Anvin <hpa@zytor.com>
>> Note: this is not in upstream since upstream is not affected due to the
>> new "BIOS glovebox" subsystem.
> 
> So it is a ".30 only" type patch?  Any older kernel versions affected?
> 

Yes, all the way back to .23 or something like that.

	-hpa

Re: [rs] Re: [stable] [PATCH] x86, setup (2.6.30-stable) fix 80x34 and 80x60 console modes

[Marc Aurele La France]

On Mon, 29 Jun 2009, H. Peter Anvin wrote:
> Greg KH wrote:
>> On Mon, Jun 29, 2009 at 06:27:40PM -0700, H. Peter Anvin wrote:
>>> Marc Aurele La France wrote:
>>>> As coded, most INT10 calls in video-vga.c allow the compiler to assume EAX
>>>> remains unchanged across them, which is not always the case.  This
>>>> triggers an optimisation issue that causes vga_set_vertical_end() to be
>>>> called with an incorrect number of scanlines.  Fix this by beefing up the
>>>> asm constraints on these calls.

>>>> Reported-by: Marc Aurele La France <tsi@xfree86.org>
>>>> Signed-off-by: Marc Aurele La France <tsi@xfree86.org>
>>>> Acked-by: H. Peter Anvin <hpa@zytor.com>

>>> Note: this is not in upstream since upstream is not affected due to the
>>> new "BIOS glovebox" subsystem.

>> So it is a ".30 only" type patch?  Any older kernel versions affected?

> Yes, all the way back to .23 or something like that.

No.  The problem can only arise in 2.6.30 and is a consequence of commit 
5f641356127712fbdce0eee120e5ce115860c17f.  It disappears with subsequent 
mainline commit cf06de7b9cdd3efee7a59dced1977b3c21d43732.

Prior to 2.6.30, vga_set_480_scanlines() was passed a byte-size value (as 
an int), which means the compiler was forced to load EAX, instead of only 
AL, even if it did assume AH still contained 0x01.  That 0x01 is what the 
last INT10 call in the vga_set_{8,14}font() functions sets AH to.

In truth, only those two INT10's need beefing up.  But I did them all for 
completeness.

Marc.

PS:   That should be "80x43", not "80x34".

+----------------------------------+----------------------------------+
|  Marc Aurele La France           |  work:   1-780-492-9310          |
|  Academic Information and        |  fax:    1-780-492-1729          |
|    Communications Technologies   |  email:  tsi@ualberta.ca         |
|  352 General Services Building   +----------------------------------+
|  University of Alberta           |                                  |
|  Edmonton, Alberta               |    Standard disclaimers apply    |
|  T6G 2H1                         |                                  |
|  CANADA                          |                                  |
+----------------------------------+----------------------------------+
XFree86 developer and VP.  ATI driver and X server internals.

Re: [rs] Re: [stable] [PATCH] x86, setup (2.6.30-stable) fix 80x34 and 80x60 console modes

[Greg KH]

On Tue, Jun 30, 2009 at 08:01:47AM -0600, Marc Aurele La France wrote:
> On Mon, 29 Jun 2009, H. Peter Anvin wrote:
> > Greg KH wrote:
> >> On Mon, Jun 29, 2009 at 06:27:40PM -0700, H. Peter Anvin wrote:
> >>> Marc Aurele La France wrote:
> >>>> As coded, most INT10 calls in video-vga.c allow the compiler to assume EAX
> >>>> remains unchanged across them, which is not always the case.  This
> >>>> triggers an optimisation issue that causes vga_set_vertical_end() to be
> >>>> called with an incorrect number of scanlines.  Fix this by beefing up the
> >>>> asm constraints on these calls.
> 
> >>>> Reported-by: Marc Aurele La France <tsi@xfree86.org>
> >>>> Signed-off-by: Marc Aurele La France <tsi@xfree86.org>
> >>>> Acked-by: H. Peter Anvin <hpa@zytor.com>
> 
> >>> Note: this is not in upstream since upstream is not affected due to the
> >>> new "BIOS glovebox" subsystem.
> 
> >> So it is a ".30 only" type patch?  Any older kernel versions affected?
> 
> > Yes, all the way back to .23 or something like that.
> 
> No.  The problem can only arise in 2.6.30 and is a consequence of commit 
> 5f641356127712fbdce0eee120e5ce115860c17f.  It disappears with subsequent 
> mainline commit cf06de7b9cdd3efee7a59dced1977b3c21d43732.
> 
> Prior to 2.6.30, vga_set_480_scanlines() was passed a byte-size value (as 
> an int), which means the compiler was forced to load EAX, instead of only 
> AL, even if it did assume AH still contained 0x01.  That 0x01 is what the 
> last INT10 call in the vga_set_{8,14}font() functions sets AH to.
> 
> In truth, only those two INT10's need beefing up.  But I did them all for 
> completeness.

Ah, thanks.  Next time, putting that information into the patch itself
would be most helpful :)

I'll go queue it up.

greg k-h