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
