Re: [PATCH -mm -v4 1/3] i386/x86_64 boot: setup data

Previous thread: [PATCH -mm -v4 2/3] i386/x86_64 boot: boot parameters export via sysfs by Huang, Ying on Monday, October 8, 2007 - 11:40 pm. (1 message)

Next thread: [PATCH -mm -v4 3/3] i386/x86_64 boot: document for 32 bit boot protocol by Huang, Ying on Monday, October 8, 2007 - 11:40 pm. (1 message)
From: Huang, Ying
Date: Monday, October 8, 2007 - 11:40 pm

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

Index: linux-2.6.23-rc8/include/asm-i386/bootparam.h
===================================================================
--- linux-2.6.23-rc8.orig/include/asm-i386/bootparam.h	2007-10-09 11:26:06.000000000 +0800
+++ linux-2.6.23-rc8/include/asm-i386/bootparam.h	2007-10-09 14:15:14.000000000 +0800
@@ -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 {
Index: linux-2.6.23-rc8/arch/i386/boot/header.S
===================================================================
--- linux-2.6.23-rc8.orig/arch/i386/boot/header.S	2007-10-09 11:26:06.000000000 +0800
+++ linux-2.6.23-rc8/arch/i386/boot/header.S	2007-10-09 11:26:08.000000000 +0800
@@ -119,7 +119,7 @@
 	# Part 2 of the header, from the old setup.S
 
 		.ascii	"HdrS"		# header signature
-		.word	0x0206		# header version number (>= ...
From: Nick Piggin
Date: Monday, October 8, 2007 - 8:25 am

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
-

From: Huang, Ying
Date: Tuesday, October 9, 2007 - 1:22 am

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

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

Best Regards,
Huang Ying
-

From: Nick Piggin
Date: Monday, October 8, 2007 - 9:06 am

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

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.
-

From: Huang, Ying
Date: Tuesday, October 9, 2007 - 1:55 am

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


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
-

From: Nick Piggin
Date: Monday, October 8, 2007 - 9:28 am

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?
-

From: Huang, Ying
Date: Tuesday, October 9, 2007 - 2:26 am

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)

Yes. I will do it.

Best Regards,
Huang Ying
-

From: Oleg Verych
Date: Tuesday, October 9, 2007 - 4:44 am

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
-

From: Andi Kleen
Date: Tuesday, October 9, 2007 - 4:13 am

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
-

From: huang ying
Date: Tuesday, October 9, 2007 - 7:00 am

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
-

From: Andi Kleen
Date: Tuesday, October 9, 2007 - 7:04 am

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 will be all freed and again the data shouldn't be that big.

-Andi

 
-

Previous thread: [PATCH -mm -v4 2/3] i386/x86_64 boot: boot parameters export via sysfs by Huang, Ying on Monday, October 8, 2007 - 11:40 pm. (1 message)

Next thread: [PATCH -mm -v4 3/3] i386/x86_64 boot: document for 32 bit boot protocol by Huang, Ying on Monday, October 8, 2007 - 11:40 pm. (1 message)