Skip to contentSkip to conversation

x86,setup: add serial_console_port_base in boot_params

PATCH In review Yinghai Lu 4 participants 12 messages Jul 15, 2010 – Aug 3, 2010

Subsystems (4)
lore
Share & tools
lore

Yinghai Lu

author
#0 lore raw

PATCH 1/1 x86,setup: add serial_console_port_base in boot_params

to save the early_serial_base.

Also bootloader could fill that field, so setup code could reuse it.

decompress code could reuse early serial console from setup code.

---
 Documentation/x86/zero-page.txt  |    1 +
 arch/x86/boot/compressed/misc.c  |   25 +++++++++++++++++++++++++
 arch/x86/boot/tty.c              |    7 ++++++-
 arch/x86/include/asm/bootparam.h |    3 ++-
 4 files changed, 34 insertions(+), 2 deletions(-)
Documentation/x86/zero-page.txt +1 -0
Index: linux-2.6/Documentation/x86/zero-page.txt
===================================================================
@@ -12,6 +12,7 @@ Offset	Proto	Name		Meaning
 000/040	ALL	screen_info	Text mode or frame buffer information
 				(struct screen_info)
 040/014	ALL	apm_bios_info	APM BIOS information (struct apm_bios_info)
+054/002 ALL	serial_console_port_base early serial console preset
 058/008	ALL	tboot_addr      Physical address of tboot shared page
 060/010	ALL	ist_info	Intel SpeedStep (IST) BIOS support information
 				(struct ist_info)
arch/x86/boot/compressed/misc.c +25 -0
Index: linux-2.6/arch/x86/boot/compressed/misc.c
===================================================================
@@ -125,6 +125,11 @@ static void error(char *m);
  */
 static struct boot_params *real_mode;		/* Pointer to real-mode data */
 static int quiet;
+static int early_serial_base;
+
+#define XMTRDY          0x20
+#define TXR             0       /*  Transmit register (WRITE) */
+#define LSR             5       /*  Line Status               */
 
 void *memset(void *s, int c, size_t n);
 void *memcpy(void *dest, const void *src, size_t n);
@@ -170,6 +175,16 @@ static void scroll(void)
 		vidmem[i] = ' ';
 }
 
