Re: [PATCH v2 3/3] multiboot2: do not set StdOut mode unconditionally

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.03.2023 11:59, Roger Pau Monne wrote:
> @@ -887,6 +881,15 @@ void __init efi_multiboot2(EFI_HANDLE ImageHandle, 
> EFI_SYSTEM_TABLE *SystemTable
>  
>          efi_arch_edid(gop_handle);
>      }
> +    else
> +    {
> +        /* If no GOP, init ConOut (StdOut) to the max supported size. */
> +        efi_console_set_mode();
> +
> +        if ( StdOut->QueryMode(StdOut, StdOut->Mode->Mode,
> +                               &cols, &rows) == EFI_SUCCESS )
> +            efi_arch_console_init(cols, rows);
> +    }

Instead of making this an "else", wouldn't you better check that a
valid gop_mode was found? efi_find_gop_mode() can return ~0 after all.

Furthermore, what if the active mode doesn't support text output? (I
consider the spec unclear in regard to whether this is possible, but
maybe I simply didn't find the right place stating it.)

Finally I think efi_arch_console_init() wants calling nevertheless.

So altogether maybe

    if ( gop_mode == ~0 ||
         StdOut->QueryMode(StdOut, StdOut->Mode->Mode,
                           &cols, &rows) != EFI_SUCCESS )
        /* If no usable GOP mode, init ConOut (StdOut) to the max supported 
size. */
        efi_console_set_mode();

    if ( StdOut->QueryMode(StdOut, StdOut->Mode->Mode,
                           &cols, &rows) == EFI_SUCCESS )
        efi_arch_console_init(cols, rows);

?

Jan