Re: [RFC PATCH v2 31/38] KVM: arm64: Manage the shadow states when virtual E2H bit enabled

Christoffer Dall
2017-07-31T09:57:27Z

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@linaro.org>
> ---
> 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