Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 28 Feb 2013 23:02:03 +0200
From:      Aleksandr Rybalko <ray@freebsd.org>
To:        Tim Kientzle <kientzle@freebsd.org>
Cc:        freebsd-arm@freebsd.org
Subject:   Re: PHYSADDR
Message-ID:  <20130228230203.e6228ff1.ray@freebsd.org>
In-Reply-To: <1F9B0ED4-7FAF-43D8-BECD-1D42792E81DF@freebsd.org>
References:  <E886046B-1612-425B-902B-72D4B0E93618@freebsd.org> <1A428660-39FB-45EB-979B-103F7E83BC4A@bsdimp.com> <1F9B0ED4-7FAF-43D8-BECD-1D42792E81DF@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 28 Feb 2013 08:56:54 -0800
Tim Kientzle <kientzle@freebsd.org> wrote:

> 
> 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
> >> 
> > 
> 
> _______________________________________________
> freebsd-arm@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-arm
> To unsubscribe, send any mail to "freebsd-arm-unsubscribe@freebsd.org"

We will have problems sometime with pc & 0xf0000000, because a lot of
hardware already on market with RAM size more than 256M. So if loader
load kernel higher than 256M with 0xf0000000 mask we will assume wrong
segment start address.

IMO we need to get such info from DTB and better to parse it
with loader(8), then pass it with kenv to kernel.

Since FDT already have info about physical RAM location you can
calculate difference and make decision to move kernel lower, like
trampoline do.

Thanks!

WBW
-- 
Aleksandr Rybalko <ray@freebsd.org>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130228230203.e6228ff1.ray>