Re: [PATCH 6/6] x86/boot: Ensure the BSS is aligned on an 8 byte boundary

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 09/04/15 16:15, Tim Deegan wrote:
> At 18:26 +0100 on 07 Apr (1428431180), Andrew Cooper wrote:
>> --- a/xen/arch/x86/boot/head.S
>> +++ b/xen/arch/x86/boot/head.S
>> @@ -127,7 +127,8 @@ __start:
>>          mov     $sym_phys(__bss_end),%ecx
>>          sub     %edi,%ecx
>>          xor     %eax,%eax
>> -        rep     stosb
>> +        shr     $2,%ecx
>> +        rep     stosl
> Should this be shr $3 and stosq?  You are aligning to 8 bytes in the
> linker runes.

It is still 32bit code here, so no stosq available.

I do however happen to know that the impending multiboot2 entry point is
64bit and is able to clear the BSS with stosq.

>
>>  
>>          /* Interrogate CPU extended features via CPUID. */
>>          mov     $0x80000000,%eax
>> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
>> index 4699a04..b1926e3 100644
>> --- a/xen/arch/x86/xen.lds.S
>> +++ b/xen/arch/x86/xen.lds.S
>> @@ -163,6 +163,7 @@ SECTIONS
>>    __init_end = .;
>>  
>>    .bss : {                     /* BSS */
>> +       . = ALIGN(8);
> Here, we're already aligned to STACK_SIZE

So we are - that should be fixed up.

That alignment is not relevant to .init, but is relevant to .bss

> , which the
> .bss.stack_aligned just below is relying on.  So on the one hand this
> new alignment comment is sort-of-harmless, but on the other hand it
> distracts from the larger and more important alignment.

I will see about fixing this up differently, but with the same overall
effect that stosl/stosq can be used.

~Andrew