[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [FOR 0.12][PATCH] Fix loading of ELF multiboot kernels
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [FOR 0.12][PATCH] Fix loading of ELF multiboot kernels |
Date: |
Wed, 16 Dec 2009 13:04:49 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) |
Kevin Wolf <address@hidden> writes:
> Am 16.12.2009 10:51, schrieb Markus Armbruster:
>> Kevin Wolf <address@hidden> writes:
>>
>>> The multiboot implementation assumed that there is only one program header
>>> (which contains the entry point) and that the entry point is at the start of
>>> the code. This doesn't hold true generally and caused too little data to be
>>> loaded.
>>
>> Out of curiosity: does this affect images people actually use?
>> Examples?
>
> It hit me, so yes. Probably no widespread kernels as Alex' tests looked
> fine (not sure what he tests, probably Xen and his OS X bootloader?). In
> my case it was a simple test kernel. I'd expect the sum of unknown
> research or hobby kernels to be a major use case for Multiboot support,
> though.
Thanks.
>>> Fix the loading code to pass the whole loaded data to the Multiboot Option
>>> ROM.
>>>
>>> Signed-off-by: Kevin Wolf <address@hidden>
>>> ---
>>> hw/loader.c | 2 --
>>> hw/pc.c | 10 ++++++----
>>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/loader.c b/hw/loader.c
>>> index 2d7a2c4..4c6981f 100644
>>> --- a/hw/loader.c
>>> +++ b/hw/loader.c
>>> @@ -718,8 +718,6 @@ int rom_copy(uint8_t *dest, target_phys_addr_t addr,
>>> size_t size)
>>> QTAILQ_FOREACH(rom, &roms, next) {
>>> if (rom->max)
>>> continue;
>>> - if (rom->min > addr)
>>> - continue;
>>> if (rom->min + rom->romsize < addr)
>>> continue;
>>> if (rom->min > end)
>>
>> I don't understand this hunk.
>
> The original code assumes that there is only one ROM that contains the
> entry point. In fact, each program header of an ELF file becomes it own
> ROM, though. So if you have a first PH which contains the entry point
> (or now the lowest loaded address) and a second PH at a higher address,
> this test would fail for the second one even though we need to load it.
>
> What we really need to test is if [addr, end] and [rom->min, rom->min +
> rom->romsize] have an intersection. This is what the following two ifs
> already check.
So rom_copy() is supposed to copy all fixed-address ROMs in the range
addr..addr+size? Makes sense then.