Patchwork multiboot: mh_load_end_addr and mh_bss_end_addr may be zero

login
register
mail settings
Submitter Göran Weinholt
Date Jan. 23, 2012, 12:49 p.m.
Message ID <87mx9ezir8.fsf@industria.weinholt.se>
Download mbox | patch
Permalink /patch/137353/
State New
Headers show

Comments

Göran Weinholt - Jan. 23, 2012, 12:49 p.m.
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 <goran@weinholt.se>
Tested-by: Alexander Graf <agraf@suse.de>
---
 hw/multiboot.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)
Kevin Wolf - Jan. 31, 2012, 2:10 p.m.
Am 23.01.2012 13:49, schrieb Göran Weinholt:
> 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 <goran@weinholt.se>
> Tested-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/multiboot.c |   15 ++++++++++++++-
>  1 files changed, 14 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/multiboot.c b/hw/multiboot.c
> index b4484a3..db28328 100644
> --- a/hw/multiboot.c
> +++ b/hw/multiboot.c
> @@ -202,10 +202,23 @@ 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;

This is only right if the OS image starts at offset 0 in the image file.
IIUC, in the general case it starts at byte i - (mh_header_addr -
mh_load_addr), so you need to subtract this from kernel_file_size.

Kevin
Göran Weinholt - Feb. 1, 2012, 12:44 a.m.
Kevin Wolf <kwolf@redhat.com> writes:

> Am 23.01.2012 13:49, schrieb Göran Weinholt:
>> 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 <goran@weinholt.se>
>> Tested-by: Alexander Graf <agraf@suse.de>
>> ---
>>  hw/multiboot.c |   15 ++++++++++++++-
>>  1 files changed, 14 insertions(+), 1 deletions(-)
>> 
>> diff --git a/hw/multiboot.c b/hw/multiboot.c
>> index b4484a3..db28328 100644
>> --- a/hw/multiboot.c
>> +++ b/hw/multiboot.c
>> @@ -202,10 +202,23 @@ 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;
>
> This is only right if the OS image starts at offset 0 in the image file.
> IIUC, in the general case it starts at byte i - (mh_header_addr -
> mh_load_addr), so you need to subtract this from kernel_file_size.

I think you're right. It would be good if someone could have a look at
fixing the patch. I will not have time to do it myself. Please Cc me so
that I can test it on my own kernel images.

Regards,

Patch

diff --git a/hw/multiboot.c b/hw/multiboot.c
index b4484a3..db28328 100644
--- a/hw/multiboot.c
+++ b/hw/multiboot.c
@@ -202,10 +202,23 @@  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;
+        }
 
         mh_entry_addr = ldl_p(header+i+28);
         mb_kernel_size = mh_bss_end_addr - mh_load_addr;
+        mb_load_size = mh_load_end_addr - mh_load_addr;
 
         /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_VBE.
         uint32_t mh_mode_type = ldl_p(header+i+32);