Re: [PATCH v8 25/38] KVM: arm64: Trap SME usage in guest

[Marc Zyngier <maz@kernel.org>]

On Tue, 25 Jan 2022 00:11:01 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> SME defines two new traps which need to be enabled for guests to ensure
> that they can't use SME, one for the main SME operations which mirrors the
> traps for SVE and another for access to TPIDR2 in SCTLR_EL2.
> 
> For VHE manage SMEN along with ZEN in activate_traps() and the FP state
> management callbacks.
> 
> For nVHE the value to be used for CPTR_EL2 in the guest is stored in
> vcpu->arch.cptr_el2, set TSM there during initialisation. It will be
> cleared in __deactivate_traps_common() by virtue of not being set in
> CPTR_EL2_DEFAULT.
> 
> For both VHE and nVHE cases handle SCTLR_EL2.EnTPIDR2 in the shared
> __active_traps_common() and __deactivate_traps_common(), there is no
> existing dynamic management of SCTLR_EL2.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/kvm/hyp/nvhe/switch.c | 30 ++++++++++++++++++++++++++++++
>  arch/arm64/kvm/hyp/vhe/switch.c  | 10 +++++++++-
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
> index 6410d21d8695..184bf6bd79b9 100644
> --- a/arch/arm64/kvm/hyp/nvhe/switch.c
> +++ b/arch/arm64/kvm/hyp/nvhe/switch.c
> @@ -47,10 +47,25 @@ static void __activate_traps(struct kvm_vcpu *vcpu)
>  		val |= CPTR_EL2_TFP | CPTR_EL2_TZ;
>  		__activate_traps_fpsimd32(vcpu);
>  	}
> +	if (IS_ENABLED(CONFIG_ARM64_SME) && cpus_have_final_cap(ARM64_SME))

Please drop the IS_ENABLED(). We purposely avoid conditional
compilation in KVM in order to avoid bitrot, and the amount of code
you save isn't significant. Having a static key is more than enough to
avoid runtime costs.

> +		val |= CPTR_EL2_TSM;
>  
>  	write_sysreg(val, cptr_el2);
>  	write_sysreg(__this_cpu_read(kvm_hyp_vector), vbar_el2);
>  
> +	if (IS_ENABLED(CONFIG_ARM64_SME) && cpus_have_final_cap(ARM64_SME) &&
> +	    cpus_have_final_cap(ARM64_HAS_FGT)) {
> +		val = read_sysreg_s(SYS_HFGRTR_EL2);
> +		val &= ~(HFGxTR_EL2_nTPIDR_EL0_MASK |
> +			 HFGxTR_EL2_nSMPRI_EL1_MASK);
> +		write_sysreg_s(val, SYS_HFGRTR_EL2);
> +
> +		val = read_sysreg_s(SYS_HFGWTR_EL2);
> +		val &= ~(HFGxTR_EL2_nTPIDR_EL0_MASK |
> +			 HFGxTR_EL2_nSMPRI_EL1_MASK);
> +		write_sysreg_s(val, SYS_HFGWTR_EL2);
> +	}

If the CPUs do not have FGT, what provides the equivalent trapping?
If FGT is mandatory when SME exists, then you should simplify the
condition.

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm