| Message ID | 1247058542-31211-8-git-send-email-glommer@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/08/2009 04:09 PM, Glauber Costa wrote: > Signed-off-by: Glauber Costa<glommer@redhat.com> > --- > kvm-all.c | 27 ++++++++++++++++++--------- > qemu-kvm.h | 6 +++--- > target-i386/kvm.c | 4 ++-- > 3 files changed, 23 insertions(+), 14 deletions(-) > > Did you test the functionality to ensure we do not regress?
On Wed, Jul 08, 2009 at 04:27:27PM +0300, Avi Kivity wrote: > On 07/08/2009 04:09 PM, Glauber Costa wrote: >> Signed-off-by: Glauber Costa<glommer@redhat.com> >> --- >> kvm-all.c | 27 ++++++++++++++++++--------- >> qemu-kvm.h | 6 +++--- >> target-i386/kvm.c | 4 ++-- >> 3 files changed, 23 insertions(+), 14 deletions(-) >> >> > > Did you test the functionality to ensure we do not regress? just briefly. To be honest, I'd feel much more confortable if Jan could give it a try. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 07/08/2009 04:39 PM, Glauber Costa wrote: > On Wed, Jul 08, 2009 at 04:27:27PM +0300, Avi Kivity wrote: > >> On 07/08/2009 04:09 PM, Glauber Costa wrote: >> >>> Signed-off-by: Glauber Costa<glommer@redhat.com> >>> --- >>> kvm-all.c | 27 ++++++++++++++++++--------- >>> qemu-kvm.h | 6 +++--- >>> target-i386/kvm.c | 4 ++-- >>> 3 files changed, 23 insertions(+), 14 deletions(-) >>> >>> >>> >> Did you test the functionality to ensure we do not regress? >> > just briefly. To be honest, I'd feel much more confortable > if Jan could give it a try. > A Jan try (+review) will definitely help.
Avi Kivity wrote: > On 07/08/2009 04:39 PM, Glauber Costa wrote: >> On Wed, Jul 08, 2009 at 04:27:27PM +0300, Avi Kivity wrote: >> >>> On 07/08/2009 04:09 PM, Glauber Costa wrote: >>> >>>> Signed-off-by: Glauber Costa<glommer@redhat.com> >>>> --- >>>> kvm-all.c | 27 ++++++++++++++++++--------- >>>> qemu-kvm.h | 6 +++--- >>>> target-i386/kvm.c | 4 ++-- >>>> 3 files changed, 23 insertions(+), 14 deletions(-) >>>> >>>> >>>> >>> Did you test the functionality to ensure we do not regress? >>> >> just briefly. To be honest, I'd feel much more confortable >> if Jan could give it a try. >> > > A Jan try (+review) will definitely help. > Queued. Jan
Jan Kiszka wrote: > Avi Kivity wrote: >> On 07/08/2009 04:39 PM, Glauber Costa wrote: >>> On Wed, Jul 08, 2009 at 04:27:27PM +0300, Avi Kivity wrote: >>> >>>> On 07/08/2009 04:09 PM, Glauber Costa wrote: >>>> >>>>> Signed-off-by: Glauber Costa<glommer@redhat.com> >>>>> --- >>>>> kvm-all.c | 27 ++++++++++++++++++--------- >>>>> qemu-kvm.h | 6 +++--- >>>>> target-i386/kvm.c | 4 ++-- >>>>> 3 files changed, 23 insertions(+), 14 deletions(-) >>>>> >>>>> >>>>> >>>> Did you test the functionality to ensure we do not regress? >>>> >>> just briefly. To be honest, I'd feel much more confortable >>> if Jan could give it a try. >>> >> A Jan try (+review) will definitely help. >> > > Queued. > Deferred until v2. My first impression is that too much upstream code is moved or touched. Glauber, if you want to use some function that is currently under KVM_UPSTREAM, don't move it, just drop the #ifdef around it. And when done, have a look at the diff between upstream and qemu-kvm to avoid unneeded variations. Another question: What prevents using CONFIG_KVM also for qemu-kvm? I would rather mask out yet unused upstream code via the well-known KVM_UPSTREAM and drop the easily confusable USE_KVM defines. Jan
On 07/08/2009 10:44 PM, Jan Kiszka wrote: > > Deferred until v2. My first impression is that too much upstream code is > moved or touched. > > Glauber, if you want to use some function that is currently under > KVM_UPSTREAM, don't move it, just drop the #ifdef around it. And when > done, have a look at the diff between upstream and qemu-kvm to avoid > unneeded variations. > Yes. I though that's what we'd agreed, but forgot to verify it. The only difference should be #endif/#ifdef pairs. > Another question: What prevents using CONFIG_KVM also for qemu-kvm? I > would rather mask out yet unused upstream code via the well-known > KVM_UPSTREAM and drop the easily confusable USE_KVM defines. > Good idea, would simplify ./configure as well.
diff --git a/kvm-all.c b/kvm-all.c index b404f76..6f92874 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -1476,6 +1476,10 @@ int kvm_qemu_init() kvm_context->no_irqchip_creation = 0; kvm_context->no_pit_creation = 0; +#ifdef KVM_CAP_SET_GUEST_DEBUG + TAILQ_INIT(&kvm_state->kvm_sw_breakpoints); +#endif + gsi_count = kvm_get_gsi_count(kvm_context); if (gsi_count > 0) { int gsi_bits, i; @@ -3434,14 +3438,13 @@ int kvm_qemu_init_env(CPUState *cenv) } #ifdef KVM_CAP_SET_GUEST_DEBUG -struct kvm_sw_breakpoint_head kvm_sw_breakpoints = - TAILQ_HEAD_INITIALIZER(kvm_sw_breakpoints); -struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(target_ulong pc) +struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *env, + target_ulong pc) { struct kvm_sw_breakpoint *bp; - TAILQ_FOREACH(bp, &kvm_sw_breakpoints, entry) { + TAILQ_FOREACH(bp, &env->kvm_state->kvm_sw_breakpoints, entry) { if (bp->pc == pc) return bp; } @@ -3476,6 +3479,11 @@ int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap) return data.err; } +int kvm_sw_breakpoints_active(CPUState *env) +{ + return !TAILQ_EMPTY(&env->kvm_state->kvm_sw_breakpoints); +} + int kvm_insert_breakpoint(CPUState *current_env, target_ulong addr, target_ulong len, int type) { @@ -3484,7 +3492,7 @@ int kvm_insert_breakpoint(CPUState *current_env, target_ulong addr, int err; if (type == GDB_BREAKPOINT_SW) { - bp = kvm_find_sw_breakpoint(addr); + bp = kvm_find_sw_breakpoint(current_env, addr); if (bp) { bp->use_count++; return 0; @@ -3502,7 +3510,8 @@ int kvm_insert_breakpoint(CPUState *current_env, target_ulong addr, return err; } - TAILQ_INSERT_HEAD(&kvm_sw_breakpoints, bp, entry); + TAILQ_INSERT_HEAD(¤t_env->kvm_state->kvm_sw_breakpoints, + bp, entry); } else { err = kvm_arch_insert_hw_breakpoint(addr, len, type); if (err) @@ -3525,7 +3534,7 @@ int kvm_remove_breakpoint(CPUState *current_env, target_ulong addr, int err; if (type == GDB_BREAKPOINT_SW) { - bp = kvm_find_sw_breakpoint(addr); + bp = kvm_find_sw_breakpoint(current_env, addr); if (!bp) return -ENOENT; @@ -3538,7 +3547,7 @@ int kvm_remove_breakpoint(CPUState *current_env, target_ulong addr, if (err) return err; - TAILQ_REMOVE(&kvm_sw_breakpoints, bp, entry); + TAILQ_REMOVE(¤t_env->kvm_state->kvm_sw_breakpoints, bp, entry); qemu_free(bp); } else { err = kvm_arch_remove_hw_breakpoint(addr, len, type); @@ -3559,7 +3568,7 @@ void kvm_remove_all_breakpoints(CPUState *current_env) struct kvm_sw_breakpoint *bp, *next; CPUState *env; - TAILQ_FOREACH_SAFE(bp, &kvm_sw_breakpoints, entry, next) { + TAILQ_FOREACH_SAFE(bp, ¤t_env->kvm_state->kvm_sw_breakpoints, entry, next) { if (kvm_arch_remove_sw_breakpoint(current_env, bp) != 0) { /* Try harder to find a CPU that currently sees the breakpoint. */ for (env = first_cpu; env != NULL; env = env->next_cpu) { diff --git a/qemu-kvm.h b/qemu-kvm.h index 4c185fd..bce80a2 100644 --- a/qemu-kvm.h +++ b/qemu-kvm.h @@ -88,12 +88,12 @@ struct kvm_sw_breakpoint { int use_count; TAILQ_ENTRY(kvm_sw_breakpoint) entry; }; -TAILQ_HEAD(kvm_sw_breakpoint_head, kvm_sw_breakpoint); -extern struct kvm_sw_breakpoint_head kvm_sw_breakpoints; +TAILQ_HEAD(kvm_sw_breakpoint_head, kvm_sw_breakpoint); int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info); -struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(target_ulong pc); +int kvm_sw_breakpoints_active(CPUState *env); +struct kvm_sw_breakpoint *kvm_find_sw_breakpoint(CPUState *env, target_ulong pc); int kvm_arch_insert_sw_breakpoint(CPUState *current_env, struct kvm_sw_breakpoint *bp); int kvm_arch_remove_sw_breakpoint(CPUState *current_env, diff --git a/target-i386/kvm.c b/target-i386/kvm.c index ab324f6..66da1ba 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -2433,7 +2433,7 @@ int kvm_arch_debug(struct kvm_debug_exit_arch *arch_info) break; } } - } else if (kvm_find_sw_breakpoint(arch_info->pc)) + } else if (kvm_find_sw_breakpoint(cpu_single_env, arch_info->pc)) handle = 1; if (!handle) @@ -2456,7 +2456,7 @@ void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg) }; int n; - if (!TAILQ_EMPTY(&kvm_sw_breakpoints)) + if (kvm_sw_breakpoints_active(env)) dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP; if (nb_hw_breakpoint > 0) {
Signed-off-by: Glauber Costa <glommer@redhat.com> --- kvm-all.c | 27 ++++++++++++++++++--------- qemu-kvm.h | 6 +++--- target-i386/kvm.c | 4 ++-- 3 files changed, 23 insertions(+), 14 deletions(-)