On Thu, May 07, 2009 at 03:26:50PM -0700, H. Peter Anvin wrote:
> From: H. Peter Anvin <hpa at zytor.com>
>
> Currently, when building a relocatable kernel, CONFIG_PHYSICAL_START
> is ignored. This is undesirable, as we would like to keep the kernel
> out of ZONE_DMA and away from the possible memory hole at 15 MB (which
> some vendors for some bizarre reason still have.)
>
> With this patch, CONFIG_PHYSICAL_START is considered the *minimum*
> address at which the kernel can be located; a relocating bootloader
> can locate it higher, but not lower. This also restores the
> originally intended behavior that CONFIG_RELOCATABLE is functionally a
> noop if used with a non-relocating bootloader.
>
> This patch also change movsb and stosb to movsl, stosl and stosq, to
> shave a small fraction off the boot time.
On the coding style side of thing...
> + movl %ebp, %ebx
> + cmpl %ebx, %eax
> + jbe 1f
> + movl %eax, %ebx
This looks messy with different idention of the operand.
Compare it to this:
> + movl %ebp, %ebx
> + cmpl %ebx, %eax
> + jbe 1f
> + movl %eax, %ebx
I like the latter more with vertical alignment of the operand.
I see that you add spaces around operators (,) - good.
> */
>
> #ifdef CONFIG_RELOCATABLE
> - movl %ebp, %ebx
> - addl $(CONFIG_PHYSICAL_ALIGN - 1), %ebx
> - andl $(~(CONFIG_PHYSICAL_ALIGN - 1)), %ebx
> + movl $LOAD_PHYSICAL_ADDR, %eax
> + movl %ebp, %ebx
> + cmpl %ebx, %eax
> + jbe 1f
> + movl %eax, %ebx
> +1:
> + addl $(CONFIG_PHYSICAL_ALIGN - 1), %ebx
> + andl $(~(CONFIG_PHYSICAL_ALIGN - 1)), %ebx
> #else
> movl $LOAD_PHYSICAL_ADDR, %ebx
> #endif
> + movl %ebx, %edi /* Save kernel target address */
>
> /* Replace the compressed data size with the uncompressed size */
> subl input_len(%ebp), %ebx
> @@ -84,27 +90,30 @@ ENTRY(startup_32)
> addl $4095, %ebx
> andl $~4095, %ebx
Not part of your patch...
But this is most likely (PAGE_SIZE - 1) so it would be better to write so.
>
> -/* Copy the compressed kernel to the end of our buffer
> +/*
> + * Set up the stack
> + */
> + leal boot_stack_end(%ebx), %esp
> + pushl %edi /* Saved kernel target address */
> +
> +/*
> + * Copy the compressed kernel to the end of our buffer
> * where decompression in place becomes safe.
> */
> pushl %esi
> - leal _ebss(%ebp), %esi
> - leal _ebss(%ebx), %edi
> - movl $(_ebss - startup_32), %ecx
> + leal (_bss-4)(%ebp), %esi
> + leal (_bss-4)(%ebx), %edi
> + movl $(_bss - startup_32), %ecx
> + shrl $2, %ecx
I do not see why you mess around with _bss here?
Do you use .bss for decompression?
Note: I do not speak x86 assembler so I have not tried to
understand the functionality.
I tired to grasp the changes around clear bss - but faild :-(
Sam
