[PATCH 1/2 v6] powerpc/kvm: support to handle sw breakpoint

2014-09-09 Thread Madhavan Srinivasan 
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 also adds support for
software breakpoint in PR KVM.

Signed-off-by: Madhavan Srinivasan 
---
 arch/powerpc/include/asm/kvm_ppc.h |  6 ++
 arch/powerpc/kvm/book3s.c  |  3 ++-
 arch/powerpc/kvm/book3s_hv.c   | 41 ++
 arch/powerpc/kvm/book3s_pr.c   |  3 +++
 arch/powerpc/kvm/emulate.c | 15 ++
 5 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_ppc.h 
b/arch/powerpc/include/asm/kvm_ppc.h
index fb86a22..dd83c9a 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -38,6 +38,12 @@
 #include 
 #endif
 
+/*
+ * KVMPPC_INST_SW_BREAKPOINT is debug Instruction
+ * for supporting software breakpoint.
+ */
+#define KVMPPC_INST_SW_BREAKPOINT  0x0000
+
 enum emulation_result {
EMULATE_DONE, /* no further processing */
EMULATE_DO_MMIO,  /* kvm_run filled with MMIO request */
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index dd03f6b..00e9c9f 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -778,7 +778,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 27cced9..000fbec 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -725,6 +725,30 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
return kvmppc_hcall_impl_hv_realmode(cmd);
 }
 
+static int kvmppc_emulate_debug_inst(struct kvm_run *run,
+   struct kvm_vcpu *vcpu)
+{
+   u32 last_inst;
+
+   if (kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
+   EMULATE_DONE) {
+   /*
+* Fetch failed, so return to guest and
+* try executing it again.
+*/
+   return RESUME_GUEST;
+   }
+
+   if (last_inst == KVMPPC_INST_SW_BREAKPOINT) {
+   run->exit_reason = KVM_EXIT_DEBUG;
+   run->debug.arch.address = kvmppc_get_pc(vcpu);
+   return RESUME_HOST;
+   } else {
+   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+   return RESUME_GUEST;
+   }
+}
+
 static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
 struct task_struct *tsk)
 {
@@ -807,12 +831,18 @@ 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.
+* If the guest debug is disabled, generate a program interrupt
+* to the guest. If guest debug is enabled, we need to check
+* whether the instruction is a software breakpoint instruction.
+* Accordingly return to Guest or Host.
 */
case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
-   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
-   r = RESUME_GUEST;
+   if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) {
+   r = kvmppc_emulate_debug_inst(run, vcpu);
+   } else {
+   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+   r = RESUME_GUEST;
+   }
break;
/*
 * This occurs if the guest (kernel or userspace), does something that
@@ -922,6 +952,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 
id,
long int i;
 
switch (id) {
+   case KVM_REG_PPC_DEBUG_INST:
+   *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT);
+   break;
case KVM_REG_PPC_HIOR:
*val = get_reg_val(id, 0);
break;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index faffb27..6d73708 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1319,6 +1319,9 @@ static int kvmppc_get_one_reg_pr(struct kvm_vcpu *vcpu, 
u64 id,
int r = 0;
 
switch (id) {
+   case KVM_REG_PPC_DEBUG_INST:
+   *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT);
+   break;
case KVM_REG_PPC_HIOR:
*val = ge

[PATCH 0/2 v6] powerpc/kvm: support to handle sw breakpoint

2014-09-09 Thread Madhavan Srinivasan 
This patchset adds ppc64 server side support for software breakpoint
and extends the use of illegal instruction as software
breakpoint across ppc platform.

Patch 1, 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 also adds support for software breakpoint
in PR KVM.

Patch 2,extends the use of illegal instruction as software
breakpoint instruction across the ppc platform. Patch extends
booke program interrupt code to support software breakpoint.

Patch 2 is only compile tested. Will really help if
someone can try it out and let me know comments.

Changes v5->v6:
 Fixed checkpatch.pl errors
 Added abort case as a seperate condition block in
  emulation function in book3s_hv
 Added debug active check for instruction emulation
 Modified return value to emulate_fail in else part
  of illegal instruction case in book3s_pr.c
 Removed KVMPPC_INST_EHPRIV_DEBUG instruction
 Moved the debug instruction as a separate block in
   program interrupt code

Changes v4->v5:
 Made changes to code comments and commit messages
 Added debugging active checks for illegal instr comparison
 Added debug instruction check in emulate code
 Extended SW breakpoint to booke

Changes v3->v4:
 Made changes to code comments and removed #define of zero opcode
 Added a new function to handle the debug instruction emulation in book3s_hv
 Rebased the code to latest upstream source.

Changes v2->v3:
 Changed the debug instructions. Using the all zero opcode in the instruction 
word
  as illegal instruction as mentioned in Power ISA instead of ABS
 Removed reg updated in emulation assist and added a call to
  kvmppc_emulate_instruction for reg update.

Changes v1->v2:

 Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also 
share it.
 Added code to use KVM get one reg infrastructure to get debug opcode.
 Updated emulate.c to include emulation of debug instruction incase of PR_KVM.
 Made changes to commit message.

Madhavan Srinivasan (2):
  powerpc/kvm: support to handle sw breakpoint
  powerpc/kvm: common sw breakpoint instr across ppc

 arch/powerpc/include/asm/kvm_booke.h |  2 --
 arch/powerpc/include/asm/kvm_ppc.h   |  6 ++
 arch/powerpc/kvm/book3s.c|  3 ++-
 arch/powerpc/kvm/book3s_hv.c | 41 
 arch/powerpc/kvm/book3s_pr.c |  3 +++
 arch/powerpc/kvm/booke.c | 19 -
 arch/powerpc/kvm/emulate.c   | 15 +
 7 files changed, 81 insertions(+), 8 deletions(-)

-- 
1.7.11.4

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2 v6] powerpc/kvm: common sw breakpoint instr across ppc

2014-09-09 Thread Madhavan Srinivasan 
This patch extends the use of illegal instruction as software
breakpoint instruction across the ppc platform. Patch extends
booke program interrupt code to support software breakpoint.

Signed-off-by: Madhavan Srinivasan 
---
Patch is only compile tested. Will really help if
someone can try it out and let me know the comments.

 arch/powerpc/include/asm/kvm_booke.h |  2 --
 arch/powerpc/kvm/booke.c | 19 ++-
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_booke.h 
b/arch/powerpc/include/asm/kvm_booke.h
index f7aa5cc..ab7123a 100644
--- a/arch/powerpc/include/asm/kvm_booke.h
+++ b/arch/powerpc/include/asm/kvm_booke.h
@@ -30,8 +30,6 @@
 #define EHPRIV_OC_SHIFT11
 /* "ehpriv 1" : ehpriv with OC = 1 is used for debug emulation */
 #define EHPRIV_OC_DEBUG1
-#define KVMPPC_INST_EHPRIV_DEBUG   (KVMPPC_INST_EHPRIV | \
-(EHPRIV_OC_DEBUG << EHPRIV_OC_SHIFT))
 
 static inline void kvmppc_set_gpr(struct kvm_vcpu *vcpu, int num, ulong val)
 {
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index b4c89fa..365e85d 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -870,6 +870,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
case BOOKE_INTERRUPT_HV_PRIV:
emulated = kvmppc_get_last_inst(vcpu, false, &last_inst);
break;
+   case BOOKE_INTERRUPT_PROGRAM:
+   /* SW breakpoints arrive as illegal instructions on HV */
+   if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
+   emulated = kvmppc_get_last_inst(vcpu, false, 
&last_inst);
+   break;
default:
break;
}
@@ -947,6 +952,18 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
break;
 
