[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/5] efi: only set a console mode if the current one is invalid


  • To: Roger Pau Monne <roger.pau@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 5 Dec 2022 15:19:13 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=MNijArCjRy+hc+Byk9CgVk4CZNDWhH5rtucEvEiZuBg=; b=Gl9dLfco0qOyX1A8fqoGVZuClnqkzo/WDcfrM29d9mqJE9wCsCixyYDZdPUvbXNvtJu+RWYDhE2m0+nvXs/f8Fogc/vKl024uapDWzmUKH4VtkJC6+t0P/3tikfJvcp5A/NM0E5r7IIPpJnqq8WBV3sk883VnehM4G0RzyXXqidh0gRRl9I84dReXBmEmJZFQ5v89K9ZMs6G009p+/GODnFt3EhnVSmfU6o2R7imaQ9k69Vf/ptcWVCcrPojxuVwWLJL3k51c3TcnwaKY3+i+sqlQkgq7BZtzGyWvPnWyZvPo1OuunL3l/Tgh0Mb7cxzAiHhwxGtFlG2WBEtbi7emg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=J1MbNssL6oEeXNQ/3d1ZvSbylySabC/Q5yKxdrt2xpJYyQAHbSf+OPl7K1epbfOi1hIRqemWVZaaIe7JrH3CE6M8/+iykgpsjQ/ZpVkavV2d2lFJXNCdIVW3PYK8KdOPLp83S4aGxL3dbLsIlA4/MIl7IDPjA/jlF1r6bYDlRGb656+8cM3TQOrVEBGJHR1Ct5OS8FdIgHT9nvSqNqEgdq12vPz2jO3zfZnkUuz/JRbEcrKtaI9yGreEn7c3KpeXVD9gJ5DR/XjrQNtFhj4D8s+Hb6w5k0bqsoQB0ds13UEcLdp09oeh6XduPwaTcP6ScMlJti8ENp1Kn8GyaWTIdg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: marmarek@xxxxxxxxxxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Mon, 05 Dec 2022 14:19:21 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.