On 17.06.14 13:07, Madhavan Srinivasan wrote:
On Tuesday 17 June 2014 02:24 PM, Alexander Graf wrote:
On 14.06.14 23:08, Madhavan Srinivasan wrote:
This patch adds kernel side support for software breakpoint.
Design is that, by using an illegal instruction, we trap to hypervisor
via Emulation Assistance interrupt, where we check for the illegal
instruction
and accordingly we return to Host or Guest. Patch mandates use of
"abs" instruction
(primary opcode 31 and extended opcode 360) as sw breakpoint instruction.
Based on PowerISA v2.01, ABS instruction has been dropped from the
architecture
and treated an illegal instruction.

Signed-off-by: Madhavan Srinivasan <ma...@linux.vnet.ibm.com>
---
   arch/powerpc/kvm/book3s.c    |  3 ++-
   arch/powerpc/kvm/book3s_hv.c | 23 +++++++++++++++++++----
   2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index c254c27..b40fe5d 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -789,7 +789,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu
*vcpu,
   int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
                       struct kvm_guest_debug *dbg)
   {
-    return -EINVAL;
+    vcpu->guest_debug = dbg->control;
+    return 0;
   }
     void kvmppc_decrementer_func(unsigned long data)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 7a12edb..688421d 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -67,6 +67,14 @@
   /* Used as a "null" value for timebase values */
   #define TB_NIL    (~(u64)0)
   +/*
+ * SW_BRK_DBG_INT is debug Instruction for supporting Software
Breakpoint.
+ * Instruction mnemonic is ABS, primary opcode is 31 and extended
opcode is 360.
+ * Based on PowerISA v2.01, ABS instruction has been dropped from the
architecture
+ * and treated an illegal instruction.
+ */
+#define SW_BRK_DBG_INT 0x7c0002d0
The instruction we use to trap needs to get exposed to user space via a
ONE_REG property.

Yes. I got to know about that from Bharat (patchset "ppc debug: Add
debug stub support"). I will change it.

Also, why don't we use twi always or something else that actually is
defined as illegal instruction? I would like to see this shared with
book3s_32 PR.

+
   static void kvmppc_end_cede(struct kvm_vcpu *vcpu);
   static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
   @@ -721,12 +729,19 @@ static int kvmppc_handle_exit_hv(struct
kvm_run *run, struct kvm_vcpu *vcpu,
           break;
       /*
        * This occurs if the guest executes an illegal instruction.
-     * We just generate a program interrupt to the guest, since
-     * we don't emulate any guest instructions at this stage.
+     * To support software breakpoint, we check for the sw breakpoint
+     * instruction to return back to host, if not we just generate a
+     * program interrupt to the guest.
        */
       case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
-        kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
-        r = RESUME_GUEST;
+        if (vcpu->arch.last_inst == SW_BRK_DBG_INT) {
Don't access last_inst directly. Instead use the provided helpers.

Ok. Will look and replace it.

+            run->exit_reason = KVM_EXIT_DEBUG;
+            run->debug.arch.address = vcpu->arch.pc;
+            r = RESUME_HOST;
+        } else {
+            kvmppc_core_queue_program(vcpu, 0x80000);
magic numbers
^^^^^
I did not understand this?

You're replacing the readable SRR1_PROGILL with the unreadable 0x80000. That's bad.


Alex

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to