| Message ID | 20220125001114.193425-27-broonie@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64/sme: Initial support for the Scalable Matrix Extension | expand |
On Tue, 25 Jan 2022 00:11:02 +0000, Mark Brown <broonie@kernel.org> 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@kernel.org> > --- > 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.
On Tue, Jan 25, 2022 at 11:59:02AM +0000, Marc Zyngier wrote: > Mark Brown <broonie@kernel.org> wrote: > > + if (has_vhe()) { > > + if (system_supports_sme()) { > nit: if (has_vhe() && system_supports_sme()) { > saves you one level of indentation. Yes, for now. IIRC there was some other stuff there when I had some of the code for doing the register switching properly. > > + /* 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. OK, it is just the KVM code that uses the plain ZEN. I'll add a cleanup patch for that at the start of the series for ZEN I guess otherwise it looks worse, though that will inflate the size of the series a bit.
On Tue, 25 Jan 2022 12:52:18 +0000, Mark Brown <broonie@kernel.org> wrote: > > [1 <text/plain; us-ascii (7bit)>] > On Tue, Jan 25, 2022 at 11:59:02AM +0000, Marc Zyngier wrote: > > Mark Brown <broonie@kernel.org> wrote: > > > > + if (has_vhe()) { > > > + if (system_supports_sme()) { > > > nit: if (has_vhe() && system_supports_sme()) { > > > saves you one level of indentation. > > Yes, for now. IIRC there was some other stuff there when I had some of > the code for doing the register switching properly. > > > > + /* 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. > > OK, it is just the KVM code that uses the plain ZEN. I'll add a cleanup > patch for that at the start of the series for ZEN I guess otherwise it > looks worse, though that will inflate the size of the series a bit. I'm happy to merge such a patch early if that helps. M.
On Tue, Jan 25, 2022 at 01:22:51PM +0000, Marc Zyngier wrote: > Mark Brown <broonie@kernel.org> wrote: > > > 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. > > OK, it is just the KVM code that uses the plain ZEN. I'll add a cleanup > > patch for that at the start of the series for ZEN I guess otherwise it > > looks worse, though that will inflate the size of the series a bit. > I'm happy to merge such a patch early if that helps. That'd be good. There's also similar stuff going on with FPEN BTW (which is I imagine where the SVE stuff came from).
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()) { + /* 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); + } + } + if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED) { if (vcpu_has_sve(vcpu)) { __vcpu_sys_reg(vcpu, ZCR_EL1) = read_sysreg_el1(SYS_ZCR);
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@kernel.org> --- arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/kvm/fpsimd.c | 38 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+)