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 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 <[email protected]> >> Cc: Paolo Bonzini <[email protected]> >> Cc: Leon Alrae <[email protected]> >> Cc: Aurelien Jarno <[email protected]> >> --- >> 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.
Well spotted, I'll fix that.
>
>> +#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.
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
>
>> err = kvm_mips_get_one_ulreg(cs, KVM_REG_MIPS_CP0_ERROREPC,
>> &env->CP0_ErrorEPC);
>> if (err < 0) {
>>
>
signature.asc
Description: OpenPGP digital signature
