[PATCH v6 30/64] KVM: arm64: nv: Configure HCR_EL2 for nested virtualization

[Alexandru Elisei]

Hi,

On Fri, Jan 28, 2022 at 12:18:38PM +0000, Marc Zyngier wrote:
> From: Jintack Lim <jintack.lim at linaro.org>
> 
> We enable nested virtualization by setting the HCR NV and NV1 bit.
> 
> When the virtual E2H bit is set, we can support EL2 register accesses
> via EL1 registers from the virtual EL2 by doing trap-and-emulate. A
> better alternative, however, is to allow the virtual EL2 to access EL2
> register states without trap. This can be easily achieved by not traping
> EL1 registers since those registers already have EL2 register states.
> 
> Signed-off-by: Jintack Lim <jintack.lim at linaro.org>
> Signed-off-by: Marc Zyngier <maz at kernel.org>
> ---
>  arch/arm64/include/asm/kvm_arm.h |  1 +
>  arch/arm64/kvm/hyp/vhe/switch.c  | 38 +++++++++++++++++++++++++++++---
>  2 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index 748c2b068d4e..d913c3fb5665 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -87,6 +87,7 @@
>  			 HCR_BSU_IS | HCR_FB | HCR_TACR | \
>  			 HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \
>  			 HCR_FMO | HCR_IMO | HCR_PTW )
> +#define HCR_GUEST_NV_FILTER_FLAGS (HCR_ATA | HCR_API | HCR_APK)
>  #define HCR_VIRT_EXCP_MASK (HCR_VSE | HCR_VI | HCR_VF)
>  #define HCR_HOST_NVHE_FLAGS (HCR_RW | HCR_API | HCR_APK | HCR_ATA)
>  #define HCR_HOST_NVHE_PROTECTED_FLAGS (HCR_HOST_NVHE_FLAGS | HCR_TSC)
> diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
> index 1e6a00e7bfb3..28845f907cfc 100644
> --- a/arch/arm64/kvm/hyp/vhe/switch.c
> +++ b/arch/arm64/kvm/hyp/vhe/switch.c
> @@ -35,9 +35,41 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
>  	u64 hcr = vcpu->arch.hcr_el2;
>  	u64 val;
>  
> -	/* Trap VM sysreg accesses if an EL2 guest is not using VHE. */
> -	if (vcpu_is_el2(vcpu) && !vcpu_el2_e2h_is_set(vcpu))
> -		hcr |= HCR_TVM | HCR_TRVM;
> +	if (is_hyp_ctxt(vcpu)) {
> +		hcr |= HCR_NV;
> +
> +		if (!vcpu_el2_e2h_is_set(vcpu)) {
> +			/*
> +			 * For a guest hypervisor on v8.0, trap and emulate

That's confusing, because FEAT_NV is available from v8.3 and newer.

> +			 * the EL1 virtual memory control register accesses.
> +			 */

Wasn't the purpose of comment to explain why something is done instead of what
something does? The bits are explained in the Arm ARM. How about something like
this instead:

"A hypervisor running in non-VHE mode writes to EL1 the memory control registers
when preparing to run a guest or the host at EL1. Since virtual EL2 is emulated
on top of physical EL1, allowing the L1 guest hypervisor to access the registers
directly would lead to the L1 guest inadvertently corrupting its own state.

The NV1 bit is set to trap accesses to the registers which aren't controlled by
the TVM and TRVM bits, and to change to translation table format used by the
EL1&0 translation regime format to the EL2 translation regime format, which is
what the L1 guest hypervisor running with E2H = 0 expects when it creates the
tables".

> +			hcr |= HCR_TVM | HCR_TRVM | HCR_NV1;
> +		} else {
> +			/*
> +			 * For a guest hypervisor on v8.1 (VHE), allow to
> +			 * access the EL1 virtual memory control registers
> +			 * natively. These accesses are to access EL2 register
> +			 * states.
> +			 * Note that we still need to respect the virtual
> +			 * HCR_EL2 state.
> +			 */
> +			u64 vhcr_el2 = __vcpu_sys_reg(vcpu, HCR_EL2);

Nitpick: I think it would be better to name this variable virtual_hcr_el2,
because vhcr_el2 looks like a valid register name.

Thanks,
Alex

> +
> +			vhcr_el2 &= ~HCR_GUEST_NV_FILTER_FLAGS;
> +
> +			/*
> +			 * We already set TVM to handle set/way cache maint
> +			 * ops traps, this somewhat collides with the nested
> +			 * virt trapping for nVHE. So turn this off for now
> +			 * here, in the hope that VHE guests won't ever do this.
> +			 * TODO: find out whether it's worth to support both
> +			 * cases at the same time.
> +			 */
> +			hcr &= ~HCR_TVM;
> +
> +			hcr |= vhcr_el2 & (HCR_TVM | HCR_TRVM);
> +		}
> +	}
>  
>  	___activate_traps(vcpu, hcr);
>  
> -- 
> 2.30.2
>