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

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 16/01/13 21:21, Marcelo Tosatti wrote:
> On Wed, Jan 16, 2013 at 09:03:20PM +0100, Christian Borntraeger wrote:
>> On 16/01/13 17:05, Marcelo Tosatti wrote:
>>
>>> The S/390 problem, from
>>> http://lists.nongnu.org/archive/html/qemu-devel/2012-11/msg02213.html:
>>>
>>> ">>> The kvm register sync needs to happen in the kvm register sync
>>>>>> function :)
>>>>> That would eliminate the whole purpose of sync regs and forces us to
>>>>> have an
>>>>> expensive ioctl on lots of exits (again). I would prefer to sync the 
>>>>> registers
>>>>> that we never need in qemu just here.
>>>>
>>>> That's why the register sync has different stages.
>>>
>>> Not the get_register. Which is called on every synchronize_state. Which
>>> happen 
>>> quite often
>>> on s390."
>>>
>>> But wait: on these S/390 codepaths, you do GET_REGS already, via
>>> cpu_synchronize_state.
>>>
>>> So on S/390
>>>
>>> - cpu_synchronize_state(env)
>>> - read any register from env
>>>
>>> Is not valid? This is what generic code assumes.
>>
>> TO recap the motiviation:
>>
>> cpu_synchronize_state on s390 currently updates any register in env that is
>> used by qemu (general purpose, prefix, psw, control and access) in the normal
>> runtime. it turns out we have all of these regs in kvm_run, so we can do 
>> synchronize states without doing an additional ioctl call.
>> Now, for life migration and dump we need some additional registers (which are
>> only accessable via onereg interface). So synchronize_state would need to
>> do 3 or 4 additional system calls on the hot path, only to take care of 
>> something that is not on the hot path at all.
>> For historic reasons, we have one exit code for almost all exits. Therefore,
>> we need to call synchronize_states almost always.
>> We could now start to have a poor mans synchronize_state in arch code, but
>> that would collide with common code synchronize_state if done at the wrong
>> time. Thus we want to make common code capable of having only a subset of
>> the register synched - by making it possible to sync the other regs later
>> on if needed without wiping the former sync.
>>
>> Makes sense?
>>
>> Christian
> 
> Yes. As noted in the last email on the thread, runtime/reset/full are to
> serapate sets of registers when writing _to_ kernel. When reading _from_
> kernel, reset and full distinctions are not appropriate (any register
> can change, as far as knowledge goes).

Hmm, I probably did not understood your point, so I will try to explain mine
and see what you respond :-)

The point of the patch set, is to allow this distinction when reading. 
In other words it allows code to state: I am only interested in regxy and dont
care if the other regs in env are out of sync.
If a full sync is necessary later on the other regs are synched as well.
If a full sync was already done before a partial get becomes a no-op. 
Why should that be not possible.

Christian