Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Xen-devel] [PATCH for-4.13 v2 0/3] EFI GOP fixes
@ 2019-10-09 20:40 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
                   ` (3 more replies)
  0 siblings, 4 replies; 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

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(-)

-- 
2.7.4


_______________________________________________
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 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	[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	[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	[flat|nested] 9+ messages in thread

* Re: [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 1/3] efi/boot: add missing pointer dereference in set_color Igor Druzhinin
@ 2019-10-10  7:07   ` Jan Beulich
  0 siblings, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2019-10-10  7:07 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:
> 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 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 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

* 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

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-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
2019-10-10  7:13   ` Jan Beulich
2019-10-10 12:23     ` Igor Druzhinin
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
2019-10-10  7:56 ` [Xen-devel] [PATCH for-4.13 v2 0/3] EFI GOP fixes Jürgen Groß

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org xen-devel@archiver.kernel.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox