Re: [Xen-devel] [PATCH 4/5] xen: fix handling framebuffer located above 4GB

[Juergen Gross <jgross@xxxxxxxx>]

On 06/05/2019 16:50, Marek Marczykowski-Górecki 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.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> ---
>  xen/arch/x86/efi/efi-boot.h |  1 +
>  xen/drivers/video/vesa.c    | 15 +++++++++++----
>  xen/include/public/xen.h    |  2 ++
>  3 files changed, 14 insertions(+), 4 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 c92497e..f22cf7f 100644
> --- a/xen/drivers/video/vesa.c
> +++ b/xen/drivers/video/vesa.c
> @@ -84,6 +84,7 @@ void __init vesa_early_init(void)
>  void __init vesa_init(void)
>  {
>      struct lfb_prop lfbp;
> +    unsigned long lfb_base;
>  
>      if ( !font )
>          return;
> @@ -97,15 +98,17 @@ 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);
> +    lfb_base = vlfb_info.lfb_base;
> +    lfb_base |= (unsigned long)vlfb_info.ext_lfb_base << 32;
> +    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, "
> +    printk(XENLOG_INFO "vesafb: framebuffer at %#lx, mapped to 0x%p, "
>             "using %uk, total %uk\n",
> -           vlfb_info.lfb_base, lfb,
> +           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,
> @@ -152,6 +155,10 @@ void __init vesa_mtrr_init(void)
>          MTRR_TYPE_WRCOMB, MTRR_TYPE_WRTHROUGH };
>      unsigned int size_total;
>      int rc, type;
> +    unsigned long lfb_base;
> +
> +    lfb_base = vlfb_info.lfb_base;
> +    lfb_base |= (unsigned long)vlfb_info.ext_lfb_base << 32;
>  
>      if ( !lfb || (vesa_mtrr == 0) || (vesa_mtrr >= ARRAY_SIZE(mtrr_types)) )
>          return;
> @@ -167,7 +174,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.h b/xen/include/public/xen.h
> index ccdffc0..b0f0f7e 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -923,6 +923,8 @@ typedef struct dom0_vga_console_info {
>              /* Mode attributes (offset 0x0, VESA command 0x4f01). */
>              uint16_t mode_attrs;
>  #endif
> +            /* high 32 bits of lfb_base */
> +            uint32_t ext_lfb_base;

You will need to put this addition into an:

#if __XEN_INTERFACE_VERSION__ >= 0x00040d00
...
#endif

section (only available from Xen 4.13 onwards).


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel