Re: [PATCH 27/59] KVM: arm64: nv: Respect virtual HCR_EL2.TVM and TRVM settings

Julien Thierry
2019-06-26T06:55:42Z

On 06/21/2019 10:38 AM, Marc Zyngier wrote:
> From: Jintack Lim <jintack.lim@linaro.org>
>
> Forward the EL1 virtual memory register traps to the virtual EL2 if they
> are not coming from the virtual EL2 and the virtual HCR_EL2.TVM or TRVM
> bit is set.
>
> This is for recursive nested virtualization.
>
> Signed-off-by: Jintack Lim <jintack.lim@linaro.org>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/kvm/sys_regs.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 582d62aa48b7..0f74b9277a86 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -436,6 +436,27 @@ static bool access_dcsw(struct kvm_vcpu *vcpu,
> return true;
> }
>
> +/* This function is to support the recursive nested virtualization */
> +static bool forward_vm_traps(struct kvm_vcpu *vcpu, struct sys_reg_params *p)
> +{
> + u64 hcr_el2 = __vcpu_sys_reg(vcpu, HCR_EL2);
> +
> + /* If a trap comes from the virtual EL2, the host hypervisor handles. */
> + if (vcpu_mode_el2(vcpu))
> + return false;
> +
> + /*
> + * If the virtual HCR_EL2.TVM or TRVM bit is set, we need to foward
> + * this trap to the virtual EL2.
> + */
> + if ((hcr_el2 & HCR_TVM) && p->is_write)
> + return true;
> + else if ((hcr_el2 & HCR_TRVM) && !p->is_write)
> + return true;
> +
> + return false;
> +}
> +
> /*
> * Generic accessor for VM registers. Only called as long as HCR_TVM
> * is set. If the guest enables the MMU, we stop trapping the VM
> @@ -452,6 +473,9 @@ static bool access_vm_reg(struct kvm_vcpu *vcpu,
> if (el12_reg(p) && forward_nv_traps(vcpu))
> return false;
>
> + if (!el12_reg(p) && forward_vm_traps(vcpu, p))
> + return kvm_inject_nested_sync(vcpu, kvm_vcpu_get_hsr(vcpu));

Since we already have forward_traps(), isn't this just:

if (!el12_reg(p) && forward_traps(vcpu, p->is_write ? HCR_TVM : HCR_TRVM))
return true;

We could maybe simplify forward_vm_traps() to just call forward_traps()
similar to forward_nv_traps().

Cheers,

Julien