On 01/03/2013 08:56 AM, Alexander Graf wrote:
static void do_kvm_cpu_synchronize_state(void *_args)
>{
> struct kvm_cpu_syncstate_args *args = _args;
>+ CPUArchState *env = args->env;
>+ int register_level = args->register_level;
>
This probably becomes more readable if we explicitly revert back to unsynced
state first:
/* Write back local modifications at our current level */
if (register_level > env->kvm_vcpu_dirty) {
kvm_arch_put_registers(...);
env->kvm_vcpu_dirty = 0;
}
and then do the sync we are requested to do:
if (!env->kvm_vcpu_dirty) {
...
}
I agree, but only if we add a second conditional to the if 1st statement
as such:
if (args->env->kvm_vcpu_dirty && register_level > env->kvm_vcpu_dirty)
This is to cover the case where the caller is asking for register level
"1" and we're already dirty at level "2". In this case, nothing should
happen and we'll need the "args->env->kvm_vcpu_dirty" to ensure that is
the case.
static void do_kvm_cpu_synchronize_state(void *_args)
{
struct kvm_cpu_syncstate_args *args = _args;
CPUArchState *env = args->env;
int register_level = args->register_level;
/* Write back local modifications at our current level */
if (args->env->kvm_vcpu_dirty && register_level >
env->kvm_vcpu_dirty) {
kvm_arch_put_registers(env, env->kvm_vcpu_dirty);
env->kvm_vcpu_dirty = 0;
}
if (!args->env->kvm_vcpu_dirty) {
kvm_arch_get_registers(env, register_level);
env->kvm_vcpu_dirty = register_level;
}
}
Do you agree? Thanks for your time. :)
--
-- Jason J. Herne (jjhe...@linux.vnet.ibm.com)
