| Message ID | 1570653603-9889-3-git-send-email-igor.druzhinin@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | EFI GOP fixes | expand |
On 09.10.2019 22:40, Igor Druzhinin wrote: > --- a/xen/arch/x86/efi/efi-boot.h > +++ b/xen/arch/x86/efi/efi-boot.h > @@ -528,9 +528,15 @@ static void __init efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, > bpp = set_color(mode_info->PixelInformation.BlueMask, bpp, > &vga_console_info.u.vesa_lfb.blue_pos, > &vga_console_info.u.vesa_lfb.blue_size); > - bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp, > - &vga_console_info.u.vesa_lfb.rsvd_pos, > - &vga_console_info.u.vesa_lfb.rsvd_size); > + if ( !mode_info->PixelInformation.ReservedMask ) > + { > + vga_console_info.u.vesa_lfb.rsvd_pos = 0; > + vga_console_info.u.vesa_lfb.rsvd_size = 0; > + } > + else > + bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp, > + &vga_console_info.u.vesa_lfb.rsvd_pos, > + &vga_console_info.u.vesa_lfb.rsvd_size); Why not simply if ( mode_info->PixelInformation.ReservedMask ) bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp, &vga_console_info.u.vesa_lfb.rsvd_pos, &vga_console_info.u.vesa_lfb.rsvd_size); ? There's nothing I can see which might have changed vga_console_info.u.vesa_lfb.rsvd_{pos,size} from its zero-initialized value. With this adjustment (which could be done while committing) or with a reason supplied for the more complex code Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
On 10/10/2019 08:13, Jan Beulich wrote: > On 09.10.2019 22:40, Igor Druzhinin wrote: >> --- a/xen/arch/x86/efi/efi-boot.h >> +++ b/xen/arch/x86/efi/efi-boot.h >> @@ -528,9 +528,15 @@ static void __init efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, >> bpp = set_color(mode_info->PixelInformation.BlueMask, bpp, >> &vga_console_info.u.vesa_lfb.blue_pos, >> &vga_console_info.u.vesa_lfb.blue_size); >> - bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp, >> - &vga_console_info.u.vesa_lfb.rsvd_pos, >> - &vga_console_info.u.vesa_lfb.rsvd_size); >> + if ( !mode_info->PixelInformation.ReservedMask ) >> + { >> + vga_console_info.u.vesa_lfb.rsvd_pos = 0; >> + vga_console_info.u.vesa_lfb.rsvd_size = 0; >> + } >> + else >> + bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp, >> + &vga_console_info.u.vesa_lfb.rsvd_pos, >> + &vga_console_info.u.vesa_lfb.rsvd_size); > > Why not simply > > if ( mode_info->PixelInformation.ReservedMask ) > bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp, > &vga_console_info.u.vesa_lfb.rsvd_pos, > &vga_console_info.u.vesa_lfb.rsvd_size); > > ? There's nothing I can see which might have changed > vga_console_info.u.vesa_lfb.rsvd_{pos,size} from its zero-initialized > value. With this adjustment (which could be done while committing) or > with a reason supplied for the more complex code > Reviewed-by: Jan Beulich <jbeulich@suse.com> > Didn't notice it was actually statically zero-initialized. Perfectly fine with the suggested change. Igor
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index a0737f9..4af6314 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -528,9 +528,15 @@ static void __init efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, bpp = set_color(mode_info->PixelInformation.BlueMask, bpp, &vga_console_info.u.vesa_lfb.blue_pos, &vga_console_info.u.vesa_lfb.blue_size); - bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp, - &vga_console_info.u.vesa_lfb.rsvd_pos, - &vga_console_info.u.vesa_lfb.rsvd_size); + if ( !mode_info->PixelInformation.ReservedMask ) + { + vga_console_info.u.vesa_lfb.rsvd_pos = 0; + vga_console_info.u.vesa_lfb.rsvd_size = 0; + } + else + bpp = set_color(mode_info->PixelInformation.ReservedMask, bpp, + &vga_console_info.u.vesa_lfb.rsvd_pos, + &vga_console_info.u.vesa_lfb.rsvd_size); if ( bpp > 0 ) break; /* fall through */
In some graphics modes firmware is allowed to return 0 in pixel reserved bitmask which doesn't go against UEFI Spec 2.8 (12.9 Graphics Output Protocol). Without this change non-TrueColor modes won't work which will cause GOP init to fail - observed while trying to boot EFI Xen with Cirrus VGA. Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> --- xen/arch/x86/efi/efi-boot.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)