Re: [PATCH] i386/linux: add support for ext_lfb_base

[Daniel Kiper]

On Wed, Jun 27, 2018 at 08:25:41PM +0530, Arindam Nath wrote:
> The EFI Graphics Output Protocol can return a 64-bit
> linear frame buffer address in some firmware/BIOS
> implementations. We currently only store the lower
> 32-bits in the lfb_base. This will eventually be
> passed to Linux kernel and the efifb driver will
> incorrectly interpret the framebuffer address as
> 32-bit address.
>
> The Linux kernel has already added support to handle
> 64-bit linear framebuffer address in the efifb driver
> since quite some time now.
>
> This patch adds the support for 64-bit linear frame
> buffer address in GRUB to address the above mentioned
> scenario.
>
> Signed-off-by: Arindam Nath <address@hidden>
> ---
>  grub-core/loader/i386/linux.c | 2 ++
>  include/grub/i386/linux.h     | 5 ++++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
> index 083f9417c..1644e8132 100644
> --- a/grub-core/loader/i386/linux.c
> +++ b/grub-core/loader/i386/linux.c
> @@ -306,6 +306,8 @@ grub_linux_setup_video (struct linux_kernel_params 
> *params)
>    params->lfb_line_len = mode_info.pitch;
>
>    params->lfb_base = (grub_size_t) framebuffer;

I think that you should add this:

#if defined (GRUB_MACHINE_EFI) && defined (__x86_64__)

> +  params->ext_lfb_base = (grub_size_t) (((grub_uint64_t)(grub_size_t) 
> framebuffer) >> 32);
> +  params->capabilities |= VIDEO_CAPABILITY_64BIT_BASE;

#endif

.. or at least #ifdef __x86_64__...

>    params->lfb_size = ALIGN_UP (params->lfb_line_len * params->lfb_height, 
> 65536);
>
>    params->red_mask_size = mode_info.red_mask_size;
> diff --git a/include/grub/i386/linux.h b/include/grub/i386/linux.h
> index da0ca3b83..e3f91e7c0 100644
> --- a/include/grub/i386/linux.h
> +++ b/include/grub/i386/linux.h
> @@ -188,8 +188,11 @@ struct linux_kernel_params
>    grub_uint16_t lfb_pages;           /* 32 */
>    grub_uint16_t vesa_attrib;         /* 34 */
>    grub_uint32_t capabilities;                /* 36 */
> +#define VIDEO_CAPABILITY_SKIP_QUIRKS (1 << 0)
> +#define VIDEO_CAPABILITY_64BIT_BASE  (1 << 1)        /* Frame buffer base is 
> 64-bit */

My I ask you to put this constants definitions at the
beginning of the file together with other constants?

Daniel