On Tue, Jul 18, 2017 at 11:58:57AM -0500, Jintack Lim wrote: In the subject: s/virtual E2H bit enabled/virtual E2H bit is set/ > When creating the shadow context for the virtual EL2 execution, we can > directly copy the EL2 register states to the shadow EL1 register states > if the virtual HCR_EL2.E2H bit is set. This is because EL1 and EL2 > system register formats compatible with E2H=1. are compatible when HCR_EL2.E2H==1. > > Now that we allow the virtual EL2 modify its EL2 registers without trap to modify without trapping, via... > via the physical EL1 system register accesses, we need to reflect the > changes made to the EL1 system registers to the virtual EL2 register > states. This is not required to the virtual EL2 without VHE, since the for virtual EL2 without... > virtual EL2 should always use _EL2 accessors, which traps to EL2. s/should always use/always uses/ > > Signed-off-by: Jintack Lim <jintack.lim@xxxxxxxxxx> > --- > arch/arm64/kvm/context.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 66 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kvm/context.c b/arch/arm64/kvm/context.c > index 39bd92d..9947bc8 100644 > --- a/arch/arm64/kvm/context.c > +++ b/arch/arm64/kvm/context.c > @@ -39,6 +39,27 @@ struct el1_el2_map { > { VBAR_EL1, VBAR_EL2 }, > }; > > +/* > + * List of pair of EL1/EL2 registers which are used to access real EL2 > + * registers in EL2 with E2H bit set. in EL1? Maybe you can just say: /* * List of system registers that can be directly mapped between VHE * EL2 system registers and EL1 system registers. */ > + */ > +static const struct el1_el2_map vhe_map[] = { > + { SCTLR_EL1, SCTLR_EL2 }, > + { CPACR_EL1, CPTR_EL2 }, > + { TTBR0_EL1, TTBR0_EL2 }, > + { TTBR1_EL1, TTBR1_EL2 }, > + { TCR_EL1, TCR_EL2}, > + { AFSR0_EL1, AFSR0_EL2 }, > + { AFSR1_EL1, AFSR1_EL2 }, > + { ESR_EL1, ESR_EL2}, > + { FAR_EL1, FAR_EL2}, > + { MAIR_EL1, MAIR_EL2 }, > + { AMAIR_EL1, AMAIR_EL2 }, > + { VBAR_EL1, VBAR_EL2 }, > + { CONTEXTIDR_EL1, CONTEXTIDR_EL2 }, > + { CNTKCTL_EL1, CNTHCTL_EL2 }, > +}; > + > static inline u64 tcr_el2_ips_to_tcr_el1_ps(u64 tcr_el2) > { > return ((tcr_el2 & TCR_EL2_PS_MASK) >> TCR_EL2_PS_SHIFT) > @@ -57,7 +78,27 @@ static inline u64 cptr_to_cpacr(u64 cptr_el2) > return cpacr_el1; > } > > -static void flush_shadow_el1_sysregs(struct kvm_vcpu *vcpu) > +static void sync_shadow_el1_sysregs(struct kvm_vcpu *vcpu) > +{ > + u64 *s_sys_regs = vcpu->arch.ctxt.shadow_sys_regs; > + int i; > + > + /* > + * In the virtual EL2 without VHE no EL1 system registers can't be no other EL1 system register then el1_non_trap_regs[] can be changed without trapping to the host hypervisor > + * changed without trap except el1_non_trap_regs[]. So we have nothing > + * to sync on exit from a guest. > + */ > + if (!vcpu_el2_e2h_is_set(vcpu)) > + return; > + > + for (i = 0; i < ARRAY_SIZE(vhe_map); i++) { > + const struct el1_el2_map *map = &vhe_map[i]; > + > + vcpu_sys_reg(vcpu, map->el2) = s_sys_regs[map->el1]; > + } > +} > + > +static void flush_shadow_el1_sysregs_nvhe(struct kvm_vcpu *vcpu) > { > u64 *s_sys_regs = vcpu->arch.ctxt.shadow_sys_regs; > u64 tcr_el2; > @@ -86,6 +127,29 @@ static void flush_shadow_el1_sysregs(struct kvm_vcpu *vcpu) > s_sys_regs[CPACR_EL1] = cptr_to_cpacr(vcpu_sys_reg(vcpu, CPTR_EL2)); > } > > +static void flush_shadow_el1_sysregs_vhe(struct kvm_vcpu *vcpu) > +{ > + u64 *s_sys_regs = vcpu->arch.ctxt.shadow_sys_regs; > + int i; > + > + /* > + * When e2h bit is set, EL2 registers becomes compatible > + * with corrensponding EL1 registers. So, no conversion required. > + */ > + for (i = 0; i < ARRAY_SIZE(vhe_map); i++) { > + const struct el1_el2_map *map = &vhe_map[i]; > + > + s_sys_regs[map->el1] = vcpu_sys_reg(vcpu, map->el2); > + } > +} > + > +static void flush_shadow_el1_sysregs(struct kvm_vcpu *vcpu) > +{ > + if (vcpu_el2_e2h_is_set(vcpu)) > + flush_shadow_el1_sysregs_vhe(vcpu); > + else > + flush_shadow_el1_sysregs_nvhe(vcpu); > +} > > /* > * List of EL0 and EL1 registers which we allow the virtual EL2 mode to access > @@ -247,6 +311,7 @@ void kvm_arm_restore_shadow_state(struct kvm_vcpu *vcpu) > if (unlikely(is_hyp_ctxt(vcpu))) { > sync_shadow_special_regs(vcpu); > sync_shadow_non_trap_el1_state(vcpu); > + sync_shadow_el1_sysregs(vcpu); > } else > sync_special_regs(vcpu); > } > -- > 1.9.1 > Thanks, -Christoffer