On 30.03.2023 17:44, Roger Pau Monné wrote: > On Mon, Dec 05, 2022 at 03:19:13PM +0100, Jan Beulich wrote: >> On 23.11.2022 16:45, Roger Pau Monne wrote: >>> Do not unconditionally set a mode in efi_console_set_mode(), do so >>> only if the currently set mode is not valid. >> >> You don't say why you want to do so. Furthermore ... >> >>> --- a/xen/common/efi/boot.c >>> +++ b/xen/common/efi/boot.c >>> @@ -799,6 +799,11 @@ static void __init efi_console_set_mode(void) >>> UINTN cols, rows, size; >>> unsigned int best, i; >>> >>> + /* Only set a mode if the current one is not valid. */ >>> + if ( StdOut->QueryMode(StdOut, StdOut->Mode->Mode, &cols, &rows) == >>> + EFI_SUCCESS ) >>> + return; >> >> ... it might be okay if you put such a check in efi_multiboot2(), but >> the call here from efi_start() is specifically guarded by a check of >> whether "-basevideo" was passed to xen.efi. This _may_ not be as >> relevant anymore today, but it certainly was 20 years ago (recall >> that we've inherited this code from a much older project of ours) - >> at that time EFI usually started in 80x25 text mode. And I think that >> even today when you end up launching xen.efi from the EFI shell, >> you'd be stuck with 80x25 text mode on at least some implementations. > > Won't you use console=vga vga=gfx-... > > To switch to a best mode?
I don't think "vga=gfx-..." is presently consumed in any way xen.efi. Doing so would require a similar hack of peeking at the (ordinary) command line options as in legacy booting, except that in xen.efi we read that command line from a file, which iirc is done only after fiddling with the video mode. >> Overall, looking at (for now) just the titles of subsequent patches, >> I'm not convinced the change here is needed at all. Or if anything it >> may want to go at the end, taking action only when "vga=current" was >> specified. > > I guess I'm slightly confused by the usage of both GOP and StdOut, I > would assume if we have a gop, and can correctly initialize it there's > no need to fiddle with StdOut also? Setting the GOP mode is done last before exiting boot services; this may be a graphics mode which doesn't support a text output protocol. Jan
