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. 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. Jan
