Re: [Qemu-devel] [PATCH] Fix -kernel on target-ppc

[Edgar E. Iglesias]

On Sun, Jan 25, 2009 at 11:03:47AM +0100, Alexander Graf wrote:
>
> On 25.01.2009, at 10:54, Aurelien Jarno wrote:
>
>> On Sun, Jan 25, 2009 at 01:28:36AM +0100, Alexander Graf wrote:
>>>
>>>
>>>
>>>
>>> On 25.01.2009, at 00:59, Aurelien Jarno <address@hidden> wrote:
>>>
>>>> On Sat, Jan 24, 2009 at 10:57:19PM +0100, Alexander Graf wrote:
>>>>>
>>>>> On 24.01.2009, at 22:48, Aurelien Jarno wrote:
>>>>>
>>>>>> On Sat, Jan 24, 2009 at 09:58:35PM +0100, Alexander Graf wrote:
>>>>>>> OpenBIOS searches for the preloaded kernel at 0x1000000, so let's
>>>>>>> put it there instead of an invalid location.
>>>>>>
>>>>>> Your patch is actually wrong, the second argument of load_elf() is
>>>>>> an
>>>>>> offset to the physical address (as found in the elf header), and
>>>>>> not a
>>>>>> load address.
>>>>>>
>>>>>> By chance the physical address of a >= 2.6.25 kernel is
>>>>>> 0x00000000, so
>>>>>> your patch works. But it will break supports for < 2.6.25 kernel as
>>>>>> their physical address is 0xc0000000. Not that they are only the
>>>>>> default
>>>>>> values, they can be changed in the .config file.
>>>>>
>>>>> Aah, that explains why :-).
>>>>>
>>>>>> I have already proposed a patch to use the virtual address of the
>>>>>> elf
>>>>>> header as done by yaboot or quik, but I have been told it is
>>>>>> actually
>>>>>> wrong.
>>>>>>
>>>>>> We have to find another way to load the elf file at a fixed address.
>>>>>
>>>>> Hm - can't we just give load_elf an override value for the base
>>>>> address?
>>>>>
>>>>>           /* address_offset is hack for kernel images that are
>>>>>              linked at the wrong physical address.  */
>>>>>           addr = ph->p_paddr + address_offset;
>>>>>
>>>>>           cpu_physical_memory_write_rom(addr, data, mem_size);
>>>>>
>>>>> Just pass another variable here that overrides addr and doesn't
>>>>> calculate it?
>>>>
>>>> Except that they can be more than one segment to load, so the last one
>>>> will overwrite the previous ones. The PowerPC kernels I have seen only
>>>> have one load segment, but I am not sure it is always the case.
>>>
>>> But then the addr hack wouldn't work either, right? It's just a question
>>> if addr_offset is relative or absolute here.
>>
>> addr_offset is just an offset that is added to the load address of the
>> elf header.
>>
>>> And fwiw in this case relative to the elf header's value doesn't make
>>> any sense at all when the firmware expects the blob on a specific
>>> address.
>>
>> As far as I understand it has been done like that to be able to support
>> multiple segments. If the elf header says that the first segment has to
>> be loaded at 0xc0000000 and the second at 0xd0000000, loading both at
>> 0x10000000 won't work. Loading them with an offset of -0xb000000 will
>> load the first one at 0x10000000 and the second one just after at
>> 0x20000000.
>
> Aaah I'm starting to see the picture now :-). Sorry, I probably shouldn't 
> do this on a weekend at night.
>
> So at least for the Linux kernel case it would be enough to know which 
> lowest address the kernel is loaded to, right? Because that's what we need 
> to use as negative offset.
>
> So if we call load_elf twice, once to get the lowest address the binary is 
> loaded to (lowaddr) and the second one to actually load it, this should at 
> least fix it for Linux without breaking multiple segment support, as long 
> as the first segment is the text segment.
>
> Am I getting this right?

I think so, that same kind of speculative loading is what I had in mind:
http://lists.gnu.org/archive/html/qemu-devel/2009-01/msg00438.html

Best regards