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? > 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? Thanks, Roger.
