From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59550) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YW6Gv-0007Fb-GT for qemu-devel@nongnu.org; Thu, 12 Mar 2015 12:47:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YW6Gs-0002jR-3L for qemu-devel@nongnu.org; Thu, 12 Mar 2015 12:47:45 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:8044 helo=imgpgp01.kl.imgtec.org) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YW6Gr-0002io-GQ for qemu-devel@nongnu.org; Thu, 12 Mar 2015 12:47:42 -0400 Message-ID: <5501C323.2000908@imgtec.com> Date: Thu, 12 Mar 2015 16:47:31 +0000 From: James Hogan 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> <5501C1C1.8060001@imgtec.com> In-Reply-To: <5501C1C1.8060001@imgtec.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="t6FqpTFiSQM3MCXEQ2TU0mskfgFPlxAte" 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: Leon Alrae , qemu-devel@nongnu.org, Paolo Bonzini Cc: Aurelien Jarno , kvm@vger.kernel.org --t6FqpTFiSQM3MCXEQ2TU0mskfgFPlxAte Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 12/03/15 16:41, Leon Alrae wrote: > On 11/03/2015 15:22, James Hogan wrote: >> Implement saving and restoring to KVM state of the Config CP0 register= s >> (namely Config, Config1, Config2, Config3, Config4, and Config5). Thes= e >> control the features available to a guest, and a few of the fields wil= l >> soon be writeable by a guest so QEMU needs to know about them so as no= t >> 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) >> =20 >> 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); >> } >> =20 >> +#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) >=20 > CP0Cx_M is 31, thus without "U" suffix 1 is left shifted into sign bit > which is undefined behaviour. Well spotted, I'll fix that. >=20 >> +#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 =3D kvm_mips_get_one_reg(cs, reg_id, &tmp); >> + if (err < 0) { >> + return err; >> + } >> + >> + /* only change bits in mask */ >> + change =3D (*addr ^ tmp) & mask; >> + if (!change) { >> + return 0; >> + } >> + >> + tmp =3D 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 *c= s, int level) >> DPRINTF("%s: Failed to put CP0_PRID (%d)\n", __func__, err); >> ret =3D err; >> } >> + err =3D 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__, e= rr); >> + ret =3D err; >> + } >> + err =3D 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 =3D err; >> + } >> + err =3D 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 =3D err; >> + } >> + err =3D 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 =3D err; >> + } >> + err =3D 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 =3D err; >> + } >> + err =3D 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 =3D err; >> + } >> err =3D 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 *c= s) >> DPRINTF("%s: Failed to get CP0_PRID (%d)\n", __func__, err); >> ret =3D err; >> } >> + err =3D kvm_mips_get_one_reg(cs, KVM_REG_MIPS_CP0_CONFIG, &env->C= P0_Config0); >> + if (err < 0) { >> + DPRINTF("%s: Failed to get CP0_CONFIG (%d)\n", __func__, err)= ; >> + ret =3D err; >> + } >> + err =3D 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 =3D err; >> + } >> + err =3D 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 =3D err; >> + } >> + err =3D 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 =3D err; >> + } >> + err =3D 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 =3D err; >> + } >> + err =3D 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 =3D err; >> + } >=20 > 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. Yes, Config4 and Config5 used to be unsigned prior to Maciej's patches, so it used get_one_ureg, just enough to make it wrap. I'll fix. Thanks for reviewing, Cheers James >=20 >> err =3D kvm_mips_get_one_ulreg(cs, KVM_REG_MIPS_CP0_ERROREPC, >> &env->CP0_ErrorEPC); >> if (err < 0) { >> >=20 --t6FqpTFiSQM3MCXEQ2TU0mskfgFPlxAte Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVAcMpAAoJEGwLaZPeOHZ6QLgQAILcb26ThqLZSUCWatUe1H6M VI+V+n0VUefGDXdhYJOJEtb9jfHgUUv0djVnTrGxoBZinpJDou9UX520APIDDGYD 3MogZGuMhfJ5QqmKXgE2gBrXWsvnt7f+DdAUHH954COKpJdf7kT/AMYGwCBSRF3G o+p6js1QiOOXJoukh2legaEyGVP2Pn8ZU2Iq5YJ0r7Jn2oR/63zwnjbU7RH70FxR YRvo1sulQlUsAIxd1ol4HP6VWb+A8Vp6oc11mDOj8hIFCJzcq1NgfK8MIwB/0PQc fY+rq3w5Y9ZjFvZeCb9x6sYEOw5yx8tNGOfY0BFmDBBLKFTW2J3fbcyLoy1IzsoF L1hl07pvoheGGxk1kA4hRE/CHKzPeTq3R64Ns5dgz6UTS4yaMNAAQtW3ByYWBbot //sRA29DgX8mmI0Hy/8UK4oLV6Yfc7oE7poeMUJMLpqsk6423sIHLMvU5QWcjIHs AeVeCrjzWSSsDiHDvLPp9oo13q/CwPRFSzbMXvI96r2fc79m18aKdsaO6bmyeTib Hqv26w8RU3nZsiP397KXLRzNc5x6FicC8NhV5OWNnenE2bCnOKPzd/21mR2zxH5b kJNlcsmz+nUMPL5+1B9BTjYtlhaJeSS0oxYOfoUWfiyG38rI/erJWWddaZ+TOIKs XZIWSMTkCyzEbE/clfQX =tLNs -----END PGP SIGNATURE----- --t6FqpTFiSQM3MCXEQ2TU0mskfgFPlxAte--