[Qemu-devel] Re: [PATCH v3] introduce on_vcpu

[Jan Kiszka]

Glauber Costa wrote:
> On Thu, Aug 27, 2009 at 07:15:28PM +0200, Jan Kiszka wrote:
>> Glauber Costa wrote:
>>> on_vcpu is a qemu-kvm function that will make sure that a specific
>>> piece of code will run on a requested cpu. We don't need that because
>>> we're restricted to -smp 1 right now, but those days are likely to end soon.
>>>
>>> So for the benefit of having qemu-kvm share more code with us, I'm
>>> introducing our own version of on_vcpu(). Right now, we either run
>>> a function on the current cpu, or abort the execution, because it would
>>> mean something is seriously wrong.
>>>
>>> As an example code, I "ported" kvm_update_guest_debug to use it,
>>> with some slight differences from qemu-kvm.
>>>
>>> This is probably 0.12 material
>>>
>>> Signed-off-by: Glauber Costa <address@hidden>
>>> CC: Jan Kiszka <address@hidden>
>>> ---
>>>  kvm-all.c |   35 +++++++++++++++++++++++++++++------
>>>  1 files changed, 29 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/kvm-all.c b/kvm-all.c
>>> index 61194b8..07a1cdb 100644
>>> --- a/kvm-all.c
>>> +++ b/kvm-all.c
>>> @@ -155,6 +155,15 @@ static void kvm_reset_vcpu(void *opaque)
>>>      }
>>>  }
>>>  
>>> +static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
>>> +{
>>> +    if (env == cpu_single_env) {
>>> +        func(data);
>>> +        return;
>>> +    }
>>> +    abort();
>> Sorry, missed this before it went in: This abort fires already now when
>> kvm_update_guest_debug is invoked by the gdbstub (where cpu_single_env
>> is 0). This completely breaks guest debugging int kvm mode.
>>
>> Moreover, if you enable I/O thread support, you already have a need for
>> true on_vcpu, don't you? Or is locking around the I/O thread still
>> broken in kvm mode? Anyway, please fix.
> It is, but I just sent two patches that would leave us in a better shape.
> Not sure what was made of them.
> 
> Anyway, I still have something almost ready for on_vcpu.
> Actually, I want to hear input on it: I was thinking the best architecture
> would be to drop it completely, and do automatically whenever we call 
> vcpu_ioctl.
> My only concern then would be speed. In this case, we could make this 
> non-blocking,
> and introduce explicit flush requests for remote cpus.
> 
> What do you think about it ?

Is on_vcpu only used for issuing vcpu_ioctl? Then combining both is
surely a reasonable step.

If you are concerned about performance, I think crafting a first version
over qemu-kvm, probably also merging it into that branch first makes
some sense. Though I think we should get along with a simple test for
the caller context in vcpu_ioctl for the fast path.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux