[v3, 3/7] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Kevin Hao]

On Tue, Jan 07, 2014 at 05:46:04PM -0600, Scott Wood wrote:
> On Sat, 2014-01-04 at 14:34 +0800, Kevin Hao wrote:
> > Actually there still have one limitation that we have to make sure
> > that the kernel and dtb are in the 64M memory mapped by the init tlb entry.
> > I booted the kernel successfully by using the following u-boot commands:
> >   setenv fdt_high 0xffffffff
> >   dhcp 6000000 128.224.162.196:/vlm-boards/p5020/uImage
> >   tftp 6f00000 128.224.162.196:/vlm-boards/p5020/p5020ds.dtb
> >   bootm 6000000 - 6f00000                                                                                                                                         
> 
> OK, that was it -- I hadn't set fdt_high and thus U-Boot was relocating
> the fdt under 64M.
> 
> We should probably be using ioremap_prot() (or some other mechanism) to

It is too early to use ioremap_prot() for this case.

> create a special mapping, rather than assuming the fdt is covered by the
> initial TLB entry.  That doesn't need to happen as part of this
> patchset, of course, as it's not a new limitation.

In order to fix this limitation we would have to create a separate map for
the dtb if it is not covered by the init 64M tlb. I would like to give it
a try if I can get some time.

> 
> > > I'm having a hard time following the logic here.  What is PAGE_OFFSET -
> > > offset supposed to be?  Why would we map anything belowe PAGE_OFFSET?
> > 
> > No, we don't map the address below PAGE_OFFSET.
> >     memstart_addr is the physical start address of RAM.
> >     start is the kernel running physical address aligned with 64M.
> > 
> >     offset = memstart_addr - start
> > 
> > So if memstart_addr < start, the offset is negative. The PAGE_OFFSET - offset
> > is the virtual start address we should use for the init 64M map. It's above
> > the PAGE_OFFSET instead of below.
> 
> Oh.  I think it'd be more readable to do "offset = start -
> memstart_addr" and add offset instead of subtracting it.

Yes, I agree. The reason that I use "offset = memstart_addr - start" is that
it seems "memstart_addr" is always greater than "start" when we are booting
a kdump kernel with a kernel option like "crashkernel=64M at 80M". :-)

> 
> Also, offset should be phys_addr_t -- even if you don't expect to
> support offsets greater than 4G on 32-bit, it's semantically the right
> type to use.  Plus, "int" would break if this code were ever used with
> 64-bit.

I thought about using phy_addr_t for the "offset" originally but gave it up
for the following reasons:
  * It will not be greater than 4G.
  * We have to use the ugly #ifdef CONFIG_PHYS_64BIT in restore_to_as0().
  * Need more registers for arguments for restore_to_as0().

Of course you can change it to phys_addr_t if you prefer.

Thanks,
Kevin
> 
> If you're OK with these changes, I can fix it while applying.
> 
> -Scott
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20140108/ff0a4881/attachment.sig>