Re: [PATCH 03/21] qemu-kvm: Clean up register access API

Jan Kiszka
2010-02-02T11:18:56Z

Gleb Natapov wrote:
> On Tue, Feb 02, 2010 at 09:18:49AM +0100, Jan Kiszka wrote:
>> qemu-kvm's functios for accessing the VCPU registers are
>> kvm_arch_load/save_regs. Use them directly instead of going through
>> various wrappers. Specifically, we do not need on_vcpu wrapping as all
>> users either already run in the related thread or call while the vm is
>> stopped.
>>
> Can we put check for that into those functions just to be sure.
> Something like:
> assert(!vm_stopped && env->thread_id != pthread_id())
>

Good idea. Will add this to a potential v2 or send an add-on patch. We
just need something else than vm_stopped (for reset, only the vcpu
threads are stopped, not the vm), probably env->stopped in qemu-kvm.

>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>> qemu-kvm.c | 37 +++----------------------------------
>> qemu-kvm.h | 11 -----------
>> target-ia64/machine.c | 4 ++--
>> 3 files changed, 5 insertions(+), 47 deletions(-)
>>
>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>> index a305907..97c098c 100644
>> --- a/qemu-kvm.c
>> +++ b/qemu-kvm.c
>> @@ -862,7 +862,7 @@ int pre_kvm_run(kvm_context_t kvm, CPUState *env)
>> kvm_arch_pre_run(env, env->kvm_run);
>>
>> if (env->kvm_cpu_state.regs_modified) {
>> - kvm_arch_put_registers(env);
>> + kvm_arch_load_regs(env);
>> env->kvm_cpu_state.regs_modified = 0;
>> }
>>
>> @@ -1532,16 +1532,11 @@ static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
>> qemu_cond_wait(&qemu_work_cond);
>> }
>>
>> -void kvm_arch_get_registers(CPUState *env)
>> -{
>> - kvm_arch_save_regs(env);
>> -}
>> -
>> static void do_kvm_cpu_synchronize_state(void *_env)
>> {
>> CPUState *env = _env;
>> if (!env->kvm_cpu_state.regs_modified) {
>> - kvm_arch_get_registers(env);
>> + kvm_arch_save_regs(env);
>> env->kvm_cpu_state.regs_modified = 1;
>> }
>> }
>> @@ -1584,32 +1579,6 @@ void kvm_update_interrupt_request(CPUState *env)
>> }
>> }
>>
>> -static void kvm_do_load_registers(void *_env)
>> -{
>> - CPUState *env = _env;
>> -
>> - kvm_arch_load_regs(env);
>> -}
>> -
>> -void kvm_load_registers(CPUState *env)
>> -{
>> - if (kvm_enabled() && qemu_system_ready)
>> - on_vcpu(env, kvm_do_load_registers, env);
>> -}
>> -
>> -static void kvm_do_save_registers(void *_env)
>> -{
>> - CPUState *env = _env;
>> -
>> - kvm_arch_save_regs(env);
>> -}
>> -
>> -void kvm_save_registers(CPUState *env)
>> -{
>> - if (kvm_enabled())
>> - on_vcpu(env, kvm_do_save_registers, env);
>> -}
>> -
>> static void kvm_do_load_mpstate(void *_env)
>> {
>> CPUState *env = _env;
>> @@ -2379,7 +2348,7 @@ static void kvm_invoke_set_guest_debug(void *data)
>> struct kvm_set_guest_debug_data *dbg_data = data;
>>
>> if (cpu_single_env->kvm_cpu_state.regs_modified) {
>> - kvm_arch_put_registers(cpu_single_env);
>> + kvm_arch_save_regs(cpu_single_env);
>> cpu_single_env->kvm_cpu_state.regs_modified = 0;
>> }
>> dbg_data->err =
>> diff --git a/qemu-kvm.h b/qemu-kvm.h
>> index 6b3e5a1..1354227 100644
>> --- a/qemu-kvm.h
>> +++ b/qemu-kvm.h
>> @@ -902,8 +902,6 @@ int kvm_main_loop(void);
>> int kvm_init_ap(void);
>> #ifndef QEMU_KVM_NO_CPU
>> int kvm_vcpu_inited(CPUState *env);
>> -void kvm_load_registers(CPUState *env);
>> -void kvm_save_registers(CPUState *env);
>> void kvm_load_mpstate(CPUState *env);
>> void kvm_save_mpstate(CPUState *env);
>> int kvm_cpu_exec(CPUState *env);
>> @@ -1068,8 +1066,6 @@ void kvm_load_tsc(CPUState *env);
>> #ifdef TARGET_I386
>> #define qemu_kvm_has_pit_state2() (0)
>> #endif
>> -#define kvm_load_registers(env) do {} while(0)
>> -#define kvm_save_registers(env) do {} while(0)
>> #define kvm_save_mpstate(env) do {} while(0)
>> #define qemu_kvm_cpu_stop(env) do {} while(0)
>> static inline void kvm_init_vcpu(CPUState *env)
>> @@ -1098,13 +1094,6 @@ static inline int kvm_sync_vcpus(void)
>> }
>>
>> #ifndef QEMU_KVM_NO_CPU
>> -void kvm_arch_get_registers(CPUState *env);
>> -
>> -static inline void kvm_arch_put_registers(CPUState *env)
>> -{
>> - kvm_load_registers(env);
>> -}
>> -
>> void kvm_cpu_synchronize_state(CPUState *env);
>>
>> static inline void cpu_synchronize_state(CPUState *env)
>> diff --git a/target-ia64/machine.c b/target-ia64/machine.c
>> index 70ef379..7d29575 100644
>> --- a/target-ia64/machine.c
>> +++ b/target-ia64/machine.c
>> @@ -9,7 +9,7 @@ void cpu_save(QEMUFile *f, void *opaque)
>> CPUState *env = opaque;
>>
>> if (kvm_enabled()) {
>> - kvm_save_registers(env);
>> + kvm_arch_save_regs(env);
>> kvm_arch_save_mpstate(env);
>> }
>> }
>> @@ -19,7 +19,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
>> CPUState *env = opaque;
>>
>> if (kvm_enabled()) {
>> - kvm_load_registers(env);
>> + kvm_arch_load_regs(env);
>> kvm_arch_load_mpstate(env);
>> }
>> return 0;
>> --
>> 1.6.0.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Gleb.

Jan

--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux