[PATCH 0/3] x86/boot: small video mode handling adjustments

[PATCH 0/3] x86/boot: small video mode handling adjustments

[Jan Beulich]

1: make "vga=current" work with graphics modes
2: simplify mode_table
3: fold branches in video handling code

Jan

[PATCH 1/3] x86/boot: make "vga=current" work with graphics modes

[Jan Beulich]

GrUB2 can be told to leave the screen in the graphics mode it has been
using (or any other one), via "set gfxpayload=keep" (or suitable
variants thereof). In this case we can avoid doing another mode switch
ourselves. This in particular avoids possibly setting the screen to a
less desirable mode: On one of my test systems the set of modes
reported available by the VESA BIOS depends on whether the interposed
KVM switch has that machine set as the active one. If it's not active,
only modes up to 1024x768 get reported, while when active 1280x1024
modes are also included. For things to always work with an explicitly
specified mode (via the "vga=" option), that mode therefore needs be a
1024x768 one.

For some reason this only works for me with "multiboot2" (and
"module2"); "multiboot" (and "module") still forces the screen into text
mode, despite my reading of the sources suggesting otherwise.

For starters I'm limiting this to graphics modes; I do think this ought
to also work for text modes, but
- I can't tell whether GrUB2 can set any text mode other than 80x25
  (I've only found plain "text" to be valid as a "gfxpayload" setting),
- I'm uncertain whether supporting that is worth it, since I'm uncertain
  how many people would be running their systems/screens in text mode,
- we're pretty close to needing a 4th trampoline page, so I'd like to
  limit the amount of added code.

For starters I'm also limiting mode information retrieval to raw BIOS
accesses. This will allow things to work (in principle) also with other
boot environments where a graphics mode can be left in place. The
downside is that this then still is dependent upon switching back to
real mode, so retrieving the needed information from multiboot info is
likely going to be desirable down the road.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I'm not convinced boot_vid_mode really needs setting here; I'm doing so
mainly because setvesabysize also does.

--- a/xen/arch/x86/boot/video.S
+++ b/xen/arch/x86/boot/video.S
@@ -575,7 +575,6 @@ set14:  movw    $0x1111, %ax
         movb    $0x01, %ah              # Define cursor scan lines 11-12
         movw    $0x0b0c, %cx
         int     $0x10
-set_current:
         stc
         ret
 
@@ -693,6 +692,39 @@ vga_modes:
         .word   VIDEO_80x60, 0x50,0x3c,0        # 80x60
 vga_modes_end:
 
+# If the current mode is a VESA graphics one, obtain its parameters.
+set_current:
+        leaw    vesa_glob_info, %di
+        movw    $0x4f00, %ax
+        int     $0x10
+        cmpw    $0x004f, %ax
+        jne     .Lsetc_done
+
+        movw    $0x4f03, %ax
+        int     $0x10
+        cmpw    $0x004f, %ax
+        jne     .Lsetc_done
+
+        leaw    vesa_mode_info, %di     # Get mode information structure
+        movw    %bx, %cx
+        movw    $0x4f01, %ax
+        int     $0x10
+        cmpw    $0x004f, %ax
+        jne     .Lsetc_done
+
+        movb    (%di), %al              # Check mode attributes
+        andb    $0x99, %al
+        cmpb    $0x99, %al
+        jne     .Lsetc_done             # Doh! No linear frame buffer
+
+        movb    $1, bootsym(graphic_mode)
+        movw    %bx, bootsym(boot_vid_mode)
+        movw    %bx, bootsym(video_mode)
+
+.Lsetc_done:
+        stc
+        ret
+
 # Detect VESA modes.
 vesa_modes:
         movw    %di, %bp                # BP=original mode table end

[PATCH 2/3] x86/boot: simplify mode_table

[Jan Beulich]

There's no point in writing 80x25 text mode information via multiple
insns all storing immediate values. The data can simply be included
first thing in the vga_modes table, allowing the already present
REP MOVSB to take care of everything in one go.

While touching this also correct a related but stale comment.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/boot/video.S
+++ b/xen/arch/x86/boot/video.S
@@ -655,8 +655,9 @@ outidx: outb    %al, %dx
 # Build the table of video modes (stored after the setup.S code at the
 # `modelist' label. Each video mode record looks like:
 #        .word        MODE-ID             (our special mode ID (see above))
-#        .byte        rows                (number of rows)
-#        .byte        columns             (number of columns)
+#        .word        rows                (number of rows)
+#        .word        columns             (number of columns)
+#        .word        0                   (color depth; gfx modes only)
 # Returns address of the end of the table in DI, the end is marked
 # with a ASK_VGA ID.
 mode_table:
@@ -665,12 +666,6 @@ mode_table:
         jnz     mtab1
 
         leaw    modelist, %di           # Store standard modes:
-        movw    $VIDEO_80x25,(%di)      # The 80x25 mode (ALL)
-        movw    $0x50,2(%di)
-        movw    $0x19,4(%di)
-        movw    $0x00,6(%di)
-        addw    $8,%di
-
         leaw    bootsym(vga_modes), %si # All modes for std VGA
         movw    $vga_modes_end-vga_modes, %cx
         rep     movsb
@@ -684,6 +679,7 @@ ret0:   ret
 
 # Modes usable on all standard VGAs
 vga_modes:
+        .word   VIDEO_80x25, 0x50,0x19,0        # 80x25
         .word   VIDEO_80x50, 0x50,0x32,0        # 80x50
         .word   VIDEO_80x43, 0x50,0x2b,0        # 80x43
         .word   VIDEO_80x28, 0x50,0x1c,0        # 80x28

[PATCH 3/3] x86/boot: fold branches in video handling code

[Jan Beulich]

Using Jcc to branch around a JMP is necessary only in pre-386 code,
where Jcc is limited to disp8. Use the opposite Jcc directly in two
places. Since it's adjacent, also convert an ORB to TESTB.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
This is an easy change reducing the overall trampoline size a little.
We're pretty close to needing a 4th page, which I'd prefer to avoid for
as long as we can.

--- a/xen/arch/x86/boot/video.S
+++ b/xen/arch/x86/boot/video.S
@@ -332,8 +332,7 @@ lment:  movb    $0, (%di)
         cmpw    $0x656d, (%si)          # 'me'
         jnz     lmhx
         cmpw    $0x756e, 2(%si)         # 'nu'
-        jnz     lmhx
-        jmp     listm
+        jz      listm
 
 lmhx:   xorw    %bx, %bx                # Else => mode ID in hex
 lmhex:  lodsb
@@ -401,10 +400,8 @@ mode_set:
         cmpb    $VIDEO_FIRST_VESA>>8, %ah
         jnc     check_vesa
 
-        orb     %ah, %ah
-        jnz     setbad
-
-        jmp     setmenu
+        testb   %ah, %ah
+        jz      setmenu
 
 setbad: clc
         ret

Re: [PATCH 3/3] x86/boot: fold branches in video handling code

[Andrew Cooper]

On 08/09/2021 14:24, Jan Beulich wrote:
> Using Jcc to branch around a JMP is necessary only in pre-386 code,
> where Jcc is limited to disp8. Use the opposite Jcc directly in two
> places. Since it's adjacent, also convert an ORB to TESTB.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> This is an easy change reducing the overall trampoline size a little.
> We're pretty close to needing a 4th page, which I'd prefer to avoid for
> as long as we can.

Honestly, it is high time we ditch all of this.

The interactive video mode menu only appears to have confused users
who've encountered it, and it is a weird thing to have in the first place.

Furthermore, the Multiboot1/2 specs have supported passing
video/edid/etc information for longer than Xen has been around.

I am not aware of anything we currently obtain via BIOS INT calls which
we can't rely on the bootloader for.  Furthermore, if the bootloader
can't provide it, we've got 0 chance of our 16bit assembly being able to
do something useful...

Dropping all of that should remove the bsp-only path from the
trampoline, taking it down to just a single page and removing a lot of
fragile code.  It gets us closer to being able to support Coreboot
(where the current logic says "not EFI?  I can make BIOS INT calls
then", and is wrong).

I know its a larger piece of work, but the wins are substantially larger.

~Andrew

Re: [PATCH 3/3] x86/boot: fold branches in video handling code

[Jan Beulich]

On 08.09.2021 15:49, Andrew Cooper wrote:
> On 08/09/2021 14:24, Jan Beulich wrote:
>> Using Jcc to branch around a JMP is necessary only in pre-386 code,
>> where Jcc is limited to disp8. Use the opposite Jcc directly in two
>> places. Since it's adjacent, also convert an ORB to TESTB.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> This is an easy change reducing the overall trampoline size a little.
>> We're pretty close to needing a 4th page, which I'd prefer to avoid for
>> as long as we can.
> 
> Honestly, it is high time we ditch all of this.
> 
> The interactive video mode menu only appears to have confused users
> who've encountered it, and it is a weird thing to have in the first place.

Iirc that's not something we've invented, but logic we've inherited
from Linux.

> Furthermore, the Multiboot1/2 specs have supported passing
> video/edid/etc information for longer than Xen has been around.

As per patch 1 you can see how successful I was with using mb1 with
grub2 when it came to graphics modes. (Just as a data point: Afaics
the tools configuring grub in our distro still use "multiboot", not
"multiboot2", and hence we'd be hosed there when gfx bits only work
in the latter case.)

> I am not aware of anything we currently obtain via BIOS INT calls which
> we can't rely on the bootloader for.

I'm unaware of replacements for what edd.S does. Which means ...

>  Furthermore, if the bootloader
> can't provide it, we've got 0 chance of our 16bit assembly being able to
> do something useful...

... I don't really agree here.

Jan