PHYSADDR

[Tim Kientzle]

On Feb 27, 2013, at 10:41 PM, Warner Losh wrote:

> 
> On Feb 27, 2013, at 11:27 PM, Tim Kientzle wrote:
> 
>> Starting to look at what is needed for a Generic ARM kernel.
>> There's a lot here; I sincerely hope I'm not the only one… ;-)
>> 
>> First up:  Can we get rid of PHYSADDR?
> 
> There's lots of places in the tree that use it, but not so many that a whack-a-mole approach wouldn't work.

It's mostly only used in locore.S and machdep.c (and, of course,
the many copies of machdep.c).

>> I think we can always compute it at runtime from PC.
>> 
>> For example, I think this works in several places:
>>  and r0, pc, #0xF0000000
> 
> This only works early in boot before we've turned on the MMU, right?

I'm still pretty new to this section of the code, so might have
missed something, but I don't think we need PHYSADDR
after the kernel has relocated itself.

Do we?

> 
>> And I've found at least one reference that I think can be simply
>> eliminated:
>> 
>> Index: arm/elf_trampoline.c
>> ===================================================================
>> --- arm/elf_trampoline.c	(revision 247250)
>> +++ arm/elf_trampoline.c	(working copy)
>> @@ -169,7 +169,7 @@
>> void
>> _startC(void)
>> {
>> -	int physaddr = KERNPHYSADDR;
>> +	unsigned int physaddr = (unsigned int)&_start & 0xfffff000;
> 
> How'd you come up with this? Perhaps we should just define KERNPHYSADDR as this gross expression :)
> 
> But isn't _start a virtual address, not a physical address?

Honestly, I'm still trying to figure that part out.  ;-)

Disassembling the compiler output, taking the
address of the current function always uses a
PC-relative address.  So &_startC here would
give the current address of the function as it's
executing.

> 
>> 	int tmp1;
>> 	unsigned int sp = ((unsigned int)&_end & ~3) + 4;
>> #if defined(FLASHADDR) && defined(LOADERRAMADDR)
>> @@ -189,10 +189,9 @@
>> 		 */
>> 		unsigned int target_addr;
>> 		unsigned int tmp_sp;
>> -		uint32_t src_addr = (uint32_t)&_start - PHYSADDR + FLASHADDR
>> -		    + (pc - FLASHADDR - ((uint32_t)&_startC - PHYSADDR)) & 0xfffff000;
>> +		uint32_t src_addr = (uint32_t)&_start;
> 
> I'm not sure how this works…

Here's my reasoning:
FLASHADDR and PHYSADDR are always multiples of 4k,
so you can pull those out of the parentheses to get:

   _start  - PHYSADDR + FLASHADDR - FLASHADD + PHYSADDR + (pc - _startC) & 0xffffff000

And since pc was sampled early in _startC, the last expression
should always be zero.

I was a little surprised by this myself, so I might have missed
something.

>> -		target_addr = (unsigned int)&_start - PHYSADDR + LOADERRAMADDR;
>> +		target_addr = (unsigned int)&_start - (pc & 0xf0000000) + LOADERRAMADDR;
> 
> This might work, but I'd suggest a PC_TO_PHYSADDR(pc) macro or something similar…

I'll try that later on.  I suspect some of these PHYSADDRs
will simply go away, since I think what we really want in
the early bootstrap stage is just "what address is the
kernel currently executing at".  I still need to piece together
some more things before I understand that.

As with the above, at least some of the expressions
using PHYSADDR seem more complex than necessary.


>> 		tmp_sp = target_addr + 0x100000 +
>> 		    (unsigned int)&_end - (unsigned int)&_start;
>> 		memcpy((char *)target_addr, (char *)src_addr,
>> 
>> 
>> Tim
>> 
>