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

From: Marc Zyngier <maz@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Shuah Khan <shuah@kernel.org>,
	Alan Hayward <alan.hayward@arm.com>,
	Luis Machado <luis.machado@arm.com>,
	Salil Akerkar <Salil.Akerkar@arm.com>,
	Basant Kumar Dwivedi <Basant.KumarDwivedi@arm.com>,
	Szabolcs Nagy <szabolcs.nagy@arm.com>,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kselftest@vger.kernel.org, kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH v8 25/38] KVM: arm64: Trap SME usage in guest
Date: Tue, 25 Jan 2022 11:27:55 +0000	[thread overview]
Message-ID: <87y2346on8.wl-maz@kernel.org> (raw)
In-Reply-To: <20220125001114.193425-26-broonie@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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel