Patchwork fix multiboot loading if load_end_addr == 0

login
register
mail settings
Submitter Scott Moser
Date March 26, 2012, 7:27 p.m.
Message ID <alpine.DEB.2.02.1203261523180.4638@brickies>
Download mbox | patch
Permalink /patch/148817/
State New
Headers show

Comments

Scott Moser - March 26, 2012, 7:27 p.m.
The previous multiboot load code did not treat the case where
load_end_addr was 0 specially.  The multiboot specification says the
following:
 * load_end_addr
   Contains the physical address of the end of the data segment.
   (load_end_addr - load_addr) specifies how much data to load. This
   implies that the text and data segments must be consecutive in the
   OS image; this is true for existing a.out executable formats. If
   this field is zero, the boot loader assumes that the text and data
   segments occupy the whole OS image file.

Signed-off-by: Scott Moser <smoser@ubuntu.com>
Kevin Wolf - May 26, 2012, 10:17 p.m.
Am Montag, 26. März 2012 21:27:00 schrieb Scott Moser:
> The previous multiboot load code did not treat the case where
> load_end_addr was 0 specially.  The multiboot specification says the
> following:
>  * load_end_addr
>    Contains the physical address of the end of the data segment.
>    (load_end_addr - load_addr) specifies how much data to load. This
>    implies that the text and data segments must be consecutive in the
>    OS image; this is true for existing a.out executable formats. If
>    this field is zero, the boot loader assumes that the text and data
>    segments occupy the whole OS image file.
>
> Signed-off-by: Scott Moser <smoser@ubuntu.com>
>
> diff --git a/hw/multiboot.c b/hw/multiboot.c
> index b4484a3..b1e04c5 100644
> --- a/hw/multiboot.c
> +++ b/hw/multiboot.c
> @@ -202,10 +202,16 @@ 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 = 0;
>          mh_entry_addr = ldl_p(header+i+28);
> -        mb_kernel_size = mh_bss_end_addr - mh_load_addr;
> +
> +        if (mh_load_end_addr) {
> +            mb_kernel_size = mh_bss_end_addr - mh_load_addr;
> +            mb_load_size = mh_load_end_addr - mh_load_addr;
> +        } else {
> +            mb_kernel_size = kernel_file_size - mb_kernel_text_offset;
> +            mb_load_size = mb_kernel_size;
> +        }
>
>          /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_VBE.
>          uint32_t mh_mode_type = ldl_p(header+i+32);


Anthony, what happened with this patch? Can we still get it into 1.1?

Kevin
Anthony Liguori - May 27, 2012, 12:52 a.m.
On 05/26/2012 05:17 PM, Kevin Wolf wrote:
> Am Montag, 26. März 2012 21:27:00 schrieb Scott Moser:
>> The previous multiboot load code did not treat the case where
>> load_end_addr was 0 specially.  The multiboot specification says the
>> following:
>>   * load_end_addr
>>     Contains the physical address of the end of the data segment.
>>     (load_end_addr - load_addr) specifies how much data to load. This
>>     implies that the text and data segments must be consecutive in the
>>     OS image; this is true for existing a.out executable formats. If
>>     this field is zero, the boot loader assumes that the text and data
>>     segments occupy the whole OS image file.
>>
>> Signed-off-by: Scott Moser<smoser@ubuntu.com>
>>
>> diff --git a/hw/multiboot.c b/hw/multiboot.c
>> index b4484a3..b1e04c5 100644
>> --- a/hw/multiboot.c
>> +++ b/hw/multiboot.c
>> @@ -202,10 +202,16 @@ 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 = 0;
>>           mh_entry_addr = ldl_p(header+i+28);
>> -        mb_kernel_size = mh_bss_end_addr - mh_load_addr;
>> +
>> +        if (mh_load_end_addr) {
>> +            mb_kernel_size = mh_bss_end_addr - mh_load_addr;
>> +            mb_load_size = mh_load_end_addr - mh_load_addr;
>> +        } else {
>> +            mb_kernel_size = kernel_file_size - mb_kernel_text_offset;
>> +            mb_load_size = mb_kernel_size;
>> +        }
>>
>>           /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_VBE.
>>           uint32_t mh_mode_type = ldl_p(header+i+32);
>
>
> Anthony, what happened with this patch?

It wasn't top posted so it got lost.

> Can we still get it into 1.1?

Yeah, I'll queue it.

Regards,

Anthony Liguori

>
> Kevin
Anthony Liguori - May 30, 2012, 2:12 a.m.
On 03/27/2012 03:27 AM, Scott Moser wrote:
> The previous multiboot load code did not treat the case where
> load_end_addr was 0 specially.  The multiboot specification says the
> following:
>   * load_end_addr
>     Contains the physical address of the end of the data segment.
>     (load_end_addr - load_addr) specifies how much data to load. This
>     implies that the text and data segments must be consecutive in the
>     OS image; this is true for existing a.out executable formats. If
>     this field is zero, the boot loader assumes that the text and data
>     segments occupy the whole OS image file.
>
> Signed-off-by: Scott Moser<smoser@ubuntu.com>

Applied.  Thanks.

Regards,

Anthony Liguori

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

Patch

diff --git a/hw/multiboot.c b/hw/multiboot.c
index b4484a3..b1e04c5 100644
--- a/hw/multiboot.c
+++ b/hw/multiboot.c
@@ -202,10 +202,16 @@  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 = 0;
         mh_entry_addr = ldl_p(header+i+28);
-        mb_kernel_size = mh_bss_end_addr - mh_load_addr;
+
+        if (mh_load_end_addr) {
+            mb_kernel_size = mh_bss_end_addr - mh_load_addr;
+            mb_load_size = mh_load_end_addr - mh_load_addr;
+        } else {
+            mb_kernel_size = kernel_file_size - mb_kernel_text_offset;
+            mb_load_size = mb_kernel_size;
+        }

         /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_VBE.
         uint32_t mh_mode_type = ldl_p(header+i+32);