Re: [Xen-devel] [PATCH v2 1/2] x86/boot: fix early error display

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.10.17 at 23:41, <cardoe@xxxxxxxxxx> wrote:
> From: David Esler <drumandstrum@xxxxxxxxx>
> 
> In 9180f5365524 a change was made to the send_chr function to take in
> C-strings and print out a character at a time until a NULL was
> encountered. However there is no code to increment the current character
> position resulting in an endless loop of the first character. This adds
> a simple increment.

This description is not accurate (it should have changed together with
the change to how you fix the issue) - with VGA the increment does
happen. Hence "display" in the title is perhaps also at least misleading.
I would be fine to adjust both while committing (and then adding my
R-b), but feel free to propose an alternative.

> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -173,10 +173,11 @@ not_multiboot:
>  .Lget_vtb:
>          mov     sym_esi(vga_text_buffer),%edi
>  .Lsend_chr:
> -        mov     (%esi),%bl
> -        test    %bl,%bl        # Terminate on '\0' sentinel
> +        lodsb
> +        test    %al,%al        # Terminate on '\0' sentinel
>          je      .Lhalt
>          mov     $0x3f8+5,%dx   # UART Line Status Register
> +        mov     %al,%bl
>  2:      in      %dx,%al
>          test    $0x20,%al      # Test THR Empty flag
>          je      2b

Daniel: What tells you that there is a UART at 0x3f8? Aren't we
risking to enter an infinite loop here if there isn't? At the very
least it would seem better to me if the VGA output was done
first - when you have a screen, you'd at least know something
was attempted to be output by seeing the first character. And
when you have neither screen nor UART (at the right port)
you're hosed anyway.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel