Re: [Qemu-devel] [PATCH v4 17/18] target-i386: block migration and savevm if invariant tsc is exposed

[Andreas Färber]

Am 15.05.2014 22:22, schrieb Andreas Färber:
> Am 30.04.2014 18:48, schrieb Eduardo Habkost:
>> From: Marcelo Tosatti <address@hidden>
>>
>> Invariant TSC documentation mentions that "invariant TSC will run at a
>> constant rate in all ACPI P-, C-. and T-states".
>>
>> This is not the case if migration to a host with different TSC frequency
>> is allowed, or if savevm is performed. So block migration/savevm.
>>
>> Also do not expose invariant tsc flag by default.
>>
>> Cc: Juan Quintela <address@hidden>
>> Signed-off-by: Marcelo Tosatti <address@hidden>
>> Reviewed-by: Eduardo Habkost <address@hidden>
>> Signed-off-by: Eduardo Habkost <address@hidden>
>> ---
>>  target-i386/cpu-qom.h |  2 +-
>>  target-i386/kvm.c     | 13 +++++++++++++
>>  target-i386/machine.c |  2 +-
>>  3 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
>> index 016f90d..473d803 100644
>> --- a/target-i386/cpu-qom.h
>> +++ b/target-i386/cpu-qom.h
>> @@ -121,7 +121,7 @@ static inline X86CPU *x86_env_get_cpu(CPUX86State *env)
>>  #define ENV_OFFSET offsetof(X86CPU, env)
>>  
>>  #ifndef CONFIG_USER_ONLY
>> -extern const struct VMStateDescription vmstate_x86_cpu;
>> +extern struct VMStateDescription vmstate_x86_cpu;
>>  #endif
>>  
>>  /**
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> index 4389959..99cc7e3 100644
>> --- a/target-i386/kvm.c
>> +++ b/target-i386/kvm.c
>> @@ -33,6 +33,8 @@
>>  #include "exec/ioport.h"
>>  #include <asm/hyperv.h>
>>  #include "hw/pci/pci.h"
>> +#include "migration/migration.h"
>> +#include "qapi/qmp/qerror.h"
>>  
>>  //#define DEBUG_KVM
>>  
>> @@ -447,6 +449,8 @@ static bool hyperv_enabled(X86CPU *cpu)
>>              cpu->hyperv_relaxed_timing);
>>  }
>>  
>> +Error *invtsc_mig_blocker;
> 
> This should be static, even if no zero-initialization is needed below.
> 
>> +
>>  #define KVM_MAX_CPUID_ENTRIES  100
>>  
>>  int kvm_arch_init_vcpu(CPUState *cs)
>> @@ -702,6 +706,15 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>                                    !!(c->ecx & CPUID_EXT_SMX);
>>      }
>>  
>> +    c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0);
>> +    if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) {
>> +        /* for migration */
>> +        error_set(&invtsc_mig_blocker, QERR_MIGRATION_NOT_SUPPORTED, "cpu");
> 
> This does not compile for me. error_setg()? With what text?

http://git.qemu-project.org/?p=qemu.git;a=blobdiff;f=include/qapi/qmp/qerror.h;h=01d1d0661c607ace1c5d3831e5c79eeab851f6b7;hp=a72bbc98503fe261bb8d2c407220252b1e6a85a4;hb=f231b88db14f13ee9a41599896f57f3594c1ca8b;hpb=d73f0beadb57f885e678bffc362864f4401262e0

-#define QERR_MIGRATION_NOT_SUPPORTED \
-    ERROR_CLASS_GENERIC_ERROR, "State blocked by non-migratable device
'%s'"

Suggesting something nicer than "device 'cpu'":

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 1fe8512..9b09a1a 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -710,7 +710,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
     c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0);
     if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) {
         /* for migration */
-        error_set(&invtsc_mig_blocker, QERR_MIGRATION_NOT_SUPPORTED,
"cpu");
+        error_setg(&invtsc_mig_blocker,
+                   "State blocked by non-migratable CPU device");
         migrate_add_blocker(invtsc_mig_blocker);
         /* for savevm */
         vmstate_x86_cpu.unmigratable = 1;

> 
> Regards,
> Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg