Hi
Two fixes for the simplefb setup code in arch/x86/. One to support Matt's 64bit
EFI fb-base-address, and one to fix a longer standing verification bug.
Neither fix is critical. Additionally, most people still use efifb/vesafb
instead of simplefb, so probably does not affect any normal distribution.
However, SimpleDRM is close to being merged, so x86-sysfb might get into
some distros.
Thanks
David
David Herrmann (2):
x86/sysfb: add support for 64bit EFI lfb_base
x86/sysfb: fix lfb_size calculation
arch/x86/kernel/sysfb_simplefb.c | 39 +++++++++++++++++++++++++++++++--------
1 file changed, 31 insertions(+), 8 deletions(-)
--
2.10.2
The screen_info object was extended to support 64bit lfb_base addresses
in:
commit ae2ee627dc87a70910de91b791b3cd0e9c6facdd
Author: Matt Fleming <[email protected]>
Date: Tue Aug 25 16:32:55 2015 +0100
efifb: Add support for 64-bit frame buffer addresses
However, the x86 simple-framebuffer setup code never made use of it. Fix
it to properly assemble and verify the lfb_base before advertising
simple-framebuffer devices.
In particular, this means if VIDEO_CAPABILITY_64BIT_BASE is set, the
screen_info->ext_lfb_base field will contain the upper 32bit of the
actual lfb_base. Make sure the address is not 0 (i.e., unset), as well as
does not overflow the physical address type.
Signed-off-by: David Herrmann <[email protected]>
---
arch/x86/kernel/sysfb_simplefb.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/sysfb_simplefb.c b/arch/x86/kernel/sysfb_simplefb.c
index 764a29f..35b8641 100644
--- a/arch/x86/kernel/sysfb_simplefb.c
+++ b/arch/x86/kernel/sysfb_simplefb.c
@@ -67,6 +67,20 @@ __init int create_simplefb(const struct screen_info *si,
struct platform_device *pd;
struct resource res;
unsigned long len;
+ u64 base;
+
+ /*
+ * If the 64BIT_BASE capability is set, ext_lfb_base will contain the
+ * upper half of the base address. Assemble the address, then make sure
+ * it is valid and we can actually access it.
+ */
+ base = si->lfb_base;
+ if (si->capabilities & VIDEO_CAPABILITY_64BIT_BASE)
+ base |= (u64)si->ext_lfb_base << 32;
+ if (!base || (u64)(resource_size_t)base != base) {
+ printk(KERN_DEBUG "sysfb: inaccessible VRAM base\n");
+ 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 */
@@ -81,8 +95,8 @@ __init int create_simplefb(const struct screen_info *si,
memset(&res, 0, sizeof(res));
res.flags = IORESOURCE_MEM | IORESOURCE_BUSY;
res.name = simplefb_resname;
- res.start = si->lfb_base;
- res.end = si->lfb_base + len - 1;
+ res.start = base;
+ res.end = res.start + len - 1;
if (res.end <= res.start)
return -EINVAL;
--
2.10.2
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 <[email protected]>
---
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
On Tue, Nov 15, 2016 at 1:01 PM, David Herrmann <[email protected]> wrote:
> The screen_info object was extended to support 64bit lfb_base addresses
> in:
>
> commit ae2ee627dc87a70910de91b791b3cd0e9c6facdd
> Author: Matt Fleming <[email protected]>
> Date: Tue Aug 25 16:32:55 2015 +0100
>
> efifb: Add support for 64-bit frame buffer addresses
>
> However, the x86 simple-framebuffer setup code never made use of it. Fix
> it to properly assemble and verify the lfb_base before advertising
> simple-framebuffer devices.
>
> In particular, this means if VIDEO_CAPABILITY_64BIT_BASE is set, the
> screen_info->ext_lfb_base field will contain the upper 32bit of the
> actual lfb_base. Make sure the address is not 0 (i.e., unset), as well as
> does not overflow the physical address type.
>
> Signed-off-by: David Herrmann <[email protected]>
Reviewed-by: Tom Gundersen <[email protected]>
> ---
> arch/x86/kernel/sysfb_simplefb.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/sysfb_simplefb.c b/arch/x86/kernel/sysfb_simplefb.c
> index 764a29f..35b8641 100644
> --- a/arch/x86/kernel/sysfb_simplefb.c
> +++ b/arch/x86/kernel/sysfb_simplefb.c
> @@ -67,6 +67,20 @@ __init int create_simplefb(const struct screen_info *si,
> struct platform_device *pd;
> struct resource res;
> unsigned long len;
> + u64 base;
> +
> + /*
> + * If the 64BIT_BASE capability is set, ext_lfb_base will contain the
> + * upper half of the base address. Assemble the address, then make sure
> + * it is valid and we can actually access it.
> + */
> + base = si->lfb_base;
> + if (si->capabilities & VIDEO_CAPABILITY_64BIT_BASE)
> + base |= (u64)si->ext_lfb_base << 32;
> + if (!base || (u64)(resource_size_t)base != base) {
> + printk(KERN_DEBUG "sysfb: inaccessible VRAM base\n");
> + 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 */
> @@ -81,8 +95,8 @@ __init int create_simplefb(const struct screen_info *si,
> memset(&res, 0, sizeof(res));
> res.flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> res.name = simplefb_resname;
> - res.start = si->lfb_base;
> - res.end = si->lfb_base + len - 1;
> + res.start = base;
> + res.end = res.start + len - 1;
> if (res.end <= res.start)
> return -EINVAL;
>
> --
> 2.10.2
>
On Tue, Nov 15, 2016 at 1:01 PM, David Herrmann <[email protected]> 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 <[email protected]>
Reviewed-by: Tom Gundersen <[email protected]>
> ---
> 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
>
Commit-ID: 9164b4ceb7b492a77c7fe770a4b9d1375c9cd45a
Gitweb: http://git.kernel.org/tip/9164b4ceb7b492a77c7fe770a4b9d1375c9cd45a
Author: David Herrmann <[email protected]>
AuthorDate: Tue, 15 Nov 2016 13:01:57 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 16 Nov 2016 09:38:22 +0100
x86/sysfb: Add support for 64bit EFI lfb_base
The screen_info object was extended to support 64-bit lfb_base addresses
in:
ae2ee627dc87 ("efifb: Add support for 64-bit frame buffer addresses")
However, the x86 simple-framebuffer setup code never made use of it. Fix
it to properly assemble and verify the lfb_base before advertising
simple-framebuffer devices.
In particular, this means if VIDEO_CAPABILITY_64BIT_BASE is set, the
screen_info->ext_lfb_base field will contain the upper 32bit of the
actual lfb_base. Make sure the address is not 0 (i.e., unset), as well as
does not overflow the physical address type.
Signed-off-by: David Herrmann <[email protected]>
Acked-by: Thomas Gleixner <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Matt Fleming <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Tom Gundersen <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/sysfb_simplefb.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/sysfb_simplefb.c b/arch/x86/kernel/sysfb_simplefb.c
index 764a29f..35b8641 100644
--- a/arch/x86/kernel/sysfb_simplefb.c
+++ b/arch/x86/kernel/sysfb_simplefb.c
@@ -67,6 +67,20 @@ __init int create_simplefb(const struct screen_info *si,
struct platform_device *pd;
struct resource res;
unsigned long len;
+ u64 base;
+
+ /*
+ * If the 64BIT_BASE capability is set, ext_lfb_base will contain the
+ * upper half of the base address. Assemble the address, then make sure
+ * it is valid and we can actually access it.
+ */
+ base = si->lfb_base;
+ if (si->capabilities & VIDEO_CAPABILITY_64BIT_BASE)
+ base |= (u64)si->ext_lfb_base << 32;
+ if (!base || (u64)(resource_size_t)base != base) {
+ printk(KERN_DEBUG "sysfb: inaccessible VRAM base\n");
+ 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 */
@@ -81,8 +95,8 @@ __init int create_simplefb(const struct screen_info *si,
memset(&res, 0, sizeof(res));
res.flags = IORESOURCE_MEM | IORESOURCE_BUSY;
res.name = simplefb_resname;
- res.start = si->lfb_base;
- res.end = si->lfb_base + len - 1;
+ res.start = base;
+ res.end = res.start + len - 1;
if (res.end <= res.start)
return -EINVAL;
Commit-ID: f96acec8c8020807429d21324547f4b904c37177
Gitweb: http://git.kernel.org/tip/f96acec8c8020807429d21324547f4b904c37177
Author: David Herrmann <[email protected]>
AuthorDate: Tue, 15 Nov 2016 13:01:58 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 16 Nov 2016 09:38:23 +0100
x86/sysfb: Fix lfb_size calculation
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 <[email protected]>
Acked-by: Thomas Gleixner <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Matt Fleming <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Tom Gundersen <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
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;