[SeaBIOS] [PATCH v2 4/6] add serial console support

[Kevin O'Connor]

On Thu, Sep 14, 2017 at 09:08:20AM +0200, Gerd Hoffmann wrote:
> Redirect int10 calls to serial console output.
> Parse serial input and queue key events.
> The serial console can work both as primary display
> and in parallel to another vga display (splitmode).

Thanks.  Looks good.  I have a few minor comments, but they can all be
fixed post commit.  (Let me know if you prefer otherwise.)

> --- a/Makefile
> +++ b/Makefile
> @@ -29,7 +29,7 @@ LD32BIT_FLAG:=-melf_i386
>  
>  # Source files
>  SRCBOTH=misc.c stacks.c output.c string.c block.c cdrom.c disk.c mouse.c kbd.c \
> -    system.c serial.c clock.c resume.c pnpbios.c vgahooks.c pcibios.c apm.c \
> +    system.c serial.c sercon.c clock.c resume.c pnpbios.c vgahooks.c pcibios.c apm.c \
>      cp437.c \

Line wrap.

> --- a/src/util.h
> +++ b/src/util.h
> @@ -234,6 +234,9 @@ void code_mutable_preinit(void);
>  // serial.c
>  void serial_setup(void);
>  void lpt_setup(void);
> +void sercon_10(struct bregs *regs);
> +void sercon_setup(u16 iobase);
> +void sercon_check_event(void);

These should be in a "// sercon.c" section (which should be above the
"serial.c" block to keep the alphabetical ordering).  Also,
sercon_10() doesn't need to be exported here.

[...]
> --- a/src/optionroms.c
> +++ b/src/optionroms.c
> @@ -12,6 +12,7 @@
>  #include "hw/pcidevice.h" // foreachpci
>  #include "hw/pci_ids.h" // PCI_CLASS_DISPLAY_VGA
>  #include "hw/pci_regs.h" // PCI_ROM_ADDRESS
> +#include "hw/serialio.h" // PORT_SERIAL1
>  #include "malloc.h" // rom_confirm
>  #include "output.h" // dprintf
>  #include "romfile.h" // romfile_loadint
> @@ -404,6 +405,8 @@ struct rom_header *VgaROM;
>  void
>  vgarom_setup(void)
>  {
> +    u16 ret, iobase = 0;
> +
>      if (! CONFIG_OPTIONROMS)
>          return;
>  
> @@ -432,11 +435,22 @@ vgarom_setup(void)
>      run_file_roms("vgaroms/", 1, NULL);
>      rom_reserve(0);
>  
> -    if (rom_get_last() == BUILD_ROM_START)
> +    ret = romfile_loadint("etc/sercon-enable", 0);
> +    if (ret)
> +        iobase = PORT_SERIAL1;
> +
> +    if (rom_get_last() == BUILD_ROM_START) {
>          // No VGA rom found
> +        if (iobase) {
> +            sercon_setup(iobase);
> +            enable_vga_console();
> +        }
>          return;
> +    }
>  
>      VgaROM = (void*)BUILD_ROM_START;
> +    if (iobase)
> +        sercon_setup(iobase);
>      enable_vga_console();
>  }

It would be preferable to not implement sercon logic in optionroms.c.
Instead, the call to enable_vga_console() can be moved into
post.c:maininit() and both it and sercon_setup() can be called
unconditionally there.

[...]
> +void VISIBLE16
> +sercon_10(struct bregs *regs)
> +{
[...]
> --- a/src/romlayout.S
> +++ b/src/romlayout.S
> @@ -414,6 +414,53 @@ __csm_return:
[...]
> +        // call sercon_10
> +        popl %eax
> +        popw %ds
> +2:      pushl $sercon_10

In keeping with the other assembler entry points, it would be
preferable to use the naming entry_sercon / handle_sercon().

-Kevin