Re: [PATCH v1 6/7] KVM: arm64: Eagerly restore host fpsimd/sve state in pKVM

From: Oliver Upton <oliver.upton@linux.dev>
To: Fuad Tabba <tabba@google.com>
Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	maz@kernel.org, will@kernel.org, qperret@google.com,
	seanjc@google.com, alexandru.elisei@arm.com,
	catalin.marinas@arm.com, philmd@linaro.org, james.morse@arm.com,
	suzuki.poulose@arm.com, mark.rutland@arm.com, broonie@kernel.org,
	joey.gouly@arm.com, rananta@google.com, yuzenghui@huawei.com
Subject: Re: [PATCH v1 6/7] KVM: arm64: Eagerly restore host fpsimd/sve state in pKVM
Date: Fri, 17 May 2024 17:09:31 +0000	[thread overview]
Message-ID: <ZkePS5jU6yEv6Nhp@linux.dev> (raw)
In-Reply-To: <20240517131814.719933-7-tabba@google.com>

Hi Fuad,

On Fri, May 17, 2024 at 02:18:13PM +0100, Fuad Tabba wrote:
>  static void kvm_hyp_save_fpsimd_host(struct kvm_vcpu *vcpu)
>  {
> -	__fpsimd_save_state(*host_data_ptr(fpsimd_state));
> +	/*
> +	 * Non-protected kvm relies on the host restoring its sve state.
> +	 * Protected kvm restores the host's sve state as not to reveal that
> +	 * fpsimd was used by a guest nor leak upper sve bits.
> +	 */
> +	if (unlikely(is_protected_kvm_enabled() && system_supports_sve())) {
> +		__hyp_sve_save_host();
> +
> +		/* Re-enable SVE traps for guests that do not support it. */
> +		if (!vcpu_has_sve(vcpu))
> +			sysreg_clear_set(cptr_el2, 0, CPTR_EL2_TZ);

This doesn't account for hVHE. I wonder we'd be better off abstracting
CPTR_EL2 behind a helper wherever it gets used in nVHE and translate
into the VHE-format behind the scenes:

static inline void __cptr_clear_set_hvhe(u64 cptr_clr, u64 cptr_set)
{
	u64 clr = 0, set = 0;

	if (cptr_clr & CPTR_EL2_TFP)
		set |= CPACR_ELx_FPEN;
	if (cptr_clr & CPTR_EL2_TZ)
		set |= CPACR_ELx_ZEN;
	if (cptr_clr & CPTR_EL2_TSM)
		set |= CPACR_ELx_SMEN;

	if (cptr_set & CPTR_EL2_TFP)
		clr |= CPACR_ELx_FPEN;
	if (cptr_set & CPTR_EL2_TZ)
		clr |= CPACR_ELx_ZEN;
	if (cptr_set & CPTR_EL2_TSM)
		clr |= CPACR_ELx_SMEN;

	sysreg_clear_set(cpacr_el1, clr, set);
}

static inline void cptr_clear_set(u64 clr, u64 set)
{
	if (has_hvhe())
		__cptr_clear_set_hvhe(clr, set);
	else
		sysreg_clear_set(cptr_el2, clr, set);
}

-- 
Thanks,
Oliver

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