On Thu, Jan 24, 2013 at 06:44:50PM -0200, Marcelo Tosatti wrote: > 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.
Accessors though are more organized, the information of which code accesses CPUState->env is then explicit. Whether to synchronize a given register using eg. GET_SREGS (say GPR set) or GET_ONE_REG (individual register) is up to architecture. What 'subtle errors' are you thinking of? It should be easy to convert as its greppable. > 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).
