Patchwork [03/15] kvm: Install optimized interrupt handlers

login
register
mail settings
Submitter Jan Kiszka
Date March 4, 2011, 10:20 a.m.
Message ID <29e857949ee4ff738c7c2f27063ee945a1af4875.1299233998.git.jan.kiszka@siemens.com>
Download mbox | patch
Permalink /patch/85399/
State New
Headers show

Comments

Jan Kiszka - March 4, 2011, 10:20 a.m.
KVM only requires to set the raised IRQ in CPUState and, if the user
space irqchip is used, to kick the receiving vcpu if it is remote.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)
Marcelo Tosatti - March 5, 2011, 3:37 p.m.
On Fri, Mar 04, 2011 at 11:20:00AM +0100, Jan Kiszka wrote:
> KVM only requires to set the raised IRQ in CPUState and, if the user
> space irqchip is used, to kick the receiving vcpu if it is remote.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  kvm-all.c |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 226843c..c460d45 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -650,6 +650,20 @@ static CPUPhysMemoryClient kvm_cpu_phys_memory_client = {
>      .log_stop = kvm_log_stop,
>  };
>  
> +static void kvm_handle_interrupt(CPUState *env, int mask)
> +{
> +    env->interrupt_request |= mask;
> +

If the env->interrupt_request request is processed in userspace, such as
MCE, the kick is still necessary for irqchip case. CPU_INTERRUPT_DEBUG
is another example, no?

> +    if (!qemu_cpu_self(env)) {
> +        qemu_cpu_kick(env);
> +    }
> +}
> +
> +static void kvm_handle_interrupt_kernel_irqchip(CPUState *env, int mask)
> +{
> +    env->interrupt_request |= mask;
> +}
> +
Jan Kiszka - March 5, 2011, 6:11 p.m.
On 2011-03-05 16:37, Marcelo Tosatti wrote:
> On Fri, Mar 04, 2011 at 11:20:00AM +0100, Jan Kiszka wrote:
>> KVM only requires to set the raised IRQ in CPUState and, if the user
>> space irqchip is used, to kick the receiving vcpu if it is remote.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  kvm-all.c |   17 +++++++++++++++++
>>  1 files changed, 17 insertions(+), 0 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 226843c..c460d45 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -650,6 +650,20 @@ static CPUPhysMemoryClient kvm_cpu_phys_memory_client = {
>>      .log_stop = kvm_log_stop,
>>  };
>>  
>> +static void kvm_handle_interrupt(CPUState *env, int mask)
>> +{
>> +    env->interrupt_request |= mask;
>> +
> 
> If the env->interrupt_request request is processed in userspace, such as
> MCE, the kick is still necessary for irqchip case. CPU_INTERRUPT_DEBUG
> is another example, no?

[this probably targeted kvm_handle_interrupt_kernel_irqchip]

In principle, you are right. But MCE must be injected synchronously over
the target VCPU, see do_inject_x86_mce, and CPU_INTERRUPT_DEBUG is also
synchronous and not even used in KVM mode.

> 
>> +    if (!qemu_cpu_self(env)) {
>> +        qemu_cpu_kick(env);
>> +    }
>> +}
>> +
>> +static void kvm_handle_interrupt_kernel_irqchip(CPUState *env, int mask)
>> +{
>> +    env->interrupt_request |= mask;
>> +}
>> +

