Re: [PATCH] libxc/arm: Correctly handle the difference between virtual and physical address

[Julien Grall <julien.grall@linaro.org>]

On 12/12/2013 10:35 AM, Ian Campbell wrote:
> On Wed, 2013-12-11 at 19:26 +0000, Julien Grall wrote:
>> On 12/11/2013 10:02 AM, Ian Campbell wrote:
>>> On Tue, 2013-12-10 at 18:16 +0000, Julien Grall wrote:
>>>> This issue is when as virt == phys, build ELF with virt != phys is very difficult.
>>>
>>> OK, so I think I was mislead by your commit message, this is not
>>> actually about virt vs phys as such (or as represented in the ELF
>>> header) but is actually more about link address vs. load address. Where
>>> link address == ELF vaddr (==paddr). And because the MMU is disable load
>>> address == some vaddr (==paddr), which may differ from the ELF vaddr.
>>>
>>> The terminology in xc_dom, which uses vaddr a lot because of PV x86,
>>> isn't helpful here but is that an accurate summary?
>>
>> Right, and this address is actually equal to ELF vaddr. But the device
>> tree base address will be computed with the physical address.
>>
>>> [...]
>>>> When the guest is creating, the ELF should loaded like zImage at the specific
>>>> physical address.
>>>
>>> So that would be my next question -- where does this load address come
>>> from?
>>
>> Load address ? Virtual ? Physical ?
>
> Load address == The literal address where the kernel image is loaded,
> e.g. on native it would be the physical address.
>
>>> I suppose it has to be the 0x80100000 kernel physical address you gave
>>> earlier?
>>>
>>> I think this means "where is dom->parms.virt_base" initialised. Have you
>>> added some ELF notes to your BSD kernel image?
>>
>> I have tried to play with the ELF notes. My current configuration is:
>>
>> XEN_ELFNOTE_VIRT_BASE    vaddr   	(0xC0000000)
>> XEN_ELFNOTE_PADDR_OFFSET vaddr		(0xC0000000)
>>
>> It's copied from Linux.
>
> XEN_ELFNOTE_PADDR_OFFSET is 0 for x86 Linux though.

Sorry I meant FreeBSD.

>
> It's possible that for ARM it should be RAMBASE or something? Or perhaps
> it should be 0 and the RAMBASE should be added by the tools. Or maybe it
> should be omitted and only VIRT_BASE is needed?

It's not clear to me what is the usage of XEN_ELFNOTE_PADDR_OFFSET. Is 
it the physical address? An offset?

> Does NetBSD absolutely require that it is loaded at precisely address
> 0x80100000 or is the requirement rambase + 0x00100000.
>
> If NetBSD has requirements about where RAM is the we may need a new note
> to designate the required RAM base address.

FreeBSD requires to know at compile time the RAM base address. I think 
it's fine now to hardcode to GUEST_RAMBASE. We can modify it later.

>> If I try to modify PADDR_OFFSET with (KERNBASE - PHYSADDR) or whatever
>> value it doesn't work. I got
>> "segment kernel too large (0x355 > 0x4000 - 0x80100 pages): Out of memory".
>
> It is very possible that the existing code buggily assumes in various
> places that RAM starts at address 0, because it has only ever been run
> on x86. You will probably need to fix this by using rambase_pfn in
> various places.

That would mean we need to fix up every seg->vstart/vend. The main issue 
is, at the end of xc_dom_alloc_segment, we will store vend to 
virt_alloc_end. This value we be retrieved to the next allocation..

I wrote the patch in this way because I think it's fine to do all the 
allocation in the virtual address space (even if MMU is turn off during 
at boot time) and then, thanks to xc_dom_alloc_segment, translate to 
physical address. It's the currently behaviour. But it works fine 
because for zImage, we made the assumption that virt_base == rambase_pfn 
<< PAGE_SHIFT.

The main change is using virt_base to allocate the device tree and 
substract different offset when the guest will start.

>>>>   Then the guest will start will MMU turn off, and during the
>>>> first instructions it will use fixup to get the right address.
>>>
>>> "fixup" == enable the MMU, or "fixup" == some sort of relocation/PIC?
>>
>> Some sort of relocation. When MMU is turn off, every time the kernel
>> needs to call asm callback, it will add an offset.
>
> Sounds a bit mad to me compared with writing PIC, but OK.

Most of the code is writing PIC. But as in Linux, for assembly code you 
can have something like that:

.word my_func

my_func will be replaced by the virtual address not the physical 
address. So when the kernel want to call the function with MMU disabled, 
it will need to add an offset. Thankfully, you don't need it everywhere.


-- 
Julien Grall