Re: [PATCH 2/2 v5] powerpc/kvm: common sw breakpoint instr across ppc
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 ma...@linux.vnet.ibm.com
---
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);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2 v5] powerpc/kvm: common sw breakpoint instr across ppc
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 ma...@linux.vnet.ibm.com
---
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? ;)
Also, please only fetch the last instruction if debugging is active.
+ 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
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.
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);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/2 v5] powerpc/kvm: common sw breakpoint instr across ppc
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 ma...@linux.vnet.ibm.com
---
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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
