| Message ID | 1570110555-24287-3-git-send-email-igor.druzhinin@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | EFI GOP fixes | expand |
On 03.10.2019 15:49, Igor Druzhinin wrote: > If a bootloader is using native driver instead of EFI GOP it might > reset graphics mode to be different from what firmware thinks it > currently is. Set chosen mode unconditionally to fix this possible > misalignment. > > Observed with EFI GRUB2 compiled with all possible video drivers where > native drivers take priority over firmware. I don't think this case can happen with just plain EFI. Therefore ... > Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> > --- > xen/common/efi/boot.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c > index 933db88..4067721 100644 > --- a/xen/common/efi/boot.c > +++ b/xen/common/efi/boot.c > @@ -1052,7 +1052,7 @@ static void __init efi_set_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, UINTN gop > UINTN info_size; > > /* Set graphics mode. */ > - if ( gop_mode < gop->Mode->MaxMode && gop_mode != gop->Mode->Mode ) > + if ( gop_mode < gop->Mode->MaxMode ) > gop->SetMode(gop, gop_mode); ... rather than deleting the right side of the && I'd like to suggest to extend to to take effect only when coming straight from EFI (i.e. EFI_LOADER set in efi_flags). The comment then should be extended to explain why this is. (Reason being that I'd prefer to avoid mode switches unless they're needed for a certain reason.) Jan
On 04/10/2019 11:40, Jan Beulich wrote: > On 03.10.2019 15:49, Igor Druzhinin wrote: >> If a bootloader is using native driver instead of EFI GOP it might >> reset graphics mode to be different from what firmware thinks it >> currently is. Set chosen mode unconditionally to fix this possible >> misalignment. >> >> Observed with EFI GRUB2 compiled with all possible video drivers where >> native drivers take priority over firmware. > > I don't think this case can happen with just plain EFI. Therefore ... > Could you clarify what you mean by "plain EFI" here? Do you mean being booted as EFI binary unlike through multiboot protocol? I think in both cases it's possible to come there through a bootloader. Igor
On 04.10.2019 13:33, Igor Druzhinin wrote: > On 04/10/2019 11:40, Jan Beulich wrote: >> On 03.10.2019 15:49, Igor Druzhinin wrote: >>> If a bootloader is using native driver instead of EFI GOP it might >>> reset graphics mode to be different from what firmware thinks it >>> currently is. Set chosen mode unconditionally to fix this possible >>> misalignment. >>> >>> Observed with EFI GRUB2 compiled with all possible video drivers where >>> native drivers take priority over firmware. >> >> I don't think this case can happen with just plain EFI. Therefore ... >> > > Could you clarify what you mean by "plain EFI" here? Do you mean being > booted as EFI binary unlike through multiboot protocol? Yes - like when running xen.efi from the EFI shell command line. > I think in both > cases it's possible to come there through a bootloader. Anything invoking an EFI application with a screwed up EFI environment is bogus. I can see how grub would violate such assumptions though, and how one might call this valid when invoking the next binary with a protocol other than what plain EFI provides (read: MB2 here). Jan
On 04/10/2019 14:04, Jan Beulich wrote: > On 04.10.2019 13:33, Igor Druzhinin wrote: >> On 04/10/2019 11:40, Jan Beulich wrote: >>> On 03.10.2019 15:49, Igor Druzhinin wrote: >>>> If a bootloader is using native driver instead of EFI GOP it might >>>> reset graphics mode to be different from what firmware thinks it >>>> currently is. Set chosen mode unconditionally to fix this possible >>>> misalignment. >>>> >>>> Observed with EFI GRUB2 compiled with all possible video drivers where >>>> native drivers take priority over firmware. >>> >>> I don't think this case can happen with just plain EFI. Therefore ... >>> >> >> Could you clarify what you mean by "plain EFI" here? Do you mean being >> booted as EFI binary unlike through multiboot protocol? > > Yes - like when running xen.efi from the EFI shell command line. > >> I think in both >> cases it's possible to come there through a bootloader. > > Anything invoking an EFI application with a screwed up EFI > environment is bogus. I can see how grub would violate such > assumptions though, and how one might call this valid when > invoking the next binary with a protocol other than what plain > EFI provides (read: MB2 here). > I'll check if it's the case and will CC Daniel if it's broken that way. Igor
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index 933db88..4067721 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -1052,7 +1052,7 @@ static void __init efi_set_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, UINTN gop UINTN info_size; /* Set graphics mode. */ - if ( gop_mode < gop->Mode->MaxMode && gop_mode != gop->Mode->Mode ) + if ( gop_mode < gop->Mode->MaxMode ) gop->SetMode(gop, gop_mode); /* Get graphics and frame buffer info. */
If a bootloader is using native driver instead of EFI GOP it might reset graphics mode to be different from what firmware thinks it currently is. Set chosen mode unconditionally to fix this possible misalignment. Observed with EFI GRUB2 compiled with all possible video drivers where native drivers take priority over firmware. Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> --- xen/common/efi/boot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)