diff mbox

[3/3] QEMU kvm/i386 : Adding KICK_VCPU capability support in i386 target.

Message ID 20111204182622.28487.98656.sendpatchset@oc5400248562.ibm.com
State New
Headers show

Commit Message

Raghavendra K T Dec. 4, 2011, 6:26 p.m. UTC 
Extend the KVM Hypervisor to enable KICK_VCPU feature that allows
a vcpu to kick the halted vcpu to continue with execution in PV ticket
spinlock.

Signed-off-by: Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>
Signed-off-by: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
---

Comments

Alexander Graf Dec. 19, 2011, 1:54 p.m. UTC | #1 
On 04.12.2011, at 19:26, Raghavendra K T wrote:

> Extend the KVM Hypervisor to enable KICK_VCPU feature that allows
> a vcpu to kick the halted vcpu to continue with execution in PV ticket
> spinlock.
> 
> Signed-off-by: Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>
> Signed-off-by: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
> ---
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 5bfc21f..69bce21 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -97,6 +97,7 @@ struct kvm_para_features {
>     { KVM_CAP_NOP_IO_DELAY, KVM_FEATURE_NOP_IO_DELAY },
>     { KVM_CAP_PV_MMU, KVM_FEATURE_MMU_OP },
>     { KVM_CAP_ASYNC_PF, KVM_FEATURE_ASYNC_PF },
> +    { KVM_CAP_KICK_VCPU, KVM_FEATURE_KICK_VCPU },

So this is handled in the kernel? Who enables the feature? Is it always on? Why bother with it in user space at all then?


Alex
Avi Kivity Dec. 19, 2011, 1:59 p.m. UTC | #2 
On 12/19/2011 03:54 PM, Alexander Graf wrote:
> On 04.12.2011, at 19:26, Raghavendra K T wrote:
>
> > Extend the KVM Hypervisor to enable KICK_VCPU feature that allows
> > a vcpu to kick the halted vcpu to continue with execution in PV ticket
> > spinlock.
> > 
> > Signed-off-by: Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>
> > Signed-off-by: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
> > ---
> > diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> > index 5bfc21f..69bce21 100644
> > --- a/target-i386/kvm.c
> > +++ b/target-i386/kvm.c
> > @@ -97,6 +97,7 @@ struct kvm_para_features {
> >     { KVM_CAP_NOP_IO_DELAY, KVM_FEATURE_NOP_IO_DELAY },
> >     { KVM_CAP_PV_MMU, KVM_FEATURE_MMU_OP },
> >     { KVM_CAP_ASYNC_PF, KVM_FEATURE_ASYNC_PF },
> > +    { KVM_CAP_KICK_VCPU, KVM_FEATURE_KICK_VCPU },
>
> So this is handled in the kernel? Who enables the feature? Is it always on? Why bother with it in user space at all then?

Backwards compatibility
Alexander Graf Dec. 19, 2011, 2:03 p.m. UTC | #3 
On 19.12.2011, at 14:59, Avi Kivity wrote:

> On 12/19/2011 03:54 PM, Alexander Graf wrote:
>> On 04.12.2011, at 19:26, Raghavendra K T wrote:
>> 
>>> Extend the KVM Hypervisor to enable KICK_VCPU feature that allows
>>> a vcpu to kick the halted vcpu to continue with execution in PV ticket
>>> spinlock.
>>> 
>>> Signed-off-by: Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>
>>> Signed-off-by: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
>>> ---
>>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>>> index 5bfc21f..69bce21 100644
>>> --- a/target-i386/kvm.c
>>> +++ b/target-i386/kvm.c
>>> @@ -97,6 +97,7 @@ struct kvm_para_features {
>>>    { KVM_CAP_NOP_IO_DELAY, KVM_FEATURE_NOP_IO_DELAY },
>>>    { KVM_CAP_PV_MMU, KVM_FEATURE_MMU_OP },
>>>    { KVM_CAP_ASYNC_PF, KVM_FEATURE_ASYNC_PF },
>>> +    { KVM_CAP_KICK_VCPU, KVM_FEATURE_KICK_VCPU },
>> 
>> So this is handled in the kernel? Who enables the feature? Is it always on? Why bother with it in user space at all then?
> 
> Backwards compatibility

If we want backwards compatibility, we need more than just a simple feature check, no? Oh, you feed that into CPUID? That's nifty. Ok, so you behave like VMX/SVM do on real hardware - you always expose the functionality but don't list it in CPUID for older user space.

Makes sense.


Alex
Jan Kiszka Dec. 19, 2011, 2:11 p.m. UTC | #4 
On 2011-12-19 15:03, Alexander Graf wrote:
> 
> On 19.12.2011, at 14:59, Avi Kivity wrote:
> 
>> On 12/19/2011 03:54 PM, Alexander Graf wrote:
>>> On 04.12.2011, at 19:26, Raghavendra K T wrote:
>>>
>>>> Extend the KVM Hypervisor to enable KICK_VCPU feature that allows
>>>> a vcpu to kick the halted vcpu to continue with execution in PV ticket
>>>> spinlock.
>>>>
>>>> Signed-off-by: Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>
>>>> Signed-off-by: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
>>>> ---
>>>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>>>> index 5bfc21f..69bce21 100644
>>>> --- a/target-i386/kvm.c
>>>> +++ b/target-i386/kvm.c
>>>> @@ -97,6 +97,7 @@ struct kvm_para_features {
>>>>    { KVM_CAP_NOP_IO_DELAY, KVM_FEATURE_NOP_IO_DELAY },
>>>>    { KVM_CAP_PV_MMU, KVM_FEATURE_MMU_OP },
>>>>    { KVM_CAP_ASYNC_PF, KVM_FEATURE_ASYNC_PF },
>>>> +    { KVM_CAP_KICK_VCPU, KVM_FEATURE_KICK_VCPU },
>>>
>>> So this is handled in the kernel? Who enables the feature? Is it always on? Why bother with it in user space at all then?
>>
>> Backwards compatibility
> 
> If we want backwards compatibility, we need more than just a simple feature check, no? Oh, you feed that into CPUID? That's nifty. Ok, so you behave like VMX/SVM do on real hardware - you always expose the functionality but don't list it in CPUID for older user space.

Do we want this to be on when providing a compat machine type ("pc-0.12"
etc.) to the guest? Then it does need more work (see the dance around
kvmclock).

Jan
Avi Kivity Dec. 26, 2011, 2:07 p.m. UTC | #5 
On 12/19/2011 04:11 PM, Jan Kiszka wrote:
> >>
> >> Backwards compatibility
> > 
> > If we want backwards compatibility, we need more than just a simple feature check, no? Oh, you feed that into CPUID? That's nifty. Ok, so you behave like VMX/SVM do on real hardware - you always expose the functionality but don't list it in CPUID for older user space.
>
> Do we want this to be on when providing a compat machine type ("pc-0.12"
> etc.) to the guest? Then it does need more work (see the dance around
> kvmclock).

We do.  I have a feeling the whole cpuid stuff, paravirt and
non-paravirt, needs some fixing in this area.  It's different than the
normal compat code since not only qemu, but also kvm and the host cpu
have a say in what's supported and what's not.
Raghavendra K T Dec. 26, 2011, 5:13 p.m. UTC | #6 
On 12/26/2011 07:37 PM, Avi Kivity wrote:
> On 12/19/2011 04:11 PM, Jan Kiszka wrote:
>>>>
>>>> Backwards compatibility
>>>
>>> If we want backwards compatibility, we need more than just a simple feature check, no? Oh, you feed that into CPUID? That's nifty. Ok, so you behave like VMX/SVM do on real hardware - you always expose the functionality but don't list it in CPUID for older user space.
>>
>> Do we want this to be on when providing a compat machine type ("pc-0.12"
>> etc.) to the guest? Then it does need more work (see the dance around
>> kvmclock).
>
> We do.  I have a feeling the whole cpuid stuff, paravirt and
> non-paravirt, needs some fixing in this area.  It's different than the
> normal compat code since not only qemu, but also kvm and the host cpu
> have a say in what's supported and what's not.
>

Sorry, missed all threads except this due to some problem with mail
client config.  Yet to explore on what is to be done, But I  Agree for
the changes and work needed in this direction.
diff mbox

Patch

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 5bfc21f..69bce21 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -97,6 +97,7 @@  struct kvm_para_features {
     { KVM_CAP_NOP_IO_DELAY, KVM_FEATURE_NOP_IO_DELAY },
     { KVM_CAP_PV_MMU, KVM_FEATURE_MMU_OP },
     { KVM_CAP_ASYNC_PF, KVM_FEATURE_ASYNC_PF },
+    { KVM_CAP_KICK_VCPU, KVM_FEATURE_KICK_VCPU },
     { -1, -1 }
 };