+static void serial_putchar(int ch)
+{
+	unsigned timeout = 0xffff;
+
+	while ((inb(early_serial_base + LSR) & XMTRDY) == 0 && --timeout)
+		cpu_relax();
+
+	outb(ch, early_serial_base + TXR);
+}
+
 static void __putstr(int error, const char *s)
 {
 	int x, y, pos;
@@ -179,6 +194,14 @@ static void __putstr(int error, const ch
 	if (!error)
 		return;
 #endif
+	if (early_serial_base) {
+		const char *str = s;
+		while (*str) {
+			if (*str == '\n')
+				serial_putchar('\r');
+			serial_putchar(*str++);
+		}
+	}
 
 	if (real_mode->screen_info.orig_video_mode == 0 &&
 	    lines == 0 && cols == 0)
@@ -319,6 +342,8 @@ asmlinkage void decompress_kernel(void *
 	lines = real_mode->screen_info.orig_video_lines;
 	cols = real_mode->screen_info.orig_video_cols;
 
+	early_serial_base = real_mode->serial_console_port_base;
+
 	free_mem_ptr     = heap;	/* Heap */
 	free_mem_end_ptr = heap + BOOT_HEAP_SIZE;
 
arch/x86/boot/tty.c +6 -1
Index: linux-2.6/arch/x86/boot/tty.c
===================================================================
@@ -266,8 +266,13 @@ static void parse_console_uart8250(void)
 
 void console_init(void)
 {
-	parse_earlyprintk();
+	early_serial_base = boot_params.serial_console_port_base;
+
+	if (!early_serial_base)
+		parse_earlyprintk();
 
 	if (!early_serial_base)
 		parse_console_uart8250();
+
+	boot_params.serial_console_port_base = early_serial_base;
 }
arch/x86/include/asm/bootparam.h +2 -1
Index: linux-2.6/arch/x86/include/asm/bootparam.h
===================================================================
@@ -93,7 +93,8 @@ struct efi_info {
 struct boot_params {
 	struct screen_info screen_info;			/* 0x000 */
 	struct apm_bios_info apm_bios_info;		/* 0x040 */
-	__u8  _pad2[4];					/* 0x054 */
+	__u16  serial_console_port_base;		/* 0x054 */
+	__u8  _pad2[2];					/* 0x056 */
 	__u64  tboot_addr;				/* 0x058 */
 	struct ist_info ist_info;			/* 0x060 */
 	__u8  _pad3[16];				/* 0x070 */
#1 lore raw
Quoted 9 lines · Yinghai Lu

to save the early_serial_base.

Also bootloader could fill that field, so setup code could reuse it.

decompress code could reuse early serial console from setup code.

What's the point with this over what we currently have?

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
author
#2 lore raw
Quoted 4 lines · H. Peter Anvin
Quoted 3 lines · Yinghai Lu

to save the early_serial_base.

Quoted 8 lines
Quoted 6 lines

Also bootloader could fill that field, so setup code could reuse it.

decompress code could reuse early serial console from setup code.

What's the point with this over what we currently have?

extend serial console coverage to boot/compressed/misc.c

before this one, we have cover in boot/*.c only.

Thanks

Yinghai

#3 lore raw
In reply to Yinghai Lu
Quoted 9 lines · Yinghai Lu

to save the early_serial_base.

Also bootloader could fill that field, so setup code could reuse it.

decompress code could reuse early serial console from setup code.

Hi Yinghai, sorry for delayed response. I like the idea much since it'll allow to log decompressor messages in case if someone would be changing exitsting code or developing new one, though this touches boot protocol so that grub/syslinux and friends will have to update their code as well which of course need some time.

-- Cyrill

#4 lore raw
Quoted 8 lines · Cyrill Gorcunov
Quoted 7 lines · Yinghai Lu

to save the early_serial_base.

Also bootloader could fill that field, so setup code could reuse it.

decompress code could reuse early serial console from setup code.

Quoted 9 lines
Quoted 2 lines

Hi Yinghai, sorry for delayed response. I like the idea much since it'll allow to log decompressor messages in case if someone would be changing exitsting code or developing new one, though this touches boot protocol so that grub/syslinux and friends will have to update their code as well which of course need some time.

No, this is the internal part of the boot protocol, so it's not an issue.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#5 lore raw
Quoted 12 lines · H. Peter Anvin
Quoted 11 lines · Cyrill Gorcunov
Quoted 9 lines · Yinghai Lu

to save the early_serial_base.

Also bootloader could fill that field, so setup code could reuse it.

decompress code could reuse early serial console from setup code.

Quoted 9 lines
Quoted 6 lines

Hi Yinghai, sorry for delayed response. I like the idea much since it'll allow to log decompressor messages in case if someone would be changing exitsting code or developing new one, though this touches boot protocol so that grub/syslinux and friends will have to update their code as well which of course need some time.

No, this is the internal part of the boot protocol, so it's not an issue.

Peter, I didn't mean any issue here, I meant that bootloaders don't know about this field yet and they will have to update own sources to pass port value at proper place of boot params. Or I miss something?

Quoted 2 lines

-hpa

-- Cyrill

#6 lore raw
Quoted 8 lines · Cyrill Gorcunov
Quoted 3 lines

No, this is the internal part of the boot protocol, so it's not an issue.

Peter, I didn't mean any issue here, I meant that bootloaders don't know about this field yet and they will have to update own sources to pass port value at proper place of boot params. Or I miss something?

Boot loaders that use the 16-bit entry point are unaffected.

Boot loaders which use the 32-bit entry point but properly clears the zero page simply will not have the feature.

Boot loaders which use the 32-bit entry point but doesn't clear the zero page are broken.

-hpa

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
#7 lore raw
Quoted 12 lines · H. Peter Anvin
Quoted 8 lines · Cyrill Gorcunov
Quoted 3 lines

No, this is the internal part of the boot protocol, so it's not an issue.

Peter, I didn't mean any issue here, I meant that bootloaders don't know about this field yet and they will have to update own sources to pass port value at proper place of boot params. Or I miss something?

Boot loaders that use the 16-bit entry point are unaffected.

Quoted 8 lines

Boot loaders which use the 32-bit entry point but properly clears the zero page simply will not have the feature.

Boot loaders which use the 32-bit entry point but doesn't clear the zero page are broken.

-hpa

Well, Peter, seems I messed boot params with setup header. So I don't see any user of this field then, hmm...

-- Cyrill

author
#8 lore raw
In reply to H. Peter Anvin
Quoted 10 lines · H. Peter Anvin
Quoted 8 lines · Cyrill Gorcunov
Quoted 3 lines

No, this is the internal part of the boot protocol, so it's not an issue.

Peter, I didn't mean any issue here, I meant that bootloaders don't know about this field yet and they will have to update own sources to pass port value at proper place of boot params. Or I miss something?

Quoted 8 lines

Boot loaders that use the 16-bit entry point are unaffected.

Boot loaders which use the 32-bit entry point but properly clears the zero page simply will not have the feature.

Boot loaders which use the 32-bit entry point but doesn't clear the zero page are broken.

can you if this one is right for kexec path?

Thanks

Yinghai

[PATCH] kexec-tools: pass serial console base for early console in kernel

when kexec is used with "console_serial", set serial_console_port_base in boot_params.

---
 doc/linux-i386-zero-page.txt      |    1 +
 include/x86/x86-linux.h           |    3 ++-
 kexec/arch/i386/kexec-x86.h       |    1 +
 kexec/arch/i386/x86-linux-setup.c |    4 ++++
 kexec/arch/x86_64/kexec-x86_64.c  |    9 +--------
 5 files changed, 9 insertions(+), 9 deletions(-)
doc/linux-i386-zero-page.txt +1 -0
Index: kexec-tools/doc/linux-i386-zero-page.txt
===================================================================
@@ -22,6 +22,7 @@ Offset	Type		Description
 			  0x90000 + contents of CL_OFFSET
 			(only taken, when CL_MAGIC = 0xA33F)
  0x40	20 bytes	struct apm_bios_info, APM_BIOS_INFO
+ 0x54   2 bytes		serial console base
  0x60	16 bytes	Intel SpeedStep (IST) BIOS support information
  0x80	16 bytes	hd0-disk-parameter from intvector 0x41
  0x90	16 bytes	hd1-disk-parameter from intvector 0x46
include/x86/x86-linux.h +2 -1
Index: kexec-tools/include/x86/x86-linux.h
===================================================================
@@ -40,7 +40,6 @@ struct apm_bios_info {
 	uint16_t cseg_len;	/* 0x4e */
 	uint16_t cseg_16_len;	/* 0x50 */
 	uint16_t dseg_len;	/* 0x52 */
-	uint8_t  reserved[44];	/* 0x54 */
 };
 
 /*
@@ -105,6 +104,8 @@ struct x86_linux_param_header {
 	uint8_t  reserved4[12];			/* 0x34 -- 0x3f reserved for future expansion */
 
 	struct apm_bios_info apm_bios_info;	/* 0x40 */
+	uint16_t  serial_console_port_base;	/* 0x54 */
+	uint8_t  reserved41[42];		/* 0x56 */
 	struct drive_info_struct drive_info;	/* 0x80 */
 	struct sys_desc_table sys_desc_table;	/* 0xa0 */
 	uint32_t alt_mem_k;			/* 0x1e0 */
kexec/arch/i386/x86-linux-setup.c +4 -0
Index: kexec-tools/kexec/arch/i386/x86-linux-setup.c
===================================================================
@@ -100,6 +100,9 @@ void setup_linux_bootloader_parameters(
 	cmdline_ptr = ((char *)real_mode) + cmdline_offset;
 	memcpy(cmdline_ptr, cmdline, cmdline_len);
 	cmdline_ptr[cmdline_len - 1] = '\0';
+
+	if (arch_options.console_serial)
+		real_mode->serial_console_port_base = arch_options.serial_base;
 }
 
 int setup_linux_vesafb(struct x86_linux_param_header *real_mode)
@@ -422,6 +425,7 @@ void setup_linux_system_parameters(struc
 
 	/* Default APM info */
 	memset(&real_mode->apm_bios_info, 0, sizeof(real_mode->apm_bios_info));
+
 	/* Default drive info */
 	memset(&real_mode->drive_info, 0, sizeof(real_mode->drive_info));
 	/* Default sysdesc table */
kexec/arch/i386/kexec-x86.h +1 -0
Index: kexec-tools/kexec/arch/i386/kexec-x86.h
===================================================================
@@ -51,6 +51,7 @@ struct arch_options_t {
 	uint8_t  	console_serial;
 	enum coretype	core_header_type;
 };
+extern struct arch_options_t arch_options;
 
 int multiboot_x86_probe(const char *buf, off_t len);
 int multiboot_x86_load(int argc, char **argv, const char *buf, off_t len,
kexec/arch/x86_64/kexec-x86_64.c +1 -8
Index: kexec-tools/kexec/arch/x86_64/kexec-x86_64.c
===================================================================
@@ -55,14 +55,7 @@ void arch_usage(void)
 		);
 }
 
-static struct {
-	uint8_t  reset_vga;
-	uint16_t serial_base;
-	uint32_t serial_baud;
-	uint8_t  console_vga;
-	uint8_t  console_serial;
-	int core_header_type;
-} arch_options = {
+struct arch_options_t arch_options = {
 	.reset_vga   = 0,
 	.serial_base = 0x3f8,
 	.serial_baud = 0,

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

#9 lore raw
Quoted 13 lines · Yinghai Lu
Quoted 12 lines · H. Peter Anvin
Quoted 8 lines · Cyrill Gorcunov
Quoted 3 lines

No, this is the internal part of the boot protocol, so it's not an issue.

Peter, I didn't mean any issue here, I meant that bootloaders don't know about this field yet and they will have to update own sources to pass port value at proper place of boot params. Or I miss something?

Boot loaders that use the 16-bit entry point are unaffected.

Quoted 7 lines
Quoted 6 lines

Boot loaders which use the 32-bit entry point but properly clears the zero page simply will not have the feature.

Boot loaders which use the 32-bit entry point but doesn't clear the zero page are broken.

can you if this one is right for kexec path?

I am walking out the door, but this seems like nonsense to me.

The kexec console_serial option today is a debugging feature for when you to debug the purgatory code. Only once in a bluemoon should it be useful, so I don't see why we would add it here. The kexec purgatory is silent by default.

Further I don't see why we would add something to the zero page when we have a perfectly good way to pass this information via the kernel command line. strstr and strtoul are trivial little functions so I don't see why anything would need to parse anything other than console= or early_printk=. The difference in code size is negligible.

This option in the zero page appears very fragile. There are a lot of dimensions that we are skipping. I admit we can detect most of them by reading the serial port. But we still have the assumption that the serial port is an 8259 serial port in i/o space and not mmio space.

*runs out the door before another silly email comes*

Eric

Quoted 102 lines

[PATCH] kexec-tools: pass serial console base for early console in kernel

when kexec is used with "console_serial", set serial_console_port_base in boot_params.

---
 doc/linux-i386-zero-page.txt      |    1 +
 include/x86/x86-linux.h           |    3 ++-
 kexec/arch/i386/kexec-x86.h       |    1 +
 kexec/arch/i386/x86-linux-setup.c |    4 ++++
 kexec/arch/x86_64/kexec-x86_64.c  |    9 +--------
 5 files changed, 9 insertions(+), 9 deletions(-)

Index: kexec-tools/doc/linux-i386-zero-page.txt
===================================================================

doc/linux-i386-zero-page.txt +1 -0
@@ -22,6 +22,7 @@ Offset	Type		Description
 			  0x90000 + contents of CL_OFFSET
 			(only taken, when CL_MAGIC = 0xA33F)
  0x40	20 bytes	struct apm_bios_info, APM_BIOS_INFO
+ 0x54   2 bytes		serial console base
  0x60	16 bytes	Intel SpeedStep (IST) BIOS support information
  0x80	16 bytes	hd0-disk-parameter from intvector 0x41
  0x90	16 bytes	hd1-disk-parameter from intvector 0x46

Index: kexec-tools/include/x86/x86-linux.h
===================================================================

include/x86/x86-linux.h +2 -1
@@ -40,7 +40,6 @@ struct apm_bios_info {
 	uint16_t cseg_len;	/* 0x4e */
 	uint16_t cseg_16_len;	/* 0x50 */
 	uint16_t dseg_len;	/* 0x52 */
-	uint8_t  reserved[44];	/* 0x54 */
 };
 
 /*
@@ -105,6 +104,8 @@ struct x86_linux_param_header {
 	uint8_t  reserved4[12];			/* 0x34 -- 0x3f reserved for future expansion */
 
 	struct apm_bios_info apm_bios_info;	/* 0x40 */
+	uint16_t  serial_console_port_base;	/* 0x54 */
+	uint8_t  reserved41[42];		/* 0x56 */
 	struct drive_info_struct drive_info;	/* 0x80 */
 	struct sys_desc_table sys_desc_table;	/* 0xa0 */
 	uint32_t alt_mem_k;			/* 0x1e0 */

Index: kexec-tools/kexec/arch/i386/x86-linux-setup.c
===================================================================

kexec/arch/i386/x86-linux-setup.c +4 -0
@@ -100,6 +100,9 @@ void setup_linux_bootloader_parameters(
 	cmdline_ptr = ((char *)real_mode) + cmdline_offset;
 	memcpy(cmdline_ptr, cmdline, cmdline_len);
 	cmdline_ptr[cmdline_len - 1] = '\0';
+
+	if (arch_options.console_serial)
+		real_mode->serial_console_port_base = arch_options.serial_base;
 }
 
 int setup_linux_vesafb(struct x86_linux_param_header *real_mode)
@@ -422,6 +425,7 @@ void setup_linux_system_parameters(struc
 
 	/* Default APM info */
 	memset(&real_mode->apm_bios_info, 0, sizeof(real_mode->apm_bios_info));
+
 	/* Default drive info */
 	memset(&real_mode->drive_info, 0, sizeof(real_mode->drive_info));
 	/* Default sysdesc table */

Index: kexec-tools/kexec/arch/i386/kexec-x86.h
===================================================================

kexec/arch/i386/kexec-x86.h +1 -0
@@ -51,6 +51,7 @@ struct arch_options_t {
 	uint8_t  	console_serial;
 	enum coretype	core_header_type;
 };
+extern struct arch_options_t arch_options;
 
 int multiboot_x86_probe(const char *buf, off_t len);
 int multiboot_x86_load(int argc, char **argv, const char *buf, off_t len,

Index: kexec-tools/kexec/arch/x86_64/kexec-x86_64.c
===================================================================

kexec/arch/x86_64/kexec-x86_64.c +1 -8
@@ -55,14 +55,7 @@ void arch_usage(void)
 		);
 }
 
-static struct {
-	uint8_t  reset_vga;
-	uint16_t serial_base;
-	uint32_t serial_baud;
-	uint8_t  console_vga;
-	uint8_t  console_serial;
-	int core_header_type;
-} arch_options = {
+struct arch_options_t arch_options = {
 	.reset_vga   = 0,
 	.serial_base = 0x3f8,
 	.serial_baud = 0,
author
#10 lore raw
Quoted 23 lines · Eric W. Biederman
Quoted 13 lines · Yinghai Lu
Quoted 12 lines · H. Peter Anvin
Quoted 8 lines · Cyrill Gorcunov
Quoted 3 lines

No, this is the internal part of the boot protocol, so it's not an issue.

Peter, I didn't mean any issue here, I meant that bootloaders don't know about this field yet and they will have to update own sources to pass port value at proper place of boot params. Or I miss something?

Boot loaders that use the 16-bit entry point are unaffected.

Quoted 7 lines
Quoted 6 lines

Boot loaders which use the 32-bit entry point but properly clears the zero page simply will not have the feature.

Boot loaders which use the 32-bit entry point but doesn't clear the zero page are broken.

can you if this one is right for kexec path?

Quoted 9 lines

I am walking out the door, but this seems like nonsense to me.

Further I don't see why we would add something to the zero page when we have a perfectly good way to pass this information via the kernel command line. strstr and strtoul are trivial little functions so I don't see why anything would need to parse anything other than console= or early_printk=. The difference in code size is negligible.

so you prefer to check command line for console info in arch/x86/boot/compressed/misc.c again?

that commandline is analyzed in arch/x86/boot/tty.c already.

Yinghai

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

#11 lore raw
Quoted 33 lines · Yinghai Lu
Quoted 23 lines · Eric W. Biederman
Quoted 13 lines · Yinghai Lu
Quoted 12 lines · H. Peter Anvin
Quoted 8 lines · Cyrill Gorcunov
Quoted 3 lines

No, this is the internal part of the boot protocol, so it's not an issue.

Peter, I didn't mean any issue here, I meant that bootloaders don't know about this field yet and they will have to update own sources to pass port value at proper place of boot params. Or I miss something?

Boot loaders that use the 16-bit entry point are unaffected.

Quoted 7 lines
Quoted 6 lines

Boot loaders which use the 32-bit entry point but properly clears the zero page simply will not have the feature.

Boot loaders which use the 32-bit entry point but doesn't clear the zero page are broken.

can you if this one is right for kexec path?

Quoted 9 lines

I am walking out the door, but this seems like nonsense to me.

Further I don't see why we would add something to the zero page when we have a perfectly good way to pass this information via the kernel command line. strstr and strtoul are trivial little functions so I don't see why anything would need to parse anything other than console= or early_printk=. The difference in code size is negligible.

Quoted 3 lines

so you prefer to check command line for console info in arch/x86/boot/compressed/misc.c again?

that commandline is analyzed in arch/x86/boot/tty.c already.

Instead of changing 2 or 3 bootloaders, waiting years for the change to propagate, and then trying to change users habits.

I definitely do.

Eric