On 07.01.2013, at 16:43, Jason J. Herne wrote:
On 01/04/2013 11:27 AM, Alexander Graf wrote:
On 04.01.2013, at 16:25, Jason J. Herne wrote:
If I've followed the conversation correctly this is what needs to be done:
1. Remove the level parameters from kvm_arch_get_registers and
kvm_arch_put_registers.
2. Add a new bitmap parameter to kvm_arch_get_registers and
kvm_arch_put_registers.
I would combine these into "replace levels with bitmap".
3. Define a bit that correlates to our current notion of "all runtime
registers". This bit, and all bits in this bitmap, would be architecture specific.
Why would that bit be architecture specific? "All runtime registers" == "registers
that gdb can access" IIRC. The implementation on what exactly that means obviously is
architecture specific, but the bit itself would not be, as the gdbstub wants to be able to
synchronize in arch independent code.
How do we want to define these bits? is it logical to break up the registers
into smaller categories and then use masks to create RUNTIME_STATE, FULL_STATE,
RESET_STATE? If so, how should we define them? Would they be arch specific
and then we'd create the _STATE masks for each architecture?
I see. So you only want to make the name arch independent, but keep its actual
backing bits arch specific. I can see how that'd end up being a useful thing to
do, yes.
So we could have archs that just define RUNTIME_STATE as ARCH_RUNTIME_STATE and
others that define it as ARCH_STATE_REGx | ARCH_STATE_REGy. That way other code
may only synchronize less than the full runtime state. Works for me :).
If we do simply define a bit for each of the above three states instead, they
should probably be 100% mutually exclusive to provide the best protection
against complicated data synchronization issues (like the original 7/7 patch
was trying to prevent). Also, if we can assume 100% mutual exclusion the sync
logic becomes trivial:
static void do_kvm_cpu_synchronize_state(void *arg)
{
struct kvm_cpu_syncstate_args *args = arg;
/* Do not sync regs that are already dirty */
int regs_to_get = args->regmap & ~cpu->kvm_vcpu_dirty;
kvm_arch_get_registers(args->cpu, regs_to_get);
args->cpu->kvm_vcpu_dirty |= regs_to_get;
}
Thoughts?
I like the idea of making the bits 100% mutually exclusive.