| Message ID | 20190228225759.21328-5-farosas@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/ppc: single step for KVM HV | expand |
On Thu, Feb 28, 2019 at 07:57:58PM -0300, Fabiano Rosas wrote: > There are four scenarios being handled in this function: > > - single stepping > - hardware breakpoints > - software breakpoints > - fallback (no debug supported) > > A future patch will add code to handle specific single step and > software breakpoints cases so let's split each scenario into its own > function now to avoid hurting readability. > > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> > Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> Again, a nice cleanup regardless of anything else. Applied. > --- > target/ppc/kvm.c | 86 ++++++++++++++++++++++++++++-------------------- > 1 file changed, 50 insertions(+), 36 deletions(-) > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 941c4e7523..9392fba192 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -1620,52 +1620,66 @@ static int kvm_handle_hw_breakpoint(CPUState *cs, > return handle; > } > > +static int kvm_handle_singlestep(void) > +{ > + return 1; > +} > + > +static int kvm_handle_sw_breakpoint(void) > +{ > + return 1; > +} > + > static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run) > { > CPUState *cs = CPU(cpu); > CPUPPCState *env = &cpu->env; > struct kvm_debug_exit_arch *arch_info = &run->debug.arch; > - int handle = 0; > > if (cs->singlestep_enabled) { > - handle = 1; > - } else if (arch_info->status) { > - handle = kvm_handle_hw_breakpoint(cs, arch_info); > - } else if (kvm_find_sw_breakpoint(cs, arch_info->address)) { > - handle = 1; > - } else { > - /* QEMU is not able to handle debug exception, so inject > - * program exception to guest; > - * Yes program exception NOT debug exception !! > - * When QEMU is using debug resources then debug exception must > - * be always set. To achieve this we set MSR_DE and also set > - * MSRP_DEP so guest cannot change MSR_DE. > - * When emulating debug resource for guest we want guest > - * to control MSR_DE (enable/disable debug interrupt on need). > - * Supporting both configurations are NOT possible. > - * So the result is that we cannot share debug resources > - * between QEMU and Guest on BOOKE architecture. > - * In the current design QEMU gets the priority over guest, > - * this means that if QEMU is using debug resources then guest > - * cannot use them; > - * For software breakpoint QEMU uses a privileged instruction; > - * So there cannot be any reason that we are here for guest > - * set debug exception, only possibility is guest executed a > - * privileged / illegal instruction and that's why we are > - * injecting a program interrupt. > - */ > + return kvm_handle_singlestep(); > + } > + > + if (arch_info->status) { > + return kvm_handle_hw_breakpoint(cs, arch_info); > + } > > - cpu_synchronize_state(cs); > - /* env->nip is PC, so increment this by 4 to use > - * ppc_cpu_do_interrupt(), which set srr0 = env->nip - 4. > - */ > - env->nip += 4; > - cs->exception_index = POWERPC_EXCP_PROGRAM; > - env->error_code = POWERPC_EXCP_INVAL; > - ppc_cpu_do_interrupt(cs); > + if (kvm_find_sw_breakpoint(cs, arch_info->address)) { > + return kvm_handle_sw_breakpoint(); > } > > - return handle; > + /* > + * QEMU is not able to handle debug exception, so inject > + * program exception to guest; > + * Yes program exception NOT debug exception !! > + * When QEMU is using debug resources then debug exception must > + * be always set. To achieve this we set MSR_DE and also set > + * MSRP_DEP so guest cannot change MSR_DE. > + * When emulating debug resource for guest we want guest > + * to control MSR_DE (enable/disable debug interrupt on need). > + * Supporting both configurations are NOT possible. > + * So the result is that we cannot share debug resources > + * between QEMU and Guest on BOOKE architecture. > + * In the current design QEMU gets the priority over guest, > + * this means that if QEMU is using debug resources then guest > + * cannot use them; > + * For software breakpoint QEMU uses a privileged instruction; > + * So there cannot be any reason that we are here for guest > + * set debug exception, only possibility is guest executed a > + * privileged / illegal instruction and that's why we are > + * injecting a program interrupt. > + */ > + cpu_synchronize_state(cs); > + /* > + * env->nip is PC, so increment this by 4 to use > + * ppc_cpu_do_interrupt(), which set srr0 = env->nip - 4. > + */ > + env->nip += 4; > + cs->exception_index = POWERPC_EXCP_PROGRAM; > + env->error_code = POWERPC_EXCP_INVAL; > + ppc_cpu_do_interrupt(cs); > + > + return 0; > } > > int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 941c4e7523..9392fba192 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -1620,52 +1620,66 @@ static int kvm_handle_hw_breakpoint(CPUState *cs, return handle; } +static int kvm_handle_singlestep(void) +{ + return 1; +} + +static int kvm_handle_sw_breakpoint(void) +{ + return 1; +} + static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run) { CPUState *cs = CPU(cpu); CPUPPCState *env = &cpu->env; struct kvm_debug_exit_arch *arch_info = &run->debug.arch; - int handle = 0; if (cs->singlestep_enabled) { - handle = 1; - } else if (arch_info->status) { - handle = kvm_handle_hw_breakpoint(cs, arch_info); - } else if (kvm_find_sw_breakpoint(cs, arch_info->address)) { - handle = 1; - } else { - /* QEMU is not able to handle debug exception, so inject - * program exception to guest; - * Yes program exception NOT debug exception !! - * When QEMU is using debug resources then debug exception must - * be always set. To achieve this we set MSR_DE and also set - * MSRP_DEP so guest cannot change MSR_DE. - * When emulating debug resource for guest we want guest - * to control MSR_DE (enable/disable debug interrupt on need). - * Supporting both configurations are NOT possible. - * So the result is that we cannot share debug resources - * between QEMU and Guest on BOOKE architecture. - * In the current design QEMU gets the priority over guest, - * this means that if QEMU is using debug resources then guest - * cannot use them; - * For software breakpoint QEMU uses a privileged instruction; - * So there cannot be any reason that we are here for guest - * set debug exception, only possibility is guest executed a - * privileged / illegal instruction and that's why we are - * injecting a program interrupt. - */ + return kvm_handle_singlestep(); + } + + if (arch_info->status) { + return kvm_handle_hw_breakpoint(cs, arch_info); + } - cpu_synchronize_state(cs); - /* env->nip is PC, so increment this by 4 to use - * ppc_cpu_do_interrupt(), which set srr0 = env->nip - 4. - */ - env->nip += 4; - cs->exception_index = POWERPC_EXCP_PROGRAM; - env->error_code = POWERPC_EXCP_INVAL; - ppc_cpu_do_interrupt(cs); + if (kvm_find_sw_breakpoint(cs, arch_info->address)) { + return kvm_handle_sw_breakpoint(); } - return handle; + /* + * QEMU is not able to handle debug exception, so inject + * program exception to guest; + * Yes program exception NOT debug exception !! + * When QEMU is using debug resources then debug exception must + * be always set. To achieve this we set MSR_DE and also set + * MSRP_DEP so guest cannot change MSR_DE. + * When emulating debug resource for guest we want guest + * to control MSR_DE (enable/disable debug interrupt on need). + * Supporting both configurations are NOT possible. + * So the result is that we cannot share debug resources + * between QEMU and Guest on BOOKE architecture. + * In the current design QEMU gets the priority over guest, + * this means that if QEMU is using debug resources then guest + * cannot use them; + * For software breakpoint QEMU uses a privileged instruction; + * So there cannot be any reason that we are here for guest + * set debug exception, only possibility is guest executed a + * privileged / illegal instruction and that's why we are + * injecting a program interrupt. + */ + cpu_synchronize_state(cs); + /* + * env->nip is PC, so increment this by 4 to use + * ppc_cpu_do_interrupt(), which set srr0 = env->nip - 4. + */ + env->nip += 4; + cs->exception_index = POWERPC_EXCP_PROGRAM; + env->error_code = POWERPC_EXCP_INVAL; + ppc_cpu_do_interrupt(cs); + + return 0; } int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)