Skip to contentSkip to conversation

i386: always clear bss

PATCH NAK Jeremy Fitzhardinge 3 participants 17 messages May 4, 2007 – May 5, 2007

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 i386: always clear bss

When the paravirt dispatcher gets run immediately on entry to startup_32, the bss isn't cleared. This happens to work if the hypervisor's domain builder loaded the complete kernel image and cleared the bss for us, but this may not always be true (for example, if we're running out of a decompressed bzImage).

Change head.S so that it unconditionally clears the bss before doing the paravirt dispatch or continuing on to normal native boot.

There are a couple of points to note:
- We can't, in general, load the segment registers before paravirt dispatch, because we could be running with a non-standard gdt and segment selectors. In practice though, all code which ends up jumping into startup_32 will have already set the segment registers up to sane values, so we don't need to do it again.
- Paging may or may not be enabled, and if enabled we may or may not be mapped to the proper kernel virtual address. To deal with this, we compare the kernel's linked address with where we're actually running, and use that to offset the bss pointer.

Cc Rusty Russell <[email protected]>
Cc Eric W. Biederman <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
---
 arch/i386/kernel/head.S |   48 ++++++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

===================================================================

arch/i386/kernel/head.S +27 -21
@@ -70,6 +70,33 @@ INIT_MAP_BEYOND_END = BOOTBITMAP_SIZE + 
  */
 .section .text.head,"ax",@progbits
 ENTRY(startup_32)
+/*
+ * Clear BSS first so that there are no surprises...
+ * This relies on the the segment registers to be set
+ * to something sensible, which will have already happened.
+ */
+	cld
+	xorl %eax,%eax
+	movl $__bss_start,%edi
+	movl $__bss_stop,%ecx
+	subl %edi,%ecx
+	shrl $2,%ecx
+	/*
+	 * Work out whether we're running mapped or not:
+	 * - call a local label
+	 * - pop the return address to get the actual eip
+	 * - subtract local label from %edi (= bss pointer)
+	 * - add in actual eip
+	 *
+	 * This will result in %edi being a virtual pointer if
+	 * we're currently mapped, or a physical pointer if we're
+	 * not (either no paging or 1:1 mapping).
+	 */
+	call 1f
+1:	popl %ebx
+	subl $1b, %edi
+	addl %ebx, %edi
+	rep ; stosl
 
 #ifdef CONFIG_PARAVIRT
         movl %cs, %eax
@@ -77,27 +104,6 @@ ENTRY(startup_32)
         jnz startup_paravirt
 #endif
 
-/*
- * Set segments to known values.
- */
-	cld
-	lgdt boot_gdt_descr - __PAGE_OFFSET
-	movl $(__BOOT_DS),%eax
-	movl %eax,%ds
-	movl %eax,%es
-	movl %eax,%fs
-	movl %eax,%gs
-
-/*
- * Clear BSS first so that there are no surprises...
- * No need to cld as DF is already clear from cld above...
- */
-	xorl %eax,%eax
-	movl $__bss_start - __PAGE_OFFSET,%edi
-	movl $__bss_stop - __PAGE_OFFSET,%ecx
-	subl %edi,%ecx
-	shrl $2,%ecx
-	rep ; stosl
 /*
  * Copy bootup parameters out of the way.
  * Note: %esi still has the pointer to the real-mode data.
#1 lore raw
Quoted 9 lines · Jeremy Fitzhardinge

When the paravirt dispatcher gets run immediately on entry to startup_32, the bss isn't cleared. This happens to work if the hypervisor's domain builder loaded the complete kernel image and cleared the bss for us, but this may not always be true (for example, if we're running out of a decompressed bzImage).

Change head.S so that it unconditionally clears the bss before doing the paravirt dispatch or continuing on to normal native boot.

Quoted 10 lines

There are a couple of points to note:
- We can't, in general, load the segment registers before paravirt dispatch, because we could be running with a non-standard gdt and segment selectors. In practice though, all code which ends up jumping into startup_32 will have already set the segment registers up to sane values, so we don't need to do it again.
- Paging may or may not be enabled, and if enabled we may or may not be mapped to the proper kernel virtual address. To deal with this, we compare the kernel's linked address with where we're actually running, and use that to offset the bss pointer.

NAK.

Skipping the segment register load is likely fine. Supporting V!=P at startup_32 is not. Assuming that we have a stack at startup_32 is not.

If you want to figure out where the kernel is loaded you can do (from arch/i386/boot/head.S)

Quoted 8 lines

/* Calculate the delta between where we were compiled to run
* at and where we were actually loaded at. This can only be done
* with a short local call on x86. Nothing else will tell us what
* address we are running at. The reserved chunk of the real-mode
* data at 0x34-0x3f are used as the stack for this calculation.
* Only 4 bytes are needed.
*/

Quoted 5 lines
	leal 0x40(%esi), %esp
	call 1f
1:	popl %ebp
	subl $1b, %ebp

Eric

#2 lore raw
Quoted 12 lines · Eric W. Biederman
Quoted 9 lines · Jeremy Fitzhardinge

When the paravirt dispatcher gets run immediately on entry to startup_32, the bss isn't cleared. This happens to work if the hypervisor's domain builder loaded the complete kernel image and cleared the bss for us, but this may not always be true (for example, if we're running out of a decompressed bzImage).

Change head.S so that it unconditionally clears the bss before doing the paravirt dispatch or continuing on to normal native boot.

Quoted 11 lines
Quoted 11 lines

There are a couple of points to note:
- We can't, in general, load the segment registers before paravirt dispatch, because we could be running with a non-standard gdt and segment selectors. In practice though, all code which ends up jumping into startup_32 will have already set the segment registers up to sane values, so we don't need to do it again.
- Paging may or may not be enabled, and if enabled we may or may not be mapped to the proper kernel virtual address. To deal with this, we compare the kernel's linked address with where we're actually running, and use that to offset the bss pointer.

BTW, I should have marked this as an RFC comment, rather than an actual submission. We don't need it for .22.

Quoted 5 lines

NAK.

Skipping the segment register load is likely fine. Supporting V!=P at startup_32 is not.

Why?

Quoted 5 lines

Assuming that we have a stack at startup_32 is not.

If you want to figure out where the kernel is loaded you can do (from arch/i386/boot/head.S)

Yes, that's more or less the same code, aside from using 0x40(%esi) as a stack. Would that be OK here?

J

#3 lore raw
Quoted 11 lines · Jeremy Fitzhardinge

BTW, I should have marked this as an RFC comment, rather than an actual submission. We don't need it for .22.

Quoted 5 lines

NAK.

Skipping the segment register load is likely fine. Supporting V!=P at startup_32 is not.

Why?

Quoted 8 lines
Quoted 5 lines

Assuming that we have a stack at startup_32 is not.

If you want to figure out where the kernel is loaded you can do (from arch/i386/boot/head.S)

Yes, that's more or less the same code, aside from using 0x40(%esi) as a stack. Would that be OK here?

Using 0x40 as a stack would be ok.

There are issues with CONFIG_RELOCATABLE and V!=P that I'm not comfortable with yet, because we can't tell the difference.

Eric

#4 lore raw
Quoted 2 lines · Eric W. Biederman

Using 0x40 as a stack would be ok.

OK.

Quoted 2 lines

There are issues with CONFIG_RELOCATABLE and V!=P that I'm not comfortable with yet, because we can't tell the difference.

But it doesn't matter in this case, does it? It just needs to find the current address, whether it be virtual or physical, of the bss. It doesn't assume any particular offset.

When does the relocation happen? Does the bzImage loader do it as part
of decompression? Or does the kernel do it to itself? (Not that it makes any difference here.)

J

#5 lore raw
Quoted 4 lines · Jeremy Fitzhardinge
Quoted 2 lines · Eric W. Biederman

Using 0x40 as a stack would be ok.

Quoted 8 lines

OK.

Quoted 2 lines

There are issues with CONFIG_RELOCATABLE and V!=P that I'm not comfortable with yet, because we can't tell the difference.

But it doesn't matter in this case, does it? It just needs to find the current address, whether it be virtual or physical, of the bss. It doesn't assume any particular offset.

For the bss that sounds correct.

Quoted 3 lines

When does the relocation happen? Does the bzImage loader do it as part
of decompression? Or does the kernel do it to itself? (Not that it makes any difference here.)

Currently right after compression just before we jump to startup_32. But if the usage of vmlinux continues to increase we should really move it to just after startup_32. Which is where we run into problems with supporting virtual addresses at our normal kernel entry point.

The relocation doesn't live just after startup_32 now because it is hard to put there.

The practical challenge is that we need to compute the delta between where we are at and where we were compiled for, and with possibility of virtual address and physical address I don't know how we would compute where we are at, in a way we could compare to our compile time physical or virtual addresses.

Xen when it comes it at a completely isolated entry point is fine because there is no pretence of code reuse, and we don't have to auto-detect how we were started.

Eric

#6 lore raw
In reply to Jeremy Fitzhardinge
Quoted 4 lines · Jeremy Fitzhardinge

Yes, that's more or less the same code, aside from using 0x40(%esi) as a stack. Would that be OK here?

I saw the 0x40(%esi) stack stuff, and I'm utterly puzzled by it. There is no reason one can't set up %esp to point to a hunk in ordinary memory and use it?

-hpa

#7 lore raw
Quoted 9 lines · H. Peter Anvin
Quoted 4 lines · Jeremy Fitzhardinge

Yes, that's more or less the same code, aside from using 0x40(%esi) as a stack. Would that be OK here?

I saw the 0x40(%esi) stack stuff, and I'm utterly puzzled by it. There is no reason one can't set up %esp to point to a hunk in ordinary memory and use it?

That is what we are doing, remind me to make certain we have this field of the boot protocol documented as permanently reserved for this.

This comes from the relocatable kernel patches where we run the kernel where the bootloader chooses to put it assuming we are >= 1M.

The problem is that we don't have any IP relative data access instructions, we don't have a stack, and so the only valid address that we know is valid is %esi. Once we compute where we are running we can setup a base address register and a stack and everything is easy, but the bootstrap to figure out where we are is just a little tricky.

Eric

#8 lore raw
Quoted 12 lines · Eric W. Biederman
Quoted 4 lines
Quoted 1 line

I saw the 0x40(%esi) stack stuff, and I'm utterly puzzled by it. There is no reason one can't set up %esp to point to a hunk in ordinary memory and use it?

That is what we are doing, remind me to make certain we have this field of the boot protocol documented as permanently reserved for this.

This comes from the relocatable kernel patches where we run the kernel where the bootloader chooses to put it assuming we are >= 1M.

Quoted 6 lines

The problem is that we don't have any IP relative data access instructions, we don't have a stack, and so the only valid address that we know is valid is %esi. Once we compute where we are running we can setup a base address register and a stack and everything is easy, but the bootstrap to figure out where we are is just a little tricky.

Oh, right. And this runs with interrupts off, so you only need one dword. That's fine, of course, although the location is a bit awkward.

-hpa

#9 lore raw
Quoted 20 lines · H. Peter Anvin
Quoted 12 lines · Eric W. Biederman
Quoted 4 lines
Quoted 1 line

I saw the 0x40(%esi) stack stuff, and I'm utterly puzzled by it. There is no reason one can't set up %esp to point to a hunk in ordinary memory and use it?

That is what we are doing, remind me to make certain we have this field of the boot protocol documented as permanently reserved for this.

This comes from the relocatable kernel patches where we run the kernel where the bootloader chooses to put it assuming we are >= 1M.

Quoted 6 lines

The problem is that we don't have any IP relative data access instructions, we don't have a stack, and so the only valid address that we know is valid is %esi. Once we compute where we are running we can setup a base address register and a stack and everything is easy, but the bootstrap to figure out where we are is just a little tricky.

Quoted 2 lines

Oh, right. And this runs with interrupts off, so you only need one dword. That's fine, of course, although the location is a bit awkward.

Yep. That is what I found when surveyed the available locations.

Eric

#10 lore raw
Quoted 4 lines · Eric W. Biederman
Quoted 2 lines

Oh, right. And this runs with interrupts off, so you only need one dword. That's fine, of course, although the location is a bit awkward.

Yep. That is what I found when surveyed the available locations.

Look at the structure definition I just posted; I think you'll find that using something not in the immediate extension area for the video area would have been preferrable, I would suggest 0x5c or 0x1e4.

-hpa

#11 lore raw
Quoted 9 lines · H. Peter Anvin
Quoted 4 lines · Eric W. Biederman
Quoted 2 lines

Oh, right. And this runs with interrupts off, so you only need one dword. That's fine, of course, although the location is a bit awkward.

Yep. That is what I found when surveyed the available locations.

Look at the structure definition I just posted; I think you'll find that using something not in the immediate extension area for the video area would have been preferrable, I would suggest 0x5c or 0x1e4.

My notes show 0x5c reserved for additional apm_bios_info, although of the top of my head I don't know how realistic that is.

0x1e4 does look available.

It has been a long time since I made that choice, and I do see that looking at struct screen_info I did remember to document that I was using 0x3c, even in your structure.

It is all internal to our boot process and external code isn't going to use it so we can change it if we feel like.

Eric

#12 lore raw
Quoted 4 lines · Eric W. Biederman

My notes show 0x5c reserved for additional apm_bios_info, although of the top of my head I don't know how realistic that is.

Quoted 9 lines

0x1e4 does look available.

It has been a long time since I made that choice, and I do see that looking at struct screen_info I did remember to document that I was using 0x3c, even in your structure.

It is all internal to our boot process and external code isn't going to use it so we can change it if we feel like.

True.

-hpa

#13 lore raw
In reply to Eric W. Biederman
Quoted 4 lines · Eric W. Biederman

My notes show 0x5c reserved for additional apm_bios_info, although of the top of my head I don't know how realistic that is.

Quoted 9 lines

0x1e4 does look available.

It has been a long time since I made that choice, and I do see that looking at struct screen_info I did remember to document that I was using 0x3c, even in your structure.

It is all internal to our boot process and external code isn't going to use it so we can change it if we feel like.

I don't see the actual instruction that does that anywhere in my tree, which was branched from Andi's "for-linus" git tree, but I have reserved 0x1e4 for that purpose as "scratch".

-hpa

#14 lore raw
Quoted 11 lines · H. Peter Anvin
Quoted 10 lines · Eric W. Biederman

My notes show 0x5c reserved for additional apm_bios_info, although of the top of my head I don't know how realistic that is.

0x1e4 does look available.

It has been a long time since I made that choice, and I do see that looking at struct screen_info I did remember to document that I was using 0x3c, even in your structure.

Quoted 7 lines
Quoted 3 lines

It is all internal to our boot process and external code isn't going to use it so we can change it if we feel like.

I don't see the actual instruction that does that anywhere in my tree, which was branched from Andi's "for-linus" git tree, but I have reserved 0x1e4 for that purpose as "scratch".

I have this vague memory of liking 0x3c because if we do happen to use more room then we intended the consequences are pretty benign.

But that is a pretty minor consequence.

Eric

#15 lore raw
Quoted 6 lines · Eric W. Biederman

I have this vague memory of liking 0x3c because if we do happen to use more room then we intended the consequences are pretty benign.

But that is a pretty minor consequence.

That's a dangerous assumption (besides, it's likely wrong, since there are only two unused bytes below it.)

-hpa

#16 lore raw
Quoted 7 lines · H. Peter Anvin
Quoted 6 lines · Eric W. Biederman

I have this vague memory of liking 0x3c because if we do happen to use more room then we intended the consequences are pretty benign.

But that is a pretty minor consequence.

I meant to say it is a pretty minor consideration.

Quoted 2 lines

That's a dangerous assumption (besides, it's likely wrong, since there are only two unused bytes below it.)

Worse case on a modern machine is that we get a messed up screen. The odds of it happening are sufficiently remote that it isn't worth worrying about.

Eric