Skip to contentSkip to conversation

x86: Handle 64bit framebuffer memory address properly

PATCH Superseded Kairui Song 3 participants 5 messages Nov 23, 2018 – May 31, 2019

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 x86: Handle 64bit framebuffer memory address properly

In a EFI system, the frame buffer address is 64bit, so currently if the address is beyound 4G, kexec will set wrong address due to truncate.

Linux kernel commit ae2ee627dc87 ('efifb: Add support for 64-bit frame buffer addresses') added support for 64bit frame buffer address, an 'ext_lfb_base' field is added as the upper 32-bits of the frame buffer, and introduced a new capability flag 'VIDEO_TYPE_CAPABILITY_64BIT_BASE' to indicate if the extend field is used.

This patch adopts this change, set proper extent address and capability flag when the address is beyound 4G.

---
 include/x86/x86-linux.h           | 7 +++++--
 kexec/arch/i386/x86-linux-setup.c | 7 ++++++-
 2 files changed, 11 insertions(+), 3 deletions(-)
include/x86/x86-linux.h +5 -2
index 7834751..352ea02 100644
@@ -107,7 +107,10 @@ struct x86_linux_param_header {
 	uint16_t vesapm_seg;			/* 0x2e */
 	uint16_t vesapm_off;			/* 0x30 */
 	uint16_t pages;				/* 0x32 */
-	uint8_t  reserved4[12];			/* 0x34 -- 0x3f reserved for future expansion */
+	uint16_t vesa_attributes;		/* 0x34 */
+	uint32_t capabilities;			/* 0x36 */
+	uint32_t ext_lfb_base;			/* 0x3a */
+	uint8_t  reserved4[2];			/* 0x3e -- 0x3f reserved for future expansion */
 
 	struct apm_bios_info apm_bios_info;	/* 0x40 */
 	struct drive_info_struct drive_info;	/* 0x80 */
@@ -199,7 +202,7 @@ struct x86_linux_param_header {
 	struct 	edd_info eddbuf[EDDMAXNR];	/* 0xd00 */
 						/* 0xeec */
 #define COMMAND_LINE_SIZE 2048
-};
+} __attribute__((packed));
 
 struct x86_linux_faked_param_header {
 	struct x86_linux_param_header hdr;	/* 0x00 */
kexec/arch/i386/x86-linux-setup.c +6 -1
index 6cda12c..dc785a2 100644
@@ -158,10 +158,15 @@ static int setup_linux_vesafb(struct x86_linux_param_header *real_mode)
 	real_mode->lfb_width      = var.xres;
 	real_mode->lfb_height     = var.yres;
 	real_mode->lfb_depth      = var.bits_per_pixel;
-	real_mode->lfb_base       = fix.smem_start;
+	real_mode->lfb_base       = fix.smem_start & 0xffffffff;
 	real_mode->lfb_linelength = fix.line_length;
 	real_mode->vesapm_seg     = 0;
 
+	if (fix.smem_start > 0xffffffff) {
+		real_mode->ext_lfb_base = fix.smem_start >> 32;
+		real_mode->capabilities |= VIDEO_CAPABILITY_64BIT_BASE;
+	}
+
 	/* FIXME: better get size from the file returned by proc_iomem() */
 	real_mode->lfb_size       = (fix.smem_len + 65535) / 65536;
 	real_mode->pages          = (fix.smem_len + 4095) / 4096;
Signature · 7 lines
2.19.1


_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
#1 lore raw

Hi Kairui,

Quoted 50 lines · Kairui Song

In a EFI system, the frame buffer address is 64bit, so currently if the address is beyound 4G, kexec will set wrong address due to truncate.

Linux kernel commit ae2ee627dc87 ('efifb: Add support for 64-bit frame buffer addresses') added support for 64bit frame buffer address, an 'ext_lfb_base' field is added as the upper 32-bits of the frame buffer, and introduced a new capability flag 'VIDEO_TYPE_CAPABILITY_64BIT_BASE' to indicate if the extend field is used.

This patch adopts this change, set proper extent address and capability flag when the address is beyound 4G.

---
 include/x86/x86-linux.h           | 7 +++++--
 kexec/arch/i386/x86-linux-setup.c | 7 ++++++-
 2 files changed, 11 insertions(+), 3 deletions(-)
include/x86/x86-linux.h +5 -2
index 7834751..352ea02 100644
@@ -107,7 +107,10 @@ struct x86_linux_param_header {
 	uint16_t vesapm_seg;			/* 0x2e */
 	uint16_t vesapm_off;			/* 0x30 */
 	uint16_t pages;				/* 0x32 */
-	uint8_t  reserved4[12];			/* 0x34 -- 0x3f reserved for future expansion */
+	uint16_t vesa_attributes;		/* 0x34 */
+	uint32_t capabilities;			/* 0x36 */
+	uint32_t ext_lfb_base;			/* 0x3a */
+	uint8_t  reserved4[2];			/* 0x3e -- 0x3f reserved for future expansion */
 
 	struct apm_bios_info apm_bios_info;	/* 0x40 */
 	struct drive_info_struct drive_info;	/* 0x80 */
@@ -199,7 +202,7 @@ struct x86_linux_param_header {
 	struct 	edd_info eddbuf[EDDMAXNR];	/* 0xd00 */
 						/* 0xeec */
 #define COMMAND_LINE_SIZE 2048
-};
+} __attribute__((packed));
 
 struct x86_linux_faked_param_header {
 	struct x86_linux_param_header hdr;	/* 0x00 */
kexec/arch/i386/x86-linux-setup.c +0 -0
index 6cda12c..dc785a2 100644

kexec/arch/i386/x86-linux-setup.c static int setup_linux_vesafb(struct x86_linux_param_header *real_mode) · line 165

Quoted 9 lines
kexec/arch/i386/x86-linux-setup.c +2 -1
@@ -158,10 +158,15 @@ static int setup_linux_vesafb(struct x86_linux_param_header *real_mode)
 	real_mode->lfb_width      = var.xres;
 	real_mode->lfb_height     = var.yres;
 	real_mode->lfb_depth      = var.bits_per_pixel;
-	real_mode->lfb_base       = fix.smem_start;
+	real_mode->lfb_base       = fix.smem_start & 0xffffffff;
 	real_mode->lfb_linelength = fix.line_length;
 	real_mode->vesapm_seg     = 0;
 
+	if (fix.smem_start > 0xffffffff) {

This might be a problem for 32 bit case? Other than this I do not see other issues of this patch.

Quoted 15 lines
kexec/arch/i386/x86-linux-setup.c +4 -0
+		real_mode->ext_lfb_base = fix.smem_start >> 32;
+		real_mode->capabilities |= VIDEO_CAPABILITY_64BIT_BASE;
+	}
+
 	/* FIXME: better get size from the file returned by proc_iomem() */
 	real_mode->lfb_size       = (fix.smem_len + 65535) / 65536;
 	real_mode->pages          = (fix.smem_len + 4095) / 4096;
Signature · 7 lines
2.19.1


_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Thanks
Dave

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#2 lore raw

Hi Dave, thanks for the review.
It should be safe, "smem_start" is defined as an unsigned long and with 32bit it will always smaller than 0xffffffff, but to make it cleaner and safer I will update the
patch to use 0xffffffffUL instead.

Quoted 85 lines · Dave Young

Hi Kairui,

Quoted 50 lines · Kairui Song

In a EFI system, the frame buffer address is 64bit, so currently if the address is beyound 4G, kexec will set wrong address due to truncate.

Linux kernel commit ae2ee627dc87 ('efifb: Add support for 64-bit frame buffer addresses') added support for 64bit frame buffer address, an 'ext_lfb_base' field is added as the upper 32-bits of the frame buffer, and introduced a new capability flag 'VIDEO_TYPE_CAPABILITY_64BIT_BASE' to indicate if the extend field is used.

This patch adopts this change, set proper extent address and capability flag when the address is beyound 4G.

---
 include/x86/x86-linux.h           | 7 +++++--
 kexec/arch/i386/x86-linux-setup.c | 7 ++++++-
 2 files changed, 11 insertions(+), 3 deletions(-)
include/x86/x86-linux.h +5 -2
index 7834751..352ea02 100644
@@ -107,7 +107,10 @@ struct x86_linux_param_header {
      uint16_t vesapm_seg;                    /* 0x2e */
      uint16_t vesapm_off;                    /* 0x30 */
      uint16_t pages;                         /* 0x32 */
-     uint8_t  reserved4[12];                 /* 0x34 -- 0x3f reserved for future expansion */
+     uint16_t vesa_attributes;               /* 0x34 */
+     uint32_t capabilities;                  /* 0x36 */
+     uint32_t ext_lfb_base;                  /* 0x3a */
+     uint8_t  reserved4[2];                  /* 0x3e -- 0x3f reserved for future expansion */
 
      struct apm_bios_info apm_bios_info;     /* 0x40 */
      struct drive_info_struct drive_info;    /* 0x80 */
@@ -199,7 +202,7 @@ struct x86_linux_param_header {
      struct  edd_info eddbuf[EDDMAXNR];      /* 0xd00 */
                                              /* 0xeec */
 #define COMMAND_LINE_SIZE 2048
-};
+} __attribute__((packed));
 
 struct x86_linux_faked_param_header {
      struct x86_linux_param_header hdr;      /* 0x00 */
kexec/arch/i386/x86-linux-setup.c +0 -0
index 6cda12c..dc785a2 100644

kexec/arch/i386/x86-linux-setup.c static int setup_linux_vesafb(struct x86_linux_param_header *real_mode) · line 165

Quoted 9 lines
kexec/arch/i386/x86-linux-setup.c +2 -1
@@ -158,10 +158,15 @@ static int setup_linux_vesafb(struct x86_linux_param_header *real_mode)
      real_mode->lfb_width      = var.xres;
      real_mode->lfb_height     = var.yres;
      real_mode->lfb_depth      = var.bits_per_pixel;
-     real_mode->lfb_base       = fix.smem_start;
+     real_mode->lfb_base       = fix.smem_start & 0xffffffff;
      real_mode->lfb_linelength = fix.line_length;
      real_mode->vesapm_seg     = 0;
 
+     if (fix.smem_start > 0xffffffff) {

This might be a problem for 32 bit case? Other than this I do not see other issues of this patch.

Quoted 11 lines
kexec/arch/i386/x86-linux-setup.c +4 -1
+             real_mode->ext_lfb_base = fix.smem_start >> 32;
+             real_mode->capabilities |= VIDEO_CAPABILITY_64BIT_BASE;
+     }
+
      /* FIXME: better get size from the file returned by proc_iomem() */
      real_mode->lfb_size       = (fix.smem_len + 65535) / 65536;
      real_mode->pages          = (fix.smem_len + 4095) / 4096;
--

2.19.1

Quoted 4 lines

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Thanks
Dave

Signature · 7 lines
Best Regards,
Kairui Song

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
#3 lore raw
In reply to Kairui Song
Quoted 11 lines · Kairui Song

In a EFI system, the frame buffer address is 64bit, so currently if the address is beyound 4G, kexec will set wrong address due to truncate.

Linux kernel commit ae2ee627dc87 ('efifb: Add support for 64-bit frame buffer addresses') added support for 64bit frame buffer address, an 'ext_lfb_base' field is added as the upper 32-bits of the frame buffer, and introduced a new capability flag 'VIDEO_TYPE_CAPABILITY_64BIT_BASE' to indicate if the extend field is used.

Quoted 4 lines

This patch adopts this change, set proper extent address and capability flag when the address is beyound 4G.

Sorry for letting this slip through the cracks.
Please let me know if this is still relevant.

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#4 lore raw
Quoted 13 lines · Simon Horman
Quoted 11 lines · Kairui Song

In a EFI system, the frame buffer address is 64bit, so currently if the address is beyound 4G, kexec will set wrong address due to truncate.

Linux kernel commit ae2ee627dc87 ('efifb: Add support for 64-bit frame buffer addresses') added support for 64bit frame buffer address, an 'ext_lfb_base' field is added as the upper 32-bits of the frame buffer, and introduced a new capability flag 'VIDEO_TYPE_CAPABILITY_64BIT_BASE' to indicate if the extend field is used.

Quoted 7 lines
Quoted 4 lines

This patch adopts this change, set proper extent address and capability flag when the address is beyound 4G.

Sorry for letting this slip through the cracks.
Please let me know if this is still relevant.

Hi Simon, I checked kexec-tools repo and this patch is merged already, maybe you replied the wrong mail?

--
Best Regards,
Kairui Song

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec