Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932827AbcKOMNp (ORCPT ); Tue, 15 Nov 2016 07:13:45 -0500 Received: from mail-ua0-f196.google.com ([209.85.217.196]:34508 "EHLO mail-ua0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751455AbcKOMNm (ORCPT ); Tue, 15 Nov 2016 07:13:42 -0500 MIME-Version: 1.0 X-Originating-IP: [2a02:fe0:c130:1430:7e7a:91ff:fe0e:3e2c] In-Reply-To: <20161115120158.15388-3-dh.herrmann@gmail.com> References: <20161115120158.15388-1-dh.herrmann@gmail.com> <20161115120158.15388-3-dh.herrmann@gmail.com> From: Tom Gundersen Date: Tue, 15 Nov 2016 13:13:21 +0100 Message-ID: Subject: Re: [PATCH 2/2] x86/sysfb: fix lfb_size calculation To: David Herrmann Cc: LKML , Matt Fleming , Ingo Molnar , "the arch/x86 maintainers" , "H. Peter Anvin" , Thomas Gleixner Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2856 Lines: 71 On Tue, Nov 15, 2016 at 1:01 PM, David Herrmann wrote: > The screen_info.lfb_size field is shifted by 16 bits *only* in case of > VBE. This has historical reasons since VBE advertised it similarly. > However, in case of EFI framebuffers, the size is no longer shifted. Fix > the x86 simple-framebuffer setup code to use the correct size in the > non-VBE case. > > While at it, avoid variable abbreviations and rename 'len' to 'length', > and use the correct types matching the screen_info definition. > > Signed-off-by: David Herrmann Reviewed-by: Tom Gundersen > --- > arch/x86/kernel/sysfb_simplefb.c | 25 +++++++++++++++++-------- > 1 file changed, 17 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kernel/sysfb_simplefb.c b/arch/x86/kernel/sysfb_simplefb.c > index 35b8641..85195d4 100644 > --- a/arch/x86/kernel/sysfb_simplefb.c > +++ b/arch/x86/kernel/sysfb_simplefb.c > @@ -66,8 +66,8 @@ __init int create_simplefb(const struct screen_info *si, > { > struct platform_device *pd; > struct resource res; > - unsigned long len; > - u64 base; > + u64 base, size; > + u32 length; > > /* > * If the 64BIT_BASE capability is set, ext_lfb_base will contain the > @@ -82,11 +82,20 @@ __init int create_simplefb(const struct screen_info *si, > return -EINVAL; > } > > - /* don't use lfb_size as it may contain the whole VMEM instead of only > - * the part that is occupied by the framebuffer */ > - len = mode->height * mode->stride; > - len = PAGE_ALIGN(len); > - if (len > (u64)si->lfb_size << 16) { > + /* > + * Don't use lfb_size as IORESOURCE size, since it may contain the > + * entire VMEM, and thus require huge mappings. Use just the part we > + * need, that is, the part where the framebuffer is located. But verify > + * that it does not exceed the advertised VMEM. > + * Note that in case of VBE, the lfb_size is shifted by 16 bits for > + * historical reasons. > + */ > + size = si->lfb_size; > + if (si->orig_video_isVGA == VIDEO_TYPE_VLFB) > + size <<= 16; > + length = mode->height * mode->stride; > + length = PAGE_ALIGN(length); > + if (length > size) { > printk(KERN_WARNING "sysfb: VRAM smaller than advertised\n"); > return -EINVAL; > } > @@ -96,7 +105,7 @@ __init int create_simplefb(const struct screen_info *si, > res.flags = IORESOURCE_MEM | IORESOURCE_BUSY; > res.name = simplefb_resname; > res.start = base; > - res.end = res.start + len - 1; > + res.end = res.start + length - 1; > if (res.end <= res.start) > return -EINVAL; > > -- > 2.10.2 >