From mboxrd@z Thu Jan 1 00:00:00 1970 From: Leon Alrae Subject: Re: [PATCH 4/9] mips/kvm: Implement Config CP0 registers Date: Thu, 12 Mar 2015 16:41:37 +0000 Message-ID: <5501C1C1.8060001@imgtec.com> References: <1426087371-16166-1-git-send-email-james.hogan@imgtec.com> <1426087371-16166-5-git-send-email-james.hogan@imgtec.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Cc: , Aurelien Jarno To: James Hogan , , Paolo Bonzini Return-path: Received: from mailapp01.imgtec.com ([195.59.15.196]:61043 "EHLO mailapp01.imgtec.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753493AbbCLQlk (ORCPT ); Thu, 12 Mar 2015 12:41:40 -0400 In-Reply-To: <1426087371-16166-5-git-send-email-james.hogan@imgtec.com> Sender: kvm-owner@vger.kernel.org List-ID: On 11/03/2015 15:22, James Hogan wrote: > Implement saving and restoring to KVM state of the Config CP0 registers > (namely Config, Config1, Config2, Config3, Config4, and Config5). These > control the features available to a guest, and a few of the fields will > soon be writeable by a guest so QEMU needs to know about them so as not > to clobber them on migration/savevm. > > Signed-off-by: James Hogan > Cc: Paolo Bonzini > Cc: Leon Alrae > Cc: Aurelien Jarno > --- > target-mips/kvm.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 108 insertions(+) > > diff --git a/target-mips/kvm.c b/target-mips/kvm.c > index 730c67e247d8..b8813a2722a3 100644 > --- a/target-mips/kvm.c > +++ b/target-mips/kvm.c > @@ -223,6 +223,12 @@ int kvm_mips_set_ipi_interrupt(MIPSCPU *cpu, int irq, int level) > #define KVM_REG_MIPS_CP0_CAUSE MIPS_CP0_32(13, 0) > #define KVM_REG_MIPS_CP0_EPC MIPS_CP0_64(14, 0) > #define KVM_REG_MIPS_CP0_PRID MIPS_CP0_32(15, 0) > +#define KVM_REG_MIPS_CP0_CONFIG MIPS_CP0_32(16, 0) > +#define KVM_REG_MIPS_CP0_CONFIG1 MIPS_CP0_32(16, 1) > +#define KVM_REG_MIPS_CP0_CONFIG2 MIPS_CP0_32(16, 2) > +#define KVM_REG_MIPS_CP0_CONFIG3 MIPS_CP0_32(16, 3) > +#define KVM_REG_MIPS_CP0_CONFIG4 MIPS_CP0_32(16, 4) > +#define KVM_REG_MIPS_CP0_CONFIG5 MIPS_CP0_32(16, 5) > #define KVM_REG_MIPS_CP0_ERROREPC MIPS_CP0_64(30, 0) > > static inline int kvm_mips_put_one_reg(CPUState *cs, uint64_t reg_id, > @@ -305,6 +311,34 @@ static inline int kvm_mips_get_one_reg64(CPUState *cs, uint64 reg_id, > return kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &cp0reg); > } > > +#define KVM_REG_MIPS_CP0_CONFIG_MASK (1 << CP0C0_M) > +#define KVM_REG_MIPS_CP0_CONFIG1_MASK (1 << CP0C1_M) > +#define KVM_REG_MIPS_CP0_CONFIG2_MASK (1 << CP0C2_M) > +#define KVM_REG_MIPS_CP0_CONFIG3_MASK (1 << CP0C3_M) > +#define KVM_REG_MIPS_CP0_CONFIG4_MASK (1 << CP0C4_M) CP0Cx_M is 31, thus without "U" suffix 1 is left shifted into sign bit which is undefined behaviour. > +#define KVM_REG_MIPS_CP0_CONFIG5_MASK 0 > + > +static inline int kvm_mips_change_one_reg(CPUState *cs, uint64_t reg_id, > + int32_t *addr, int32_t mask) > +{ > + int err; > + int32_t tmp, change; > + > + err = kvm_mips_get_one_reg(cs, reg_id, &tmp); > + if (err < 0) { > + return err; > + } > + > + /* only change bits in mask */ > + change = (*addr ^ tmp) & mask; > + if (!change) { > + return 0; > + } > + > + tmp = tmp ^ change; > + return kvm_mips_put_one_reg(cs, reg_id, &tmp); > +} > + > /* > * We freeze the KVM timer when either the VM clock is stopped or the state is > * saved (the state is dirty). > @@ -527,6 +561,48 @@ static int kvm_mips_put_cp0_registers(CPUState *cs, int level) > DPRINTF("%s: Failed to put CP0_PRID (%d)\n", __func__, err); > ret = err; > } > + err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG, > + &env->CP0_Config0, > + KVM_REG_MIPS_CP0_CONFIG_MASK); > + if (err < 0) { > + DPRINTF("%s: Failed to change CP0_CONFIG (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG1, > + &env->CP0_Config1, > + KVM_REG_MIPS_CP0_CONFIG1_MASK); > + if (err < 0) { > + DPRINTF("%s: Failed to change CP0_CONFIG1 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG2, > + &env->CP0_Config2, > + KVM_REG_MIPS_CP0_CONFIG2_MASK); > + if (err < 0) { > + DPRINTF("%s: Failed to change CP0_CONFIG2 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG3, > + &env->CP0_Config3, > + KVM_REG_MIPS_CP0_CONFIG3_MASK); > + if (err < 0) { > + DPRINTF("%s: Failed to change CP0_CONFIG3 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG4, > + &env->CP0_Config4, > + KVM_REG_MIPS_CP0_CONFIG4_MASK); > + if (err < 0) { > + DPRINTF("%s: Failed to change CP0_CONFIG4 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG5, > + &env->CP0_Config5, > + KVM_REG_MIPS_CP0_CONFIG5_MASK); > + if (err < 0) { > + DPRINTF("%s: Failed to change CP0_CONFIG5 (%d)\n", __func__, err); > + ret = err; > + } > err = kvm_mips_put_one_ulreg(cs, KVM_REG_MIPS_CP0_ERROREPC, > &env->CP0_ErrorEPC); > if (err < 0) { > @@ -618,6 +694,38 @@ static int kvm_mips_get_cp0_registers(CPUState *cs) > DPRINTF("%s: Failed to get CP0_PRID (%d)\n", __func__, err); > ret = err; > } > + err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG, &env->CP0_Config0); > + if (err < 0) { > + DPRINTF("%s: Failed to get CP0_CONFIG (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG1, &env->CP0_Config1); > + if (err < 0) { > + DPRINTF("%s: Failed to get CP0_CONFIG1 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG2, &env->CP0_Config2); > + if (err < 0) { > + DPRINTF("%s: Failed to get CP0_CONFIG2 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG3, &env->CP0_Config3); > + if (err < 0) { > + DPRINTF("%s: Failed to get CP0_CONFIG3 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG4, > + &env->CP0_Config4); > + if (err < 0) { > + DPRINTF("%s: Failed to get CP0_CONFIG4 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG5, > + &env->CP0_Config5); > + if (err < 0) { > + DPRINTF("%s: Failed to get CP0_CONFIG5 (%d)\n", __func__, err); > + ret = err; > + } There's a minor style inconsistency here - for Config4 and Config5 the last argument has been moved to a new line whereas for Config{0,1,2,3} all arguments are in the same line. > err = kvm_mips_get_one_ulreg(cs, KVM_REG_MIPS_CP0_ERROREPC, > &env->CP0_ErrorEPC); > if (err < 0) { > From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58173) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YW6B6-0007PC-2n for qemu-devel@nongnu.org; Thu, 12 Mar 2015 12:41:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YW6B2-0000je-Lt for qemu-devel@nongnu.org; Thu, 12 Mar 2015 12:41:43 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:27673) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YW6B2-0000j9-7g for qemu-devel@nongnu.org; Thu, 12 Mar 2015 12:41:40 -0400 Message-ID: <5501C1C1.8060001@imgtec.com> Date: Thu, 12 Mar 2015 16:41:37 +0000 From: Leon Alrae MIME-Version: 1.0 References: <1426087371-16166-1-git-send-email-james.hogan@imgtec.com> <1426087371-16166-5-git-send-email-james.hogan@imgtec.com> In-Reply-To: <1426087371-16166-5-git-send-email-james.hogan@imgtec.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 4/9] mips/kvm: Implement Config CP0 registers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: James Hogan , qemu-devel@nongnu.org, Paolo Bonzini Cc: Aurelien Jarno , kvm@vger.kernel.org On 11/03/2015 15:22, James Hogan wrote: > Implement saving and restoring to KVM state of the Config CP0 registers > (namely Config, Config1, Config2, Config3, Config4, and Config5). These > control the features available to a guest, and a few of the fields will > soon be writeable by a guest so QEMU needs to know about them so as not > to clobber them on migration/savevm. > > Signed-off-by: James Hogan > Cc: Paolo Bonzini > Cc: Leon Alrae > Cc: Aurelien Jarno > --- > target-mips/kvm.c | 108 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 108 insertions(+) > > diff --git a/target-mips/kvm.c b/target-mips/kvm.c > index 730c67e247d8..b8813a2722a3 100644 > --- a/target-mips/kvm.c > +++ b/target-mips/kvm.c > @@ -223,6 +223,12 @@ int kvm_mips_set_ipi_interrupt(MIPSCPU *cpu, int irq, int level) > #define KVM_REG_MIPS_CP0_CAUSE MIPS_CP0_32(13, 0) > #define KVM_REG_MIPS_CP0_EPC MIPS_CP0_64(14, 0) > #define KVM_REG_MIPS_CP0_PRID MIPS_CP0_32(15, 0) > +#define KVM_REG_MIPS_CP0_CONFIG MIPS_CP0_32(16, 0) > +#define KVM_REG_MIPS_CP0_CONFIG1 MIPS_CP0_32(16, 1) > +#define KVM_REG_MIPS_CP0_CONFIG2 MIPS_CP0_32(16, 2) > +#define KVM_REG_MIPS_CP0_CONFIG3 MIPS_CP0_32(16, 3) > +#define KVM_REG_MIPS_CP0_CONFIG4 MIPS_CP0_32(16, 4) > +#define KVM_REG_MIPS_CP0_CONFIG5 MIPS_CP0_32(16, 5) > #define KVM_REG_MIPS_CP0_ERROREPC MIPS_CP0_64(30, 0) > > static inline int kvm_mips_put_one_reg(CPUState *cs, uint64_t reg_id, > @@ -305,6 +311,34 @@ static inline int kvm_mips_get_one_reg64(CPUState *cs, uint64 reg_id, > return kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &cp0reg); > } > > +#define KVM_REG_MIPS_CP0_CONFIG_MASK (1 << CP0C0_M) > +#define KVM_REG_MIPS_CP0_CONFIG1_MASK (1 << CP0C1_M) > +#define KVM_REG_MIPS_CP0_CONFIG2_MASK (1 << CP0C2_M) > +#define KVM_REG_MIPS_CP0_CONFIG3_MASK (1 << CP0C3_M) > +#define KVM_REG_MIPS_CP0_CONFIG4_MASK (1 << CP0C4_M) CP0Cx_M is 31, thus without "U" suffix 1 is left shifted into sign bit which is undefined behaviour. > +#define KVM_REG_MIPS_CP0_CONFIG5_MASK 0 > + > +static inline int kvm_mips_change_one_reg(CPUState *cs, uint64_t reg_id, > + int32_t *addr, int32_t mask) > +{ > + int err; > + int32_t tmp, change; > + > + err = kvm_mips_get_one_reg(cs, reg_id, &tmp); > + if (err < 0) { > + return err; > + } > + > + /* only change bits in mask */ > + change = (*addr ^ tmp) & mask; > + if (!change) { > + return 0; > + } > + > + tmp = tmp ^ change; > + return kvm_mips_put_one_reg(cs, reg_id, &tmp); > +} > + > /* > * We freeze the KVM timer when either the VM clock is stopped or the state is > * saved (the state is dirty). > @@ -527,6 +561,48 @@ static int kvm_mips_put_cp0_registers(CPUState *cs, int level) > DPRINTF("%s: Failed to put CP0_PRID (%d)\n", __func__, err); > ret = err; > } > + err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG, > + &env->CP0_Config0, > + KVM_REG_MIPS_CP0_CONFIG_MASK); > + if (err < 0) { > + DPRINTF("%s: Failed to change CP0_CONFIG (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG1, > + &env->CP0_Config1, > + KVM_REG_MIPS_CP0_CONFIG1_MASK); > + if (err < 0) { > + DPRINTF("%s: Failed to change CP0_CONFIG1 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG2, > + &env->CP0_Config2, > + KVM_REG_MIPS_CP0_CONFIG2_MASK); > + if (err < 0) { > + DPRINTF("%s: Failed to change CP0_CONFIG2 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG3, > + &env->CP0_Config3, > + KVM_REG_MIPS_CP0_CONFIG3_MASK); > + if (err < 0) { > + DPRINTF("%s: Failed to change CP0_CONFIG3 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG4, > + &env->CP0_Config4, > + KVM_REG_MIPS_CP0_CONFIG4_MASK); > + if (err < 0) { > + DPRINTF("%s: Failed to change CP0_CONFIG4 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_change_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG5, > + &env->CP0_Config5, > + KVM_REG_MIPS_CP0_CONFIG5_MASK); > + if (err < 0) { > + DPRINTF("%s: Failed to change CP0_CONFIG5 (%d)\n", __func__, err); > + ret = err; > + } > err = kvm_mips_put_one_ulreg(cs, KVM_REG_MIPS_CP0_ERROREPC, > &env->CP0_ErrorEPC); > if (err < 0) { > @@ -618,6 +694,38 @@ static int kvm_mips_get_cp0_registers(CPUState *cs) > DPRINTF("%s: Failed to get CP0_PRID (%d)\n", __func__, err); > ret = err; > } > + err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG, &env->CP0_Config0); > + if (err < 0) { > + DPRINTF("%s: Failed to get CP0_CONFIG (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG1, &env->CP0_Config1); > + if (err < 0) { > + DPRINTF("%s: Failed to get CP0_CONFIG1 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG2, &env->CP0_Config2); > + if (err < 0) { > + DPRINTF("%s: Failed to get CP0_CONFIG2 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG3, &env->CP0_Config3); > + if (err < 0) { > + DPRINTF("%s: Failed to get CP0_CONFIG3 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG4, > + &env->CP0_Config4); > + if (err < 0) { > + DPRINTF("%s: Failed to get CP0_CONFIG4 (%d)\n", __func__, err); > + ret = err; > + } > + err = kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG5, > + &env->CP0_Config5); > + if (err < 0) { > + DPRINTF("%s: Failed to get CP0_CONFIG5 (%d)\n", __func__, err); > + ret = err; > + } There's a minor style inconsistency here - for Config4 and Config5 the last argument has been moved to a new line whereas for Config{0,1,2,3} all arguments are in the same line. > err = kvm_mips_get_one_ulreg(cs, KVM_REG_MIPS_CP0_ERROREPC, > &env->CP0_ErrorEPC); > if (err < 0) { >