[PATCH 02/14] x86, boot: honor CONFIG_PHYSICAL_START when relocatable
H. Peter Anvin
hpa at zytor.com
Fri May 8 12:58:20 EDT 2009
Sam Ravnborg wrote:
>
> 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.
head_32.S was already written in the style with space (rather than tab)
between opcode and operands. There was a total of three instructions
that didn't follow the pattern. Re-styling the whole file is possible,
of course, but should be a separate patch entirely.
>> 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?
This chunk of code is about moving the initialized segments into place.
Moving the contents of the bss is a bug (and it clobbers the stack, the
setup of which I moved earlier in the code.)
However, using _edata would give an unaligned symbol, so use _bss
instead as being an aligned symbol at the beginning of the _bss; we thus
move the range from startup_32 to _bss.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
More information about the kexec
mailing list