Patchwork fix multiboot loading if load_end_addr == 0 (fwd)

login
register
mail settings
Submitter Scott Moser
Date March 18, 2012, 5:45 p.m.
Message ID <alpine.DEB.2.02.1203181344200.4638@brickies>
Download mbox | patch
Permalink /patch/147408/
State New
Headers show

Comments

Scott Moser - March 18, 2012, 5:45 p.m.
Re-sending to qemu-devel.  I'd originally sent this to kvm mailing list.


---------- Forwarded message ----------
Date: Sat, 17 Mar 2012 00:08:06
From: Scott Moser <smoser@ubuntu.com>
To: kvm@vger.kernel.org
Subject: [PATCH] fix multiboot loading if load_end_addr == 0

The previous 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.

This was raised initially as launchpad bug
https://bugs.launchpad.net/qemu/+bug/957622
Serge E. Hallyn - March 20, 2012, 3:04 p.m.
Quoting Scott Moser (smoser@ubuntu.com):
> Re-sending to qemu-devel.  I'd originally sent this to kvm mailing list.
> 
> 
> ---------- Forwarded message ----------
> Date: Sat, 17 Mar 2012 00:08:06
> From: Scott Moser <smoser@ubuntu.com>
> To: kvm@vger.kernel.org
> Subject: [PATCH] fix multiboot loading if load_end_addr == 0
> 
> The previous 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.
> 
> This was raised initially as launchpad bug
> https://bugs.launchpad.net/qemu/+bug/957622
> 

Tested-by: Serge Hallyn <serge.hallyn@canonical.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 Liguori - March 26, 2012, 7:17 p.m.
On 03/18/2012 12:45 PM, Scott Moser wrote:
> Re-sending to qemu-devel.  I'd originally sent this to kvm mailing list.

Can you submit as a proper patch with a Signed-off-by?

Regards,

Anthony Liguori

>
> ---------- Forwarded message ----------
> Date: Sat, 17 Mar 2012 00:08:06
> From: Scott Moser<smoser@ubuntu.com>
> To: kvm@vger.kernel.org
> Subject: [PATCH] fix multiboot loading if load_end_addr == 0
>
> The previous 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.
>
> This was raised initially as launchpad bug
> https://bugs.launchpad.net/qemu/+bug/957622
>
> 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);