Hi Catalin, Vladimir,

On 09/10/17 16:23, Catalin Marinas wrote:

On Mon, Oct 09, 2017 at 01:55:32PM +0100, Vladimir Murzin wrote:

quoted

This patch adds support for Common Not Private translations on
different exceptions levels:

quoted

(2) For EL1 we postpone setting CNP till all cpus are up and rely on
    cpufeature framework to 1) patch the code which is sensitive to
    CNP and 2) update TTBR1_EL1 with CNP bit set. The only case where
    TTBR1_EL1 can be reprogrammed is hibirnation, so the code there is
    changed to save raw TTBR1_EL1 and blindly restore it on resume.

Even if you do this when all the CPUs are up, that's not always true.
Starting with maxcpus=1 allows something like systemd to bring up new
CPUs once user space starts.

For secondary CPUs cpu_enable_cnp() will be called to set CNP on TTBR1_EL1. But
what about cpuidle? This also resets the TTBR1_EL1 value.

The problem we have is that we don't know
what the firmware is doing, whether it's setting CnP or not. Maybe we
should add some statement in Documentation/arm64/booting.txt that
firmware must not use CnP at all.

quoted

diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
index 095d3c1..1d056f3 100644
--- a/arch/arm64/kernel/hibernate.c
+++ b/arch/arm64/kernel/hibernate.c

@@ -124,7 +124,7 @@ int arch_hibernation_header_save(void *addr, unsigned int max_size)
 		return -EOVERFLOW;
 
 	arch_hdr_invariants(&hdr->invariants);
-	hdr->ttbr1_el1		= __pa_symbol(swapper_pg_dir);
+	hdr->ttbr1_el1		= read_sysreg(ttbr1_el1);
 	hdr->reenter_kernel	= _cpu_resume;
 
 	/* We can't use __hyp_get_vectors() because kvm may still be loaded */

Are all the CPUs up when coming out of hibernation and restoring
ttbr1_el1?

'nonboot' CPUs are powered off around hibernate, so this only runs on one CPU.

Restoring with the CNP set like this will share all the TTBR1 mappings using the
reserved ASID out of TTBR0. Hibernate then calls cpu_uninstall_idmap() via
__cpu_suspend_exit(), which will restore the original ttbr0 value and CNP bit.


Thanks,

James