Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1393766 > unrolled thread

[PATCH 2/3] x86/sysfb_efi: Fix valid BAR address range check

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/3] x86/sysfb_efi: Fix valid BAR address range check Matt Fleming <matt@codeblueprint.co.uk> - 2016-05-03 21:40 +0200
    Re: [PATCH 2/3] x86/sysfb_efi: Fix valid BAR address range check Ingo Molnar <mingo@kernel.org> - 2016-05-04 08:40 +0200
      Re: [PATCH 2/3] x86/sysfb_efi: Fix valid BAR address range check Matt Fleming <matt@codeblueprint.co.uk> - 2016-05-04 11:30 +0200
        Re: [PATCH 2/3] x86/sysfb_efi: Fix valid BAR address range check Ingo Molnar <mingo@kernel.org> - 2016-05-04 12:30 +0200
          Re: [PATCH 2/3] x86/sysfb_efi: Fix valid BAR address range check Matt Fleming <matt@codeblueprint.co.uk> - 2016-05-04 12:30 +0200
      Re: [PATCH 2/3] x86/sysfb_efi: Fix valid BAR address range check Wang YanQing <udknight@gmail.com> - 2016-05-04 17:50 +0200

#1393766 — [PATCH 2/3] x86/sysfb_efi: Fix valid BAR address range check

From: Wang YanQing <udknight@gmail.com>

We can't just break out when meet start is equal to zero,
this will cause us to miss valid address ranges in later BARs.

On the other hand, it isn't enough to test start only
for below situation:

  0(start) <= lfb_base < end

Due to the BUG this patch fix, I can't use video=efifb:
boot parameter to get efifb on my new ThinkPad E550 for
my old linux system hard disk with 3.10 kernel. In 3.10,
efifb is the only choice due to DRM/I915 in it doesn't
support the GPU.

This patch also add a trivial optimization, break out
after we find the address range is valid without test
later BARs.

Signed-off-by: Wang YanQing <udknight@gmail.com>
Reviewed-by: Peter Jones <pjones@redhat.com>
Cc: David Herrmann <dh.herrmann@gmail.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: <stable@vger.kernel.org>
[ Updated changelog ]
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 arch/x86/kernel/sysfb_efi.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/sysfb_efi.c b/arch/x86/kernel/sysfb_efi.c
index b285d4e8c68e..5da924bbf0a0 100644
--- a/arch/x86/kernel/sysfb_efi.c
+++ b/arch/x86/kernel/sysfb_efi.c
@@ -106,14 +106,24 @@ static int __init efifb_set_system(const struct dmi_system_id *id)
 					continue;
 				for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
 					resource_size_t start, end;
+					unsigned long flags;
+
+					flags = pci_resource_flags(dev, i);
+					if (!(flags & IORESOURCE_MEM))
+						continue;
+
+					if (flags & IORESOURCE_UNSET)
+						continue;
+
+					if (pci_resource_len(dev, i) == 0)
+						continue;
 
 					start = pci_resource_start(dev, i);
-					if (start == 0)
-						break;
 					end = pci_resource_end(dev, i);
 					if (screen_info.lfb_base >= start &&
 					    screen_info.lfb_base < end) {
 						found_bar = 1;
+						break;
 					}
 				}
 			}
-- 
2.7.3

[toc] | [next] | [standalone]

#1394018

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

> From: Wang YanQing <udknight@gmail.com>
> 
> We can't just break out when meet start is equal to zero,

Hm, wot?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1394114

On Wed, 04 May, at 08:35:24AM, Ingo Molnar wrote:
> 
> * Matt Fleming <matt@codeblueprint.co.uk> wrote:
> 
> > From: Wang YanQing <udknight@gmail.com>
> > 
> > We can't just break out when meet start is equal to zero,
> 
> Hm, wot?

The existing code treats address 0x0 as invalid for a PCI BAR range
start address, but 0x0 is actually possible and legitimate, so we
shouldn't be breaking out of the loop.

[toc] | [prev] | [next] | [standalone]

#1394132

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

> On Wed, 04 May, at 08:35:24AM, Ingo Molnar wrote:
> > 
> > * Matt Fleming <matt@codeblueprint.co.uk> wrote:
> > 
> > > From: Wang YanQing <udknight@gmail.com>
> > > 
> > > We can't just break out when meet start is equal to zero,
> > 
> > Hm, wot?
> 
> The existing code treats address 0x0 as invalid for a PCI BAR range
> start address, but 0x0 is actually possible and legitimate, so we
> shouldn't be breaking out of the loop.

Yeah, so I just don't understand the 'when meet start is equal to zero' part - 
what does it mean?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1394133

On Wed, 04 May, at 12:23:40PM, Ingo Molnar wrote:
> 
> * Matt Fleming <matt@codeblueprint.co.uk> wrote:
> 
> > On Wed, 04 May, at 08:35:24AM, Ingo Molnar wrote:
> > > 
> > > * Matt Fleming <matt@codeblueprint.co.uk> wrote:
> > > 
> > > > From: Wang YanQing <udknight@gmail.com>
> > > > 
> > > > We can't just break out when meet start is equal to zero,
> > > 
> > > Hm, wot?
> > 
> > The existing code treats address 0x0 as invalid for a PCI BAR range
> > start address, but 0x0 is actually possible and legitimate, so we
> > shouldn't be breaking out of the loop.
> 
> Yeah, so I just don't understand the 'when meet start is equal to zero' part - 
> what does it mean?

I suspect it means "when start is equal to zero" or "when we encounter
the scenario where start is equal to zero".

Wang?

[toc] | [prev] | [next] | [standalone]

#1394485

On Wed, May 04, 2016 at 08:35:24AM +0200, Ingo Molnar wrote:
> 
> * Matt Fleming <matt@codeblueprint.co.uk> wrote:
> 
> > From: Wang YanQing <udknight@gmail.com>
> > 
> > We can't just break out when meet start is equal to zero,
> 
> Hm, wot?
> 
> Thanks,
> 
> 	Ingo

Sorry for my poor English ,and poor commit message, this bring
trouble for more than one maintainer, I guess.

The old code use below comparion as condition to terminate valid 
range check without care whether there are more ranges in later
BARs:
"
        if (start == 0)
            break;
"

I guess original author think (or make a mistake) when we meet a 
address range begin from zero means it is a invalid address range, 
and no valid address ranges in remain BARs.

So I said:
We can't break the loop when meet range whose start address is zero,
without check it and remaining BARs' range.

Thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web