qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 4/7 v2] KVM regsync: Add register bitmap paramet


From: Marcelo Tosatti
Subject: Re: [Qemu-devel] [PATCH 4/7 v2] KVM regsync: Add register bitmap parameter to do_kvm_cpu_synchronize_state
Date: Thu, 24 Jan 2013 18:44:50 -0200
User-agent: Mutt/1.5.21 (2010-09-15)

On Thu, Jan 24, 2013 at 01:40:49PM +0100, Alexander Graf wrote:
> > read_reg(x)
> >     if x not cached
> >             arch_get_regs(RUNTIME_STATE) (*)
> > 
> > write_reg(x, val)
> >     read_reg(x)
> >     cpustate->x = val;
> >     mark_dirty(x)
> > 
> > Which is basically the pattern used in KVM x86 (but instead of
> > ioctl(KVM_RUN) there is VMENTRY).
> 
> But that would mean that any code in QEMU that accesses registers can't 
> access env-> ,but instead needs to go through an accessor function. That's a 
> lot of potential for subtile error, no?

I do not see why. It has the potential to catch users of
env->reg which do not call cpu_synchronize_state().

> I think for now the best choice for get_regs() would be to ignore the 
> FULL/RESET bits and always keep the syncing as it happens today under the 
> RUNTIME umbrella only. So all of get_regs() only checks for RUNTIME.

Well the interface "kvm_arch_get_regs" is supposed to synchronize the
entire state ATM. So for example, "info registers" has

- cpu_synchronize_state()
- proceed assuming env-> is an uptodate copy of VCPU registers.

> Whenever get_xxx() happens, a bit gets set for set_xxx(). Up to this point, 
> only the RUNTIME bit is ever set, because that's what 
> cpu_synchronize_registers() sets.

There is no parameter to cpu_synchronize_registers().

> Then s390 can add special separate bits for "sync GPRs" and "sync CRs". 
> SYNC_RUNTIME would include those bits. The kvm hypercall exit calls a new 
> synchronize_registers() function with a parameter telling it to only sync 
> GPRs. This marks GPRs dirty, but not RUNTIME. The set_registers() function in 
> s390 specific code could handle this particular case specially.
> 
> That way everything's solved and scalable, no?

Yes, creating a new subset GPR which is part of RUNTIME is valid. 

S/390 not synchronizing the env-> copy of the FULL register set is still
a bug, though (because the FULL set is what "cpu_synchronize_state" with
no parameter implies).




reply via email to

[Prev in Thread] Current Thread [Next in Thread]