* [PATCH 1/5 v2] KVM: PPC: BOOKE: allow debug interrupt at "debug level"
2014-08-04 8:02 [PATCH 0/5 v2] Guest debug emulation Bharat Bhushan
@ 2014-08-04 8:02 ` Bharat Bhushan
2014-08-04 8:02 ` [PATCH 2/5 v2] KVM: PPC: BOOKE : Emulate rfdi instruction Bharat Bhushan
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Bharat Bhushan @ 2014-08-04 8:02 UTC (permalink / raw)
To: agraf, kvm-ppc; +Cc: kvm, scottwood, stuart.yoder, Bharat Bhushan
Debug interrupt can be either "critical level" or "debug level".
There are separate set of save/restore registers used for different level.
Example: DSRR0/DSRR1 are used for "debug level" and CSRR0/CSRR1
are used for critical level debug interrupt.
Using CPU_FTR_DEBUG_LVL_EXC to decide which interrupt level to be used.
Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v1->v2
- No change
arch/powerpc/kvm/booke.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index b4c89fa..322da7d 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -377,7 +377,11 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
allowed = vcpu->arch.shared->msr & MSR_DE;
allowed = allowed && !crit;
msr_mask = MSR_ME;
- int_class = INT_CLASS_CRIT;
+ if (cpu_has_feature(CPU_FTR_DEBUG_LVL_EXC))
+ int_class = INT_CLASS_DBG;
+ else
+ int_class = INT_CLASS_CRIT;
+
break;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/5 v2] KVM: PPC: BOOKE : Emulate rfdi instruction
2014-08-04 8:02 [PATCH 0/5 v2] Guest debug emulation Bharat Bhushan
2014-08-04 8:02 ` [PATCH 1/5 v2] KVM: PPC: BOOKE: allow debug interrupt at "debug level" Bharat Bhushan
@ 2014-08-04 8:02 ` Bharat Bhushan
2014-08-04 8:02 ` [PATCH 3/5 v2] KVM: PPC: BOOKE: Allow guest to change MSR_DE Bharat Bhushan
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Bharat Bhushan @ 2014-08-04 8:02 UTC (permalink / raw)
To: agraf, kvm-ppc; +Cc: kvm, scottwood, stuart.yoder, Bharat Bhushan
This patch adds "rfdi" instruction emulation which is required for
guest debug hander on BOOKE-HV
Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v1->v2
-msr update based on guest_debug done under _set_msr(); so removed same check here
arch/powerpc/include/asm/kvm_host.h | 1 +
arch/powerpc/kvm/booke_emulate.c | 13 +++++++++++++
2 files changed, 14 insertions(+)
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 98d9dd5..636b230 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -144,6 +144,7 @@ enum kvm_exit_types {
EMULATED_TLBWE_EXITS,
EMULATED_RFI_EXITS,
EMULATED_RFCI_EXITS,
+ EMULATED_RFDI_EXITS,
DEC_EXITS,
EXT_INTR_EXITS,
HALT_WAKEUP,
diff --git a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c
index 28c1588..4b9a079 100644
--- a/arch/powerpc/kvm/booke_emulate.c
+++ b/arch/powerpc/kvm/booke_emulate.c
@@ -25,6 +25,7 @@
#define OP_19_XOP_RFI 50
#define OP_19_XOP_RFCI 51
+#define OP_19_XOP_RFDI 39
#define OP_31_XOP_MFMSR 83
#define OP_31_XOP_WRTEE 131
@@ -37,6 +38,12 @@ static void kvmppc_emul_rfi(struct kvm_vcpu *vcpu)
kvmppc_set_msr(vcpu, vcpu->arch.shared->srr1);
}
+static void kvmppc_emul_rfdi(struct kvm_vcpu *vcpu)
+{
+ vcpu->arch.pc = vcpu->arch.dsrr0;
+ kvmppc_set_msr(vcpu, vcpu->arch.dsrr1);
+}
+
static void kvmppc_emul_rfci(struct kvm_vcpu *vcpu)
{
vcpu->arch.pc = vcpu->arch.csrr0;
@@ -65,6 +72,12 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
*advance = 0;
break;
+ case OP_19_XOP_RFDI:
+ kvmppc_emul_rfdi(vcpu);
+ kvmppc_set_exit_type(vcpu, EMULATED_RFDI_EXITS);
+ *advance = 0;
+ break;
+
default:
emulated = EMULATE_FAIL;
break;
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/5 v2] KVM: PPC: BOOKE: Allow guest to change MSR_DE
2014-08-04 8:02 [PATCH 0/5 v2] Guest debug emulation Bharat Bhushan
2014-08-04 8:02 ` [PATCH 1/5 v2] KVM: PPC: BOOKE: allow debug interrupt at "debug level" Bharat Bhushan
2014-08-04 8:02 ` [PATCH 2/5 v2] KVM: PPC: BOOKE : Emulate rfdi instruction Bharat Bhushan
@ 2014-08-04 8:02 ` Bharat Bhushan
2014-08-04 8:02 ` [PATCH 4/5 v2] KVM: PPC: BOOKE: Clear guest dbsr in userspace exit KVM_EXIT_DEBUG Bharat Bhushan
2014-08-04 8:02 ` [PATCH 5/5 v2] KVM: PPC: BOOKE: Emulate debug registers and exception Bharat Bhushan
4 siblings, 0 replies; 9+ messages in thread
From: Bharat Bhushan @ 2014-08-04 8:02 UTC (permalink / raw)
To: agraf, kvm-ppc; +Cc: kvm, scottwood, stuart.yoder, Bharat Bhushan
This patch changes the default behavior of MSRP_DEP, that is
guest is not allowed to change the MSR_DE, to guest can change
MSR_DE. When userspace is debugging guest then it override the
default behavior and set MSRP_DEP. This stops guest to change
MSR_DE when userspace is debugging guest.
Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v1->v2
- No Change
arch/powerpc/kvm/e500mc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index 164bad2..000cf82 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -194,7 +194,7 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu)
#ifdef CONFIG_64BIT
vcpu->arch.shadow_epcr |= SPRN_EPCR_ICM;
#endif
- vcpu->arch.shadow_msrp = MSRP_UCLEP | MSRP_DEP | MSRP_PMMP;
+ vcpu->arch.shadow_msrp = MSRP_UCLEP | MSRP_PMMP;
vcpu->arch.eplc = EPC_EGS | (vcpu->kvm->arch.lpid << EPC_ELPID_SHIFT);
vcpu->arch.epsc = vcpu->arch.eplc;
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/5 v2] KVM: PPC: BOOKE: Clear guest dbsr in userspace exit KVM_EXIT_DEBUG
2014-08-04 8:02 [PATCH 0/5 v2] Guest debug emulation Bharat Bhushan
` (2 preceding siblings ...)
2014-08-04 8:02 ` [PATCH 3/5 v2] KVM: PPC: BOOKE: Allow guest to change MSR_DE Bharat Bhushan
@ 2014-08-04 8:02 ` Bharat Bhushan
2014-08-04 8:02 ` [PATCH 5/5 v2] KVM: PPC: BOOKE: Emulate debug registers and exception Bharat Bhushan
4 siblings, 0 replies; 9+ messages in thread
From: Bharat Bhushan @ 2014-08-04 8:02 UTC (permalink / raw)
To: agraf, kvm-ppc; +Cc: kvm, scottwood, stuart.yoder, Bharat Bhushan
Dbsr is not visible to userspace and we do not think any need to
expose this to userspace because:
Userspace cannot inject debug interrupt to guest (as this
does not know guest ability to handle debug interrupt), so
userspace will always clear DBSR.
Now if userspace has to always clear DBSR in KVM_EXIT_DEBUG
handling then clearing dbsr in kernel looks simple as this
avoid doing SET_SREGS/set_one_reg() to clear DBSR
Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v1->v2
- New patch (separated from guest debug emulation
- Added description
arch/powerpc/kvm/booke.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 322da7d..5c2e26a 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -735,6 +735,17 @@ static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu *vcpu)
struct debug_reg *dbg_reg = &(vcpu->arch.shadow_dbg_reg);
u32 dbsr = vcpu->arch.dbsr;
+ /* Clear guest dbsr (vcpu->arch.dbsr).
+ * dbsr is not visible to userspace and we do not think any
+ * need to expose this to userspace because:
+ * Userspace cannot inject debug interrupt to guest (as this does
+ * not know guest ability to handle debug interrupt), so userspace
+ * will always clear DBSR.
+ * Now if userspace has to always clear DBSR in KVM_EXIT_DEBUG
+ * handling then clearing here looks simple as this
+ * avoid doing SET_SREGS/set_one_reg() to clear DBSR
+ */
+ vcpu->arch.dbsr = 0;
run->debug.arch.status = 0;
run->debug.arch.address = vcpu->arch.pc;
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/5 v2] KVM: PPC: BOOKE: Emulate debug registers and exception
2014-08-04 8:02 [PATCH 0/5 v2] Guest debug emulation Bharat Bhushan
` (3 preceding siblings ...)
2014-08-04 8:02 ` [PATCH 4/5 v2] KVM: PPC: BOOKE: Clear guest dbsr in userspace exit KVM_EXIT_DEBUG Bharat Bhushan
@ 2014-08-04 8:02 ` Bharat Bhushan
2014-08-04 22:53 ` Scott Wood
4 siblings, 1 reply; 9+ messages in thread
From: Bharat Bhushan @ 2014-08-04 8:02 UTC (permalink / raw)
To: agraf, kvm-ppc; +Cc: kvm, scottwood, stuart.yoder, Bharat Bhushan
This patch emulates debug registers and debug exception
to support guest using debug resource. This enables running
gdb/kgdb etc in guest.
On BOOKE architecture we cannot share debug resources between QEMU and
guest because:
When QEMU is using debug resources then debug exception must
be always enabled. 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).
So above mentioned two configuration cannot be supported
at the same time. So the result is that we cannot share
debug resources between QEMU and Guest on BOOKE architecture.
In the current design QEMU gets priority over guest, this means that if
QEMU is using debug resources then guest cannot use them and if guest is
using debug resource then QEMU can overwrite them.
Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v1->v2
- Clear DBCR0_EDM when userspace release debug resource
- change order of debug register and update in h/w register
- no debug interrupt on MRR/IDE debug event
arch/powerpc/include/asm/kvm_ppc.h | 3 +
arch/powerpc/include/asm/reg_booke.h | 2 +
arch/powerpc/kvm/booke.c | 35 +++++++-
arch/powerpc/kvm/booke_emulate.c | 157 +++++++++++++++++++++++++++++++++++
4 files changed, 196 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index fb86a22..05e58b6 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -206,6 +206,9 @@ extern int kvmppc_xics_get_xive(struct kvm *kvm, u32 irq, u32 *server,
extern int kvmppc_xics_int_on(struct kvm *kvm, u32 irq);
extern int kvmppc_xics_int_off(struct kvm *kvm, u32 irq);
+void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu);
+void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu);
+
union kvmppc_one_reg {
u32 wval;
u64 dval;
diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
index 464f108..150d485 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -307,6 +307,8 @@
* DBSR bits which have conflicting definitions on true Book E versus IBM 40x.
*/
#ifdef CONFIG_BOOKE
+#define DBSR_IDE 0x80000000 /* Imprecise Debug Event */
+#define DBSR_MRR 0x30000000 /* Most Recent Reset */
#define DBSR_IC 0x08000000 /* Instruction Completion */
#define DBSR_BT 0x04000000 /* Branch Taken */
#define DBSR_IRPT 0x02000000 /* Exception Debug Event */
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 5c2e26a..bd7d93f 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -267,6 +267,16 @@ static void kvmppc_core_dequeue_watchdog(struct kvm_vcpu *vcpu)
clear_bit(BOOKE_IRQPRIO_WATCHDOG, &vcpu->arch.pending_exceptions);
}
+void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu)
+{
+ kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG);
+}
+
+void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu)
+{
+ clear_bit(BOOKE_IRQPRIO_DEBUG, &vcpu->arch.pending_exceptions);
+}
+
static void set_guest_srr(struct kvm_vcpu *vcpu, unsigned long srr0, u32 srr1)
{
kvmppc_set_srr0(vcpu, srr0);
@@ -735,7 +745,27 @@ static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu *vcpu)
struct debug_reg *dbg_reg = &(vcpu->arch.shadow_dbg_reg);
u32 dbsr = vcpu->arch.dbsr;
- /* Clear guest dbsr (vcpu->arch.dbsr).
+ if (vcpu->guest_debug == 0) {
+ /*
+ * Debug resources belong to Guest.
+ * Imprecise debug event are not injected
+ */
+ if (dbsr & DBSR_IDE)
+ return RESUME_GUEST;
+
+ if (dbsr && (vcpu->arch.shared->msr & MSR_DE) &&
+ (vcpu->arch.dbg_reg.dbcr0 & DBCR0_IDM))
+ kvmppc_core_queue_debug(vcpu);
+
+ /* Inject a program interrupt if trap debug is not allowed */
+ if ((dbsr & DBSR_TIE) && !(vcpu->arch.shared->msr & MSR_DE))
+ kvmppc_core_queue_program(vcpu, ESR_PTR);
+
+ return RESUME_GUEST;
+ }
+
+ /* Debug resource owned by userspace.
+ * Clear guest dbsr (vcpu->arch.dbsr).
* dbsr is not visible to userspace and we do not think any
* need to expose this to userspace because:
* Userspace cannot inject debug interrupt to guest (as this does
@@ -828,6 +858,8 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
case BOOKE_INTERRUPT_DEBUG:
/* Save DBSR before preemption is enabled */
vcpu->arch.dbsr = mfspr(SPRN_DBSR);
+ /* MASK out DBSR_MRR */
+ vcpu->arch.dbsr &= ~DBSR_MRR;
kvmppc_clear_dbsr();
break;
}
@@ -1858,6 +1890,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
if (!(dbg->control & KVM_GUESTDBG_ENABLE)) {
vcpu->arch.shadow_dbg_reg.dbcr0 = 0;
+ vcpu->arch.dbg_reg.dbcr0 = 0;
vcpu->guest_debug = 0;
kvm_guest_protect_msr(vcpu, MSR_DE, false);
return 0;
diff --git a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c
index 4b9a079..c605897 100644
--- a/arch/powerpc/kvm/booke_emulate.c
+++ b/arch/powerpc/kvm/booke_emulate.c
@@ -131,6 +131,7 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
{
int emulated = EMULATE_DONE;
+ bool debug_inst = false;
switch (sprn) {
case SPRN_DEAR:
@@ -145,14 +146,137 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
case SPRN_CSRR1:
vcpu->arch.csrr1 = spr_val;
break;
+ case SPRN_DSRR0:
+ vcpu->arch.dsrr0 = spr_val;
+ break;
+ case SPRN_DSRR1:
+ vcpu->arch.dsrr1 = spr_val;
+ break;
+ case SPRN_IAC1:
+ /*
+ * If userspace is debugging guest then guest
+ * can not access debug registers.
+ */
+ if (vcpu->guest_debug)
+ break;
+
+ debug_inst = true;
+ vcpu->arch.dbg_reg.iac1 = spr_val;
+ vcpu->arch.shadow_dbg_reg.iac1 = spr_val;
+ break;
+ case SPRN_IAC2:
+ /*
+ * If userspace is debugging guest then guest
+ * can not access debug registers.
+ */
+ if (vcpu->guest_debug)
+ break;
+
+ debug_inst = true;
+ vcpu->arch.dbg_reg.iac2 = spr_val;
+ vcpu->arch.shadow_dbg_reg.iac2 = spr_val;
+ break;
+#if CONFIG_PPC_ADV_DEBUG_IACS > 2
+ case SPRN_IAC3:
+ /*
+ * If userspace is debugging guest then guest
+ * can not access debug registers.
+ */
+ if (vcpu->guest_debug)
+ break;
+
+ debug_inst = true;
+ vcpu->arch.dbg_reg.iac3 = spr_val;
+ vcpu->arch.shadow_dbg_reg.iac3 = spr_val;
+ break;
+ case SPRN_IAC4:
+ /*
+ * If userspace is debugging guest then guest
+ * can not access debug registers.
+ */
+ if (vcpu->guest_debug)
+ break;
+
+ debug_inst = true;
+ vcpu->arch.dbg_reg.iac4 = spr_val;
+ vcpu->arch.shadow_dbg_reg.iac4 = spr_val;
+ break;
+#endif
+ case SPRN_DAC1:
+ /*
+ * If userspace is debugging guest then guest
+ * can not access debug registers.
+ */
+ if (vcpu->guest_debug)
+ break;
+
+ debug_inst = true;
+ vcpu->arch.dbg_reg.dac1 = spr_val;
+ vcpu->arch.shadow_dbg_reg.dac1 = spr_val;
+ break;
+ case SPRN_DAC2:
+ /*
+ * If userspace is debugging guest then guest
+ * can not access debug registers.
+ */
+ if (vcpu->guest_debug)
+ break;
+
+ debug_inst = true;
+ vcpu->arch.dbg_reg.dac2 = spr_val;
+ vcpu->arch.shadow_dbg_reg.dac2 = spr_val;
+ break;
case SPRN_DBCR0:
+ /*
+ * If userspace is debugging guest then guest
+ * can not access debug registers.
+ */
+ if (vcpu->guest_debug)
+ break;
+
+ debug_inst = true;
+ spr_val &= (DBCR0_IDM | DBCR0_IC | DBCR0_BT | DBCR0_TIE |
+ DBCR0_IAC1 | DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4 |
+ DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W);
+
vcpu->arch.dbg_reg.dbcr0 = spr_val;
+ vcpu->arch.shadow_dbg_reg.dbcr0 = spr_val;
break;
case SPRN_DBCR1:
+ /*
+ * If userspace is debugging guest then guest
+ * can not access debug registers.
+ */
+ if (vcpu->guest_debug)
+ break;
+
+ debug_inst = true;
vcpu->arch.dbg_reg.dbcr1 = spr_val;
+ vcpu->arch.shadow_dbg_reg.dbcr1 = spr_val;
+ break;
+ case SPRN_DBCR2:
+ /*
+ * If userspace is debugging guest then guest
+ * can not access debug registers.
+ */
+ if (vcpu->guest_debug)
+ break;
+
+ debug_inst = true;
+ vcpu->arch.dbg_reg.dbcr2 = spr_val;
+ vcpu->arch.shadow_dbg_reg.dbcr2 = spr_val;
break;
case SPRN_DBSR:
+ /*
+ * If userspace is debugging guest then guest
+ * can not access debug registers.
+ */
+ if (vcpu->guest_debug)
+ break;
+
vcpu->arch.dbsr &= ~spr_val;
+ if (!(vcpu->arch.dbsr & ~DBSR_IDE))
+ kvmppc_core_dequeue_debug(vcpu);
break;
case SPRN_TSR:
kvmppc_clr_tsr_bits(vcpu, spr_val);
@@ -265,6 +389,10 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val)
emulated = EMULATE_FAIL;
}
+ if (debug_inst) {
+ current->thread.debug = vcpu->arch.shadow_dbg_reg;
+ switch_booke_debug_regs(&vcpu->arch.shadow_dbg_reg);
+ }
return emulated;
}
@@ -291,12 +419,41 @@ int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val)
case SPRN_CSRR1:
*spr_val = vcpu->arch.csrr1;
break;
+ case SPRN_DSRR0:
+ *spr_val = vcpu->arch.dsrr0;
+ break;
+ case SPRN_DSRR1:
+ *spr_val = vcpu->arch.dsrr1;
+ break;
+ case SPRN_IAC1:
+ *spr_val = vcpu->arch.dbg_reg.iac1;
+ break;
+ case SPRN_IAC2:
+ *spr_val = vcpu->arch.dbg_reg.iac2;
+ break;
+#if CONFIG_PPC_ADV_DEBUG_IACS > 2
+ case SPRN_IAC3:
+ *spr_val = vcpu->arch.dbg_reg.iac3;
+ break;
+ case SPRN_IAC4:
+ *spr_val = vcpu->arch.dbg_reg.iac4;
+ break;
+#endif
+ case SPRN_DAC1:
+ *spr_val = vcpu->arch.dbg_reg.dac1;
+ break;
+ case SPRN_DAC2:
+ *spr_val = vcpu->arch.dbg_reg.dac2;
+ break;
case SPRN_DBCR0:
*spr_val = vcpu->arch.dbg_reg.dbcr0;
break;
case SPRN_DBCR1:
*spr_val = vcpu->arch.dbg_reg.dbcr1;
break;
+ case SPRN_DBCR2:
+ *spr_val = vcpu->arch.dbg_reg.dbcr2;
+ break;
case SPRN_DBSR:
*spr_val = vcpu->arch.dbsr;
break;
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 5/5 v2] KVM: PPC: BOOKE: Emulate debug registers and exception
2014-08-04 8:02 ` [PATCH 5/5 v2] KVM: PPC: BOOKE: Emulate debug registers and exception Bharat Bhushan
@ 2014-08-04 22:53 ` Scott Wood
2014-08-05 3:41 ` Bharat.Bhushan
0 siblings, 1 reply; 9+ messages in thread
From: Scott Wood @ 2014-08-04 22:53 UTC (permalink / raw)
To: Bharat Bhushan; +Cc: agraf, kvm-ppc, kvm, stuart.yoder
On Mon, 2014-08-04 at 13:32 +0530, Bharat Bhushan wrote:
> @@ -735,7 +745,27 @@ static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu *vcpu)
> struct debug_reg *dbg_reg = &(vcpu->arch.shadow_dbg_reg);
> u32 dbsr = vcpu->arch.dbsr;
>
> - /* Clear guest dbsr (vcpu->arch.dbsr).
> + if (vcpu->guest_debug == 0) {
> + /*
> + * Debug resources belong to Guest.
> + * Imprecise debug event are not injected
> + */
> + if (dbsr & DBSR_IDE)
> + return RESUME_GUEST;
This is incorrect. DBSR_IDE shouldn't *cause* an injection, but it
shouldn't inhibit it either.
> @@ -828,6 +858,8 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
> case BOOKE_INTERRUPT_DEBUG:
> /* Save DBSR before preemption is enabled */
> vcpu->arch.dbsr = mfspr(SPRN_DBSR);
> + /* MASK out DBSR_MRR */
> + vcpu->arch.dbsr &= ~DBSR_MRR;
> kvmppc_clear_dbsr();
> break;
> }
DBSR[MRR] can only be set once per host system reset. There's no need
to filter it out here; just make sure the host clears it at some point
before this point. The MRR value doesn't currently survive past
kvmppc_clear_dbsr(), so this isn't helping to preserve it for the host's
benefit...
> @@ -1858,6 +1890,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>
> if (!(dbg->control & KVM_GUESTDBG_ENABLE)) {
> vcpu->arch.shadow_dbg_reg.dbcr0 = 0;
> + vcpu->arch.dbg_reg.dbcr0 = 0;
Again, it's not clear why we need shadow debug registers here. "Just in
case we implement something that can't be implemented" isn't a good
reason to keep complexity around.
-Scott
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [PATCH 5/5 v2] KVM: PPC: BOOKE: Emulate debug registers and exception
2014-08-04 22:53 ` Scott Wood
@ 2014-08-05 3:41 ` Bharat.Bhushan
2014-08-05 21:41 ` Scott Wood
0 siblings, 1 reply; 9+ messages in thread
From: Bharat.Bhushan @ 2014-08-05 3:41 UTC (permalink / raw)
To: Scott Wood
Cc: agraf@suse.de, kvm-ppc@vger.kernel.org, kvm@vger.kernel.org,
Stuart Yoder
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, August 05, 2014 4:23 AM
> To: Bhushan Bharat-R65777
> Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Yoder Stuart-
> B08248
> Subject: Re: [PATCH 5/5 v2] KVM: PPC: BOOKE: Emulate debug registers and
> exception
>
> On Mon, 2014-08-04 at 13:32 +0530, Bharat Bhushan wrote:
> > @@ -735,7 +745,27 @@ static int kvmppc_handle_debug(struct kvm_run *run,
> struct kvm_vcpu *vcpu)
> > struct debug_reg *dbg_reg = &(vcpu->arch.shadow_dbg_reg);
> > u32 dbsr = vcpu->arch.dbsr;
> >
> > - /* Clear guest dbsr (vcpu->arch.dbsr).
> > + if (vcpu->guest_debug == 0) {
> > + /*
> > + * Debug resources belong to Guest.
> > + * Imprecise debug event are not injected
> > + */
> > + if (dbsr & DBSR_IDE)
> > + return RESUME_GUEST;
>
> This is incorrect. DBSR_IDE shouldn't *cause* an injection, but it shouldn't
> inhibit it either.
Will this work ?
If ((dbsr & DBSR_IDE) && !(dbsr & ~DBSR_IDE))
Return RESUME_GUEST;
>
> > @@ -828,6 +858,8 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu
> *vcpu,
> > case BOOKE_INTERRUPT_DEBUG:
> > /* Save DBSR before preemption is enabled */
> > vcpu->arch.dbsr = mfspr(SPRN_DBSR);
> > + /* MASK out DBSR_MRR */
> > + vcpu->arch.dbsr &= ~DBSR_MRR;
> > kvmppc_clear_dbsr();
> > break;
> > }
>
> DBSR[MRR] can only be set once per host system reset. There's no need to filter
> it out here; just make sure the host clears it at some point before this point.
Can you please suggest where ? somewhere in KVM initialization ?
> The MRR value doesn't currently survive past kvmppc_clear_dbsr(), so this isn't
> helping to preserve it for the host's benefit...
>
> > @@ -1858,6 +1890,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct
> > kvm_vcpu *vcpu,
> >
> > if (!(dbg->control & KVM_GUESTDBG_ENABLE)) {
> > vcpu->arch.shadow_dbg_reg.dbcr0 = 0;
> > + vcpu->arch.dbg_reg.dbcr0 = 0;
>
> Again, it's not clear why we need shadow debug registers here. "Just in case we
> implement something that can't be implemented" isn't a good reason to keep
> complexity around.
One reason was that setting EDM in guest visible register, For this we need shadow_reg is used to save/restore state in h/w register (which does not have DBCR0_EDM) but debug_reg have DBCR0_EDM.
Thanks
-Bharat
>
> -Scott
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 5/5 v2] KVM: PPC: BOOKE: Emulate debug registers and exception
2014-08-05 3:41 ` Bharat.Bhushan
@ 2014-08-05 21:41 ` Scott Wood
0 siblings, 0 replies; 9+ messages in thread
From: Scott Wood @ 2014-08-05 21:41 UTC (permalink / raw)
To: Bhushan Bharat-R65777
Cc: agraf@suse.de, kvm-ppc@vger.kernel.org, kvm@vger.kernel.org,
Yoder Stuart-B08248
On Mon, 2014-08-04 at 22:41 -0500, Bhushan Bharat-R65777 wrote:
>
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, August 05, 2014 4:23 AM
> > To: Bhushan Bharat-R65777
> > Cc: agraf@suse.de; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; Yoder Stuart-
> > B08248
> > Subject: Re: [PATCH 5/5 v2] KVM: PPC: BOOKE: Emulate debug registers and
> > exception
> >
> > On Mon, 2014-08-04 at 13:32 +0530, Bharat Bhushan wrote:
> > > @@ -735,7 +745,27 @@ static int kvmppc_handle_debug(struct kvm_run *run,
> > struct kvm_vcpu *vcpu)
> > > struct debug_reg *dbg_reg = &(vcpu->arch.shadow_dbg_reg);
> > > u32 dbsr = vcpu->arch.dbsr;
> > >
> > > - /* Clear guest dbsr (vcpu->arch.dbsr).
> > > + if (vcpu->guest_debug == 0) {
> > > + /*
> > > + * Debug resources belong to Guest.
> > > + * Imprecise debug event are not injected
> > > + */
> > > + if (dbsr & DBSR_IDE)
> > > + return RESUME_GUEST;
> >
> > This is incorrect. DBSR_IDE shouldn't *cause* an injection, but it shouldn't
> > inhibit it either.
>
> Will this work ?
> If ((dbsr & DBSR_IDE) && !(dbsr & ~DBSR_IDE))
> Return RESUME_GUEST;
I suppose it could, but it would be cleaner to just change "dbsr" to
"(dbsr & ~DBSR_IDE)" in the next if-statement (maybe factoring out each
&& term of that if-statement to variables to make it more readable).
> > > @@ -828,6 +858,8 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu
> > *vcpu,
> > > case BOOKE_INTERRUPT_DEBUG:
> > > /* Save DBSR before preemption is enabled */
> > > vcpu->arch.dbsr = mfspr(SPRN_DBSR);
> > > + /* MASK out DBSR_MRR */
> > > + vcpu->arch.dbsr &= ~DBSR_MRR;
> > > kvmppc_clear_dbsr();
> > > break;
> > > }
> >
> > DBSR[MRR] can only be set once per host system reset. There's no need to filter
> > it out here; just make sure the host clears it at some point before this point.
>
> Can you please suggest where ? somewhere in KVM initialization ?
Sure, KVM init works given that there's no real reason for non-KVM code
to care.
> > The MRR value doesn't currently survive past kvmppc_clear_dbsr(), so this isn't
> > helping to preserve it for the host's benefit...
> >
> > > @@ -1858,6 +1890,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct
> > > kvm_vcpu *vcpu,
> > >
> > > if (!(dbg->control & KVM_GUESTDBG_ENABLE)) {
> > > vcpu->arch.shadow_dbg_reg.dbcr0 = 0;
> > > + vcpu->arch.dbg_reg.dbcr0 = 0;
> >
> > Again, it's not clear why we need shadow debug registers here. "Just in case we
> > implement something that can't be implemented" isn't a good reason to keep
> > complexity around.
>
> One reason was that setting EDM in guest visible register, For this we
> need shadow_reg is used to save/restore state in h/w register (which
> does not have DBCR0_EDM) but debug_reg have DBCR0_EDM.
If that's the only reason, then I'd get rid of the shadow and just OR in
DCBR0_EDM when reading the register, if vcpu->guest_debug is nonzero.
-Scott
^ permalink raw reply [flat|nested] 9+ messages in thread