diff mbox

multiboot: Fix bss segment support

Message ID 87ei1ggf36.fsf@industria.weinholt.se
State New
Headers show

Commit Message

Göran Weinholt July 24, 2011, 10:14 a.m. UTC
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.

Signed-off-by: Göran Weinholt <goran@weinholt.se>
---
 hw/multiboot.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

Comments

Alexander Graf July 24, 2011, 1:02 p.m. UTC | #1
On 24.07.2011, at 12:14, 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 still doesn't zero the BSS region out, no? Does it work properly after system_reset?


Alex
Göran Weinholt July 24, 2011, 3:29 p.m. UTC | #2
Alexander Graf <agraf@suse.de> writes:

> On 24.07.2011, at 12:14, 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 still doesn't zero the BSS region out, no? Does it work
> properly after system_reset?

Thank you, that's a good point. The problem could even arise on the
first boot, because the buffer that holds the kernel is allocated with
qemu_malloc(). I'll send a new version of the patch shortly.

Regards,
diff mbox

Patch

diff --git a/hw/multiboot.c b/hw/multiboot.c
index 2426e84..4cff6aa 100644
--- a/hw/multiboot.c
+++ b/hw/multiboot.c
@@ -198,11 +198,13 @@  int load_multiboot(void *fw_cfg,
     } else {
         /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_ADDR. */
         uint32_t mh_header_addr = ldl_p(header+i+12);
+        uint32_t mh_load_end_addr = ldl_p(header+i+20);
+        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);
 
         mh_entry_addr = ldl_p(header+i+28);
-        mb_kernel_size = kernel_file_size - mb_kernel_text_offset;
+        mb_kernel_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);
@@ -212,8 +214,8 @@  int load_multiboot(void *fw_cfg,
 
         mb_debug("multiboot: mh_header_addr = %#x\n", mh_header_addr);
         mb_debug("multiboot: mh_load_addr = %#x\n", mh_load_addr);
-        mb_debug("multiboot: mh_load_end_addr = %#x\n", ldl_p(header+i+20));
-        mb_debug("multiboot: mh_bss_end_addr = %#x\n", ldl_p(header+i+24));
+        mb_debug("multiboot: mh_load_end_addr = %#x\n", mh_load_end_addr);
+        mb_debug("multiboot: mh_bss_end_addr = %#x\n", mh_bss_end_addr);
         mb_debug("qemu: loading multiboot kernel (%#x bytes) at %#x\n",
                  mb_kernel_size, mh_load_addr);
 
@@ -224,6 +226,7 @@  int load_multiboot(void *fw_cfg,
             exit(1);
         }
         fclose(f);
+        mb_kernel_size = mh_bss_end_addr - mh_load_addr;
     }
 
     mbs.mb_buf_phys = mh_load_addr;