* [Xen-devel] [PATCH for-4.13 v2 1/3] efi/boot: add missing pointer dereference in set_color
2019-10-09 20:40 [Xen-devel] [PATCH for-4.13 v2 0/3] EFI GOP fixes Igor Druzhinin
@ 2019-10-09 20:40 ` Igor Druzhinin
2019-10-10 7:07 ` Jan Beulich
2019-10-09 20:40 ` [Xen-devel] [PATCH for-4.13 v2 2/3] x86/efi: properly handle 0 in pixel reserved bitmask Igor Druzhinin
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Igor Druzhinin @ 2019-10-09 20:40 UTC (permalink / raw)
To: xen-devel; +Cc: jgross, Igor Druzhinin, wl, andrew.cooper3, jbeulich, roger.pau
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 9a89414..6cef429 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1116,7 +1116,7 @@ static int __init __maybe_unused set_color(u32 mask, int bpp, u8 *pos, u8 *sz)
return -EINVAL;
for ( *pos = 0; !(mask & 1); ++*pos )
mask >>= 1;
- for ( *sz = 0; mask & 1; ++sz)
+ for ( *sz = 0; mask & 1; ++*sz)
mask >>= 1;
if ( mask )
return -EINVAL;
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Xen-devel] [PATCH for-4.13 v2 2/3] x86/efi: properly handle 0 in pixel reserved bitmask
2019-10-09 20:40 [Xen-devel] [PATCH for-4.13 v2 0/3] EFI GOP fixes Igor Druzhinin
2019-10-09 20:40 ` [Xen-devel] [PATCH for-4.13 v2 1/3] efi/boot: add missing pointer dereference in set_color Igor Druzhinin
@ 2019-10-09 20:40 ` Igor Druzhinin
2019-10-10 7:13 ` Jan Beulich
2019-10-09 20:40 ` [Xen-devel] [PATCH for-4.13 v2 3/3] efi/boot: make sure graphics mode is set while booting through MB2 Igor Druzhinin
2019-10-10 7:56 ` [Xen-devel] [PATCH for-4.13 v2 0/3] EFI GOP fixes Jürgen Groß
3 siblings, 1 reply; 9+ messages in thread
From: Igor Druzhinin @ 2019-10-09 20:40 UTC (permalink / raw)
To: xen-devel; +Cc: jgross, Igor Druzhinin, wl, andrew.cooper3, jbeulich, roger.pau
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(-)
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 */
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Xen-devel] [PATCH for-4.13 v2 2/3] x86/efi: properly handle 0 in pixel reserved bitmask
2019-10-09 20:40 ` [Xen-devel] [PATCH for-4.13 v2 2/3] x86/efi: properly handle 0 in pixel reserved bitmask Igor Druzhinin
@ 2019-10-10 7:13 ` Jan Beulich
2019-10-10 12:23 ` Igor Druzhinin
0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2019-10-10 7:13 UTC (permalink / raw)
To: Igor Druzhinin; +Cc: Juergen Gross, xen-devel, roger.pau, wl, andrew.cooper3
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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Xen-devel] [PATCH for-4.13 v2 2/3] x86/efi: properly handle 0 in pixel reserved bitmask
2019-10-10 7:13 ` Jan Beulich
@ 2019-10-10 12:23 ` Igor Druzhinin
0 siblings, 0 replies; 9+ messages in thread
From: Igor Druzhinin @ 2019-10-10 12:23 UTC (permalink / raw)
To: Jan Beulich; +Cc: Juergen Gross, xen-devel, roger.pau, wl, andrew.cooper3
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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Xen-devel] [PATCH for-4.13 v2 3/3] efi/boot: make sure graphics mode is set while booting through MB2
2019-10-09 20:40 [Xen-devel] [PATCH for-4.13 v2 0/3] EFI GOP fixes Igor Druzhinin
2019-10-09 20:40 ` [Xen-devel] [PATCH for-4.13 v2 1/3] efi/boot: add missing pointer dereference in set_color Igor Druzhinin
2019-10-09 20:40 ` [Xen-devel] [PATCH for-4.13 v2 2/3] x86/efi: properly handle 0 in pixel reserved bitmask Igor Druzhinin
@ 2019-10-09 20:40 ` Igor Druzhinin
2019-10-10 7:14 ` Jan Beulich
2019-10-10 7:56 ` [Xen-devel] [PATCH for-4.13 v2 0/3] EFI GOP fixes Jürgen Groß
3 siblings, 1 reply; 9+ messages in thread
From: Igor Druzhinin @ 2019-10-09 20:40 UTC (permalink / raw)
To: xen-devel; +Cc: jgross, Igor Druzhinin, wl, andrew.cooper3, jbeulich, roger.pau
If a bootloader is using native driver instead of EFI GOP it might
reset graphics mode to be different from what has been originally set
by firmware. While booting through MB2 Xen either need to parse video
setting passed by MB2 and use them instead of what GOP reports or
reset the mode to synchronise it with firmware - prefer the latter.
Observed while booting Xen using MB2 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 | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 6cef429..6b069c4 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1051,8 +1051,12 @@ static void __init efi_set_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, UINTN gop
EFI_STATUS status;
UINTN info_size;
- /* Set graphics mode. */
- if ( gop_mode < gop->Mode->MaxMode && gop_mode != gop->Mode->Mode )
+ /*
+ * Set graphics mode to a selected one and reset it if we didn't come
+ * directly from EFI loader as video settings might have been already modified.
+ */
+ if ( gop_mode < gop->Mode->MaxMode &&
+ (gop_mode != gop->Mode->Mode || !efi_enabled(EFI_LOADER)) )
gop->SetMode(gop, gop_mode);
/* Get graphics and frame buffer info. */
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Xen-devel] [PATCH for-4.13 v2 3/3] efi/boot: make sure graphics mode is set while booting through MB2
2019-10-09 20:40 ` [Xen-devel] [PATCH for-4.13 v2 3/3] efi/boot: make sure graphics mode is set while booting through MB2 Igor Druzhinin
@ 2019-10-10 7:14 ` Jan Beulich
0 siblings, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2019-10-10 7:14 UTC (permalink / raw)
To: Igor Druzhinin; +Cc: Juergen Gross, xen-devel, roger.pau, wl, andrew.cooper3
On 09.10.2019 22:40, Igor Druzhinin wrote:
> If a bootloader is using native driver instead of EFI GOP it might
> reset graphics mode to be different from what has been originally set
> by firmware. While booting through MB2 Xen either need to parse video
> setting passed by MB2 and use them instead of what GOP reports or
> reset the mode to synchronise it with firmware - prefer the latter.
>
> Observed while booting Xen using MB2 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>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Xen-devel] [PATCH for-4.13 v2 0/3] EFI GOP fixes
2019-10-09 20:40 [Xen-devel] [PATCH for-4.13 v2 0/3] EFI GOP fixes Igor Druzhinin
` (2 preceding siblings ...)
2019-10-09 20:40 ` [Xen-devel] [PATCH for-4.13 v2 3/3] efi/boot: make sure graphics mode is set while booting through MB2 Igor Druzhinin
@ 2019-10-10 7:56 ` Jürgen Groß
3 siblings, 0 replies; 9+ messages in thread
From: Jürgen Groß @ 2019-10-10 7:56 UTC (permalink / raw)
To: Igor Druzhinin, xen-devel; +Cc: andrew.cooper3, wl, Jan Beulich, roger.pau
On 09.10.19 22:40, Igor Druzhinin wrote:
> Igor Druzhinin (3):
> efi/boot: add missing pointer dereference in set_color
> x86/efi: properly handle 0 in pixel reserved bitmask
> efi/boot: make sure graphics mode is set while booting through MB2
>
> xen/arch/x86/efi/efi-boot.h | 12 +++++++++---
> xen/common/efi/boot.c | 10 +++++++---
> 2 files changed, 16 insertions(+), 6 deletions(-)
>
For the series:
Release-acked-by: Juergen Gross <jgross@suse.com>
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread