i386/x86_64 boot: setup data

PATCH In review Huang, Ying 5 participants 11 messages Oct 8, 2007 – Oct 9, 2007

Subsystems (2)

Memory management MM core

lore

Share & tools

lore

Huang, Ying

author

2007-10-09 06:38 UTC #0 lore raw

PATCH 1/3 i386/x86_64 boot: 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/i386/Kconfig            |    3 -
 arch/i386/boot/header.S      |    8 +++
 arch/i386/kernel/setup.c     |   92 +++++++++++++++++++++++++++++++++++++++++++
 arch/x86_64/kernel/setup.c   |   37 +++++++++++++++++
 include/asm-i386/bootparam.h |   15 +++++++
 include/asm-i386/io.h        |    7 +++
 include/linux/mm.h           |    2 
 mm/memory.c                  |   24 +++++++++++
 8 files changed, 184 insertions(+), 4 deletions(-)

include/asm-i386/bootparam.h +15 -0

Index: linux-2.6.23-rc8/include/asm-i386/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];
+} __attribute__((packed));
+
 struct setup_header {
 	u8	setup_sects;
 	u16	root_flags;

@@ -41,6 +52,10 @@
 	u32	initrd_addr_max;
 	u32	kernel_alignment;
 	u8	relocatable_kernel;
+	u8	_pad2[3];
+	u32	cmdline_size;
+	u32	_pad3;
+	u64	setup_data;
 } __attribute__((packed));
 
 struct sys_desc_table {

arch/i386/boot/header.S +7 -1

Index: linux-2.6.23-rc8/arch/i386/boot/header.S
===================================================================
@@ -119,7 +119,7 @@
 	# Part 2 of the header, from the old setup.S
 
 		.ascii	"HdrS"		# header signature
-		.word	0x0206		# header version number (>= 0x0105)
+		.word	0x0207		# header version number (>= 0x0105)
 					# or else old loadlin-1.5 will fail)
 		.globl realmode_swtch
 realmode_swtch:	.word	0, 0		# default_switch, SETUPSEG

@@ -214,6 +214,12 @@
                                                 #added with boot protocol
                                                 #version 2.06
 
+pad4:			.long 0
+
+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_64/kernel/setup.c +37 -0

Index: linux-2.6.23-rc8/arch/x86_64/kernel/setup.c
===================================================================
@@ -250,6 +250,40 @@
 		ebda_size = 64*1024;
 }
 