case BOOKE_INTERRUPT_PROGRAM:
+   if ((vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) &&
+   (last_inst == KVMPPC_INST_SW_BREAKPOINT)) {
+   /*
+* We are here because of an SW breakpoint instr,
+* so lets return to host to handle.
+*/
+   r = kvmppc_handle_debug(run, vcpu);
+   run->exit_reason = KVM_EXIT_DEBUG;
+   kvmppc_account_exit(vcpu, DEBUG_EXITS);
+   break;
+   }
+
if (vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) {
/*
 * Program traps generated by user-level software must
@@ -1505,7 +1522,7 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, 
struct kvm_one_reg *reg)
val = get_reg_val(reg->id, vcpu->arch.tsr);
break;
case KVM_REG_PPC_DEBUG_INST:
-   val = get_reg_val(reg->id, KVMPPC_INST_EHPRIV_DEBUG);
+   val = get_reg_val(reg->id, KVMPPC_INST_SW_BREAKPOINT);
break;
case KVM_REG_PPC_VRSAVE:
val = get_reg_val(reg->id, vcpu->arch.vrsave);
-- 
1.7.11.4

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2 v5] powerpc/kvm: common sw breakpoint instr across ppc

2014-09-09 Thread Madhavan Srinivasan 
On Monday 08 September 2014 06:39 PM, Alexander Graf wrote:
> 
> 
> On 07.09.14 18:31, Madhavan Srinivasan wrote:
>> This patch extends the use of illegal instruction as software
>> breakpoint instruction across the ppc platform. Patch extends
>> booke program interrupt code to support software breakpoint.
>>
>> Signed-off-by: Madhavan Srinivasan 
>> ---
>>
>> Patch is only compile tested. Will really help if
>> someone can try it out and let me know comments.
>>
>>  arch/powerpc/kvm/booke.c | 18 --
>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
>> index b4c89fa..1b84853 100644
>> --- a/arch/powerpc/kvm/booke.c
>> +++ b/arch/powerpc/kvm/booke.c
>> @@ -870,6 +870,10 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
>> kvm_vcpu *vcpu,
>>  case BOOKE_INTERRUPT_HV_PRIV:
>>  emulated = kvmppc_get_last_inst(vcpu, false, &last_inst);
>>  break;
>> +case BOOKE_INTERRUPT_PROGRAM:
>> +/*SW breakpoints arrive as illegal instructions on HV */
> 
> Is it my email client or is there a space missing again? ;)
> 

Facepalm. Will fix it.

> Also, please only fetch the last instruction if debugging is active.
> 

Will change it.

>> +emulated = kvmppc_get_last_inst(vcpu, false, &last_inst);
>> +break;
>>  default:
>>  break;
>>  }
>> @@ -947,7 +951,17 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
>> kvm_vcpu *vcpu,
>>  break;
>>  
>>  case BOOKE_INTERRUPT_PROGRAM:
>> -if (vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) {
>> +if ((vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) &&
>> +(last_inst == KVMPPC_INST_SW_BREAKPOINT)) {
> 
> I think this is changing the logic from "if the guest is in user mode or
> we're in HV, deflect" to "if the guest is in user mode or an HV guest
> and the instruction is a breakpoint, treat it as debug. Otherwise
> deflect". So you're essentially breaking PR KVM here from what I can tell.
> 
> Why don't you just split the whole thing out to the beginning of
> BOOKE_INTERRUPT_PROGRAM and check for
> 
>   a) debug is enabled
>   b) instruction is sw breakpoint
> 
This is what we pretty much do for the server side. Will changes it.

