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 <[email protected]>
Signed-off-by: Marc Aurele La France <[email protected]>
Acked-by: H. Peter Anvin <[email protected]>
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: [email protected] |
| 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.
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 <[email protected]>
> Signed-off-by: Marc Aurele La France <[email protected]>
> Acked-by: H. Peter Anvin <[email protected]>
Note: this is not in upstream since upstream is not affected due to the
new "BIOS glovebox" subsystem.
-hpa
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 <[email protected]>
> > Signed-off-by: Marc Aurele La France <[email protected]>
> > Acked-by: H. Peter Anvin <[email protected]>
>
> 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
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 <[email protected]>
>>> Signed-off-by: Marc Aurele La France <[email protected]>
>>> Acked-by: H. Peter Anvin <[email protected]>
>> 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
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 <[email protected]>
>>>> Signed-off-by: Marc Aurele La France <[email protected]>
>>>> Acked-by: H. Peter Anvin <[email protected]>
>>> 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: [email protected] |
| 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.
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 <[email protected]>
> >>>> Signed-off-by: Marc Aurele La France <[email protected]>
> >>>> Acked-by: H. Peter Anvin <[email protected]>
>
> >>> 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