lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 05 Jul 2020 12:42:16 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     David Brazdil <dbrazdil@...gle.com>
Cc:     Will Deacon <will@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        James Morse <james.morse@....com>,
        Julien Thierry <julien.thierry.kdev@...il.com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, android-kvm@...gle.com,
        kernel-team@...roid.com, Andrew Scull <ascull@...gle.com>
Subject: Re: [PATCH v4 07/15] arm64: kvm: Move hyp-init.S to nVHE

Hi David,

On Thu, 25 Jun 2020 14:14:12 +0100,
David Brazdil <dbrazdil@...gle.com> wrote:
> 
> From: Andrew Scull <ascull@...gle.com>
> 
> hyp-init.S contains the identity mapped initialisation code for the
> non-VHE code that runs at EL2. It is only used for non-VHE.
> 
> Adjust code that calls into this to use the prefixed symbol name.
> 
> Signed-off-by: Andrew Scull <ascull@...gle.com>
> 
> [David: pass idmap_t0sz as an argument]

It is unclear to me why moving the way idmap_t0sz is passed is
required at this stage. I understand that you want to minimise the
amount of shared data between EL1 and EL2, but it hardly seems
relevant here.

Or is it, as I expect, to avoid yet another symbol renaming issue?
If so, it would be preferable to have the symbol alias, keep the setup
hypercall as is, and have a later, separate patch that deals with the
the idmap. And I am pretty sure that, as we move to a more autonomous
EL2, we won't have to deal with it at all and we'll simply delete this
code.

I'm planning to squash the following diff into this patch, effectively
reverting the idmap_t0sz related changes. Let me know if you're OK
with it.

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 8ba32bff7bb2..9e897c500237 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -83,6 +83,9 @@ KVM_NVHE_ALIAS(panic);
 /* Vectors installed by hyp-init on reset HVC. */
 KVM_NVHE_ALIAS(__hyp_stub_vectors);
 
+/* IDMAP TCR_EL1.T0SZ as computed by the EL1 init code */
+KVM_NVHE_ALIAS(idmap_t0sz);
+
 /* Kernel symbol used by icache_is_vpipt(). */
 KVM_NVHE_ALIAS(__icache_flags);
 
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 8ca2c111cec2..0bf2cf5614c6 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1296,7 +1296,7 @@ static void cpu_init_hyp_mode(void)
 	 * cpus_have_const_cap() wrapper.
 	 */
 	BUG_ON(!system_capabilities_finalized());
-	__kvm_call_hyp((void *)pgd_ptr, hyp_stack_ptr, vector_ptr, tpidr_el2, idmap_t0sz);
+	__kvm_call_hyp((void *)pgd_ptr, hyp_stack_ptr, vector_ptr, tpidr_el2);
 
 	/*
 	 * Disabling SSBD on a non-VHE system requires us to enable SSBS
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-init.S b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
index 7bb75acbede0..6e6ed5581eed 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-init.S
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-init.S
@@ -47,24 +47,23 @@ __invalid:
 	 * x1: HYP stack
 	 * x2: HYP vectors
 	 * x3: per-CPU offset
-	 * x4: idmap_t0sz
 	 */
 __do_hyp_init:
 	/* Check for a stub HVC call */
 	cmp	x0, #HVC_STUB_HCALL_NR
 	b.lo	__kvm_handle_stub_hvc
 
-	phys_to_ttbr x5, x0
+	phys_to_ttbr x4, x0
 alternative_if ARM64_HAS_CNP
-	orr	x5, x5, #TTBR_CNP_BIT
+	orr	x4, x4, #TTBR_CNP_BIT
 alternative_else_nop_endif
-	msr	ttbr0_el2, x5
+	msr	ttbr0_el2, x4
 
-	mrs	x5, tcr_el1
-	mov_q	x6, TCR_EL2_MASK
-	and	x5, x5, x6
-	mov	x6, #TCR_EL2_RES1
-	orr	x5, x5, x6
+	mrs	x4, tcr_el1
+	mov_q	x5, TCR_EL2_MASK
+	and	x4, x4, x5
+	mov	x5, #TCR_EL2_RES1
+	orr	x4, x4, x5
 
 	/*
 	 * The ID map may be configured to use an extended virtual address
@@ -80,14 +79,15 @@ alternative_else_nop_endif
 	 *
 	 * So use the same T0SZ value we use for the ID map.
 	 */
-	bfi	x5, x4, TCR_T0SZ_OFFSET, TCR_TxSZ_WIDTH
+	ldr_l	x5, idmap_t0sz
+	bfi	x4, x5, TCR_T0SZ_OFFSET, TCR_TxSZ_WIDTH
 
 	/*
 	 * Set the PS bits in TCR_EL2.
 	 */
-	tcr_compute_pa_size x5, #TCR_EL2_PS_SHIFT, x4, x6
+	tcr_compute_pa_size x4, #TCR_EL2_PS_SHIFT, x5, x6
 
-	msr	tcr_el2, x5
+	msr	tcr_el2, x4
 
 	mrs	x4, mair_el1
 	msr	mair_el2, x4

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Powered by blists - more mailing lists