On Tue, 25 Jan 2022 00:11:02 +0000,
Mark Brown <broonie@xxxxxxxxxx> wrote:
>
> While we don't currently support SME in guests we do currently support it
> for the host system so we need to take care of SME's impact, including
> the floating point register state, when running guests. Simiarly to SVE
> we need to manage the traps in CPACR_RL1, what is new is the handling of
> streaming mode and ZA.
>
> Normally we defer any handling of the floating point register state until
> the guest first uses it however if the system is in streaming mode FPSIMD
> and SVE operations may generate SME traps which we would need to distinguish
> from actual attempts by the guest to use SME. Rather than do this for the
> time being if we are in streaming mode when entering the guest we force
> the floating point state to be saved immediately and exit streaming mode,
> meaning that the guest won't generate SME traps for supported operations.
>
> We could handle ZA in the access trap similarly to the FPSIMD/SVE state
> without the disruption caused by streaming mode but for simplicity
> handle it the same way as streaming mode for now.
>
> This will be revisited when we support SME for guests (hopefully before SME
> hardware becomes available), for now it will only incur additional cost on
> systems with SME and even there only if streaming mode or ZA are enabled.
>
> Signed-off-by: Mark Brown <broonie@xxxxxxxxxx>
> ---
> arch/arm64/include/asm/kvm_host.h | 1 +
> arch/arm64/kvm/fpsimd.c | 38 +++++++++++++++++++++++++++++++
> 2 files changed, 39 insertions(+)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 7dc85d5a6552..404b7358ba96 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -438,6 +438,7 @@ struct kvm_vcpu_arch {
> #define KVM_ARM64_DEBUG_STATE_SAVE_SPE (1 << 12) /* Save SPE context if active */
> #define KVM_ARM64_DEBUG_STATE_SAVE_TRBE (1 << 13) /* Save TRBE context if active */
> #define KVM_ARM64_FP_FOREIGN_FPSTATE (1 << 14)
> +#define KVM_ARM64_HOST_SME_ENABLED (1 << 15) /* SME enabled for EL0 */
>
> #define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | \
> KVM_GUESTDBG_USE_SW_BP | \
> diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
> index 338733ac63f8..cecaddb644ce 100644
> --- a/arch/arm64/kvm/fpsimd.c
> +++ b/arch/arm64/kvm/fpsimd.c
> @@ -82,6 +82,26 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
>
> if (read_sysreg(cpacr_el1) & CPACR_EL1_ZEN_EL0EN)
> vcpu->arch.flags |= KVM_ARM64_HOST_SVE_ENABLED;
> +
> + /*
> + * We don't currently support SME guests but if we leave
> + * things in streaming mode then when the guest starts running
> + * FPSIMD or SVE code it may generate SME traps so as a
> + * special case if we are in streaming mode we force the host
> + * state to be saved now and exit streaming mode so that we
> + * don't have to handle any SME traps for valid guest
> + * operations. Do this for ZA as well for now for simplicity.
> + */
> + if (system_supports_sme()) {
> + if (read_sysreg(cpacr_el1) & CPACR_EL1_SMEN_EL0EN)
> + vcpu->arch.flags |= KVM_ARM64_HOST_SME_ENABLED;
> +
> + if (read_sysreg_s(SYS_SVCR_EL0) &
> + (SYS_SVCR_EL0_SM_MASK | SYS_SVCR_EL0_ZA_MASK)) {
> + vcpu->arch.flags &= ~KVM_ARM64_FP_HOST;
> + fpsimd_save_and_flush_cpu_state();
> + }
> + }
> }
>
> void kvm_arch_vcpu_ctxflush_fp(struct kvm_vcpu *vcpu)
> @@ -129,6 +149,24 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
>
> local_irq_save(flags);
>
> + /*
> + * If we have VHE then the Hyp code will reset CPACR_EL1 to
> + * CPACR_EL1_DEFAULT and we need to reenable SME.
> + */
> + if (has_vhe()) {
> + if (system_supports_sme()) {
nit: if (has_vhe() && system_supports_sme()) {
saves you one level of indentation.
> + /* Also restore EL0 state seen on entry */
> + if (vcpu->arch.flags & KVM_ARM64_HOST_SME_ENABLED)
> + sysreg_clear_set(CPACR_EL1, 0,
> + CPACR_EL1_SMEN_EL0EN |
> + CPACR_EL1_SMEN_EL1EN);
> + else
> + sysreg_clear_set(CPACR_EL1,
> + CPACR_EL1_SMEN_EL0EN,
> + CPACR_EL1_SMEN_EL1EN);
I find the use of CPACR_EL1_SMEN in some cases and its individual bits
in some others pretty confusing. I understand that you have modelled
it after the SVE code, but maybe this is a mistake we don't need to
repeat. I'd be in favour of directly exposing the individual bits in
all cases.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
