[PATCH -mm 2/4] x86_64 boot: Add linked list of struct setup_data

[PATCH -mm 2/4] x86_64 boot: Add linked list of struct setup_data

[Huang, Ying]

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 <ying.huang@intel.com>

---
 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(-)

--- a/include/asm-x86/bootparam.h
+++ b/include/asm-x86/bootparam.h
@@ -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 {
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -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"
--- a/arch/x86/kernel/setup_64.c
+++ b/arch/x86/kernel/setup_64.c
@@ -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
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -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

Re: [PATCH -mm 2/4] x86_64 boot: Add linked list of struct setup_data

[Paul Jackson]

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;

???

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH -mm 2/4] x86_64 boot: Add linked list of struct setup_data

[Paul Jackson]

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.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH -mm 2/4] x86_64 boot: Add linked list of struct setup_data

[Huang, Ying]

On Thu, 2008-03-27 at 06:25 -0500, Paul Jackson wrote:
> 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;

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

Re: [PATCH -mm 2/4] x86_64 boot: Add linked list of struct setup_data

[Huang, Ying]

On Thu, 2008-03-27 at 06:33 -0500, Paul Jackson wrote:
> 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.

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

Re: [PATCH -mm 2/4] x86_64 boot: Add linked list of struct setup_data

[Paul Jackson]

Huang wrote:
> This break will break the switch instead of while.

Right you are ... sorry for my confusion.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH -mm 2/4] x86_64 boot: Add linked list of struct setup_data

[Paul Jackson]

Huang wrote:
> 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?

>  So a type field is used to distinguish various setup_data.

What various types of 'setup_data' will be distinguished?

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.940.382.4214

Re: [PATCH -mm 2/4] x86_64 boot: Add linked list of struct setup_data

[H. Peter Anvin]

Paul Jackson wrote:
> Huang wrote:
>> 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