On 01/24/2013 07:40 AM, Alexander Graf wrote:
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.
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.
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?
Alex
Ok, based on the discussions we've had I think I have a plan of attack
based on Alex's above suggestion. I believe it also satisfies the
concerns Marcelo pointed out. Please correct me if I'm wrong.
kvm_arch_get_registers() stays exactly as is for all architectures
(reads RUNTIME state only). No new parameters.
Each architecture defines arch specific bits for runtime/reset/full states:
#define KVM_REGSYNC_I386_RUNTIME_BIT (1 << 1)
#define KVM_REGSYNC_I386_RESET_BIT (1 << 2)
#define KVM_REGSYNC_I386_FULL_BIT (1 << 3)
Each architecture defines generic bits (for use in platform agnostic
code: kvm-all.c) for runtime/reset/full states:
#define KVM_REGSYNC_RUNTIME_STATE KVM_REGSYNC_I386_RUNTIME_BIT
#define KVM_REGSYNC_RESET_STATE KVM_REGSYNC_I386_RESET_BIT
#define KVM_REGSYNC_FULL_STATE KVM_REGSYNC_I386_FULL_BIT
S390: replace KVM_REGSYNC_S390_RUNTIME_BIT with two new bits so the S390
arch specific bits look like this:
#define KVM_REGSYNC_S390_RUNTIME_SOME_BIT (1 << 1)
#define KVM_REGSYNC_S390_RUNTIME_REST_BIT (1 << 2)
#define KVM_REGSYNC_S390_RESET_BIT (1 << 3)
#define KVM_REGSYNC_S390_FULL_BIT (1 << 4)
The idea being that SOME represents the set of RUNTIME registers we
always want to read when we exit from KVM. And REST represents the set
of RUNTIME registers we want to read for migration/dump and potentially
other special cases. My understanding is that SOME and REST should be
mutually exclusive. I think they need better names as well :).
S390 defines it's generic bits like this:
#define KVM_REGSYNC_RUNTIME_STATE
(KVM_REGSYNC_S390_RUNTIME_SOME_BIT |
KVM_REGSYNC_S390_RUNTIME_REST_BIT)
#define KVM_REGSYNC_RESET_STATE KVM_REGSYNC_S390_RESET_BIT
#define KVM_REGSYNC_FULL_STATE KVM_REGSYNC_S390_FULL_BIT
S390: A new function is created: s390_sync_partial_runtime_registers(int
bitmap). The bitmap argument indicates which of the SOME/REST register
sets to read. Either this new function or perhaps the caller will
update the cpu->kvm_vcpu_dirty bitmap to indicate which regs are now dirty.
S390: On the hot paths we call
s390_sync_partial_runtime_registers(KVM_REGSYNC_S390_RUNTIME_SOME_BIT)
instead of cpu_synchronize_state() to read only the set of runtime
registers we need on the hot path. If at some later point
cpu_synchronize_state() happens to be called then the S390 version of
kvm_arch_get_registers() needs to be smart enough to avoid data loss. So
we make it write back all dirty registers (env->kvm_vcpu_dirty) before
getting anything.
I think this works. Comments please and thank you!! :)