[GIT PULL] EFI urgent fix

[GIT PULL] EFI urgent fix

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

Please pull the following fix from Zoltán which addresses a bug that
resulted in the the secondary GOP display being garbled when booting
using the EFI boot stub.

The following changes since commit 825fcfce81921c9cc4ef801d844793815721e458:

  MAINTAINERS: Change Matt Fleming's email address (2015-10-11 09:54:29 +0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git tags/efi-urgent

for you to fetch changes up to dc1ea95fd23c36dd6be284334a0c8327d11d8c52:

  x86/efi: Fix multiple GOP device support (2015-10-11 11:40:54 +0100)

----------------------------------------------------------------
 * Fix booting using the EFI boot stub on platforms with multiple
   Graphics Output Protocol devices because currently the secondary
   display will be garbled on such systems - Zoltán Kővágó

----------------------------------------------------------------
Kővágó, Zoltán (1):
      x86/efi: Fix multiple GOP device support

 arch/x86/boot/compressed/eboot.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

[PATCH] x86/efi: Fix multiple GOP device support

[Matt Fleming]

From: Kővágó, Zoltán <dirty.ice.hu@gmail.com>

When multiple GOP devices exists, but none of them implements ConOut,
the code should just choose the first GOP (according to the comments).
But currently fb_base will refer to the last GOP, while other parameters
to the first GOP, which will likely result in a garbled display.

