Re: [RFC PATCH 3/3] arm64: hibernate: idmap the single page that holds the copy page routines

From: James Morse <james.morse@arm.com>
To: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC PATCH 3/3] arm64: hibernate: idmap the single page that holds the copy page routines
Date: Wed, 25 Mar 2020 09:58:02 +0000	[thread overview]
Message-ID: <b983dfbf-49e2-d32c-e3a5-781df4f9ea4e@arm.com> (raw)
In-Reply-To: <CA+CK2bAEyp7Kcc_qEOfchNvyEHSVnjAXD-LB9NGbUOJ_xZV1AQ@mail.gmail.com>

Hi Pavel,

On 3/20/20 9:22 PM, Pavel Tatashin wrote:
> Soon, I will send out updated MMU enabled kexec series which will have
> this work included. I appreciate your help with this.
> 
>> Today the address it uses for this mapping is arbitrary, but to allow
>> kexec to reuse this code, it needs to be idmapped. To idmap the page
>> we must avoid the kernel helpers that have VA_BITS baked in.
> 
> Makes sense.

>> Convert create_single_mapping() to take a single PA, and idmap it.
> 
> I like the idea of using idmap in both places!

This is the only way this should work. Both hibernate and kexec replace
all of memory, with the MMU on, while using a temporary set of page tables.

As much of the code that does this should be shared.

Hibernate already does all of this, so kexec should re-use that code.

>> The page tables are built in the reverse order to normal using
>> pfn_pte() to stir in any bits between 52:48. T0SZ is always increased
>> to cover 48bits, or 52 if the copy code has bits 52:48 in its PA.
> 
> I do not think this will work for kexec case. In hibernate we map only
> one page, so we can allocate every level from bottom to top, but in
> kexec we map many pages. So, upper levels might already exist. I think
> we will  need to modify the loop to still go from top to bottom.

No.

We should not have a second set of helpers for building page tables for
kexec, its an unnecessary maintenance burden.

You keep coming back to this because you are trying to idmap all memory
on arm64. You do not need to do this.

You only need one page idmaped so you can switch TTBR1_EL1, and turn the
MMU off.

You can do the copy of memory using a copy of the linear map in
TTBR1_EL1. For an example: hibernate does exactly this.

This saves all the hassle of nomap, reserved-firmware pages and the risk
of introducing mismatched attributes. (which would lead to mysterious
coherency issues for the next kernel)

Your answer is going to be that kexec's data structures are physically
addressed. The linear map, is linear: You can convert the
kexec:physical-address to a KASLR'd linear-map virtual address, with
addition. (beware, the kaslr offset is _signed_, it can be negative!)

The code in this RFC was particularly tricky to test as its behaviour
depends on which bits of a pointer are set.

This code is complicated, and impossible to debug if it goes wrong.
(photograph of a screen with the word 'Bye' on it anyone?). Worse: it
must not introduce coherency issues into the next kernel.

It must be as simple as possible. What you are proposing is not.

Thanks,

James

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel