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

Jan Beulich posted 3 patches 1 month, 2 weeks ago

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

Posted by Jan Beulich 1 month, 2 weeks ago
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

Posted by Andrew Cooper 1 month, 2 weeks ago
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

Posted by Jan Beulich 1 month, 2 weeks ago
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