+static void __init parse_setup_data(void)
+{
+	struct setup_data *data;
+	unsigned long pa_data;
+
+	if (boot_params.hdr.version < 0x0207)
+		return;
+	pa_data = boot_params.hdr.setup_data;
+	while (pa_data) {
+		data = early_ioremap(pa_data, PAGE_SIZE);
+		switch (data->type) {
+		default:
+			break;
+		}
+		pa_data = data->next;
+		early_iounmap(data, PAGE_SIZE);
+	}
+}
+
+static void __init reserve_setup_data(void)
+{
+	struct setup_data *data;
+	unsigned long pa_data;
+
+	if (boot_params.hdr.version < 0x0207)
+		return;
+	pa_data = boot_params.hdr.setup_data;
+	while (pa_data) {
+		data = __va(pa_data);
+		reserve_bootmem_generic(pa_data, sizeof(*data)+data->len);
+		pa_data = data->next;
+	}
+}
+
 void __init setup_arch(char **cmdline_p)
 {
 	printk(KERN_INFO "Command line: %s\n", boot_command_line);

@@ -285,6 +319,8 @@
 	strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
 	*cmdline_p = command_line;
 
+	parse_setup_data();
+
 	parse_early_param();
 
 	finish_e820_parsing();

@@ -373,6 +409,7 @@
         */
        acpi_reserve_bootmem();
 #endif
+	reserve_setup_data();
 	/*
 	 * Find and reserve possible boot-time SMP configuration:
 	 */

arch/i386/kernel/setup.c +92 -0

116 lines not on this page · raw message

include/asm-i386/io.h +7 -0

Index: linux-2.6.23-rc8/include/asm-i386/io.h
===================================================================
@@ -126,6 +126,13 @@
 extern void iounmap(volatile void __iomem *addr);
 
 /*
+ * boot_ioremap is for temporary early boot-time mappings, before
+ * paging_init(), when the boot-time page tables are still in use.
+ * The mapping is currently limited to at most 4 pages.
+ */
+extern void *boot_ioremap(unsigned long offset, unsigned long size);
+
+/*
  * bt_ioremap() and bt_iounmap() are for temporary early boot-time
  * mappings, before the real ioremap() is functional.
  * A boot-time mapping is currently limited to at most 16 pages.

arch/i386/Kconfig +0 -3

Index: linux-2.6.23-rc8/arch/i386/Kconfig
===================================================================
@@ -784,11 +784,8 @@
  	  The default yes will allow the kernel to do irq load balancing.
 	  Saying no will keep the kernel from doing irq load balancing.
 
-# turning this on wastes a bunch of space.
-# Summit needs it only when NUMA is on
 config BOOT_IOREMAP
 	bool
-	depends on (((X86_SUMMIT || X86_GENERICARCH) && NUMA) || (X86 && EFI))
 	default y
 
 config SECCOMP

include/linux/mm.h +2 -0

Index: linux-2.6.23-rc8/include/linux/mm.h
===================================================================
@@ -942,6 +942,8 @@
 extern void show_mem(void);
 extern void si_meminfo(struct sysinfo * val);
 extern void si_meminfo_node(struct sysinfo *val, int nid);
+extern unsigned long copy_from_phys(void *to, unsigned long from_phys,
+				    unsigned long n);
 
 #ifdef CONFIG_NUMA
 extern void setup_per_cpu_pageset(void);

mm/memory.c +24 -0

Index: linux-2.6.23-rc8/mm/memory.c
===================================================================
@@ -2788,3 +2788,27 @@
 	return buf - old_buf;
 }
 EXPORT_SYMBOL_GPL(access_process_vm);
+
+unsigned long copy_from_phys(void *to, unsigned long from_phys,
+			     unsigned long n)
+{
+	struct page *page;
+	void *from;
+	unsigned long remain = n, offset, trunck;
+
+	while (remain) {
+		page = pfn_to_page(from_phys >> PAGE_SHIFT);
+		from = kmap_atomic(page, KM_USER0);
+		offset = from_phys & ~PAGE_MASK;
+		if (remain > PAGE_SIZE - offset)
+			trunck = PAGE_SIZE - offset;
+		else
+			trunck = remain;
+		memcpy(to, from + offset, trunck);
+		kunmap_atomic(from, KM_USER0);
+		to += trunck;
+		from_phys += trunck;
+		remain -= trunck;
+	}
+	return n;
+}

Nick Piggin

2007-10-08 15:25 UTC #1 lore raw

Quoted 13 lines · Ying

mm/memory.c +13 -0

+unsigned long copy_from_phys(void *to, unsigned long from_phys,
+			     unsigned long n)
+{
+	struct page *page;
+	void *from;
+	unsigned long remain = n, offset, trunck;
+
+	while (remain) {
+		page = pfn_to_page(from_phys >> PAGE_SHIFT);
+		from = kmap_atomic(page, KM_USER0);
+		offset = from_phys & ~PAGE_MASK;
+		if (remain > PAGE_SIZE - offset)
+			trunck = PAGE_SIZE - offset;

mm/memory.c line 2814

Quoted 10 lines

mm/memory.c +10 -0

+		else
+			trunck = remain;
+		memcpy(to, from + offset, trunck);
+		kunmap_atomic(from, KM_USER0);
+		to += trunck;
+		from_phys += trunck;
+		remain -= trunck;
+	}
+	return n;
+}

I suppose that's not unreasonable to put in mm/memory.c, although it's not really considered a problem to do this kind of stuff in a low level arch file...

You have no kernel virtual mapping for the source data?

Should it be __init?

Care to add a line of documentation if you keep it in mm/memory.c?

Thanks,
Nick

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Huang, Ying

author

2007-10-09 08:20 UTC #2 lore raw

Quoted 31 lines · Nick Piggin

Quoted 13 lines · Ying

mm/memory.c +13 -0

+unsigned long copy_from_phys(void *to, unsigned long from_phys,
+			     unsigned long n)
+{
+	struct page *page;
+	void *from;
+	unsigned long remain = n, offset, trunck;
+
+	while (remain) {
+		page = pfn_to_page(from_phys >> PAGE_SHIFT);
+		from = kmap_atomic(page, KM_USER0);
+		offset = from_phys & ~PAGE_MASK;
+		if (remain > PAGE_SIZE - offset)
+			trunck = PAGE_SIZE - offset;

mm/memory.c line 2814

Quoted 10 lines

mm/memory.c +10 -0

+		else
+			trunck = remain;
+		memcpy(to, from + offset, trunck);
+		kunmap_atomic(from, KM_USER0);
+		to += trunck;
+		from_phys += trunck;
+		remain -= trunck;
+	}
+	return n;
+}

I suppose that's not unreasonable to put in mm/memory.c, although it's not really considered a problem to do this kind of stuff in a low level arch file...

Quoted 2 lines

You have no kernel virtual mapping for the source data?

On 32-bit platform such as i386. Some memory zones have no kernel virtual mapping (highmem region etc). So I think this may be useful as a universal way to access physical memory. But it can be more efficient to implement it in arch file for some arch. Should this implementation be used as a fall back implementation with attribute "weak"?

Quoted 4 lines

Should it be __init?
Care to add a line of documentation if you keep it in mm/memory.c?

OK, I will add the document in the next version.

Best Regards,
Huang Ying

Nick Piggin

2007-10-08 16:06 UTC #3 lore raw

Quoted 9 lines · Ying

Quoted 8 lines · Nick Piggin
mm/memory.c line 2793
Quoted 2 lines · Ying
mm/memory.c +2 -0
+unsigned long copy_from_phys(void *to, unsigned long from_phys,
+			     unsigned long n)
I suppose that's not unreasonable to put in mm/memory.c, although it's not really considered a problem to do this kind of stuff in a low level arch file...

Quoted 4 lines

Quoted 1 line
You have no kernel virtual mapping for the source data?
On 32-bit platform such as i386. Some memory zones have no kernel virtual mapping (highmem region etc).

I'm just wondering whether you really need to access highmem in boot code...

Quoted 4 lines

So I think this may be useful as a
universal way to access physical memory. But it can be more efficient to implement it in arch file for some arch. Should this implementation be used as a fall back implementation with attribute "weak"?

Definitely on most architectures it would just amount to memcpy(dst, __va(phys), n);, right? However I don't know if it's worth the trouble of overriding it unless there is some non-__init user of it.

Huang, Ying

author

2007-10-09 08:53 UTC #4 lore raw

Quoted 10 lines · Nick Piggin

Quoted 9 lines · Ying
Quoted 8 lines · Nick Piggin
mm/memory.c line 2793
Quoted 2 lines · Ying
mm/memory.c +2 -0
+unsigned long copy_from_phys(void *to, unsigned long from_phys,
+			     unsigned long n)
I suppose that's not unreasonable to put in mm/memory.c, although it's not really considered a problem to do this kind of stuff in a low level arch file...

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
You have no kernel virtual mapping for the source data?
On 32-bit platform such as i386. Some memory zones have no kernel virtual mapping (highmem region etc).
I'm just wondering whether you really need to access highmem in boot code...

Because the zero page (boot_parameters) of i386 boot protocol has 4k limitation, a linked list style boot parameter passing mechanism (struct setup_data) is proposed by Peter Anvin. The linked list is provided by bootloader, so it is possible to be in highmem region.

Quoted 8 lines

Quoted 5 lines
So I think this may be useful as a
universal way to access physical memory. But it can be more efficient to implement it in arch file for some arch. Should this implementation be used as a fall back implementation with attribute "weak"?
Definitely on most architectures it would just amount to memcpy(dst, __va(phys), n);, right? However I don't know if

Yes.

Quoted 2 lines

it's worth the trouble of overriding it unless there is some non-__init user of it.

To support debugging and kexec, the boot parameters include the linked list above are exported into sysfs. This function is used there too. The patch is the No. 2 of the series.

Best Regards,
Huang Ying

Nick Piggin

2007-10-08 16:28 UTC #5 lore raw

Quoted 9 lines · Ying

Quoted 2 lines · Nick Piggin
I'm just wondering whether you really need to access highmem in boot code...
Because the zero page (boot_parameters) of i386 boot protocol has 4k limitation, a linked list style boot parameter passing mechanism (struct setup_data) is proposed by Peter Anvin. The linked list is provided by bootloader, so it is possible to be in highmem region.

OK. I don't really know the code, but I trust you ;)

Quoted 11 lines

Quoted 2 lines
Definitely on most architectures it would just amount to memcpy(dst, __va(phys), n);, right? However I don't know if
Yes.
Quoted 2 lines
it's worth the trouble of overriding it unless there is some non-__init user of it.
To support debugging and kexec, the boot parameters include the linked list above are exported into sysfs. This function is used there too. The patch is the No. 2 of the series.

Ah, I see. I missed that.

OK, well rather than make it weak, and have everyone except i386 override it, can you just ifdef CONFIG_HIGHMEM?

After that, I guess most other architectures wouldn't even use the function. Maybe it can go into lib/ instead so that it can be discarded by the linker if it isn't used?

Huang, Ying

author

2007-10-09 09:24 UTC #6 lore raw

Quoted 6 lines · Nick Piggin

Quoted 5 lines · Ying
Quoted 2 lines · Nick Piggin
I'm just wondering whether you really need to access highmem in boot code...

Quoted 6 lines

Quoted 4 lines
Because the zero page (boot_parameters) of i386 boot protocol has 4k limitation, a linked list style boot parameter passing mechanism (struct setup_data) is proposed by Peter Anvin. The linked list is provided by bootloader, so it is possible to be in highmem region.
OK. I don't really know the code, but I trust you ;)

Thank you :)

Quoted 9 lines

Quoted 9 lines
Quoted 3 lines
Definitely on most architectures it would just amount to memcpy(dst, __va(phys), n);, right? However I don't know if
Yes.
Quoted 2 lines
it's worth the trouble of overriding it unless there is some non-__init user of it.

Quoted 8 lines

Quoted 3 lines
To support debugging and kexec, the boot parameters include the linked list above are exported into sysfs. This function is used there too. The patch is the No. 2 of the series.
Ah, I see. I missed that.
OK, well rather than make it weak, and have everyone except i386 override it, can you just ifdef CONFIG_HIGHMEM?

Yes. This is better. I will do it. Maybe it can be defined as a macro for these architectures, as follow:

/* in linux/mm.h */
#ifdef CONFIG_HIGHMEM
void *copy_from_phys(void *to, unsigned long from_phys, size_t n); #else
#define copy_from_phys(dst, phys, n) memcpy(dst, __va(phys), n)
#endif

Quoted 3 lines

After that, I guess most other architectures wouldn't even use the function. Maybe it can go into lib/ instead so that it can be discarded by the linker if it isn't used?

Yes. I will do it.

Best Regards,
Huang Ying

Oleg Verych

2007-10-09 11:30 UTC #7 lore raw

In reply to Huang, Ying

* Tue, 09 Oct 2007 16:55:23 +0800

Quoted 3 lines · Nick Piggin

Quoted 1 line · Ying

[]

Quoted 7 lines

Quoted 2 lines
I'm just wondering whether you really need to access highmem in boot code...
Because the zero page (boot_parameters) of i386 boot protocol has 4k limitation, a linked list style boot parameter passing mechanism (struct setup_data) is proposed by Peter Anvin. The linked list is provided by bootloader, so it is possible to be in highmem region.

Can it be explained, why boot protocol and boot line must be expanded? This amount of code for what?

 arch/i386/Kconfig            |    3 -
 arch/i386/boot/header.S      |    8 +++
 arch/i386/kernel/setup.c     |   92 +++++++++++++++++++++++++++++++++++++++++++
 arch/x86_64/kernel/setup.c   |   37 +++++++++++++++++
 include/asm-i386/bootparam.h |   15 +++++++
 include/asm-i386/io.h        |    7 +++
 include/linux/mm.h           |    2
 mm/memory.c                  |   24 +++++++++++

If it is proposed for passing ACPI makeup language bugfixes by boot line for ACPI parser in the kernel, or "telling to kernel what to do via EFI" then it's kind of very nasty red flag.

I'd suggest to have initramfs image ready with all possible data/options/actions based on very small amount of possible boot line information.

Any _right_ use-cases explained for dummies are appreciated.

Thanks.
--
-o--=O`C
#oo'L O
<___=E M

Andi Kleen

2007-10-09 11:13 UTC #8 lore raw

In reply to Nick Piggin

Quoted 1 line

Care to add a line of documentation if you keep it in mm/memory.c?

It would be better to just use early_ioremap() (or ioremap())

That is how ACPI who has similar issues accessing its tables solves this.

-Andi

huang ying

2007-10-09 14:00 UTC #9 lore raw

Quoted 6 lines · Andi Kleen

Quoted 2 lines
Care to add a line of documentation if you keep it in mm/memory.c?
It would be better to just use early_ioremap() (or ioremap())
That is how ACPI who has similar issues accessing its tables solves this.

Yes. That is another solution. But there is some problem about early_ioremap (boot_ioremap, bt_ioremap for i386) or ioremap.

- ioremap can not be used before mem_init.
- For i386, boot_ioremap can map at most 4 pages, bt_ioremap can map at most 16 pages. This will be an unnecessary constrains for size of setup_data.
- For i386, the virtual memory space of ioremap is limited too.

Best Regards,
Huang Ying

Andi Kleen

2007-10-09 14:04 UTC #10 lore raw

Quoted 8 lines · On Tuesday 09 October 2007 16:00:57 huang ying wrote:

Quoted 6 lines · Andi Kleen
Quoted 2 lines
Care to add a line of documentation if you keep it in mm/memory.c?
It would be better to just use early_ioremap() (or ioremap())
That is how ACPI who has similar issues accessing its tables solves this.

Quoted 7 lines

Yes. That is another solution. But there is some problem about early_ioremap (boot_ioremap, bt_ioremap for i386) or ioremap.
- ioremap can not be used before mem_init.
- For i386, boot_ioremap can map at most 4 pages, bt_ioremap can map at most 16 pages. This will be an unnecessary constrains for size of setup_data.

That could be easily extended if needed. But I don't see why we would need that much setup data anyways. Limiting it to let's say 16KB seems entirely reasonable. And if some kernel ever needs more it can be still extended.

The biggest item is probably the command line and i don't see why that should be more than a one or two KB.

Quoted 1 line

- For i386, the virtual memory space of ioremap is limited too.

That will be all freed and again the data shouldn't be that big.

-Andi