Jan
Marcelo Tosatti - March 6, 2011, 2:13 a.m.
On Sat, Mar 05, 2011 at 07:11:53PM +0100, Jan Kiszka wrote:
> On 2011-03-05 16:37, Marcelo Tosatti wrote:
> > On Fri, Mar 04, 2011 at 11:20:00AM +0100, Jan Kiszka wrote:
> >> KVM only requires to set the raised IRQ in CPUState and, if the user
> >> space irqchip is used, to kick the receiving vcpu if it is remote.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>  kvm-all.c |   17 +++++++++++++++++
> >>  1 files changed, 17 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/kvm-all.c b/kvm-all.c
> >> index 226843c..c460d45 100644
> >> --- a/kvm-all.c
> >> +++ b/kvm-all.c
> >> @@ -650,6 +650,20 @@ static CPUPhysMemoryClient kvm_cpu_phys_memory_client = {
> >>      .log_stop = kvm_log_stop,
> >>  };
> >>  
> >> +static void kvm_handle_interrupt(CPUState *env, int mask)
> >> +{
> >> +    env->interrupt_request |= mask;
> >> +
> > 
> > If the env->interrupt_request request is processed in userspace, such as
> > MCE, the kick is still necessary for irqchip case. CPU_INTERRUPT_DEBUG
> > is another example, no?
> 
> [this probably targeted kvm_handle_interrupt_kernel_irqchip]
> 
> In principle, you are right. But MCE must be injected synchronously over
> the target VCPU, see do_inject_x86_mce, and CPU_INTERRUPT_DEBUG is also
> synchronous and not even used in KVM mode.

CPU_INTERRUPT_NMI from monitor?

Don't see what gain you expect from avoiding the signal in this case.
Jan Kiszka - March 7, 2011, 8 a.m.
On 2011-03-06 03:13, Marcelo Tosatti wrote:
> On Sat, Mar 05, 2011 at 07:11:53PM +0100, Jan Kiszka wrote:
>> On 2011-03-05 16:37, Marcelo Tosatti wrote:
>>> On Fri, Mar 04, 2011 at 11:20:00AM +0100, Jan Kiszka wrote:
>>>> KVM only requires to set the raised IRQ in CPUState and, if the user
>>>> space irqchip is used, to kick the receiving vcpu if it is remote.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>  kvm-all.c |   17 +++++++++++++++++
>>>>  1 files changed, 17 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/kvm-all.c b/kvm-all.c
>>>> index 226843c..c460d45 100644
>>>> --- a/kvm-all.c
>>>> +++ b/kvm-all.c
>>>> @@ -650,6 +650,20 @@ static CPUPhysMemoryClient kvm_cpu_phys_memory_client = {
>>>>      .log_stop = kvm_log_stop,
>>>>  };
>>>>  
>>>> +static void kvm_handle_interrupt(CPUState *env, int mask)
>>>> +{
>>>> +    env->interrupt_request |= mask;
>>>> +
>>>
>>> If the env->interrupt_request request is processed in userspace, such as
>>> MCE, the kick is still necessary for irqchip case. CPU_INTERRUPT_DEBUG
>>> is another example, no?
>>
>> [this probably targeted kvm_handle_interrupt_kernel_irqchip]
>>
>> In principle, you are right. But MCE must be injected synchronously over
>> the target VCPU, see do_inject_x86_mce, and CPU_INTERRUPT_DEBUG is also
>> synchronous and not even used in KVM mode.
> 
> CPU_INTERRUPT_NMI from monitor?
> 
> Don't see what gain you expect from avoiding the signal in this case.

Well, looking at this from a different angle again, I cannot identify my
original optimization anymore. I guess I was under the wrong impression
that cpu_interrupt is still a frequently used service even with
in-kernel irqchip. But that's by far not the case.

Will drop this.

Jan

Patch

diff --git a/kvm-all.c b/kvm-all.c
index 226843c..c460d45 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -650,6 +650,20 @@  static CPUPhysMemoryClient kvm_cpu_phys_memory_client = {
     .log_stop = kvm_log_stop,
 };
 
+static void kvm_handle_interrupt(CPUState *env, int mask)
+{
+    env->interrupt_request |= mask;
+
+    if (!qemu_cpu_self(env)) {
+        qemu_cpu_kick(env);
+    }
+}
+
+static void kvm_handle_interrupt_kernel_irqchip(CPUState *env, int mask)
+{
+    env->interrupt_request |= mask;
+}
+
 int kvm_init(void)
 {
     static const char upgrade_note[] =
@@ -758,6 +772,9 @@  int kvm_init(void)
 
     s->many_ioeventfds = kvm_check_many_ioeventfds();
 
+    cpu_interrupt_handler = kvm_irqchip_in_kernel() ?
+        kvm_handle_interrupt_kernel_irqchip : kvm_handle_interrupt;
+
     return 0;
 
 err: