Re: [SeaBIOS] [PATCH 5/5] [wip] sercon: initial split-output implementation

[Kevin O'Connor]

On Thu, Jul 14, 2016 at 10:53:02AM +0200, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann
>
> src/optionroms.c |  2 ++
>  src/romlayout.S  | 39 ++++++++++++++++++++++
>  src/sercon.c     | 99 +++++++++++++++++++++++++++++++++++++++-----------------
>  3 files changed, 111 insertions(+), 29 deletions(-)
>
> diff --git a/src/optionroms.c b/src/optionroms.c
> index f9e9593..f08fcb1 100644
> --- a/src/optionroms.c
> +++ b/src/optionroms.c
> @@ -442,6 +442,8 @@ vgarom_setup(void)
>      }
>  VgaROM = (void*)BUILD_ROM_START;
> if (romfile_loadint("etc/sercon-enable", 0))
>    sercon_enable();

Minor nit, but why not unconditionally call sercon_enable() and let
sercon_enable() check romfile_loadint().

[...]
> --- a/src/romlayout.S
> +++ b/src/romlayout.S
> @@ -522,6 +522,45 @@ irqentry_arg:
>          DECL_IRQ_ENTRY hwpic1
>          DECL_IRQ_ENTRY hwpic2
>    // hooked int10, for sercon
>    DECLFUNC entry_10_hooked
>
> +entry_10_hooked:
> pushl $ handle_10
>
> +#if CONFIG_ENTRY_EXTRASTACK
> // FIXME: too much cut+paste

I'm okay with the cut-and-paste.  But, another option would be to use
the iretw at the end of the existing irqentry_extrastack to implement
the ljmpw into the main vgabios.  Something like (totally untested):

entry_10_hooked:
        pushfw                          // Setup for iretw in irqentry_arg
        pushl %cs:sercon_int10_hook_resume

pushl $handle_10
#if CONFIG_ENTRY_EXTRASTACK
        jmp irqentry_arg_extrastack
#else
        jmp irqentry_arg
#endif

Separately, have you considered choosing a separate entry point for
entry_10_hooked.  That is, changing the above pushl to
$handle_sercon_hooked and introducing that function in sercon.c.  It
seems it would reduce a number of "if (!sercon_splitmode())" checks in
the main code, as handle_sercon_hooked() could just use a smaller
switch statement and ignore requests it doesn't need to support.

Finally, one high level observation is that we know there are a number
of quirks in various vgabios emulators.  For example, we know some
emulators don't handle certain 32bit instructions when in 16bit mode
(hence scripts/vgafixup.py), we know some versions of Windows use an
emulator that doesn't like some stack relative instructions (hence the
vgabios is compiled without -fomit-frame-pointer), and we know Windows
Vista doesn't like the extra stack in high ram (the skifree bug).  Any
thoughts on what happens with these quirks if the main seabios code
hooks int10?

-Kevin