On Wed, 2014-01-08 at 10:42 +0800, Kevin Hao wrote:

On Tue, Jan 07, 2014 at 05:46:04PM -0600, Scott Wood wrote:

quoted

On Sat, 2014-01-04 at 14:34 +0800, Kevin Hao wrote:

quoted

quoted

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@80M". :-)

...so there is a situation where you map below PAGE_OFFSET. :-)
 

quoted

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.

I'd at least like to make it "long".

-Scott