Re: [PATCH v6 06/21] arm64: Move VHE-specific SPE setup to mutate_to_vhe()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Will,

On 2021-02-03 21:13, Will Deacon wrote:
Hi Marc,

On Mon, Feb 01, 2021 at 11:56:22AM +0000, Marc Zyngier wrote:
There isn't much that a VHE kernel needs on top of whatever has
been done for nVHE, so let's move the little we need to the
VHE stub (the SPE setup), and drop the init_el2_state macro.

No expected functional change.

Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
Acked-by: David Brazdil <dbrazdil@xxxxxxxxxx>
Acked-by: Catalin Marinas <catalin.marinas@xxxxxxx>
---
 arch/arm64/kernel/hyp-stub.S | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
index 373ed2213e1d..6b5c73cf9d52 100644
--- a/arch/arm64/kernel/hyp-stub.S
+++ b/arch/arm64/kernel/hyp-stub.S
@@ -92,9 +92,6 @@ SYM_CODE_START_LOCAL(mutate_to_vhe)
 	msr	hcr_el2, x0
 	isb

-	// Doesn't do much on VHE, but still, worth a shot
-	init_el2_state vhe
-
 	// Use the EL1 allocated stack, per-cpu offset
 	mrs	x0, sp_el1
 	mov	sp, x0
@@ -107,6 +104,31 @@ SYM_CODE_START_LOCAL(mutate_to_vhe)
 	mrs_s	x0, SYS_VBAR_EL12
 	msr	vbar_el1, x0

+	// Fixup SPE configuration, if supported...
+	mrs	x1, id_aa64dfr0_el1
+	ubfx	x1, x1, #ID_AA64DFR0_PMSVER_SHIFT, #4
+	mov	x2, xzr
+	cbz	x1, skip_spe
+
+	// ... and not owned by EL3
+	mrs_s	x0, SYS_PMBIDR_EL1
+	and	x0, x0, #(1 << SYS_PMBIDR_EL1_P_SHIFT)
+	cbnz	x0, skip_spe
+
+	// Let the SPE driver in control of the sampling
+	mrs_s	x0, SYS_PMSCR_EL1
+	bic	x0, x0, #(1 << SYS_PMSCR_EL2_PCT_SHIFT)
+	bic	x0, x0, #(1 << SYS_PMSCR_EL2_PA_SHIFT)
+	msr_s	SYS_PMSCR_EL1, x0

Why do we need to touch pmscr_el1 at all? The SPE driver should take care of
that, no? If you drop the pmscr_el1 accesses, I think you can drop the
pmbidr_el1 check as well.

That's definitely a brain fart, and is what we need on the nVHE path,
not here. Doing the same thing twice isn't exactly helpful.

And actually, then why even check dfr0? Doing the
bic for the mdcr_el1.e2pb bits is harmless.

+	mov	x2, #MDCR_EL2_TPMS
+
+skip_spe:
+	// For VHE, use EL2 translation and disable access from EL1
+	mrs	x0, mdcr_el2
+	bic	x0, x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)
+	orr	x0, x0, x2
+	msr	mdcr_el2, x0

Doesn't this undo the setting of mdcr_el2.hpmn if SPE is not present or
unavailable? (This wouldn't be an issue if we removed the skip_spe paths
altogether).

I don't think it does. We only clear the E2PB bits (harmless, as you point
out above), and OR something that is either 0 (no SPE) or MDCR_EL2_TPMS.
In any case, the HPMN bits are preserved (having been set during the nVHE
setup).

I think the following patch addresses the above issue, which I'll squash
with the original patch. Please shout if I missed anything.

Thanks,

        M.

diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
index aa7e8d592295..3e08dcc924b5 100644
--- a/arch/arm64/kernel/hyp-stub.S
+++ b/arch/arm64/kernel/hyp-stub.S
@@ -115,29 +115,9 @@ SYM_CODE_START_LOCAL(mutate_to_vhe)
 	mrs_s	x0, SYS_VBAR_EL12
 	msr	vbar_el1, x0

-	// Fixup SPE configuration, if supported...
-	mrs	x1, id_aa64dfr0_el1
-	ubfx	x1, x1, #ID_AA64DFR0_PMSVER_SHIFT, #4
-	mov	x2, xzr
-	cbz	x1, skip_spe
-
-	// ... and not owned by EL3
-	mrs_s	x0, SYS_PMBIDR_EL1
-	and	x0, x0, #(1 << SYS_PMBIDR_EL1_P_SHIFT)
-	cbnz	x0, skip_spe
-
-	// Let the SPE driver in control of the sampling
-	mrs_s	x0, SYS_PMSCR_EL1
-	bic	x0, x0, #(1 << SYS_PMSCR_EL2_PCT_SHIFT)
-	bic	x0, x0, #(1 << SYS_PMSCR_EL2_PA_SHIFT)
-	msr_s	SYS_PMSCR_EL1, x0
-	mov	x2, #MDCR_EL2_TPMS
-
-skip_spe:
-	// For VHE, use EL2 translation and disable access from EL1
+	// Use EL2 translations for SPE and disable access from EL1
 	mrs	x0, mdcr_el2
 	bic	x0, x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)
-	orr	x0, x0, x2
 	msr	mdcr_el2, x0

 	// Transfer the MM state from EL1 to EL2

--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm



[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux