Skip to contentSkip to conversation

x86_64 boot: Add linked list of struct setup_data

PATCH In review Huang, Ying 3 participants 8 messages Mar 27, 2008 – Mar 29, 2008 x86

lore
Share & tools
lore

Huang, Ying

author
#0 lore raw

PATCH 2/4 x86_64 boot: Add linked list of struct setup_data

This patch add a field of 64-bit physical pointer to NULL terminated single linked list of struct setup_data to real-mode kernel header. This is used as a more extensible boot parameters passing mechanism.

Signed-off-by Huang Ying <[email protected]>
---
 arch/x86/boot/header.S      |    6 +++++-
 arch/x86/kernel/head64.c    |   20 ++++++++++++++++++++
 arch/x86/kernel/setup_64.c  |   22 ++++++++++++++++++++++
 include/asm-x86/bootparam.h |   14 ++++++++++++++
 4 files changed, 61 insertions(+), 1 deletion(-)
include/asm-x86/bootparam.h +14 -0
@@ -9,6 +9,17 @@
 #include <asm/ist.h>
 #include <video/edid.h>
 
+/* setup data types */
+#define SETUP_NONE			0
+
+/* extensible setup data list node */
+struct setup_data {
+	u64 next;
+	u32 type;
+	u32 len;
+	u8 data[0];
+};
+
 struct setup_header {
 	__u8	setup_sects;
 	__u16	root_flags;

@@ -46,6 +57,9 @@ struct setup_header {
 	__u32	cmdline_size;
 	__u32	hardware_subarch;
 	__u64	hardware_subarch_data;
+	__u32	payload_offset;
+	__u32	payload_length;
+	__u64	setup_data;
 } __attribute__((packed));
 
 struct sys_desc_table {
arch/x86/boot/header.S +5 -1
@@ -120,7 +120,7 @@ _start:
 	# Part 2 of the header, from the old setup.S
 
 		.ascii	"HdrS"		# header signature
-		.word	0x0208		# header version number (>= 0x0105)
+		.word	0x0209		# header version number (>= 0x0105)
 					# or else old loadlin-1.5 will fail)
 		.globl realmode_swtch
 realmode_swtch:	.word	0, 0		# default_switch, SETUPSEG

@@ -227,6 +227,10 @@ hardware_subarch_data:	.quad 0
 payload_offset:		.long input_data
 payload_length:		.long input_data_end-input_data
 
+setup_data:		.quad 0			# 64-bit physical pointer to
+						# single linked list of
+						# struct setup_data
+
 # End of setup header #####################################################
 
 	.section ".inittext", "ax"
arch/x86/kernel/setup_64.c +22 -0
@@ -266,6 +266,26 @@ void __attribute__((weak)) __init memory
        machine_specific_memory_setup();
 }
 
+static void __init parse_setup_data(void)
+{
+	struct setup_data *data;
+	unsigned long pa_data;
+
+	if (boot_params.hdr.version < 0x0209)
+		return;
+	pa_data = boot_params.hdr.setup_data;
+	while (pa_data) {
+		data = early_ioremap(pa_data, PAGE_SIZE);
+		switch (data->type) {
+		default:
+			break;
+		}
+		free_early(pa_data, pa_data+sizeof(*data)+data->len);
+		pa_data = data->next;
+		early_iounmap(data, PAGE_SIZE);
+	}
+}
+
 /*
  * setup_arch - architecture-specific boot-time initializations
  *

@@ -318,6 +338,8 @@ void __init setup_arch(char **cmdline_p)
 	strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
 	*cmdline_p = command_line;
 
+	parse_setup_data();
+
 	parse_early_param();
 
 #ifdef CONFIG_PROVIDE_OHCI1394_DMA_INIT
arch/x86/kernel/head64.c +20 -0
@@ -11,6 +11,7 @@
 #include <linux/string.h>
 #include <linux/percpu.h>
 #include <linux/start_kernel.h>
+#include <linux/io.h>
 
 #include <asm/processor.h>
 #include <asm/proto.h>

@@ -105,6 +106,24 @@ static void __init reserve_ebda_region(v
 	reserve_early(lowmem, 0x100000, "BIOS reserved");
 }
 
+static void __init reserve_setup_data(void)
+{
+	struct setup_data *data;
+	unsigned long pa_data;
+	char buf[32];
+
+	if (boot_params.hdr.version < 0x0209)
+		return;
+	pa_data = boot_params.hdr.setup_data;
+	while (pa_data) {
+		data = early_ioremap(pa_data, sizeof(*data));
+		sprintf(buf, "setup data %x", data->type);
+		reserve_early(pa_data, pa_data+sizeof(*data)+data->len, buf);
+		pa_data = data->next;
+		early_iounmap(data, sizeof(*data));
+	}
+}
+
 void __init x86_64_start_kernel(char * real_mode_data)
 {
 	int i;

@@ -159,6 +178,7 @@ void __init x86_64_start_kernel(char * r
 	}
 
 	reserve_ebda_region();
+	reserve_setup_data();
 
 	/*
 	 * At this point everything still needed from the boot loader
#1 lore raw

Huang wrote:
+ while (pa_data) {
+ data = early_ioremap(pa_data, PAGE_SIZE);
+ switch (data->type) {
+ default:
+ break;
+ }

Isn't that switch statement equivalent to -always- breaking, as in:

+ while (pa_data) {
+ data = early_ioremap(pa_data, PAGE_SIZE);
+ break;

I doubt you want to do that. I suppose what you want to do is check for data == NULL instead, as in:

+ while (pa_data) {
+ data = early_ioremap(pa_data, PAGE_SIZE);
+ if (!data)
+ break;

???

Signature · 3 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 1.940.382.4214
author
#2 lore raw
Quoted 17 lines · Paul Jackson

Huang wrote:

arch/x86/kernel/setup_64.c +6 -0
+	while (pa_data) {
+		data = early_ioremap(pa_data, PAGE_SIZE);
+		switch (data->type) {
+		default:
+			break;
+		}

Isn't that switch statement equivalent to -always- breaking, as in:

arch/x86/kernel/setup_64.c +3 -0
+	while (pa_data) {
+		data = early_ioremap(pa_data, PAGE_SIZE);
+		break;

I doubt you want to do that. I suppose what you want to do is check for data == NULL instead, as in:

arch/x86/kernel/setup_64.c void __attribute__((weak)) __init memory · line 281

Quoted 4 lines
arch/x86/kernel/setup_64.c +4 -0
+	while (pa_data) {
+		data = early_ioremap(pa_data, PAGE_SIZE);
+		if (!data)
+			break;

This break will break the switch instead of while. When setup_data processing code is added, the code will looks like:

#define SETUP_DATA_E820_EXT 1

while (pa_data) {
	data = early_ioremap(pa_data, PAGE_SIZE);
	switch (data->type) {
	case SETUP_DATA_E820_EXT:
		parse_e820_ext(data, pa_data);
		break;
	default:
		break;
	}

...

Best Regards,
Huang Ying

#3 lore raw
Quoted 1 line · Huang

This break will break the switch instead of while.

Right you are ... sorry for my confusion.

Signature · 3 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 1.940.382.4214
#4 lore raw
In reply to Huang, Ying

Huang wrote:
+/* setup data types */
+#define SETUP_NONE 0
+
+/* extensible setup data list node */
+struct setup_data {
+ u64 next;
+ u32 type;
+ u32 len;
+ u8 data[0];
+};

I don't understand what purpose the 'type' field serves.

Signature · 3 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 1.940.382.4214
author
#5 lore raw
Quoted 13 lines · Paul Jackson

Huang wrote:

include/asm-x86/bootparam.h +10 -0
+/* setup data types */
+#define SETUP_NONE			0
+
+/* extensible setup data list node */
+struct setup_data {
+	u64 next;
+	u32 type;
+	u32 len;
+	u8 data[0];
+};
Quoted 1 line

I don't understand what purpose the 'type' field serves.

The setup_data will not only be used for E820 extension. So a type field is used to distinguish various setup_data.

Best Regards,
Huang Ying

#6 lore raw
Quoted 1 line · Huang

The setup_data will not only be used for E820 extension.

I'm still unsure of your meaning.

Are you saying that 'setup_data' is not used in your current patch set?

Are you also saying that 'setup_data' will be used in the future?

Quoted 1 line

So a type field is used to distinguish various setup_data.

What various types of 'setup_data' will be distinguished?

Signature · 3 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 1.940.382.4214
reviewer
#7 lore raw
Quoted 9 lines · Paul Jackson
Quoted 1 line · Huang

The setup_data will not only be used for E820 extension.

I'm still unsure of your meaning.

Are you saying that 'setup_data' is not used in your current patch set?

Are you also saying that 'setup_data' will be used in the future?

They will be used for other things in the future. EDD extension is one we already know about -- the current EDD code doesn't carry as much information as we want.

-hpa