I can reliably reproduce this bug using my ASRock Z87M Extreme4
motherboard with CSM and integrated GPU disabled, and two PCIe video
cards (NVidia GT640 and GTX980), booting from efi-stub (booting from
grub works fine).  On the primary display the asrock logo remains and on
the secondary screen is garbled up completely.

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/x86/boot/compressed/eboot.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index ee1b6d346b98..db51c1f27446 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -667,6 +667,7 @@ setup_gop32(struct screen_info *si, efi_guid_t *proto,
 		bool conout_found = false;
 		void *dummy = NULL;
 		u32 h = handles[i];
+		u32 current_fb_base;
 
 		status = efi_call_early(handle_protocol, h,
 					proto, (void **)&gop32);
@@ -678,7 +679,7 @@ setup_gop32(struct screen_info *si, efi_guid_t *proto,
 		if (status == EFI_SUCCESS)
 			conout_found = true;
 
-		status = __gop_query32(gop32, &info, &size, &fb_base);
+		status = __gop_query32(gop32, &info, &size, &current_fb_base);
 		if (status == EFI_SUCCESS && (!first_gop || conout_found)) {
 			/*
 			 * Systems that use the UEFI Console Splitter may
@@ -692,6 +693,7 @@ setup_gop32(struct screen_info *si, efi_guid_t *proto,
 			pixel_format = info->pixel_format;
 			pixel_info = info->pixel_information;
 			pixels_per_scan_line = info->pixels_per_scan_line;
+			fb_base = current_fb_base;
 
 			/*
 			 * Once we've found a GOP supporting ConOut,
@@ -770,6 +772,7 @@ setup_gop64(struct screen_info *si, efi_guid_t *proto,
 		bool conout_found = false;
 		void *dummy = NULL;
 		u64 h = handles[i];
+		u32 current_fb_base;
 
 		status = efi_call_early(handle_protocol, h,
 					proto, (void **)&gop64);
@@ -781,7 +784,7 @@ setup_gop64(struct screen_info *si, efi_guid_t *proto,
 		if (status == EFI_SUCCESS)
 			conout_found = true;
 
-		status = __gop_query64(gop64, &info, &size, &fb_base);
+		status = __gop_query64(gop64, &info, &size, &current_fb_base);
 		if (status == EFI_SUCCESS && (!first_gop || conout_found)) {
 			/*
 			 * Systems that use the UEFI Console Splitter may
@@ -795,6 +798,7 @@ setup_gop64(struct screen_info *si, efi_guid_t *proto,
 			pixel_format = info->pixel_format;
 			pixel_info = info->pixel_information;
 			pixels_per_scan_line = info->pixels_per_scan_line;
+			fb_base = current_fb_base;
 
 			/*
 			 * Once we've found a GOP supporting ConOut,
-- 
2.1.0

Re: [PATCH] x86/efi: Fix multiple GOP device support

[Ingo Molnar]

* Matt Fleming <matt@codeblueprint.co.uk> wrote:

> From: Kővágó, Zoltán <dirty.ice.hu@gmail.com>
> 
> When multiple GOP devices exists, but none of them implements ConOut,
> the code should just choose the first GOP (according to the comments).
> But currently fb_base will refer to the last GOP, while other parameters
> to the first GOP, which will likely result in a garbled display.
> 
> I can reliably reproduce this bug using my ASRock Z87M Extreme4
> motherboard with CSM and integrated GPU disabled, and two PCIe video
> cards (NVidia GT640 and GTX980), booting from efi-stub (booting from
> grub works fine).  On the primary display the asrock logo remains and on
> the secondary screen is garbled up completely.
> 
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> ---
>  arch/x86/boot/compressed/eboot.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
> index ee1b6d346b98..db51c1f27446 100644
> --- a/arch/x86/boot/compressed/eboot.c
> +++ b/arch/x86/boot/compressed/eboot.c
> @@ -667,6 +667,7 @@ setup_gop32(struct screen_info *si, efi_guid_t *proto,
>  		bool conout_found = false;
>  		void *dummy = NULL;
>  		u32 h = handles[i];
> +		u32 current_fb_base;

Sigh, fb_base is u64...

> @@ -770,6 +772,7 @@ setup_gop64(struct screen_info *si, efi_guid_t *proto,
>  		bool conout_found = false;
>  		void *dummy = NULL;
>  		u64 h = handles[i];
> +		u32 current_fb_base;

Ditto.

So I've applied it with that obvious bug fixed, but could you guys please double 
check how on earth this patch could possibly have worked fine in testing, without 
crashing 64-bit kernels?

Thanks,

	Ingo

Re: [PATCH] x86/efi: Fix multiple GOP device support

[Ingo Molnar]

* Ingo Molnar <mingo@kernel.org> wrote:

> * Matt Fleming <matt@codeblueprint.co.uk> wrote:
> 
> > From: Kővágó, Zoltán <dirty.ice.hu@gmail.com>
> > 
> > When multiple GOP devices exists, but none of them implements ConOut,
> > the code should just choose the first GOP (according to the comments).
> > But currently fb_base will refer to the last GOP, while other parameters
> > to the first GOP, which will likely result in a garbled display.
> > 
> > I can reliably reproduce this bug using my ASRock Z87M Extreme4
> > motherboard with CSM and integrated GPU disabled, and two PCIe video
> > cards (NVidia GT640 and GTX980), booting from efi-stub (booting from
> > grub works fine).  On the primary display the asrock logo remains and on
> > the secondary screen is garbled up completely.
> > 
> > Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> > Cc: Matthew Garrett <mjg59@srcf.ucam.org>
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> > ---
> >  arch/x86/boot/compressed/eboot.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
> > index ee1b6d346b98..db51c1f27446 100644
> > --- a/arch/x86/boot/compressed/eboot.c
> > +++ b/arch/x86/boot/compressed/eboot.c
> > @@ -667,6 +667,7 @@ setup_gop32(struct screen_info *si, efi_guid_t *proto,
> >  		bool conout_found = false;
> >  		void *dummy = NULL;
> >  		u32 h = handles[i];
> > +		u32 current_fb_base;
> 
> Sigh, fb_base is u64...
> 
> > @@ -770,6 +772,7 @@ setup_gop64(struct screen_info *si, efi_guid_t *proto,
> >  		bool conout_found = false;
> >  		void *dummy = NULL;
> >  		u64 h = handles[i];
> > +		u32 current_fb_base;
> 
> Ditto.
> 
> So I've applied it with that obvious bug fixed, but could you guys please double 
> check how on earth this patch could possibly have worked fine in testing, without 
> crashing 64-bit kernels?

Ah, I see, this is a subtle semantic conflict with pending v4.4 EFI changes in 
tip:core/efi, which changed fb_base from u32 to u64:

  ae2ee627dc87 ("efifb: Add support for 64-bit frame buffer addresses")

(Interestingly there was no textual conflict between this patch and that commit.)

So the fix patch is fine as-is for v4.3, but needs a conflict resolution for the 
pending v4.4 commit.

I've applied it that way.

Thanks,

	Ingo

Re: [PATCH] x86/efi: Fix multiple GOP device support

[Matt Fleming]

On Wed, 14 Oct, at 04:51:04PM, Ingo Molnar wrote:
> 
> Ah, I see, this is a subtle semantic conflict with pending v4.4 EFI changes in 
> tip:core/efi, which changed fb_base from u32 to u64:
> 
>   ae2ee627dc87 ("efifb: Add support for 64-bit frame buffer addresses")
 
Yeah, that's exactly the issue. I should have given you a heads up
about this but I forgot that there were two patches to this area in
separate branches. 

> (Interestingly there was no textual conflict between this patch and that commit.)
> 
> So the fix patch is fine as-is for v4.3, but needs a conflict resolution for the 
> pending v4.4 commit.
> 
> I've applied it that way.

Do you need me to send a patch on top or have you taken care of the
semantic conflict for v4.4? (the change you originally proposed,
s/u32/u64/, looked fine)

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH] x86/efi: Fix multiple GOP device support

[Ingo Molnar]

* Matt Fleming <matt@codeblueprint.co.uk> wrote:

> > So the fix patch is fine as-is for v4.3, but needs a conflict resolution for 
> > the pending v4.4 commit.
> > 
> > I've applied it that way.
> 
> Do you need me to send a patch on top or have you taken care of the
> semantic conflict for v4.4? (the change you originally proposed,
> s/u32/u64/, looked fine)

So to not break bisection in hard to debug ways, I made this fixup in the merge 
commit of your changes, and documented it all in the merge commit message:

commit 790a2ee2427852cff50993c98f15ed88511e9af0
Merge: c7d77a7980e4 0f96a99dab36
Author: Ingo Molnar <mingo@kernel.org>
Date:   Wed Oct 14 16:05:40 2015 +0200

    Merge tag 'efi-next' of git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi into core/efi
    
    Pull v4.4 EFI updates from Matt Fleming:
    
      - Make the EFI System Resource Table (ESRT) driver explicitly
        non-modular by ripping out the module_* code since Kconfig doesn't
        allow it to be built as a module anyway. (Paul Gortmaker)
    
      - Make the x86 efi=debug kernel parameter, which enables EFI debug
        code and output, generic and usable by arm64. (Leif Lindholm)
    
      - Add support to the x86 EFI boot stub for 64-bit Graphics Output
        Protocol frame buffer addresses. (Matt Fleming)
    
      - Detect when the UEFI v2.5 EFI_PROPERTIES_TABLE feature is enabled
        in the firmware and set an efi.flags bit so the kernel knows when
        it can apply more strict runtime mapping attributes - Ard Biesheuvel
    
      - Auto-load the efi-pstore module on EFI systems, just like we
        currently do for the efivars module. (Ben Hutchings)
    
      - Add "efi_fake_mem" kernel parameter which allows the system's EFI
        memory map to be updated with additional attributes for specific
        memory ranges. This is useful for testing the kernel code that handles
        the EFI_MEMORY_MORE_RELIABLE memmap bit even if your firmware
        doesn't include support. (Taku Izumi)
    
    Note: there is a semantic conflict between the following two commits:
    
      8a53554e12e9 ("x86/efi: Fix multiple GOP device support")
      ae2ee627dc87 ("efifb: Add support for 64-bit frame buffer addresses")
    
    I fixed up the interaction in the merge commit, changing the type of
    current_fb_base from u32 to u64.
    
    Signed-off-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

Re: [PATCH] x86/efi: Fix multiple GOP device support

[Matt Fleming]

On Wed, 14 Oct, at 05:04:26PM, Ingo Molnar wrote:
> 
> * Matt Fleming <matt@codeblueprint.co.uk> wrote:
> 
> > > So the fix patch is fine as-is for v4.3, but needs a conflict resolution for 
> > > the pending v4.4 commit.
> > > 
> > > I've applied it that way.
> > 
> > Do you need me to send a patch on top or have you taken care of the
> > semantic conflict for v4.4? (the change you originally proposed,
> > s/u32/u64/, looked fine)
> 
> So to not break bisection in hard to debug ways, I made this fixup in the merge 
> commit of your changes, and documented it all in the merge commit message:
> 
> commit 790a2ee2427852cff50993c98f15ed88511e9af0
> Merge: c7d77a7980e4 0f96a99dab36
> Author: Ingo Molnar <mingo@kernel.org>
> Date:   Wed Oct 14 16:05:40 2015 +0200

[...]

>     Note: there is a semantic conflict between the following two commits:
>     
>       8a53554e12e9 ("x86/efi: Fix multiple GOP device support")
>       ae2ee627dc87 ("efifb: Add support for 64-bit frame buffer addresses")
>     
>     I fixed up the interaction in the merge commit, changing the type of
>     current_fb_base from u32 to u64.
>     
>     Signed-off-by: Ingo Molnar <mingo@kernel.org>

Awesome, thanks Ingo.

-- 
Matt Fleming, Intel Open Source Technology Center

[tip:core/efi] x86/efi: Fix multiple GOP device support

[tip-bot for Kővágó, Zoltán]

Commit-ID:  8a53554e12e98d1759205afd7b8e9e2ea0936f48
Gitweb:     http://git.kernel.org/tip/8a53554e12e98d1759205afd7b8e9e2ea0936f48
Author:     Kővágó, Zoltán <dirty.ice.hu@gmail.com>
AuthorDate: Mon, 12 Oct 2015 15:13:56 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 14 Oct 2015 16:02:43 +0200

x86/efi: Fix multiple GOP device support

When multiple GOP devices exists, but none of them implements
ConOut, the code should just choose the first GOP (according to
the comments). But currently 'fb_base' will refer to the last GOP,
while other parameters to the first GOP, which will likely
result in a garbled display.

I can reliably reproduce this bug using my ASRock Z87M Extreme4
motherboard with CSM and integrated GPU disabled, and two PCIe
video cards (NVidia GT640 and GTX980), booting from efi-stub
(booting from grub works fine).  On the primary display the
ASRock logo remains and on the secondary screen it is garbled
up completely.

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
Cc: <stable@vger.kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1444659236-24837-2-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/boot/compressed/eboot.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index ee1b6d3..db51c1f 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -667,6 +667,7 @@ setup_gop32(struct screen_info *si, efi_guid_t *proto,
 		bool conout_found = false;
 		void *dummy = NULL;
 		u32 h = handles[i];
+		u32 current_fb_base;
 
 		status = efi_call_early(handle_protocol, h,
 					proto, (void **)&gop32);
@@ -678,7 +679,7 @@ setup_gop32(struct screen_info *si, efi_guid_t *proto,
 		if (status == EFI_SUCCESS)
 			conout_found = true;
 
-		status = __gop_query32(gop32, &info, &size, &fb_base);
+		status = __gop_query32(gop32, &info, &size, &current_fb_base);
 		if (status == EFI_SUCCESS && (!first_gop || conout_found)) {
 			/*
 			 * Systems that use the UEFI Console Splitter may
@@ -692,6 +693,7 @@ setup_gop32(struct screen_info *si, efi_guid_t *proto,
 			pixel_format = info->pixel_format;
 			pixel_info = info->pixel_information;
 			pixels_per_scan_line = info->pixels_per_scan_line;
+			fb_base = current_fb_base;
 
 			/*
 			 * Once we've found a GOP supporting ConOut,
@@ -770,6 +772,7 @@ setup_gop64(struct screen_info *si, efi_guid_t *proto,
 		bool conout_found = false;
 		void *dummy = NULL;
 		u64 h = handles[i];
+		u32 current_fb_base;
 
 		status = efi_call_early(handle_protocol, h,
 					proto, (void **)&gop64);
@@ -781,7 +784,7 @@ setup_gop64(struct screen_info *si, efi_guid_t *proto,
 		if (status == EFI_SUCCESS)
 			conout_found = true;
 
-		status = __gop_query64(gop64, &info, &size, &fb_base);
+		status = __gop_query64(gop64, &info, &size, &current_fb_base);
 		if (status == EFI_SUCCESS && (!first_gop || conout_found)) {
 			/*
 			 * Systems that use the UEFI Console Splitter may
@@ -795,6 +798,7 @@ setup_gop64(struct screen_info *si, efi_guid_t *proto,
 			pixel_format = info->pixel_format;
 			pixel_info = info->pixel_information;
 			pixels_per_scan_line = info->pixels_per_scan_line;
+			fb_base = current_fb_base;
 
 			/*
 			 * Once we've found a GOP supporting ConOut,