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.
Alex
