* [PATCH v3 0/1] Fixes for large framebuffer, placed above 4GB @ 2019-05-16 14:07 ` Marek Marczykowski-Górecki 0 siblings, 0 replies; 12+ messages in thread From: Marek Marczykowski-Górecki @ 2019-05-16 14:07 UTC (permalink / raw) To: xen-devel Cc: Olaf Hering, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Marek Marczykowski-Górecki, Tim Deegan, Julien Grall, Jan Beulich A bunch of fixes for booting Xen on Thinkpad P52, through grub2-efi + multiboot2. Most of them can be applied independently. Changes in v3: - updated "xen: fix handling framebuffer located above 4GB" - dropped already applied patches --- cc: Andrew Cooper <andrew.cooper3@citrix.com> cc: George Dunlap <George.Dunlap@eu.citrix.com> cc: Ian Jackson <ian.jackson@eu.citrix.com> cc: Jan Beulich <jbeulich@suse.com> cc: Julien Grall <julien.grall@arm.com> cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> cc: Stefano Stabellini <sstabellini@kernel.org> cc: Tim Deegan <tim@xen.org> cc: Wei Liu <wei.liu2@citrix.com> cc: Olaf Hering <olaf@aepfle.de> Marek Marczykowski-Górecki (1): xen: fix handling framebuffer located above 4GB xen/arch/x86/efi/efi-boot.h | 1 + xen/drivers/video/vesa.c | 14 +++++++++----- xen/include/public/xen-compat.h | 2 +- xen/include/public/xen.h | 5 +++++ 4 files changed, 16 insertions(+), 6 deletions(-) base-commit: 9dc8043ba18409e7a2057d8cccbf0ddaff71eb7e -- git-series 0.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [Xen-devel] [PATCH v3 0/1] Fixes for large framebuffer, placed above 4GB @ 2019-05-16 14:07 ` Marek Marczykowski-Górecki 0 siblings, 0 replies; 12+ messages in thread From: Marek Marczykowski-Górecki @ 2019-05-16 14:07 UTC (permalink / raw) To: xen-devel Cc: Olaf Hering, Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Marek Marczykowski-Górecki, Tim Deegan, Julien Grall, Jan Beulich A bunch of fixes for booting Xen on Thinkpad P52, through grub2-efi + multiboot2. Most of them can be applied independently. Changes in v3: - updated "xen: fix handling framebuffer located above 4GB" - dropped already applied patches --- cc: Andrew Cooper <andrew.cooper3@citrix.com> cc: George Dunlap <George.Dunlap@eu.citrix.com> cc: Ian Jackson <ian.jackson@eu.citrix.com> cc: Jan Beulich <jbeulich@suse.com> cc: Julien Grall <julien.grall@arm.com> cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> cc: Stefano Stabellini <sstabellini@kernel.org> cc: Tim Deegan <tim@xen.org> cc: Wei Liu <wei.liu2@citrix.com> cc: Olaf Hering <olaf@aepfle.de> Marek Marczykowski-Górecki (1): xen: fix handling framebuffer located above 4GB xen/arch/x86/efi/efi-boot.h | 1 + xen/drivers/video/vesa.c | 14 +++++++++----- xen/include/public/xen-compat.h | 2 +- xen/include/public/xen.h | 5 +++++ 4 files changed, 16 insertions(+), 6 deletions(-) base-commit: 9dc8043ba18409e7a2057d8cccbf0ddaff71eb7e -- git-series 0.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 1/1] xen: fix handling framebuffer located above 4GB @ 2019-05-16 14:07 ` Marek Marczykowski-Górecki 0 siblings, 0 replies; 12+ messages in thread From: Marek Marczykowski-Górecki @ 2019-05-16 14:07 UTC (permalink / raw) To: xen-devel Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Marek Marczykowski-Górecki, Tim Deegan, Julien Grall, Jan Beulich, Roger Pau Monné On some machines (for example Thinkpad P52), UEFI GOP reports framebuffer located above 4GB (0x4000000000 on that machine). This address does not fit in {xen,dom0}_vga_console_info.u.vesa_lfb.lfb_base field, which is 32bit. The overflow here cause all kind of memory corruption when anything tries to write something on the screen, starting with zeroing the whole framebuffer in vesa_init(). Fix this similar to how it's done in Linux: add ext_lfb_base field at the end of the structure, to hold upper 32bits of the address. Since the field is added at the end of the structure, it will work with older Linux versions too (other than using possibly truncated address - no worse than without this change). Thanks to ABI containing size of the structure (start_info.console.dom0.info_size), Linux can detect when this field is present and use it appropriately then. Since this change public interface and use __XEN_INTERFACE_VERSION__, bump __XEN_LATEST_INTERFACE_VERSION__. Note: if/when backporting this change to Xen <= 4.12, #if in xen.h needs to be extended with " || defined(__XEN__)". Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2: - guard ext_lfb_base with #if __XEN_INTERFACE_VERSION__, but always include whe building Xen itself - add a helper function for lfb_base Changes in v3: - add padding field - add parentheses around << - unwrap format string per coding style, restore 0x - drop #if defined(__XEN__) needed only in backport - bump __XEN_LATEST_INTERFACE_VERSION__ --- xen/arch/x86/efi/efi-boot.h | 1 + xen/drivers/video/vesa.c | 14 +++++++++----- xen/include/public/xen-compat.h | 2 +- xen/include/public/xen.h | 5 +++++ 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index 5789d2c..7a13a30 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -550,6 +550,7 @@ static void __init efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, vga_console_info.u.vesa_lfb.bytes_per_line = (mode_info->PixelsPerScanLine * bpp + 7) >> 3; vga_console_info.u.vesa_lfb.lfb_base = gop->Mode->FrameBufferBase; + vga_console_info.u.vesa_lfb.ext_lfb_base = gop->Mode->FrameBufferBase >> 32; vga_console_info.u.vesa_lfb.lfb_size = (gop->Mode->FrameBufferSize + 0xffff) >> 16; } diff --git a/xen/drivers/video/vesa.c b/xen/drivers/video/vesa.c index 26d4962..9cf4bad 100644 --- a/xen/drivers/video/vesa.c +++ b/xen/drivers/video/vesa.c @@ -40,6 +40,11 @@ static int __init parse_font_height(const char *s) } custom_param("font", parse_font_height); +static inline paddr_t lfb_base(void) +{ + return (paddr_t)((vlfb_info.ext_lfb_base) << 32) | vlfb_info.lfb_base; +} + void __init vesa_early_init(void) { unsigned int vram_vmode; @@ -97,15 +102,14 @@ void __init vesa_init(void) lfbp.text_columns = vlfb_info.width / font->width; lfbp.text_rows = vlfb_info.height / font->height; - lfbp.lfb = lfb = ioremap(vlfb_info.lfb_base, vram_remap); + lfbp.lfb = lfb = ioremap(lfb_base(), vram_remap); if ( !lfb ) return; memset(lfb, 0, vram_remap); - printk(XENLOG_INFO "vesafb: framebuffer at %#x, mapped to 0x%p, " - "using %uk, total %uk\n", - vlfb_info.lfb_base, lfb, + printk(XENLOG_INFO "vesafb: framebuffer at 0x%" PRIpaddr ", mapped to 0x%p, using %uk, total %uk\n", + lfb_base(), lfb, vram_remap >> 10, vram_total >> 10); printk(XENLOG_INFO "vesafb: mode is %dx%dx%u, linelength=%d, font %ux%u\n", vlfb_info.width, vlfb_info.height, @@ -167,7 +171,7 @@ void __init vesa_mtrr_init(void) /* Try and find a power of two to add */ do { - rc = mtrr_add(vlfb_info.lfb_base, size_total, type, 1); + rc = mtrr_add(lfb_base(), size_total, type, 1); size_total >>= 1; } while ( (size_total >= PAGE_SIZE) && (rc == -EINVAL) ); } diff --git a/xen/include/public/xen-compat.h b/xen/include/public/xen-compat.h index 6fabca1..6708132 100644 --- a/xen/include/public/xen-compat.h +++ b/xen/include/public/xen-compat.h @@ -27,7 +27,7 @@ #ifndef __XEN_PUBLIC_XEN_COMPAT_H__ #define __XEN_PUBLIC_XEN_COMPAT_H__ -#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040a00 +#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040d00 #if defined(__XEN__) || defined(__XEN_TOOLS__) /* Xen is built with matching headers and implements the latest interface. */ diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index ccdffc0..4676521 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -923,6 +923,11 @@ typedef struct dom0_vga_console_info { /* Mode attributes (offset 0x0, VESA command 0x4f01). */ uint16_t mode_attrs; #endif +#if __XEN_INTERFACE_VERSION__ >= 0x00040d00 + uint16_t _pad; + /* high 32 bits of lfb_base */ + uint32_t ext_lfb_base; +#endif } vesa_lfb; } u; } dom0_vga_console_info_t; -- git-series 0.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Xen-devel] [PATCH v3 1/1] xen: fix handling framebuffer located above 4GB @ 2019-05-16 14:07 ` Marek Marczykowski-Górecki 0 siblings, 0 replies; 12+ messages in thread From: Marek Marczykowski-Górecki @ 2019-05-16 14:07 UTC (permalink / raw) To: xen-devel Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Marek Marczykowski-Górecki, Tim Deegan, Julien Grall, Jan Beulich, Roger Pau Monné On some machines (for example Thinkpad P52), UEFI GOP reports framebuffer located above 4GB (0x4000000000 on that machine). This address does not fit in {xen,dom0}_vga_console_info.u.vesa_lfb.lfb_base field, which is 32bit. The overflow here cause all kind of memory corruption when anything tries to write something on the screen, starting with zeroing the whole framebuffer in vesa_init(). Fix this similar to how it's done in Linux: add ext_lfb_base field at the end of the structure, to hold upper 32bits of the address. Since the field is added at the end of the structure, it will work with older Linux versions too (other than using possibly truncated address - no worse than without this change). Thanks to ABI containing size of the structure (start_info.console.dom0.info_size), Linux can detect when this field is present and use it appropriately then. Since this change public interface and use __XEN_INTERFACE_VERSION__, bump __XEN_LATEST_INTERFACE_VERSION__. Note: if/when backporting this change to Xen <= 4.12, #if in xen.h needs to be extended with " || defined(__XEN__)". Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2: - guard ext_lfb_base with #if __XEN_INTERFACE_VERSION__, but always include whe building Xen itself - add a helper function for lfb_base Changes in v3: - add padding field - add parentheses around << - unwrap format string per coding style, restore 0x - drop #if defined(__XEN__) needed only in backport - bump __XEN_LATEST_INTERFACE_VERSION__ --- xen/arch/x86/efi/efi-boot.h | 1 + xen/drivers/video/vesa.c | 14 +++++++++----- xen/include/public/xen-compat.h | 2 +- xen/include/public/xen.h | 5 +++++ 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index 5789d2c..7a13a30 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -550,6 +550,7 @@ static void __init efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, vga_console_info.u.vesa_lfb.bytes_per_line = (mode_info->PixelsPerScanLine * bpp + 7) >> 3; vga_console_info.u.vesa_lfb.lfb_base = gop->Mode->FrameBufferBase; + vga_console_info.u.vesa_lfb.ext_lfb_base = gop->Mode->FrameBufferBase >> 32; vga_console_info.u.vesa_lfb.lfb_size = (gop->Mode->FrameBufferSize + 0xffff) >> 16; } diff --git a/xen/drivers/video/vesa.c b/xen/drivers/video/vesa.c index 26d4962..9cf4bad 100644 --- a/xen/drivers/video/vesa.c +++ b/xen/drivers/video/vesa.c @@ -40,6 +40,11 @@ static int __init parse_font_height(const char *s) } custom_param("font", parse_font_height); +static inline paddr_t lfb_base(void) +{ + return (paddr_t)((vlfb_info.ext_lfb_base) << 32) | vlfb_info.lfb_base; +} + void __init vesa_early_init(void) { unsigned int vram_vmode; @@ -97,15 +102,14 @@ void __init vesa_init(void) lfbp.text_columns = vlfb_info.width / font->width; lfbp.text_rows = vlfb_info.height / font->height; - lfbp.lfb = lfb = ioremap(vlfb_info.lfb_base, vram_remap); + lfbp.lfb = lfb = ioremap(lfb_base(), vram_remap); if ( !lfb ) return; memset(lfb, 0, vram_remap); - printk(XENLOG_INFO "vesafb: framebuffer at %#x, mapped to 0x%p, " - "using %uk, total %uk\n", - vlfb_info.lfb_base, lfb, + printk(XENLOG_INFO "vesafb: framebuffer at 0x%" PRIpaddr ", mapped to 0x%p, using %uk, total %uk\n", + lfb_base(), lfb, vram_remap >> 10, vram_total >> 10); printk(XENLOG_INFO "vesafb: mode is %dx%dx%u, linelength=%d, font %ux%u\n", vlfb_info.width, vlfb_info.height, @@ -167,7 +171,7 @@ void __init vesa_mtrr_init(void) /* Try and find a power of two to add */ do { - rc = mtrr_add(vlfb_info.lfb_base, size_total, type, 1); + rc = mtrr_add(lfb_base(), size_total, type, 1); size_total >>= 1; } while ( (size_total >= PAGE_SIZE) && (rc == -EINVAL) ); } diff --git a/xen/include/public/xen-compat.h b/xen/include/public/xen-compat.h index 6fabca1..6708132 100644 --- a/xen/include/public/xen-compat.h +++ b/xen/include/public/xen-compat.h @@ -27,7 +27,7 @@ #ifndef __XEN_PUBLIC_XEN_COMPAT_H__ #define __XEN_PUBLIC_XEN_COMPAT_H__ -#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040a00 +#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040d00 #if defined(__XEN__) || defined(__XEN_TOOLS__) /* Xen is built with matching headers and implements the latest interface. */ diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index ccdffc0..4676521 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -923,6 +923,11 @@ typedef struct dom0_vga_console_info { /* Mode attributes (offset 0x0, VESA command 0x4f01). */ uint16_t mode_attrs; #endif +#if __XEN_INTERFACE_VERSION__ >= 0x00040d00 + uint16_t _pad; + /* high 32 bits of lfb_base */ + uint32_t ext_lfb_base; +#endif } vesa_lfb; } u; } dom0_vga_console_info_t; -- git-series 0.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3 1/1] xen: fix handling framebuffer located above 4GB @ 2019-05-16 14:59 ` Jan Beulich 0 siblings, 0 replies; 12+ messages in thread From: Jan Beulich @ 2019-05-16 14:59 UTC (permalink / raw) To: Marek Marczykowski Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall, xen-devel, Roger Pau Monne >>> On 16.05.19 at 16:07, <marmarek@invisiblethingslab.com> wrote: > --- a/xen/drivers/video/vesa.c > +++ b/xen/drivers/video/vesa.c > @@ -40,6 +40,11 @@ static int __init parse_font_height(const char *s) > } > custom_param("font", parse_font_height); > > +static inline paddr_t lfb_base(void) > +{ > + return (paddr_t)((vlfb_info.ext_lfb_base) << 32) | vlfb_info.lfb_base; I'm afraid you've misplaced the parentheses here. I wonder if this has worked for you at all. return ((paddr_t)vlfb_info.ext_lfb_base << 32) | vlfb_info.lfb_base; > --- a/xen/include/public/xen.h > +++ b/xen/include/public/xen.h > @@ -923,6 +923,11 @@ typedef struct dom0_vga_console_info { > /* Mode attributes (offset 0x0, VESA command 0x4f01). */ > uint16_t mode_attrs; > #endif > +#if __XEN_INTERFACE_VERSION__ >= 0x00040d00 > + uint16_t _pad; > + /* high 32 bits of lfb_base */ > + uint32_t ext_lfb_base; > +#endif Strictly speaking the padding field belongs ahead of the earlier #endif. Both aspects can be fixed while committing, but confirmation on the first wrt it working for you would still be nice. With them in place 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] 12+ messages in thread
* Re: [Xen-devel] [PATCH v3 1/1] xen: fix handling framebuffer located above 4GB @ 2019-05-16 14:59 ` Jan Beulich 0 siblings, 0 replies; 12+ messages in thread From: Jan Beulich @ 2019-05-16 14:59 UTC (permalink / raw) To: Marek Marczykowski Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall, xen-devel, Roger Pau Monne >>> On 16.05.19 at 16:07, <marmarek@invisiblethingslab.com> wrote: > --- a/xen/drivers/video/vesa.c > +++ b/xen/drivers/video/vesa.c > @@ -40,6 +40,11 @@ static int __init parse_font_height(const char *s) > } > custom_param("font", parse_font_height); > > +static inline paddr_t lfb_base(void) > +{ > + return (paddr_t)((vlfb_info.ext_lfb_base) << 32) | vlfb_info.lfb_base; I'm afraid you've misplaced the parentheses here. I wonder if this has worked for you at all. return ((paddr_t)vlfb_info.ext_lfb_base << 32) | vlfb_info.lfb_base; > --- a/xen/include/public/xen.h > +++ b/xen/include/public/xen.h > @@ -923,6 +923,11 @@ typedef struct dom0_vga_console_info { > /* Mode attributes (offset 0x0, VESA command 0x4f01). */ > uint16_t mode_attrs; > #endif > +#if __XEN_INTERFACE_VERSION__ >= 0x00040d00 > + uint16_t _pad; > + /* high 32 bits of lfb_base */ > + uint32_t ext_lfb_base; > +#endif Strictly speaking the padding field belongs ahead of the earlier #endif. Both aspects can be fixed while committing, but confirmation on the first wrt it working for you would still be nice. With them in place 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] 12+ messages in thread
* Re: [PATCH v3 1/1] xen: fix handling framebuffer located above 4GB @ 2019-05-16 15:45 ` Marek Marczykowski 0 siblings, 0 replies; 12+ messages in thread From: Marek Marczykowski @ 2019-05-16 15:45 UTC (permalink / raw) To: Jan Beulich Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall, xen-devel, Roger Pau Monne [-- Attachment #1.1: Type: text/plain, Size: 1713 bytes --] On Thu, May 16, 2019 at 08:59:36AM -0600, Jan Beulich wrote: > >>> On 16.05.19 at 16:07, <marmarek@invisiblethingslab.com> wrote: > > --- a/xen/drivers/video/vesa.c > > +++ b/xen/drivers/video/vesa.c > > @@ -40,6 +40,11 @@ static int __init parse_font_height(const char *s) > > } > > custom_param("font", parse_font_height); > > > > +static inline paddr_t lfb_base(void) > > +{ > > + return (paddr_t)((vlfb_info.ext_lfb_base) << 32) | vlfb_info.lfb_base; > > I'm afraid you've misplaced the parentheses here. I wonder if > this has worked for you at all. Indeed it's messed up... > return ((paddr_t)vlfb_info.ext_lfb_base << 32) | vlfb_info.lfb_base; This works fine. > > --- a/xen/include/public/xen.h > > +++ b/xen/include/public/xen.h > > @@ -923,6 +923,11 @@ typedef struct dom0_vga_console_info { > > /* Mode attributes (offset 0x0, VESA command 0x4f01). */ > > uint16_t mode_attrs; > > #endif > > +#if __XEN_INTERFACE_VERSION__ >= 0x00040d00 > > + uint16_t _pad; And also compat checker don't like this name. I've changed it to "pad" (v4 upcoming). > > + /* high 32 bits of lfb_base */ > > + uint32_t ext_lfb_base; > > +#endif > > Strictly speaking the padding field belongs ahead of the earlier > #endif. > > Both aspects can be fixed while committing, but confirmation on > the first wrt it working for you would still be nice. With them in > place > Reviewed-by: Jan Beulich <jbeulich@suse.com> -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xen-devel] [PATCH v3 1/1] xen: fix handling framebuffer located above 4GB @ 2019-05-16 15:45 ` Marek Marczykowski 0 siblings, 0 replies; 12+ messages in thread From: Marek Marczykowski @ 2019-05-16 15:45 UTC (permalink / raw) To: Jan Beulich Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall, xen-devel, Roger Pau Monne [-- Attachment #1.1: Type: text/plain, Size: 1713 bytes --] On Thu, May 16, 2019 at 08:59:36AM -0600, Jan Beulich wrote: > >>> On 16.05.19 at 16:07, <marmarek@invisiblethingslab.com> wrote: > > --- a/xen/drivers/video/vesa.c > > +++ b/xen/drivers/video/vesa.c > > @@ -40,6 +40,11 @@ static int __init parse_font_height(const char *s) > > } > > custom_param("font", parse_font_height); > > > > +static inline paddr_t lfb_base(void) > > +{ > > + return (paddr_t)((vlfb_info.ext_lfb_base) << 32) | vlfb_info.lfb_base; > > I'm afraid you've misplaced the parentheses here. I wonder if > this has worked for you at all. Indeed it's messed up... > return ((paddr_t)vlfb_info.ext_lfb_base << 32) | vlfb_info.lfb_base; This works fine. > > --- a/xen/include/public/xen.h > > +++ b/xen/include/public/xen.h > > @@ -923,6 +923,11 @@ typedef struct dom0_vga_console_info { > > /* Mode attributes (offset 0x0, VESA command 0x4f01). */ > > uint16_t mode_attrs; > > #endif > > +#if __XEN_INTERFACE_VERSION__ >= 0x00040d00 > > + uint16_t _pad; And also compat checker don't like this name. I've changed it to "pad" (v4 upcoming). > > + /* high 32 bits of lfb_base */ > > + uint32_t ext_lfb_base; > > +#endif > > Strictly speaking the padding field belongs ahead of the earlier > #endif. > > Both aspects can be fixed while committing, but confirmation on > the first wrt it working for you would still be nice. With them in > place > Reviewed-by: Jan Beulich <jbeulich@suse.com> -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4] xen: fix handling framebuffer located above 4GB @ 2019-05-16 15:46 ` Marek Marczykowski-Górecki 0 siblings, 0 replies; 12+ messages in thread From: Marek Marczykowski-Górecki @ 2019-05-16 15:46 UTC (permalink / raw) To: xen-devel Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Marek Marczykowski-Górecki, Tim Deegan, Julien Grall, Jan Beulich, Roger Pau Monné On some machines (for example Thinkpad P52), UEFI GOP reports framebuffer located above 4GB (0x4000000000 on that machine). This address does not fit in {xen,dom0}_vga_console_info.u.vesa_lfb.lfb_base field, which is 32bit. The overflow here cause all kind of memory corruption when anything tries to write something on the screen, starting with zeroing the whole framebuffer in vesa_init(). Fix this similar to how it's done in Linux: add ext_lfb_base field at the end of the structure, to hold upper 32bits of the address. Since the field is added at the end of the structure, it will work with older Linux versions too (other than using possibly truncated address - no worse than without this change). Thanks to ABI containing size of the structure (start_info.console.dom0.info_size), Linux can detect when this field is present and use it appropriately then. Since this change public interface and use __XEN_INTERFACE_VERSION__, bump __XEN_LATEST_INTERFACE_VERSION__. Note: if/when backporting this change to Xen <= 4.12, #if in xen.h needs to be extended with " || defined(__XEN__)". Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2: - guard ext_lfb_base with #if __XEN_INTERFACE_VERSION__, but always include whe building Xen itself - add a helper function for lfb_base Changes in v3: - add padding field - add parentheses around << - unwrap format string per coding style, restore 0x - drop #if defined(__XEN__) needed only in backport - bump __XEN_LATEST_INTERFACE_VERSION__ Changes in v4: - fix parentheses - fix padding --- xen/arch/x86/efi/efi-boot.h | 1 + xen/drivers/video/vesa.c | 14 +++++++++----- xen/include/public/xen-compat.h | 2 +- xen/include/public/xen.h | 5 +++++ 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index 5789d2cb70..7a13a30bc0 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -550,6 +550,7 @@ static void __init efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, vga_console_info.u.vesa_lfb.bytes_per_line = (mode_info->PixelsPerScanLine * bpp + 7) >> 3; vga_console_info.u.vesa_lfb.lfb_base = gop->Mode->FrameBufferBase; + vga_console_info.u.vesa_lfb.ext_lfb_base = gop->Mode->FrameBufferBase >> 32; vga_console_info.u.vesa_lfb.lfb_size = (gop->Mode->FrameBufferSize + 0xffff) >> 16; } diff --git a/xen/drivers/video/vesa.c b/xen/drivers/video/vesa.c index 26d4962b0e..fd2cb1312d 100644 --- a/xen/drivers/video/vesa.c +++ b/xen/drivers/video/vesa.c @@ -40,6 +40,11 @@ static int __init parse_font_height(const char *s) } custom_param("font", parse_font_height); +static inline paddr_t lfb_base(void) +{ + return ((paddr_t)vlfb_info.ext_lfb_base << 32) | vlfb_info.lfb_base; +} + void __init vesa_early_init(void) { unsigned int vram_vmode; @@ -97,15 +102,14 @@ void __init vesa_init(void) lfbp.text_columns = vlfb_info.width / font->width; lfbp.text_rows = vlfb_info.height / font->height; - lfbp.lfb = lfb = ioremap(vlfb_info.lfb_base, vram_remap); + lfbp.lfb = lfb = ioremap(lfb_base(), vram_remap); if ( !lfb ) return; memset(lfb, 0, vram_remap); - printk(XENLOG_INFO "vesafb: framebuffer at %#x, mapped to 0x%p, " - "using %uk, total %uk\n", - vlfb_info.lfb_base, lfb, + printk(XENLOG_INFO "vesafb: framebuffer at 0x%" PRIpaddr ", mapped to 0x%p, using %uk, total %uk\n", + lfb_base(), lfb, vram_remap >> 10, vram_total >> 10); printk(XENLOG_INFO "vesafb: mode is %dx%dx%u, linelength=%d, font %ux%u\n", vlfb_info.width, vlfb_info.height, @@ -167,7 +171,7 @@ void __init vesa_mtrr_init(void) /* Try and find a power of two to add */ do { - rc = mtrr_add(vlfb_info.lfb_base, size_total, type, 1); + rc = mtrr_add(lfb_base(), size_total, type, 1); size_total >>= 1; } while ( (size_total >= PAGE_SIZE) && (rc == -EINVAL) ); } diff --git a/xen/include/public/xen-compat.h b/xen/include/public/xen-compat.h index 6fabca1889..6708132394 100644 --- a/xen/include/public/xen-compat.h +++ b/xen/include/public/xen-compat.h @@ -27,7 +27,7 @@ #ifndef __XEN_PUBLIC_XEN_COMPAT_H__ #define __XEN_PUBLIC_XEN_COMPAT_H__ -#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040a00 +#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040d00 #if defined(__XEN__) || defined(__XEN_TOOLS__) /* Xen is built with matching headers and implements the latest interface. */ diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index ccdffc0ad1..cb2917e74b 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -922,6 +922,11 @@ typedef struct dom0_vga_console_info { uint32_t gbl_caps; /* Mode attributes (offset 0x0, VESA command 0x4f01). */ uint16_t mode_attrs; + uint16_t pad; +#endif +#if __XEN_INTERFACE_VERSION__ >= 0x00040d00 + /* high 32 bits of lfb_base */ + uint32_t ext_lfb_base; #endif } vesa_lfb; } u; -- 2.17.2 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Xen-devel] [PATCH v4] xen: fix handling framebuffer located above 4GB @ 2019-05-16 15:46 ` Marek Marczykowski-Górecki 0 siblings, 0 replies; 12+ messages in thread From: Marek Marczykowski-Górecki @ 2019-05-16 15:46 UTC (permalink / raw) To: xen-devel Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Marek Marczykowski-Górecki, Tim Deegan, Julien Grall, Jan Beulich, Roger Pau Monné On some machines (for example Thinkpad P52), UEFI GOP reports framebuffer located above 4GB (0x4000000000 on that machine). This address does not fit in {xen,dom0}_vga_console_info.u.vesa_lfb.lfb_base field, which is 32bit. The overflow here cause all kind of memory corruption when anything tries to write something on the screen, starting with zeroing the whole framebuffer in vesa_init(). Fix this similar to how it's done in Linux: add ext_lfb_base field at the end of the structure, to hold upper 32bits of the address. Since the field is added at the end of the structure, it will work with older Linux versions too (other than using possibly truncated address - no worse than without this change). Thanks to ABI containing size of the structure (start_info.console.dom0.info_size), Linux can detect when this field is present and use it appropriately then. Since this change public interface and use __XEN_INTERFACE_VERSION__, bump __XEN_LATEST_INTERFACE_VERSION__. Note: if/when backporting this change to Xen <= 4.12, #if in xen.h needs to be extended with " || defined(__XEN__)". Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2: - guard ext_lfb_base with #if __XEN_INTERFACE_VERSION__, but always include whe building Xen itself - add a helper function for lfb_base Changes in v3: - add padding field - add parentheses around << - unwrap format string per coding style, restore 0x - drop #if defined(__XEN__) needed only in backport - bump __XEN_LATEST_INTERFACE_VERSION__ Changes in v4: - fix parentheses - fix padding --- xen/arch/x86/efi/efi-boot.h | 1 + xen/drivers/video/vesa.c | 14 +++++++++----- xen/include/public/xen-compat.h | 2 +- xen/include/public/xen.h | 5 +++++ 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index 5789d2cb70..7a13a30bc0 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -550,6 +550,7 @@ static void __init efi_arch_video_init(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop, vga_console_info.u.vesa_lfb.bytes_per_line = (mode_info->PixelsPerScanLine * bpp + 7) >> 3; vga_console_info.u.vesa_lfb.lfb_base = gop->Mode->FrameBufferBase; + vga_console_info.u.vesa_lfb.ext_lfb_base = gop->Mode->FrameBufferBase >> 32; vga_console_info.u.vesa_lfb.lfb_size = (gop->Mode->FrameBufferSize + 0xffff) >> 16; } diff --git a/xen/drivers/video/vesa.c b/xen/drivers/video/vesa.c index 26d4962b0e..fd2cb1312d 100644 --- a/xen/drivers/video/vesa.c +++ b/xen/drivers/video/vesa.c @@ -40,6 +40,11 @@ static int __init parse_font_height(const char *s) } custom_param("font", parse_font_height); +static inline paddr_t lfb_base(void) +{ + return ((paddr_t)vlfb_info.ext_lfb_base << 32) | vlfb_info.lfb_base; +} + void __init vesa_early_init(void) { unsigned int vram_vmode; @@ -97,15 +102,14 @@ void __init vesa_init(void) lfbp.text_columns = vlfb_info.width / font->width; lfbp.text_rows = vlfb_info.height / font->height; - lfbp.lfb = lfb = ioremap(vlfb_info.lfb_base, vram_remap); + lfbp.lfb = lfb = ioremap(lfb_base(), vram_remap); if ( !lfb ) return; memset(lfb, 0, vram_remap); - printk(XENLOG_INFO "vesafb: framebuffer at %#x, mapped to 0x%p, " - "using %uk, total %uk\n", - vlfb_info.lfb_base, lfb, + printk(XENLOG_INFO "vesafb: framebuffer at 0x%" PRIpaddr ", mapped to 0x%p, using %uk, total %uk\n", + lfb_base(), lfb, vram_remap >> 10, vram_total >> 10); printk(XENLOG_INFO "vesafb: mode is %dx%dx%u, linelength=%d, font %ux%u\n", vlfb_info.width, vlfb_info.height, @@ -167,7 +171,7 @@ void __init vesa_mtrr_init(void) /* Try and find a power of two to add */ do { - rc = mtrr_add(vlfb_info.lfb_base, size_total, type, 1); + rc = mtrr_add(lfb_base(), size_total, type, 1); size_total >>= 1; } while ( (size_total >= PAGE_SIZE) && (rc == -EINVAL) ); } diff --git a/xen/include/public/xen-compat.h b/xen/include/public/xen-compat.h index 6fabca1889..6708132394 100644 --- a/xen/include/public/xen-compat.h +++ b/xen/include/public/xen-compat.h @@ -27,7 +27,7 @@ #ifndef __XEN_PUBLIC_XEN_COMPAT_H__ #define __XEN_PUBLIC_XEN_COMPAT_H__ -#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040a00 +#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040d00 #if defined(__XEN__) || defined(__XEN_TOOLS__) /* Xen is built with matching headers and implements the latest interface. */ diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index ccdffc0ad1..cb2917e74b 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -922,6 +922,11 @@ typedef struct dom0_vga_console_info { uint32_t gbl_caps; /* Mode attributes (offset 0x0, VESA command 0x4f01). */ uint16_t mode_attrs; + uint16_t pad; +#endif +#if __XEN_INTERFACE_VERSION__ >= 0x00040d00 + /* high 32 bits of lfb_base */ + uint32_t ext_lfb_base; #endif } vesa_lfb; } u; -- 2.17.2 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4] xen: fix handling framebuffer located above 4GB @ 2019-05-16 16:02 ` Jan Beulich 0 siblings, 0 replies; 12+ messages in thread From: Jan Beulich @ 2019-05-16 16:02 UTC (permalink / raw) To: Marek Marczykowski Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall, xen-devel, Roger Pau Monne >>> On 16.05.19 at 17:46, <marmarek@invisiblethingslab.com> wrote: > On some machines (for example Thinkpad P52), UEFI GOP reports > framebuffer located above 4GB (0x4000000000 on that machine). This > address does not fit in {xen,dom0}_vga_console_info.u.vesa_lfb.lfb_base > field, which is 32bit. The overflow here cause all kind of memory > corruption when anything tries to write something on the screen, > starting with zeroing the whole framebuffer in vesa_init(). > > Fix this similar to how it's done in Linux: add ext_lfb_base field at > the end of the structure, to hold upper 32bits of the address. Since the > field is added at the end of the structure, it will work with older > Linux versions too (other than using possibly truncated address - no > worse than without this change). Thanks to ABI containing size of the > structure (start_info.console.dom0.info_size), Linux can detect when > this field is present and use it appropriately then. > > Since this change public interface and use __XEN_INTERFACE_VERSION__, > bump __XEN_LATEST_INTERFACE_VERSION__. > > Note: if/when backporting this change to Xen <= 4.12, #if in xen.h needs > to be extended with " || defined(__XEN__)". > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.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] 12+ messages in thread
* Re: [Xen-devel] [PATCH v4] xen: fix handling framebuffer located above 4GB @ 2019-05-16 16:02 ` Jan Beulich 0 siblings, 0 replies; 12+ messages in thread From: Jan Beulich @ 2019-05-16 16:02 UTC (permalink / raw) To: Marek Marczykowski Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan, Julien Grall, xen-devel, Roger Pau Monne >>> On 16.05.19 at 17:46, <marmarek@invisiblethingslab.com> wrote: > On some machines (for example Thinkpad P52), UEFI GOP reports > framebuffer located above 4GB (0x4000000000 on that machine). This > address does not fit in {xen,dom0}_vga_console_info.u.vesa_lfb.lfb_base > field, which is 32bit. The overflow here cause all kind of memory > corruption when anything tries to write something on the screen, > starting with zeroing the whole framebuffer in vesa_init(). > > Fix this similar to how it's done in Linux: add ext_lfb_base field at > the end of the structure, to hold upper 32bits of the address. Since the > field is added at the end of the structure, it will work with older > Linux versions too (other than using possibly truncated address - no > worse than without this change). Thanks to ABI containing size of the > structure (start_info.console.dom0.info_size), Linux can detect when > this field is present and use it appropriately then. > > Since this change public interface and use __XEN_INTERFACE_VERSION__, > bump __XEN_LATEST_INTERFACE_VERSION__. > > Note: if/when backporting this change to Xen <= 4.12, #if in xen.h needs > to be extended with " || defined(__XEN__)". > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.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] 12+ messages in thread
end of thread, other threads:[~2019-05-16 16:02 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-05-16 14:07 [PATCH v3 0/1] Fixes for large framebuffer, placed above 4GB Marek Marczykowski-Górecki 2019-05-16 14:07 ` [Xen-devel] " Marek Marczykowski-Górecki 2019-05-16 14:07 ` [PATCH v3 1/1] xen: fix handling framebuffer located " Marek Marczykowski-Górecki 2019-05-16 14:07 ` [Xen-devel] " Marek Marczykowski-Górecki 2019-05-16 14:59 ` Jan Beulich 2019-05-16 14:59 ` [Xen-devel] " Jan Beulich 2019-05-16 15:45 ` Marek Marczykowski 2019-05-16 15:45 ` [Xen-devel] " Marek Marczykowski 2019-05-16 15:46 ` [PATCH v4] " Marek Marczykowski-Górecki 2019-05-16 15:46 ` [Xen-devel] " Marek Marczykowski-Górecki 2019-05-16 16:02 ` Jan Beulich 2019-05-16 16:02 ` [Xen-devel] " Jan Beulich
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.