[Qemu-devel] Re: [Bochs-developers] [PATCH v2 4/6] Execute rombios32 code from romaddress 0xe0000.

[Gleb Natapov]

On Sat, Nov 08, 2008 at 12:20:56AM +0100, Sebastian Herbszt wrote:
>> index c7f6066..113a2c0 100644
>> --- a/bios/rombios32.ld
>> +++ b/bios/rombios32.ld
>> @@ -3,14 +3,12 @@ OUTPUT_ARCH(i386)
>> ENTRY(_start);
>> SECTIONS
>> {
>> -        . = 0x00040000;
>> +        . = 0x000e0000;
>>         .text     : { *(.text)    }
>>         .rodata    : { *(.rodata) }
>> -        . = ALIGN(4096);
>> -        .data     : { *(.data)    }
>> -        __bss_start = . ;
>> -        .bss      : { *(.bss) *(COMMON) }
>>         _end = . ;
>> +        .data 0x700 : AT (_end) { __data_start = .; *(.data); __data_end = 
>> .;}
>> +        .bss      : { __bss_start = .; *(.bss) *(COMMON); __bss_end = .;}
>>         /DISCARD/ : { *(.stab)
>>                      *(.stabstr)
>>                      *(.comment)
>
> The .data section is currently empty because all data is read-only and put
> into .rodata, .rodata.str1.1 and .rodata.str1.4. As soon as we put something
> into .data we get a link error because .data now overlaps .rodata.str1.1.
> The error is gone if we put all .rodata input sections into .rodata output
> section with ".rodata    : { *(.rodata*)".
>
Oops. I'll fix that. Worked for me in KVM bios even though its .data is
not empty, but now I see that it worked accidentally because there is ALIGN
after .rodata there.

>> diff --git a/bios/rombios32start.S b/bios/rombios32start.S
>> index 601e2b0..1900261 100644
>> --- a/bios/rombios32start.S
>> +++ b/bios/rombios32start.S
>> @@ -32,10 +32,17 @@ _start:
>>   /* clear bss section */
>>   xor %eax, %eax
>>   mov $__bss_start, %edi
>> -  mov $_end, %ecx
>> +  mov $__bss_end, %ecx
>>   sub %edi, %ecx
>>   rep stosb
>
> We do overwrite data at 0x700 here. Is this acceptable for S3 resume?
>
First page is reserved for BIOS use.

>> +  /* copy data section */
>> +  mov $_end, %esi
>> +  mov $__data_start, %edi
>> +  mov $__data_end, %ecx
>> +  sub %edi, %ecx
>> +  rep movsb
>> +
>
> As described above this is currently a no-op, because
> __data_start = __data_end = 0x700.
>
I know, but do we want to remember to add this after changing C code?
There is no harm to add it here and let it do nothing, and don't worry
about adding .data variable later.

--
                        Gleb.