[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] multiboot: Fix bss segment support
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [PATCH v2] multiboot: Fix bss segment support |
Date: |
Tue, 20 Dec 2011 15:07:32 +0100 |
On 20.12.2011, at 12:53, Göran Weinholt wrote:
> Alexander Graf <address@hidden> writes:
>
>> On 19.12.2011, at 23:01, Anthony Liguori wrote:
>>
>> On 12/19/2011 11:35 AM, Alexander Graf wrote:
>>
>> On 24.07.2011, at 17:55, Göran Weinholt wrote:
>>
>> Multiboot images can specify a bss segment. The boot loader must
>> clear
>>
>> the memory of the bss and ensure that no modules or structures are
>>
>> allocated inside it. Several fields are provided in the Multiboot
>>
>> header that were previously not used properly. The header is now
>> used
>>
>> to determine how much data should be read from the image and how
>> much
>>
>> memory should be reserved to the bss segment.
>>
>> This patch breaks the OSX booter:
>>
>> http://people.exactcode.de/~rene/mac/boot
>>
>> How is this licensed? Is there source available?
>>
>> Yes, it's the XNU booter, but binaries are easier to debug when it comes to
>> reading out the bootloader info:
>>
>> http://tgwbd.org/darwin/boot.html
>>
>> I don't know if the binary above is 1:1 built from those sources, but it's
>> definitely close enough.
>
> I was unable to build that program from source, but I found a pre-built
> binary that triggered the same bug. Please test the following patch.
>
>
> Subject: [PATCH] multiboot: mh_load_end_addr and mh_bss_end_addr may be zero
>
> There are two special cases in the address fields of the multiboot
> format. If mh_load_end_addr is zero then the whole image file should
> be loaded and if mh_bss_end_addr is zero then there is no bss segment.
> With this change it is again possible to boot kernels where these
> fields are zero.
>
> Signed-off-by: Göran Weinholt <address@hidden>
Yes, this patch makes things work again :). Thanks a lot!
The only thing I could nitpick on would be the coding style - checkpatch.pl
complains :). Could you please resend with braces?
Justin, Please also queue this for 1.0-stable when it comes in its final form.
Tested-by: Alexander Graf <address@hidden>
Alex
> ---
> hw/multiboot.c | 13 ++++++++++++-
> 1 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/hw/multiboot.c b/hw/multiboot.c
> index b4484a3..a43d56c 100644
> --- a/hw/multiboot.c
> +++ b/hw/multiboot.c
> @@ -202,10 +202,21 @@ int load_multiboot(void *fw_cfg,
> uint32_t mh_bss_end_addr = ldl_p(header+i+24);
> mh_load_addr = ldl_p(header+i+16);
> uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr);
> - uint32_t mb_load_size = mh_load_end_addr - mh_load_addr;
> + uint32_t mb_load_size;
> +
> + /* A load end address of zero indicates that the whole file
> + * should be loaded. */
> + if (!mh_load_end_addr)
> + mh_load_end_addr = kernel_file_size + mh_load_addr;
> +
> + /* A bss end address of zero indicates that there is no bss
> + * segment. */
> + if (!mh_bss_end_addr)
> + mh_bss_end_addr = mh_load_end_addr;
WARNING: braces {} are necessary for all arms of this statement
#83: FILE: hw/multiboot.c:209:
+ if (!mh_load_end_addr)
[...]
WARNING: braces {} are necessary for all arms of this statement
#88: FILE: hw/multiboot.c:214:
+ if (!mh_bss_end_addr)
[...]