| Submitter | Jan Kiszka |
|---|---|
| Date | March 15, 2011, 11:26 a.m. |
| Message ID | <b8c5dfe9505a48556ee17545a53e8f6245e4b262.1300188374.git.jan.kiszka@siemens.com> |
| Download | mbox | patch |
| Permalink | /patch/86950/ |
| State | New |
| Headers | show |
Comments
On Tue, Mar 15, 2011 at 12:26:17PM +0100, Jan Kiszka wrote: > KVM only requires to set the raised IRQ in CPUState and to kick the > receiving vcpu if it is remote. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > kvm-all.c | 11 +++++++++++ > 1 files changed, 11 insertions(+), 0 deletions(-) > > diff --git a/kvm-all.c b/kvm-all.c > index 226843c..25ab545 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -650,6 +650,15 @@ 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_is_self(env)) { > + qemu_cpu_kick(env); > + } > +} > + Not sure its worthwhile to allow different handlers. The advantage over tcg version is that its shorter, without cpu_unlink_tb and icount handler?
On 2011-03-15 18:10, Marcelo Tosatti wrote: > On Tue, Mar 15, 2011 at 12:26:17PM +0100, Jan Kiszka wrote: >> KVM only requires to set the raised IRQ in CPUState and to kick the >> receiving vcpu if it is remote. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> kvm-all.c | 11 +++++++++++ >> 1 files changed, 11 insertions(+), 0 deletions(-) >> >> diff --git a/kvm-all.c b/kvm-all.c >> index 226843c..25ab545 100644 >> --- a/kvm-all.c >> +++ b/kvm-all.c >> @@ -650,6 +650,15 @@ 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_is_self(env)) { >> + qemu_cpu_kick(env); >> + } >> +} >> + > > Not sure its worthwhile to allow different handlers. The advantage over > tcg version is that its shorter, without cpu_unlink_tb and icount > handler? I thought about this again as well before posting, and I came to the conclusion that an important advantage is avoiding TCG surprises in KVM code paths. This way, KVM does not need to bother if cpu_unlink_tb or icount related code changes. Maybe I should have added this to the commit message. Jan
On 2011-03-15 21:12, Jan Kiszka wrote: > On 2011-03-15 18:10, Marcelo Tosatti wrote: >> On Tue, Mar 15, 2011 at 12:26:17PM +0100, Jan Kiszka wrote: >>> KVM only requires to set the raised IRQ in CPUState and to kick the >>> receiving vcpu if it is remote. >>> >>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>> --- >>> kvm-all.c | 11 +++++++++++ >>> 1 files changed, 11 insertions(+), 0 deletions(-) >>> >>> diff --git a/kvm-all.c b/kvm-all.c >>> index 226843c..25ab545 100644 >>> --- a/kvm-all.c >>> +++ b/kvm-all.c >>> @@ -650,6 +650,15 @@ 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_is_self(env)) { >>> + qemu_cpu_kick(env); >>> + } >>> +} >>> + >> >> Not sure its worthwhile to allow different handlers. The advantage over >> tcg version is that its shorter, without cpu_unlink_tb and icount >> handler? > > I thought about this again as well before posting, and I came to the > conclusion that an important advantage is avoiding TCG surprises in KVM > code paths. This way, KVM does not need to bother if cpu_unlink_tb or > icount related code changes. Maybe I should have added this to the > commit message. What's your opinion on this? Should I repost the remaining three with comments adjusted? Jan
On Fri, Mar 18, 2011 at 11:18:40AM +0100, Jan Kiszka wrote: > On 2011-03-15 21:12, Jan Kiszka wrote: > > On 2011-03-15 18:10, Marcelo Tosatti wrote: > >> On Tue, Mar 15, 2011 at 12:26:17PM +0100, Jan Kiszka wrote: > >>> KVM only requires to set the raised IRQ in CPUState and to kick the > >>> receiving vcpu if it is remote. > >>> > >>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > >>> --- > >>> kvm-all.c | 11 +++++++++++ > >>> 1 files changed, 11 insertions(+), 0 deletions(-) > >>> > >>> diff --git a/kvm-all.c b/kvm-all.c > >>> index 226843c..25ab545 100644 > >>> --- a/kvm-all.c > >>> +++ b/kvm-all.c > >>> @@ -650,6 +650,15 @@ 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_is_self(env)) { > >>> + qemu_cpu_kick(env); > >>> + } > >>> +} > >>> + > >> > >> Not sure its worthwhile to allow different handlers. The advantage over > >> tcg version is that its shorter, without cpu_unlink_tb and icount > >> handler? > > > > I thought about this again as well before posting, and I came to the > > conclusion that an important advantage is avoiding TCG surprises in KVM > > code paths. This way, KVM does not need to bother if cpu_unlink_tb or > > icount related code changes. Maybe I should have added this to the > > commit message. > > What's your opinion on this? Should I repost the remaining three with > comments adjusted? > > Jan > Its up to you. Your argument above makes sense.
Patch
diff --git a/kvm-all.c b/kvm-all.c index 226843c..25ab545 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -650,6 +650,15 @@ 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_is_self(env)) { + qemu_cpu_kick(env); + } +} + int kvm_init(void) { static const char upgrade_note[] = @@ -758,6 +767,8 @@ int kvm_init(void) s->many_ioeventfds = kvm_check_many_ioeventfds(); + cpu_interrupt_handler = kvm_handle_interrupt; + return 0; err:
KVM only requires to set the raised IRQ in CPUState and to kick the receiving vcpu if it is remote. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- kvm-all.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-)