From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Martin Subject: Re: [PATCH 1/2] KVM: arm/arm64: Add save/restore support for firmware workaround state Date: Tue, 22 Jan 2019 15:17:14 +0000 Message-ID: <20190122151714.GG3578@e103592.cambridge.arm.com> References: <20190107120537.184252-1-andre.przywara@arm.com> <20190107120537.184252-2-andre.przywara@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 51B3E4A3C0 for ; Tue, 22 Jan 2019 10:17:20 -0500 (EST) Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id e6zH9b77WZ7F for ; Tue, 22 Jan 2019 10:17:19 -0500 (EST) Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 0F04B4A376 for ; Tue, 22 Jan 2019 10:17:19 -0500 (EST) Content-Disposition: inline In-Reply-To: <20190107120537.184252-2-andre.przywara@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu To: Andre Przywara Cc: Marc Zyngier , kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu List-Id: kvmarm@lists.cs.columbia.edu On Mon, Jan 07, 2019 at 12:05:36PM +0000, Andre Przywara wrote: > KVM implements the firmware interface for mitigating cache speculation > vulnerabilities. Guests may use this interface to ensure mitigation is > active. > If we want to migrate such a guest to a host with a different support > level for those workarounds, migration might need to fail, to ensure that > critical guests don't loose their protection. > > Introduce a way for userland to save and restore the workarounds state. > On restoring we do checks that make sure we don't downgrade our > mitigation level. > > Signed-off-by: Andre Przywara > --- > arch/arm/include/asm/kvm_emulate.h | 10 ++ > arch/arm/include/uapi/asm/kvm.h | 9 ++ > arch/arm64/include/asm/kvm_emulate.h | 14 +++ > arch/arm64/include/uapi/asm/kvm.h | 9 ++ > virt/kvm/arm/psci.c | 138 ++++++++++++++++++++++++++- > 5 files changed, 178 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/include/asm/kvm_emulate.h b/arch/arm/include/asm/kvm_emulate.h > index 77121b713bef..2255c50debab 100644 > --- a/arch/arm/include/asm/kvm_emulate.h > +++ b/arch/arm/include/asm/kvm_emulate.h > @@ -275,6 +275,16 @@ static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu) > return vcpu_cp15(vcpu, c0_MPIDR) & MPIDR_HWID_BITMASK; > } > > +static inline bool kvm_arm_get_vcpu_workaround_2_flag(struct kvm_vcpu *vcpu) > +{ > + return false; > +} > + > +static inline void kvm_arm_set_vcpu_workaround_2_flag(struct kvm_vcpu *vcpu, > + bool flag) > +{ > +} > + > static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu) > { > *vcpu_cpsr(vcpu) |= PSR_E_BIT; > diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h > index 4602464ebdfb..02c93b1d8f6d 100644 > --- a/arch/arm/include/uapi/asm/kvm.h > +++ b/arch/arm/include/uapi/asm/kvm.h > @@ -214,6 +214,15 @@ struct kvm_vcpu_events { > #define KVM_REG_ARM_FW_REG(r) (KVM_REG_ARM | KVM_REG_SIZE_U64 | \ > KVM_REG_ARM_FW | ((r) & 0xffff)) > #define KVM_REG_ARM_PSCI_VERSION KVM_REG_ARM_FW_REG(0) > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_1 KVM_REG_ARM_FW_REG(1) > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_1_NOT_AVAIL 0 > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_1_AVAIL 1 > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2 KVM_REG_ARM_FW_REG(2) > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_MASK 0x3 > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_NOT_AVAIL 0 > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_AVAIL 1 > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_UNAFFECTED 2 > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_ENABLED 4 > > /* Device Control API: ARM VGIC */ > #define KVM_DEV_ARM_VGIC_GRP_ADDR 0 > diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h > index 506386a3edde..a44f07f68da4 100644 > --- a/arch/arm64/include/asm/kvm_emulate.h > +++ b/arch/arm64/include/asm/kvm_emulate.h > @@ -336,6 +336,20 @@ static inline unsigned long kvm_vcpu_get_mpidr_aff(struct kvm_vcpu *vcpu) > return vcpu_read_sys_reg(vcpu, MPIDR_EL1) & MPIDR_HWID_BITMASK; > } > > +static inline bool kvm_arm_get_vcpu_workaround_2_flag(struct kvm_vcpu *vcpu) > +{ > + return vcpu->arch.workaround_flags & VCPU_WORKAROUND_2_FLAG; > +} > + > +static inline void kvm_arm_set_vcpu_workaround_2_flag(struct kvm_vcpu *vcpu, > + bool flag) > +{ > + if (flag) > + vcpu->arch.workaround_flags |= VCPU_WORKAROUND_2_FLAG; > + else > + vcpu->arch.workaround_flags &= ~VCPU_WORKAROUND_2_FLAG; > +} > + > static inline void kvm_vcpu_set_be(struct kvm_vcpu *vcpu) > { > if (vcpu_mode_is_32bit(vcpu)) { > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index 97c3478ee6e7..4a19ef199a99 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -225,6 +225,15 @@ struct kvm_vcpu_events { > #define KVM_REG_ARM_FW_REG(r) (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \ > KVM_REG_ARM_FW | ((r) & 0xffff)) > #define KVM_REG_ARM_PSCI_VERSION KVM_REG_ARM_FW_REG(0) > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_1 KVM_REG_ARM_FW_REG(1) > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_1_NOT_AVAIL 0 > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_1_AVAIL 1 > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2 KVM_REG_ARM_FW_REG(2) > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_MASK 0x3 > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_NOT_AVAIL 0 > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_AVAIL 1 > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_UNAFFECTED 2 > +#define KVM_REG_ARM_SMCCC_ARCH_WORKAROUND_2_ENABLED 4 If this is the first exposure of this information to userspace, I wonder if we can come up with some common semantics that avoid having to add new ad-hoc code (and bugs) every time a new vulnerability/workaround is defined. We seem to have at least the two following independent properties for a vulnerability, with the listed values for each: * vulnerability (Vulnerable, Unknown, Not Vulnerable) * mitigation support (Not Requestable, Requestable) Migrations must not move to the left in _either_ list for any vulnerability. If we want to hedge out bets we could follow the style of the ID registers and allocate to each theoretical vulnerability a pair of signed 2- or (for more expansion room if we think we might need it) 4-bit fields. We could perhaps allocate as follows: * -1=Vulnerable, 0=Unknown, 1=Not Vulnerable * 0=Mitigation not requestable, 1=Mitigation requestable Checking code wouldn't need to know which fields describe mitigation mechanisms and which describe vulnerabilities: we'd just do a strict >= comparison on each. Further, if a register is never written before the vcpu is first run, we should imply a write of 0 to it as part of KVM_RUN (so that if the destination node has a negative value anywhere, KVM_RUN barfs cleanly. (Those semantics should apply equally to the CPU ID registers, though we don't currently do that.) Thoughts? [...] Cheers ---Dave