[PATCH 4/4] arm64: align PHYS_OFFSET to block size

On Mon, Mar 30, 2015 at 08:08:52PM +0200, Ard Biesheuvel wrote:

On 30 March 2015 at 17:00, Catalin Marinas [off-list ref] wrote:

quoted

On Mon, Mar 30, 2015 at 04:00:31PM +0200, Ard Biesheuvel wrote:

quoted

On 30 March 2015 at 15:49, Catalin Marinas [off-list ref] wrote:

quoted

Can we defer the setting of PHYS_OFFSET until we parse the DT memory
nodes?

I experimented a bit with that, but it is quite hairy. Any
manipulation of the page tables goes through __va/__pa, so you need a
valid PHYS_OFFSET there to ensure they point at the right physical
region.

Yes, so we need set PHYS_OFFSET before we start using __va/__pa.

quoted

But PHYS_OFFSET also needs to be small enough for the DT
parsing code not to disregard regions that are below it.

With PHYS_OFFSET as 0 initially, no regions would be dropped. But we
could write a simplified early_init_dt_add_memory_arch() which avoids
the PHYS_OFFSET check while setting the actual PHYS_OFFSET to the
minimal address detected. I can see the generic function only uses
__pa() to get the PHYS_OFFSET.

Excellent idea.

quoted

quoted

And then
there is the memblock limit to ensure that early dynamically allocated
page tables come from a region that is already mapped.

By the time we start using memblock allocations, we have a PHYS_OFFSET
set. The early DT parsing does not require any memory allocations AFAIK.
We need to make sure that setup_machine_fdt creates a VA mapping of the
DT and does not require the __va() macro (I thought I've seen some
patches to use fixmap for DT).

Yes, so did I :-)

But using early_fixmap() implies using the ordinary page tables
manipulation code, which uses __pa/__va
So instead, I should refactor those patches to simply pick a VA offset
and map the FDT there from head.S

I haven't dug out those patches yet but in principle you should not care
about __va, just __pa for populating the pmd/pud/pgd. Since with fixmap
we pre-allocate the page tables in the kernel data section
(bm_pud/pmd/pte), a __pa implementation that takes KERNEL_PAGE_OFFSET
into account (as per these patches) is enough, you don't really care
about the linear PAGE_OFFSET at this stage since you would not provide
__pa with such virtual address until after setup_machine_fdt().

quoted

quoted

I think it may be doable, but it would require some significant
hacking, e.g., call early_init_scan_dt() at its physical address with
only the ID map loaded and the MMU and caches on, and only after that
start populating the virtual address space. Or at least only populate
the lower half, i.e., mappings below PAGE_OFFSET for the kernel and
the FDT

With some form of your patches, we already decouple the PAGE_OFFSET from
the kernel text mapping. We map the latter at some very high
KERNEL_PAGE_OFFSET, the DT via fixmap (which is part of the kernel data
section, so mapped at the high KERNEL_PAGE_OFFSET). Once we start
calling early_init_dt_add_memory_arch(), we set the real PAGE_OFFSET and
are free to use __pa/__va after setup_machine_fdt(). The actual linear
mappings will be created later via paging_init().

OK, that sounds like it could work.

The only thing to note is (and this should answer Mark's question in
the other thread) is that the pgdir region should be mapped via the
linear mapping as well.

I'm not sure I followed Mark's comments fully but why can't use just
access swapper_pg_dir only via the KERNEL_PAGE_OFFSET mapping? Such
kernel text/data mapping is not going away. There are some weird things
at some point as functions like pmd_page_vaddr() would return the linear
mapping. I don't think anything would break but I cannot guarantee (I
don't think such vaddr functions would be called before we have the
memory mapped and PHYS_OFFSET set anyway).

The alternative is to make __va() check whether the input physical
address falls within the pgdir region, and subtract the PAGE_OFFSET -
KERNEL_PAGE_OFFSET from the return value, but this is less trivial
than the __pa() counterpart which only needs to check a single bit.

It looks ugly.

So I propose we take defines KERNEL_PAGE_OFFSET as (PAGE_OFFSET -
SZ_64M) and FDT_PAGE_OFFSET (or whatever you want to call it) as
(PAGE_OFFSET - SZ_2M).
That way, the kernel, fdt and module space always share the same 128
MB window, which we can move around in the vmalloc space later if we
want to implement kASLR.

The only downside is that to make __pa only a bit test requires
PAGE_OFFSET to be always aligned to a power of two. Right now we kind of
use (waste) half of the VA space on vmalloc + I/O + modules but there
are no restrictions on PAGE_OFFSET placement. If we decouple the
KERNEL_PAGE_OFFSET from PAGE_OFFSET, we could make PAGE_OFFSET start at
the beginning of the VA space and the kernel at the top 64MB (+ 64MB
below it for modules). The vmalloc + I/O can leave somewhere in between
but this way we allow the PAGE_OFFSET to grow beyond half the VA space
easily and we still have a single bit check in __pa (only that it's a
higher bit).

-- 
Catalin