Message ID | 29e857949ee4ff738c7c2f27063ee945a1af4875.1299233998.git.jan.kiszka@siemens.com |
---|---|
State | New |
Headers | show |
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; > +} > +
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
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.
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
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:
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(-)