> instead?
> 
>> +/*
>> + * We are here because of an SW breakpoint instr,
>> + * so lets return to host to handle.
>> + */
>> +r = kvmppc_handle_debug(run, vcpu);
>> +run->exit_reason = KVM_EXIT_DEBUG;
>> +kvmppc_account_exit(vcpu, DEBUG_EXITS);
>> +break;
>> +} else {
>>  /*
>>   * Program traps generated by user-level software must
>>   * be handled by the guest kernel.
>> @@ -1505,7 +1519,7 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, 
>> struct kvm_one_reg *reg)
>>  val = get_reg_val(reg->id, vcpu->arch.tsr);
>>  break;
>>  case KVM_REG_PPC_DEBUG_INST:
>> -val = get_reg_val(reg->id, KVMPPC_INST_EHPRIV_DEBUG);
> 
> Please also remove the definition of EHPRIV_DEBUG.
> 
OK. Will do.


Thanks for review
Maddy

> 
> Alex
> 
>> +val = get_reg_val(reg->id, KVMPPC_INST_SW_BREAKPOINT);
>>  break;
>>  case KVM_REG_PPC_VRSAVE:
>>  val = get_reg_val(reg->id, vcpu->arch.vrsave);
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2 v5] powerpc/kvm: support to handle sw breakpoint

2014-09-09 Thread Madhavan Srinivasan 
On Monday 08 September 2014 06:35 PM, Alexander Graf wrote:
> 
> 
> On 07.09.14 18:31, 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 also adds support for
>> software breakpoint in PR KVM.
>>
>> Signed-off-by: Madhavan Srinivasan 
>> ---
>>  arch/powerpc/include/asm/kvm_ppc.h |  6 ++
>>  arch/powerpc/kvm/book3s.c  |  3 ++-
>>  arch/powerpc/kvm/book3s_hv.c   | 41 
>> ++
>>  arch/powerpc/kvm/book3s_pr.c   |  3 +++
>>  arch/powerpc/kvm/emulate.c | 18 +
>>  5 files changed, 66 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h 
>> b/arch/powerpc/include/asm/kvm_ppc.h
>> index fb86a22..dd83c9a 100644
>> --- a/arch/powerpc/include/asm/kvm_ppc.h
>> +++ b/arch/powerpc/include/asm/kvm_ppc.h
>> @@ -38,6 +38,12 @@
>>  #include 
>>  #endif
>>  
>> +/*
>> + * KVMPPC_INST_SW_BREAKPOINT is debug Instruction
>> + * for supporting software breakpoint.
>> + */
>> +#define KVMPPC_INST_SW_BREAKPOINT   0x0000
>> +
>>  enum emulation_result {
>>  EMULATE_DONE, /* no further processing */
>>  EMULATE_DO_MMIO,  /* kvm_run filled with MMIO request */
>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>> index dd03f6b..00e9c9f 100644
>> --- a/arch/powerpc/kvm/book3s.c
>> +++ b/arch/powerpc/kvm/book3s.c
>> @@ -778,7 +778,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 27cced9..3a2414c 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -725,6 +725,30 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
>>  return kvmppc_hcall_impl_hv_realmode(cmd);
>>  }
>>  
>> +static int kvmppc_emulate_debug_inst(struct kvm_run *run,
>> +struct kvm_vcpu *vcpu)
>> +{
>> +u32 last_inst;
>> +
>> +if(kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
>> +EMULATE_DONE) {
>> +/*
>> + * Fetch failed, so return to guest and
>> + * try executing it again.
>> + */
>> +return RESUME_GUEST;
> 
> Please end the if() here. In the kernel we usually treat abort
> situations as separate.
> 

OK. Will change it.

>> +} else {
>> +if (last_inst == KVMPPC_INST_SW_BREAKPOINT) {
>> +run->exit_reason = KVM_EXIT_DEBUG;
>> +run->debug.arch.address = kvmppc_get_pc(vcpu);
>> +return RESUME_HOST;
>> +} else {
>> +kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
>> +return RESUME_GUEST;
>> +}
>> +}
>> +}
>> +
>>  static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
>>   struct task_struct *tsk)
>>  {
>> @@ -807,12 +831,18 @@ 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.
>> + * If the guest debug is disabled, generate a program interrupt
>> + * to the guest. If guest debug is enabled, we need to check
>> + * whether the instruction is a software breakpoint instruction.
>> + * Accordingly return to Guest or Host.
>>   */
>>  case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
>> -kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
>> -r = RESUME_GUEST;
>> +if(vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) {
>> +r = kvmppc_emulate_debug_inst(run, vcp

[PATCH 0/2 v5] powerpc/kvm: support to handle sw breakpoint

2014-09-07 Thread Madhavan Srinivasan 
This patchset adds ppc64 server side support for software breakpoint
and extends the use of illegal instruction as software
breakpoint across ppc platform.

Patch 1, 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 also adds support for software breakpoint
in PR KVM.

Patch 2,extends the use of illegal instruction as software
breakpoint instruction across the ppc platform. Patch extends
booke program interrupt code to support software breakpoint.

Patch 2 is only compile tested. Will really help if
someone can try it out and let me know comments.

Changes v4->v5:
 Made changes to code comments and commit messages
 Added debugging active checks for illegal instr comparison
 Added debug instruction check in emulate code
 Extended SW breakpoint to booke

Changes v3->v4:
 Made changes to code comments and removed #define of zero opcode
 Added a new function to handle the debug instruction emulation in book3s_hv
 Rebased the code to latest upstream source.

Changes v2->v3:
 Changed the debug instructions. Using the all zero opcode in the instruction 
word
  as illegal instruction as mentioned in Power ISA instead of ABS
 Removed reg updated in emulation assist and added a call to
  kvmppc_emulate_instruction for reg update.

Changes v1->v2:

 Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also 
share it.
 Added code to use KVM get one reg infrastructure to get debug opcode.
 Updated emulate.c to include emulation of debug instruction incase of PR_KVM.
 Made changes to commit message.

Madhavan Srinivasan (2):
  powerpc/kvm: support to handle sw breakpoint
  powerpc/kvm: common sw breakpoint instr across ppc

 arch/powerpc/include/asm/kvm_ppc.h |  6 ++
 arch/powerpc/kvm/book3s.c  |  3 ++-
 arch/powerpc/kvm/book3s_hv.c   | 41 ++
 arch/powerpc/kvm/book3s_pr.c   |  3 +++
 arch/powerpc/kvm/booke.c   | 18 +++--
 arch/powerpc/kvm/emulate.c | 18 +
 6 files changed, 82 insertions(+), 7 deletions(-)

-- 
1.7.11.4

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2 v5] powerpc/kvm: support to handle sw breakpoint

2014-09-07 Thread Madhavan Srinivasan 
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 also adds support for
software breakpoint in PR KVM.

Signed-off-by: Madhavan Srinivasan 
---
 arch/powerpc/include/asm/kvm_ppc.h |  6 ++
 arch/powerpc/kvm/book3s.c  |  3 ++-
 arch/powerpc/kvm/book3s_hv.c   | 41 ++
 arch/powerpc/kvm/book3s_pr.c   |  3 +++
 arch/powerpc/kvm/emulate.c | 18 +
 5 files changed, 66 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_ppc.h 
b/arch/powerpc/include/asm/kvm_ppc.h
index fb86a22..dd83c9a 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -38,6 +38,12 @@
 #include 
 #endif
 
+/*
+ * KVMPPC_INST_SW_BREAKPOINT is debug Instruction
+ * for supporting software breakpoint.
+ */
+#define KVMPPC_INST_SW_BREAKPOINT  0x0000
+
 enum emulation_result {
EMULATE_DONE, /* no further processing */
EMULATE_DO_MMIO,  /* kvm_run filled with MMIO request */
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index dd03f6b..00e9c9f 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -778,7 +778,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 27cced9..3a2414c 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -725,6 +725,30 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
return kvmppc_hcall_impl_hv_realmode(cmd);
 }
 
+static int kvmppc_emulate_debug_inst(struct kvm_run *run,
+   struct kvm_vcpu *vcpu)
+{
+   u32 last_inst;
+
+   if(kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
+   EMULATE_DONE) {
+   /*
+* Fetch failed, so return to guest and
+* try executing it again.
+*/
+   return RESUME_GUEST;
+   } else {
+   if (last_inst == KVMPPC_INST_SW_BREAKPOINT) {
+   run->exit_reason = KVM_EXIT_DEBUG;
+   run->debug.arch.address = kvmppc_get_pc(vcpu);
+   return RESUME_HOST;
+   } else {
+   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+   return RESUME_GUEST;
+   }
+   }
+}
+
 static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
 struct task_struct *tsk)
 {
@@ -807,12 +831,18 @@ 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.
+* If the guest debug is disabled, generate a program interrupt
+* to the guest. If guest debug is enabled, we need to check
+* whether the instruction is a software breakpoint instruction.
+* Accordingly return to Guest or Host.
 */
case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
-   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
-   r = RESUME_GUEST;
+   if(vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) {
+   r = kvmppc_emulate_debug_inst(run, vcpu);
+   } else {
+   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+   r = RESUME_GUEST;
+   }
break;
/*
 * This occurs if the guest (kernel or userspace), does something that
@@ -922,6 +952,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 
id,
long int i;
 
switch (id) {
+   case KVM_REG_PPC_DEBUG_INST:
+   *val = get_reg_val(id, KVMPPC_INST_SW_BREAKPOINT);
+   break;
case KVM_REG_PPC_HIOR:
*val = get_reg_val(id, 0);
break;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index faffb27..6d73708 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1319,6 +1319,9 @@ static int kvmppc_get_one_reg_pr(struct kvm_vcpu *vcpu, 
u64 id,
int r = 0;
 
switch (id) {
+   case KVM_REG_PPC_DEBUG_INST:
+   *val = get_reg_val(id, KVMPPC_INST_SW_BREAKP

[PATCH 2/2 v5] powerpc/kvm: common sw breakpoint instr across ppc

2014-09-07 Thread Madhavan Srinivasan 
This patch extends the use of illegal instruction as software
breakpoint instruction across the ppc platform. Patch extends
booke program interrupt code to support software breakpoint.

Signed-off-by: Madhavan Srinivasan 
---

Patch is only compile tested. Will really help if
someone can try it out and let me know comments.

 arch/powerpc/kvm/booke.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index b4c89fa..1b84853 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -870,6 +870,10 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
case BOOKE_INTERRUPT_HV_PRIV:
emulated = kvmppc_get_last_inst(vcpu, false, &last_inst);
break;
+   case BOOKE_INTERRUPT_PROGRAM:
+   /*SW breakpoints arrive as illegal instructions on HV */
+   emulated = kvmppc_get_last_inst(vcpu, false, &last_inst);
+   break;
default:
break;
}
@@ -947,7 +951,17 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
break;
 
case BOOKE_INTERRUPT_PROGRAM:
-   if (vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) {
+   if ((vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) &&
+   (last_inst == KVMPPC_INST_SW_BREAKPOINT)) {
+   /*
+* We are here because of an SW breakpoint instr,
+* so lets return to host to handle.
+*/
+   r = kvmppc_handle_debug(run, vcpu);
+   run->exit_reason = KVM_EXIT_DEBUG;
+   kvmppc_account_exit(vcpu, DEBUG_EXITS);
+   break;
+   } else {
/*
 * Program traps generated by user-level software must
 * be handled by the guest kernel.
@@ -1505,7 +1519,7 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, 
struct kvm_one_reg *reg)
val = get_reg_val(reg->id, vcpu->arch.tsr);
break;
case KVM_REG_PPC_DEBUG_INST:
-   val = get_reg_val(reg->id, KVMPPC_INST_EHPRIV_DEBUG);
+   val = get_reg_val(reg->id, KVMPPC_INST_SW_BREAKPOINT);
break;
case KVM_REG_PPC_VRSAVE:
val = get_reg_val(reg->id, vcpu->arch.vrsave);
-- 
1.7.11.4

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] powerpc/kvm: support to handle sw breakpoint

2014-08-23 Thread Madhavan Srinivasan 
On Thursday 21 August 2014 02:40 PM, Alexander Graf wrote:
> 
> 
> On 20.08.14 07:52, 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 also adds support for
>> software breakpoint in PR KVM.
>>
>> Changes v3->v4:
>>  Made changes to code comments and removed #define of zero opcode
>>  Added a new function to handle the debug instruction emulation in book3s_hv
>>  Rebased the code to latest upstream source.
>>
>> Changes v2->v3:
>>  Changed the debug instructions. Using the all zero opcode in the 
>> instruction word
>>   as illegal instruction as mentioned in Power ISA instead of ABS
>>  Removed reg updated in emulation assist and added a call to
>>   kvmppc_emulate_instruction for reg update.
>>
>> Changes v1->v2:
>>
>>  Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also 
>> share it.
>>  Added code to use KVM get one reg infrastructure to get debug opcode.
>>  Updated emulate.c to include emulation of debug instruction incase of 
>> PR_KVM.
>>  Made changes to commit message.
>>
>> Signed-off-by: Madhavan Srinivasan 
>> ---
>>  arch/powerpc/include/asm/kvm_book3s.h |  7 +++
>>  arch/powerpc/kvm/book3s.c |  3 ++-
>>  arch/powerpc/kvm/book3s_hv.c  | 32 ++--
>>  arch/powerpc/kvm/book3s_pr.c  |  3 +++
>>  arch/powerpc/kvm/emulate.c| 11 +++
>>  5 files changed, 53 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h 
>> b/arch/powerpc/include/asm/kvm_book3s.h
>> index 6acf0c2..a1944f8 100644
>> --- a/arch/powerpc/include/asm/kvm_book3s.h
>> +++ b/arch/powerpc/include/asm/kvm_book3s.h
>> @@ -24,6 +24,13 @@
>>  #include 
>>  #include 
>>  
>> +/*
>> + * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting Software 
>> Breakpoint.
>> + * Based on PowerISA v2.07, Instruction with primary opcode 0 will be 
>> treated as illegal
>> + * instruction.
>> + */
>> +#define KVMPPC_INST_BOOK3S_DEBUG0x0000
> 
> Please change the BookE version of this as well, put the define in a
> common header and use a non book specific name.
> 

I first wanted to get the server side in and then take up this, but i
can do it with this, just concerned incase of booke testing :(

>> +
>>  struct kvmppc_bat {
>>  u64 raw;
>>  u32 bepi;
>> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
>> index dd03f6b..00e9c9f 100644
>> --- a/arch/powerpc/kvm/book3s.c
>> +++ b/arch/powerpc/kvm/book3s.c
>> @@ -778,7 +778,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 27cced9..0a92e45 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -725,6 +725,14 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
>>  return kvmppc_hcall_impl_hv_realmode(cmd);
>>  }
>>  
>> +static int kvmppc_emulate_debug_instruction_hv(struct kvm_run *run,
>> +struct kvm_vcpu *vcpu)
>> +{
>> +run->exit_reason = KVM_EXIT_DEBUG;
>> +run->debug.arch.address = kvmppc_get_pc(vcpu);
>> +return 0;
>> +}
>> +
>>  static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
>>   struct task_struct *tsk)
>>  {
>> @@ -811,9 +819,26 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, 
>> struct kvm_vcpu *vcpu,
>>   * we don't emulate any guest instructions at this stage.
> 
> This comment is no longer true, it should get changed.
> 

Will change it.

>>   */
>>  case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
>> -kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
>> -r = RESUME_GUEST;
>> +{
>> +u32 last_inst;
>> +if(kvmppc_get_last_inst(vcpu, INST_G

[PATCH v4] powerpc/kvm: support to handle sw breakpoint

2014-08-19 Thread Madhavan Srinivasan 
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 also adds support for
software breakpoint in PR KVM.

Changes v3->v4:
 Made changes to code comments and removed #define of zero opcode
 Added a new function to handle the debug instruction emulation in book3s_hv
 Rebased the code to latest upstream source.

Changes v2->v3:
 Changed the debug instructions. Using the all zero opcode in the instruction 
word
  as illegal instruction as mentioned in Power ISA instead of ABS
 Removed reg updated in emulation assist and added a call to
  kvmppc_emulate_instruction for reg update.

Changes v1->v2:

 Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also 
share it.
 Added code to use KVM get one reg infrastructure to get debug opcode.
 Updated emulate.c to include emulation of debug instruction incase of PR_KVM.
 Made changes to commit message.

Signed-off-by: Madhavan Srinivasan 
---
 arch/powerpc/include/asm/kvm_book3s.h |  7 +++
 arch/powerpc/kvm/book3s.c |  3 ++-
 arch/powerpc/kvm/book3s_hv.c  | 32 ++--
 arch/powerpc/kvm/book3s_pr.c  |  3 +++
 arch/powerpc/kvm/emulate.c| 11 +++
 5 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_book3s.h 
b/arch/powerpc/include/asm/kvm_book3s.h
index 6acf0c2..a1944f8 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -24,6 +24,13 @@
 #include 
 #include 
 
+/*
+ * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting Software 
Breakpoint.
+ * Based on PowerISA v2.07, Instruction with primary opcode 0 will be treated 
as illegal
+ * instruction.
+ */
+#define KVMPPC_INST_BOOK3S_DEBUG   0x0000
+
 struct kvmppc_bat {
u64 raw;
u32 bepi;
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index dd03f6b..00e9c9f 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -778,7 +778,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 27cced9..0a92e45 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -725,6 +725,14 @@ static int kvmppc_hcall_impl_hv(unsigned long cmd)
return kvmppc_hcall_impl_hv_realmode(cmd);
 }
 
+static int kvmppc_emulate_debug_instruction_hv(struct kvm_run *run,
+   struct kvm_vcpu *vcpu)
+{
+   run->exit_reason = KVM_EXIT_DEBUG;
+   run->debug.arch.address = kvmppc_get_pc(vcpu);
+   return 0;
+}
+
 static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
 struct task_struct *tsk)
 {
@@ -811,9 +819,26 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, 
struct kvm_vcpu *vcpu,
 * we don't emulate any guest instructions at this stage.
 */
case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
-   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
-   r = RESUME_GUEST;
+   {
+   u32 last_inst;
+   if(kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
+   EMULATE_DONE) {
+   /*
+* Fetch failed, so return to guest and
+* try executing it again.
+*/
+   r = RESUME_GUEST;
+   } else {
+   if (last_inst == KVMPPC_INST_BOOK3S_DEBUG) {
+   kvmppc_emulate_debug_instruction_hv(run, vcpu);
+   r = RESUME_HOST;
+   } else {
+   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+   r = RESUME_GUEST;
+   }
+   }
break;
+   }
/*
 * This occurs if the guest (kernel or userspace), does something that
 * is prohibited by HFSCR.  We just generate a program interrupt to
@@ -922,6 +947,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 
id,
long int i;
 
switch (id) {
+   case KVM_REG_PPC_DEBUG_INST:
+   *val = get_reg_val(id, KVMPPC_INST_BOOK3S_DEBUG);
+   break;
case KVM_REG_PPC_HIOR:
*val = get_reg_val(id, 0);
break;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc

Re: [PATCH v3] powerpc/kvm: support to handle sw breakpoint

2014-08-12 Thread Madhavan Srinivasan 
On Tuesday 12 August 2014 05:45 PM, Alexander Graf wrote:
> 
> On 12.08.14 13:35, Madhavan Srinivasan wrote:
>> On Tuesday 12 August 2014 04:49 PM, Alexander Graf wrote:
>>> On 12.08.14 07:17, Madhavan Srinivasan wrote:
>>>> On Monday 11 August 2014 02:45 PM, Alexander Graf wrote:
>>>>> On 11.08.14 10:51, Benjamin Herrenschmidt wrote:
>>>>>> On Mon, 2014-08-11 at 09:26 +0200, Alexander Graf wrote:
>>>>>>>> diff --git a/arch/powerpc/kvm/emulate.c
>>>>>>>> b/arch/powerpc/kvm/emulate.c
>>>>>>>> index da86d9b..d95014e 100644
>>>>>>>> --- a/arch/powerpc/kvm/emulate.c
>>>>>>>> +++ b/arch/powerpc/kvm/emulate.c
>>>>>>> This should be book3s_emulate.c.
>>>>>> Any reason we can't make that 0000 opcode as breakpoint common to
>>>>>> all powerpc variants ?
>>>>> I can't think of a good reason. We use a hypercall on booke (which
>>>>> traps
>>>>> into an illegal instruction for pr) today, but I don't think it has to
>>>>> be that way.
>>>>>
>>>>> Given that the user space API allows us to change it dynamically,
>>>>> there
>>>>> should be nothing blocking us from going with 0000 always.
>>>>>
>>>> Kindly correct me if i am wrong. So we can still have a common code in
>>>> emulate.c to set the env for both HV and pr incase of illegal
>>>> instruction (i will rebase latest src). But suggestion here to use
>>>> 0000, in that case current path in embed is kvmppc_handle_exit
>>>> (booke.c) -> BOOKE_INTERRUPT_HV_PRIV -> emulation_exit ->
>>>> kvmppc_emulate_instruction, will change to kvmppc_handle_exit (booke.c)
>>>> -> BOOKE_INTERRUPT_PROGRAM -> if debug instr call emulation_exit else
>>>> send to guest?
>>> I can't follow your description above.
>>>
>> My bad.
>>
>>> With the latest git version HV KVM does not include emulate.c anymore.
>>>
>>> Also, it would make a lot of sense of have the same soft breakpoint
>>> instruction across all ppc targets, so it would make sense to change it
>>> to 0x0000 for booke as well.
>>>
>> Got it. Was describing the current control flow with respect to booke
>> and where changes needed (for same software breakpoint inst). This is
>> for my understanding and wanted verify.
>>
>> kvmppc_handle_exit(booke.c)
>> -> BOOKE_INTERRUPT_HV_PRIV
>> -> emulation_exit
>> ->kvmppc_emulate_instruction
>>
>> Incase of using the same software breakpoint instruction (0x0000),
>> then we need to add code in booke something like this
>>
>> kvmppc_handle_exit (booke.c)
>> -> BOOKE_INTERRUPT_PROGRAM
>> ->if debug instr
>> ->emulation_exit
>> else
>> ->send to guest?
> 
> Bleks. I see your point. I guess you need something like this for booke:
> 
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 074b7fc..1fdeee0 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -876,6 +876,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
> kvm_vcpu *vcpu,
>  case BOOKE_INTERRUPT_HV_PRIV:
>  emulated = kvmppc_get_last_inst(vcpu, false, &last_inst);
>  break;
> +case BOOKE_INTERRUPT_PROGRAM:
> +/* SW breakpoints arrive as illegal instructions on HV */
> +if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP)
> +emulated = kvmppc_get_last_inst(vcpu, false, &last_inst);
> +break;
>  default:
>  break;
>  }
> @@ -953,7 +958,8 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
> kvm_vcpu *vcpu,
>  break;
> 
>  case BOOKE_INTERRUPT_PROGRAM:
> -if (vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) {
> +if ((vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) &&
> +(last_inst != KVMPPC_INST_SOFT_BREAKPOINT)) {
>  /*
>   * Program traps generated by user-level software must
>   * be handled by the guest kernel.
> 
> 
> 

Ok make sense.

Regards
Maddy

>>
>>> Basically you would have handling code in emulate.c and book3s_hv.c at
>>> the end of the day.
>>>
>> Yes. Will resend the patch with updated code.
> 
> Thanks,
> 
> 
> Alex
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] powerpc/kvm: support to handle sw breakpoint

2014-08-12 Thread Madhavan Srinivasan 
On Tuesday 12 August 2014 04:49 PM, Alexander Graf wrote:
> 
> On 12.08.14 07:17, Madhavan Srinivasan wrote:
>> On Monday 11 August 2014 02:45 PM, Alexander Graf wrote:
>>> On 11.08.14 10:51, Benjamin Herrenschmidt wrote:
>>>> On Mon, 2014-08-11 at 09:26 +0200, Alexander Graf wrote:
>>>>>> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
>>>>>> index da86d9b..d95014e 100644
>>>>>> --- a/arch/powerpc/kvm/emulate.c
>>>>>> +++ b/arch/powerpc/kvm/emulate.c
>>>>> This should be book3s_emulate.c.
>>>> Any reason we can't make that 0000 opcode as breakpoint common to
>>>> all powerpc variants ?
>>> I can't think of a good reason. We use a hypercall on booke (which traps
>>> into an illegal instruction for pr) today, but I don't think it has to
>>> be that way.
>>>
>>> Given that the user space API allows us to change it dynamically, there
>>> should be nothing blocking us from going with 0000 always.
>>>
>> Kindly correct me if i am wrong. So we can still have a common code in
>> emulate.c to set the env for both HV and pr incase of illegal
>> instruction (i will rebase latest src). But suggestion here to use
>> 0000, in that case current path in embed is kvmppc_handle_exit
>> (booke.c) -> BOOKE_INTERRUPT_HV_PRIV -> emulation_exit ->
>> kvmppc_emulate_instruction, will change to kvmppc_handle_exit (booke.c)
>> -> BOOKE_INTERRUPT_PROGRAM -> if debug instr call emulation_exit else
>> send to guest?
> 
> I can't follow your description above.
> 
My bad.

> With the latest git version HV KVM does not include emulate.c anymore.
> 
> Also, it would make a lot of sense of have the same soft breakpoint
> instruction across all ppc targets, so it would make sense to change it
> to 0x0000 for booke as well.
> 
Got it. Was describing the current control flow with respect to booke
and where changes needed (for same software breakpoint inst). This is
for my understanding and wanted verify.

kvmppc_handle_exit(booke.c)
-> BOOKE_INTERRUPT_HV_PRIV
-> emulation_exit
->kvmppc_emulate_instruction

Incase of using the same software breakpoint instruction (0x0000),
then we need to add code in booke something like this

kvmppc_handle_exit (booke.c)
-> BOOKE_INTERRUPT_PROGRAM
->  if debug instr
->emulation_exit
else
->send to guest?

> Basically you would have handling code in emulate.c and book3s_hv.c at
> the end of the day.
> 
Yes. Will resend the patch with updated code.

> 
> Alex
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] powerpc/kvm: support to handle sw breakpoint

2014-08-11 Thread Madhavan Srinivasan 
On Monday 11 August 2014 02:45 PM, Alexander Graf wrote:
> 
> On 11.08.14 10:51, Benjamin Herrenschmidt wrote:
>> On Mon, 2014-08-11 at 09:26 +0200, Alexander Graf wrote:
 diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
 index da86d9b..d95014e 100644
 --- a/arch/powerpc/kvm/emulate.c
 +++ b/arch/powerpc/kvm/emulate.c
>>> This should be book3s_emulate.c.
>> Any reason we can't make that 0000 opcode as breakpoint common to
>> all powerpc variants ?
> 
> I can't think of a good reason. We use a hypercall on booke (which traps
> into an illegal instruction for pr) today, but I don't think it has to
> be that way.
> 
> Given that the user space API allows us to change it dynamically, there
> should be nothing blocking us from going with 0000 always.
> 
Kindly correct me if i am wrong. So we can still have a common code in
emulate.c to set the env for both HV and pr incase of illegal
instruction (i will rebase latest src). But suggestion here to use
0000, in that case current path in embed is kvmppc_handle_exit
(booke.c) -> BOOKE_INTERRUPT_HV_PRIV -> emulation_exit ->
kvmppc_emulate_instruction, will change to kvmppc_handle_exit (booke.c)
-> BOOKE_INTERRUPT_PROGRAM -> if debug instr call emulation_exit else
send to guest?

Thanks for review
regards
Maddy

> 
> Alex
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] powerpc/kvm: support to handle sw breakpoint

2014-08-10 Thread Madhavan Srinivasan 
On Sunday 03 August 2014 09:21 PM, Segher Boessenkool wrote:
>> +/*
>> + * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting Software 
>> Breakpoint.
>> + * Based on PowerISA v2.07, Instruction with opcode 0s will be treated as 
>> illegal
>> + * instruction.
>> + */
> 
> "primary opcode 0" instead?
> 
ok sure.

>> +#define OP_ZERO 0x0
> 
> Using 0x0 where you mean 0, making a #define for 0 in the first place...
> This all looks rather silly doesn't it.
> 

I wanted to avoid zero mentioned in the case statement, but can add a
comment explaining it.

>> +case OP_ZERO:
>> +if((inst & 0x0000) == KVMPPC_INST_BOOK3S_DEBUG) {
> 
> You either shouldn't mask at all here, or the mask is wrong (the primary
> op is the top six bits, not the top eight).
>
Yes. I guess I dont need to check here. Will resend the patch.

Thanks for review
Regards
Maddy

> 
> Segher
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] powerpc/kvm: support to handle sw breakpoint

2014-07-31 Thread Madhavan Srinivasan 
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 also adds support for
software breakpoint in PR KVM.

Changes v2->v3:
 Changed the debug instructions. Using the all zero opcode in the instruction 
word
  as illegal instruction as mentioned in Power ISA instead of ABS
 Removed reg updated in emulation assist and added a call to
  kvmppc_emulate_instruction for reg update.

Changes v1->v2:

 Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also 
share it.
 Added code to use KVM get one reg infrastructure to get debug opcode.
 Updated emulate.c to include emulation of debug instruction incase of PR_KVM.
 Made changes to commit message.

Signed-off-by: Madhavan Srinivasan 
---
 arch/powerpc/include/asm/kvm_book3s.h |  7 +++
 arch/powerpc/include/asm/ppc-opcode.h |  5 +
 arch/powerpc/kvm/book3s.c |  3 ++-
 arch/powerpc/kvm/book3s_hv.c  | 12 ++--
 arch/powerpc/kvm/book3s_pr.c  |  3 +++
 arch/powerpc/kvm/emulate.c|  9 +
 6 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_book3s.h 
b/arch/powerpc/include/asm/kvm_book3s.h
index f52f656..f17e3fd 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -24,6 +24,13 @@
 #include 
 #include 
 
+/*
+ * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting Software 
Breakpoint.
+ * Based on PowerISA v2.07, Instruction with opcode 0s will be treated as 
illegal
+ * instruction.
+ */
+#define KVMPPC_INST_BOOK3S_DEBUG   0x0000
+
 struct kvmppc_bat {
u64 raw;
u32 bepi;
diff --git a/arch/powerpc/include/asm/ppc-opcode.h 
b/arch/powerpc/include/asm/ppc-opcode.h
index 3132bb9..56739b3 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -111,6 +111,11 @@
 #define OP_31_XOP_LHBRX 790
 #define OP_31_XOP_STHBRX918
 
+/* KVMPPC_INST_BOOK3S_DEBUG -- Software breakpoint Instruction
+ * 0x0000 -- Primary opcode is 0s
+ */
+#define OP_ZERO 0x0
+
 #define OP_LWZ  32
 #define OP_LD   58
 #define OP_LWZU 33
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..7c16f4f 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -725,8 +725,13 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, 
struct kvm_vcpu *vcpu,
 * we don't emulate any guest instructions at this stage.
 */
case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
-   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
-   r = RESUME_GUEST;
+   if (kvmppc_get_last_inst(vcpu) == KVMPPC_INST_BOOK3S_DEBUG) {
+   kvmppc_emulate_instruction(run, vcpu);
+   r = RESUME_HOST;
+   } else {
+   kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
+   r = RESUME_GUEST;
+   }
break;
/*
 * This occurs if the guest (kernel or userspace), does something that
@@ -831,6 +836,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 
id,
long int i;
 
switch (id) {
+   case KVM_REG_PPC_DEBUG_INST:
+   *val = get_reg_val(id, KVMPPC_INST_BOOK3S_DEBUG);
+   break;
case KVM_REG_PPC_HIOR:
*val = get_reg_val(id, 0);
break;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 8eef1e5..27f5234 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1229,6 +1229,9 @@ static int kvmppc_get_one_reg_pr(struct kvm_vcpu *vcpu, 
u64 id,
int r = 0;
 
switch (id) {
+   case KVM_REG_PPC_DEBUG_INST:
+   *val = get_reg_val(id, KVMPPC_INST_BOOK3S_DEBUG);
+   break;
case KVM_REG_PPC_HIOR:
*val = get_reg_val(id, to_book3s(vcpu)->hior);
break;
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index da86d9b..d95014e 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -458,6 +458,15 @@ int kvmppc_emulate_instruction(struct kvm_run *run, struct 
kvm_vcpu *vcpu)
kvm

Re: [PATCH v2] powerpc/kvm: support to handle sw breakpoint

2014-07-04 Thread Madhavan Srinivasan 
On Thursday 03 July 2014 05:21 PM, Alexander Graf wrote:
> 
> On 01.07.14 10:41, 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 also adds support for
>> software breakpoint in PR KVM.
>>
>> Patch mandates use of "abs" instruction as sw breakpoint instruction
>> (primary opcode 31 and extended opcode 360). Based on PowerISA v2.01,
>> ABS instruction has been dropped from the architecture and treated an
>> illegal instruction.
>>
>> Changes v1->v2:
>>
>>   Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM
>> can also share it.
>>   Added code to use KVM get one reg infrastructure to get debug opcode.
>>   Updated emulate.c to include emulation of debug instruction incase
>> of PR_KVM.
>>   Made changes to commit message.
>>
>> Signed-off-by: Madhavan Srinivasan 
>> ---
>>   arch/powerpc/include/asm/kvm_book3s.h |8 
>>   arch/powerpc/include/asm/ppc-opcode.h |5 +
>>   arch/powerpc/kvm/book3s.c |3 ++-
>>   arch/powerpc/kvm/book3s_hv.c  |9 +
>>   arch/powerpc/kvm/book3s_pr.c  |3 +++
>>   arch/powerpc/kvm/emulate.c|   10 ++
>>   6 files changed, 37 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h
>> b/arch/powerpc/include/asm/kvm_book3s.h
>> index f52f656..180d549 100644
>> --- a/arch/powerpc/include/asm/kvm_book3s.h
>> +++ b/arch/powerpc/include/asm/kvm_book3s.h
>> @@ -24,6 +24,14 @@
>>   #include 
>>   #include 
>>   +/*
>> + * KVMPPC_INST_BOOK3S_DEBUG 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 KVMPPC_INST_BOOK3S_DEBUG0x7c0002d0
> 
> This will still break with LE guests.
> 

I am told to try with all 0s opcode. So rewriting the patch.

>> +
>>   struct kvmppc_bat {
>>   u64 raw;
>>   u32 bepi;
>> diff --git a/arch/powerpc/include/asm/ppc-opcode.h
>> b/arch/powerpc/include/asm/ppc-opcode.h
>> index 3132bb9..3fbb4c1 100644
>> --- a/arch/powerpc/include/asm/ppc-opcode.h
>> +++ b/arch/powerpc/include/asm/ppc-opcode.h
>> @@ -111,6 +111,11 @@
>>   #define OP_31_XOP_LHBRX 790
>>   #define OP_31_XOP_STHBRX918
>>   +/* KVMPPC_INST_BOOK3S_DEBUG -- Software breakpoint Instruction
>> + * Instruction mnemonic is ABS, primary opcode is 31 and extended
>> opcode is 360.
>> + */
>> +#define OP_31_XOP_ABS360
>> +
>>   #define OP_LWZ  32
>>   #define OP_LD   58
>>   #define OP_LWZU 33
>> 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..402c1ec 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -725,8 +725,14 @@ static int kvmppc_handle_exit_hv(struct kvm_run
>> *run, struct kvm_vcpu *vcpu,
>>* we don't emulate any guest instructions at this stage.
>>*/
>>   case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
>> +if (kvmppc_get_last_inst(vcpu) == KVMPPC_INST_BOOK3S_DEBUG ) {
>> +run->exit_reason = KVM_EXIT_DEBUG;
>> +run->debug.arch.address = kvmppc_get_pc(vcpu);
>> +r = RESUME_HOST;
> 
> Phew - why can't we just go into the normal instruction emulator for
> EMUL_ASSIST?
> 

IIUC, using the emulation_assist_interrupt function (kernel/trap.c) ?

Thanks for review
Regards
Maddy

> 
> Alex
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] powerpc/kvm: support to handle sw breakpoint

2014-07-04 Thread Madhavan Srinivasan 
On Friday 04 July 2014 12:18 PM, Alexander Graf wrote:
> 
> On 04.07.14 06:34, Madhavan Srinivasan wrote:
>> On Thursday 03 July 2014 05:21 PM, Alexander Graf wrote:
>>> On 01.07.14 10:41, 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 also adds support for
>>>> software breakpoint in PR KVM.
>>>>
>>>> Patch mandates use of "abs" instruction as sw breakpoint instruction
>>>> (primary opcode 31 and extended opcode 360). Based on PowerISA v2.01,
>>>> ABS instruction has been dropped from the architecture and treated an
>>>> illegal instruction.
>>>>
>>>> Changes v1->v2:
>>>>
>>>>Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM
>>>> can also share it.
>>>>Added code to use KVM get one reg infrastructure to get debug
>>>> opcode.
>>>>Updated emulate.c to include emulation of debug instruction incase
>>>> of PR_KVM.
>>>>Made changes to commit message.
>>>>
>>>> Signed-off-by: Madhavan Srinivasan 
>>>> ---
>>>>arch/powerpc/include/asm/kvm_book3s.h |8 
>>>>arch/powerpc/include/asm/ppc-opcode.h |5 +
>>>>arch/powerpc/kvm/book3s.c |3 ++-
>>>>arch/powerpc/kvm/book3s_hv.c  |9 +
>>>>arch/powerpc/kvm/book3s_pr.c  |3 +++
>>>>arch/powerpc/kvm/emulate.c|   10 ++
>>>>6 files changed, 37 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/kvm_book3s.h
>>>> b/arch/powerpc/include/asm/kvm_book3s.h
>>>> index f52f656..180d549 100644
>>>> --- a/arch/powerpc/include/asm/kvm_book3s.h
>>>> +++ b/arch/powerpc/include/asm/kvm_book3s.h
>>>> @@ -24,6 +24,14 @@
>>>>#include 
>>>>#include 
>>>>+/*
>>>> + * KVMPPC_INST_BOOK3S_DEBUG 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 KVMPPC_INST_BOOK3S_DEBUG0x7c0002d0
>>> This will still break with LE guests.
>>>
>> I am told to try with all 0s opcode. So rewriting the patch.
> 
> The problem with "all 0s" is that it's reasonably likely to occur on
> real world code. Hence Segher was proposing something like 0x0000
> which should be the same regardless of endianness, but has a certain
> appeal of intentional placement ;).
> 

Ok Sure.

>>
>>>> +
>>>>struct kvmppc_bat {
>>>>u64 raw;
>>>>u32 bepi;
>>>> diff --git a/arch/powerpc/include/asm/ppc-opcode.h
>>>> b/arch/powerpc/include/asm/ppc-opcode.h
>>>> index 3132bb9..3fbb4c1 100644
>>>> --- a/arch/powerpc/include/asm/ppc-opcode.h
>>>> +++ b/arch/powerpc/include/asm/ppc-opcode.h
>>>> @@ -111,6 +111,11 @@
>>>>#define OP_31_XOP_LHBRX 790
>>>>#define OP_31_XOP_STHBRX918
>>>>+/* KVMPPC_INST_BOOK3S_DEBUG -- Software breakpoint Instruction
>>>> + * Instruction mnemonic is ABS, primary opcode is 31 and extended
>>>> opcode is 360.
>>>> + */
>>>> +#define OP_31_XOP_ABS360
>>>> +
>>>>#define OP_LWZ  32
>>>>#define OP_LD   58
>>>>#define OP_LWZU 33
>>>> 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;

[PATCH v2] powerpc/kvm: support to handle sw breakpoint

2014-07-01 Thread Madhavan Srinivasan 
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 also adds support for
software breakpoint in PR KVM.

Patch mandates use of "abs" instruction as sw breakpoint instruction
(primary opcode 31 and extended opcode 360). Based on PowerISA v2.01,
ABS instruction has been dropped from the architecture and treated an
illegal instruction.

Changes v1->v2:

 Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also 
share it.
 Added code to use KVM get one reg infrastructure to get debug opcode.
 Updated emulate.c to include emulation of debug instruction incase of PR_KVM.
 Made changes to commit message.

Signed-off-by: Madhavan Srinivasan 
---
 arch/powerpc/include/asm/kvm_book3s.h |8 
 arch/powerpc/include/asm/ppc-opcode.h |5 +
 arch/powerpc/kvm/book3s.c |3 ++-
 arch/powerpc/kvm/book3s_hv.c  |9 +
 arch/powerpc/kvm/book3s_pr.c  |3 +++
 arch/powerpc/kvm/emulate.c|   10 ++
 6 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/kvm_book3s.h 
b/arch/powerpc/include/asm/kvm_book3s.h
index f52f656..180d549 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -24,6 +24,14 @@
 #include 
 #include 
 
+/*
+ * KVMPPC_INST_BOOK3S_DEBUG 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 KVMPPC_INST_BOOK3S_DEBUG   0x7c0002d0
+
 struct kvmppc_bat {
u64 raw;
u32 bepi;
diff --git a/arch/powerpc/include/asm/ppc-opcode.h 
b/arch/powerpc/include/asm/ppc-opcode.h
index 3132bb9..3fbb4c1 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -111,6 +111,11 @@
 #define OP_31_XOP_LHBRX 790
 #define OP_31_XOP_STHBRX918
 
+/* KVMPPC_INST_BOOK3S_DEBUG -- Software breakpoint Instruction
+ * Instruction mnemonic is ABS, primary opcode is 31 and extended opcode is 
360.
+ */
+#define OP_31_XOP_ABS  360
+
 #define OP_LWZ  32
 #define OP_LD   58
 #define OP_LWZU 33
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..402c1ec 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -725,8 +725,14 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, 
struct kvm_vcpu *vcpu,
 * we don't emulate any guest instructions at this stage.
 */
case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
+   if (kvmppc_get_last_inst(vcpu) == KVMPPC_INST_BOOK3S_DEBUG ) {
+   run->exit_reason = KVM_EXIT_DEBUG;
+   run->debug.arch.address = kvmppc_get_pc(vcpu);
+   r = RESUME_HOST;
+   } else {
kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
r = RESUME_GUEST;
+   }
break;
/*
 * This occurs if the guest (kernel or userspace), does something that
@@ -831,6 +837,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 
id,
long int i;
 
switch (id) {
+   case KVM_REG_PPC_DEBUG_INST:
+   *val = get_reg_val(id, KVMPPC_INST_BOOK3S_DEBUG);
+   break;
case KVM_REG_PPC_HIOR:
*val = get_reg_val(id, 0);
break;
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 8eef1e5..27f5234 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1229,6 +1229,9 @@ static int kvmppc_get_one_reg_pr(struct kvm_vcpu *vcpu, 
u64 id,
int r = 0;
 
switch (id) {
+   case KVM_REG_PPC_DEBUG_INST:
+   *val = get_reg_val(id, KVMPPC_INST_BOOK3S_DEBUG);
+   break;
case KVM_REG_PPC_HIOR:
*val = get_reg_val(id, to_book3s(vcpu)->hior);
break;
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index da86d9b..13fba51 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -363,6 +363,16 @@ int kvmppc_emulate_instruction(struct kvm

Re: [PATCH] powerpc/kvm: support to handle sw breakpoint

2014-06-17 Thread Madhavan Srinivasan 
On Tuesday 17 June 2014 04:38 PM, Alexander Graf wrote:
> 
> 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 
>>>> ---
>>>>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, 0x8);
>>> magic numbers
>> ^
>> I did not understand this?
> 
> You're replacing the readable SRR1_PROGILL with the unreadable 0x8.
> That's bad.
> 

Oops. My bad. Will undo that. I guess I messed up when was re basing it.

> 
> Alex
> 
Thanks for review
Regards
Maddy

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] powerpc/kvm: support to handle sw breakpoint

2014-06-17 Thread Madhavan Srinivasan 
On Tuesday 17 June 2014 03:13 PM, Alexander Graf wrote:
> 
> On 17.06.14 11:32, Benjamin Herrenschmidt wrote:
>> On Tue, 2014-06-17 at 11:25 +0200, Alexander Graf wrote:
>>> On 17.06.14 11:22, Benjamin Herrenschmidt wrote:
 On Tue, 2014-06-17 at 10:54 +0200, Alexander Graf wrote:
> 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.
 twi will be directed to the guest on HV no ? We want a real illegal
 because those go to the host (for potential emulation by the HV).
>>> Ah, good point. I guess we need different one for PR and HV then to
>>> ensure compatibility with older ISAs on PR.
>> Well, we also need to be careful with what happens if a PR guest puts
>> that instruction in, do that stop its HV guest/host ?
>>
>> What if it's done in userspace ? Do that stop the kernel ? :-)
> 
> The way SW breakpointing is handled is that when we see one, it gets
> deflected into user space. User space then has an array of breakpoints
> it configured itself. If the breakpoint is part of that list, it
> consumes it. If not, it injects a debug interrupt (program in this case)
> into the guest.
> 
> That way we can overlay that one instruction with as many layers as we
> like :). We only get a performance hit on execution of that instruction.
> 
>> Maddy, I haven't checked, does your patch ensure that we only ever stop
>> if the instruction is at a recorded bkpt address ? It still means that a
>> userspace process can practically DOS its kernel by issuing a lot of
>> these causing a crapload of exits.
> 
> Only user space knows about its breakpoint addresses, so we have to
> deflect. However since time still ticks on, we only increase jitter of
> the guest. The process would still get scheduled away after the same
 ^^^ Where is this taken care. I am still trying to understand. Kindly
can you explain or point to the code. Will help.

> amount of real time, no?
> 
> 
> Alex
> 
Thanks for review.
Regards
Maddy



--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] powerpc/kvm: support to handle sw breakpoint

2014-06-17 Thread Madhavan Srinivasan 
On Tuesday 17 June 2014 03:02 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2014-06-17 at 11:25 +0200, Alexander Graf wrote:
>> On 17.06.14 11:22, Benjamin Herrenschmidt wrote:
>>> On Tue, 2014-06-17 at 10:54 +0200, Alexander Graf wrote:
 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.
>>> twi will be directed to the guest on HV no ? We want a real illegal
>>> because those go to the host (for potential emulation by the HV).
>>
>> Ah, good point. I guess we need different one for PR and HV then to 
>> ensure compatibility with older ISAs on PR.
> 
> Well, we also need to be careful with what happens if a PR guest puts
> that instruction in, do that stop its HV guest/host ?
> 

Damn, my mail client is messed up. did not see the mail till now.


I havent tried this incase of PR guest kernel. I will need to try this
before commenting.

> What if it's done in userspace ? Do that stop the kernel ? :-)
> 

Basically flow is that, when we see this instruction, we return to host,
and host checks for address in the SW array and if not it returns to kernel.

> Maddy, I haven't checked, does your patch ensure that we only ever stop
> if the instruction is at a recorded bkpt address ? It still means that a
> userspace process can practically DOS its kernel by issuing a lot of
> these causing a crapload of exits.
> 
This is valid, userspace can create a mess, need to handle this, meaning
incase if we dont find a valid SW breakpoint for this address in the
HOST, we need to route it to guest and kill it at app.

Regards
Maddy

> Cheers,
> Ben.
> 
>> Alex
>>
>>> I'm
>>> trying to see if I can get the architect to set one in stone in a future
>>> proof way.
>>>
>>> Cheers,
>>> Ben.
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] powerpc/kvm: support to handle sw breakpoint

2014-06-17 Thread Madhavan Srinivasan 
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 
>> ---
>>   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, 0x8);
> 
> magic numbers
^
I did not understand this?

>> +r = RESUME_GUEST;
>> +}
>>   break;
>>   /*
>>* This occurs if the guest (kernel or userspace), does
>> something that
> 
> Please enable PR KVM as well while you're at it.
> 
My bad, I did not try the PR KVM. I will try it out.

> 
> Alex
> 
Thanks for review
Regards
Maddy

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] powerpc/kvm: support to handle sw breakpoint

2014-06-14 Thread Madhavan Srinivasan 
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 
---
 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
+
 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) {
+   run->exit_reason = KVM_EXIT_DEBUG;
+   run->debug.arch.address = vcpu->arch.pc;
+   r = RESUME_HOST;
+   } else {
+   kvmppc_core_queue_program(vcpu, 0x8);
+   r = RESUME_GUEST;
+   }
break;
/*
 * This occurs if the guest (kernel or userspace), does something that
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html