Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-02-07 Thread Alexander Graf

On 01.02.2013, at 10:07, Bhushan Bharat-R65777 wrote:

 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, February 01, 2013 1:36 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 01.02.2013, at 06:04, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: kvm-ppc-ow...@vger.kernel.org
 [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf
 Sent: Thursday, January 31, 2013 10:38 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, January 31, 2013 5:47 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:13 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan
 Bharat-R65777
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
 From: Bharat Bhushan bharat.bhus...@freescale.com
 
 Installed debug handler will be used for guest debug support and
 debug facility emulation features (patches for these features
 will follow this patch).
 
 Signed-off-by: Liu Yu yu@freescale.com
 [bharat.bhus...@freescale.com: Substantial changes]
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 arch/powerpc/include/asm/kvm_host.h |1 +
 arch/powerpc/kernel/asm-offsets.c   |1 +
 arch/powerpc/kvm/booke_interrupts.S |   49
 ++-
 --
 --
 3 files changed, 44 insertions(+), 7 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_host.h
 b/arch/powerpc/include/asm/kvm_host.h
 index 8a72d59..f4ba881 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
   u32 tlbcfg[4];
   u32 mmucfg;
   u32 epr;
 + u32 crit_save;
   struct kvmppc_booke_debug_reg dbg_reg; #endif
   gpa_t paddr_accessed;
 diff --git a/arch/powerpc/kernel/asm-offsets.c
 b/arch/powerpc/kernel/asm-offsets.c
 index 46f6afd..02048f3 100644
 --- a/arch/powerpc/kernel/asm-offsets.c
 +++ b/arch/powerpc/kernel/asm-offsets.c
 @@ -562,6 +562,7 @@ int main(void)
   DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, 
 arch.last_inst));
   DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu,
 arch.fault_dear));
   DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu,
 arch.fault_esr));
 + DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu,
 +arch.crit_save));
 #endif /* CONFIG_PPC_BOOK3S */
 #endif /* CONFIG_KVM */
 
 diff --git a/arch/powerpc/kvm/booke_interrupts.S
 b/arch/powerpc/kvm/booke_interrupts.S
 index eae8483..dd9c5d4 100644
 --- a/arch/powerpc/kvm/booke_interrupts.S
 +++ b/arch/powerpc/kvm/booke_interrupts.S
 @@ -52,12 +52,7 @@
(1BOOKE_INTERRUPT_PROGRAM) | \
(1BOOKE_INTERRUPT_DTLB_MISS))
 
 -.macro KVM_HANDLER ivor_nr scratch srr0
 -_GLOBAL(kvmppc_handler_\ivor_nr)
 - /* Get pointer to vcpu and record exit number. */
 - mtspr   \scratch , r4
 - mfspr   r4, SPRN_SPRG_THREAD
 - lwz r4, THREAD_KVM_VCPU(r4)
 +.macro __KVM_HANDLER ivor_nr scratch srr0
   stw r3, VCPU_GPR(R3)(r4)
   stw r5, VCPU_GPR(R5)(r4)
   stw r6, VCPU_GPR(R6)(r4)
 @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
   bctr
 .endm
 
 +.macro KVM_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 + /* Get pointer to vcpu and record exit number. */
 + mtspr   \scratch , r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
 +
 +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 + mtspr   \scratch, r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + stw r3, VCPU_CRIT_SAVE(r4)
 + mfcrr3
 + mfspr   r4, SPRN_CSRR1
 + andi.   r4, r4, MSR_PR
 + bne 1f
 
 
 + /* debug interrupt happened in enter/exit path */
 + mfspr   r4, SPRN_CSRR1
 + rlwinm  r4, r4, 0, ~MSR_DE
 + mtspr   SPRN_CSRR1, r4
 + lis r4, 0x
 + ori r4, r4, 0x
 + mtspr   SPRN_DBSR, r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + mtcrr3
 + lwz r3, VCPU_CRIT_SAVE(r4)
 + mfspr   r4, \scratch
 + rfci
 
 What is this part doing? Try to ignore the debug exit?
 
 As BOOKE doesn't have hardware support for virtualization,
 hardware never know
 current pc

RE: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-02-07 Thread Bhushan Bharat-R65777
  -Original Message-
  From: Alexander Graf [mailto:ag...@suse.de]
  Sent: Friday, January 25, 2013 5:13 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan
  Bharat-R65777
  Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
  On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
  From: Bharat Bhushan bharat.bhus...@freescale.com
 
  Installed debug handler will be used for guest debug support
  and debug facility emulation features (patches for these
  features will follow this patch).
 
  Signed-off-by: Liu Yu yu@freescale.com
  [bharat.bhus...@freescale.com: Substantial changes]
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  arch/powerpc/include/asm/kvm_host.h |1 +
  arch/powerpc/kernel/asm-offsets.c   |1 +
  arch/powerpc/kvm/booke_interrupts.S |   49
  ++-
  --
  --
  3 files changed, 44 insertions(+), 7 deletions(-)
 
  diff --git a/arch/powerpc/include/asm/kvm_host.h
  b/arch/powerpc/include/asm/kvm_host.h
  index 8a72d59..f4ba881 100644
  --- a/arch/powerpc/include/asm/kvm_host.h
  +++ b/arch/powerpc/include/asm/kvm_host.h
  @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
  u32 tlbcfg[4];
  u32 mmucfg;
  u32 epr;
  +   u32 crit_save;
  struct kvmppc_booke_debug_reg dbg_reg; #endif
  gpa_t paddr_accessed;
  diff --git a/arch/powerpc/kernel/asm-offsets.c
  b/arch/powerpc/kernel/asm-offsets.c
  index 46f6afd..02048f3 100644
  --- a/arch/powerpc/kernel/asm-offsets.c
  +++ b/arch/powerpc/kernel/asm-offsets.c
  @@ -562,6 +562,7 @@ int main(void)
  DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, 
  arch.last_inst));
  DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu,
  arch.fault_dear));
  DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu,
  arch.fault_esr));
  +   DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu,
  +arch.crit_save));
  #endif /* CONFIG_PPC_BOOK3S */ #endif /* CONFIG_KVM */
 
  diff --git a/arch/powerpc/kvm/booke_interrupts.S
  b/arch/powerpc/kvm/booke_interrupts.S
  index eae8483..dd9c5d4 100644
  --- a/arch/powerpc/kvm/booke_interrupts.S
  +++ b/arch/powerpc/kvm/booke_interrupts.S
  @@ -52,12 +52,7 @@
 (1BOOKE_INTERRUPT_PROGRAM) | \
 (1BOOKE_INTERRUPT_DTLB_MISS))
 
  -.macro KVM_HANDLER ivor_nr scratch srr0
  -_GLOBAL(kvmppc_handler_\ivor_nr)
  -   /* Get pointer to vcpu and record exit number. */
  -   mtspr   \scratch , r4
  -   mfspr   r4, SPRN_SPRG_THREAD
  -   lwz r4, THREAD_KVM_VCPU(r4)
  +.macro __KVM_HANDLER ivor_nr scratch srr0
  stw r3, VCPU_GPR(R3)(r4)
  stw r5, VCPU_GPR(R5)(r4)
  stw r6, VCPU_GPR(R6)(r4)
  @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
  bctr
  .endm
 
  +.macro KVM_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  +   /* Get pointer to vcpu and record exit number. */
  +   mtspr   \scratch , r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
  +
  +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  +   mtspr   \scratch, r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   stw r3, VCPU_CRIT_SAVE(r4)
  +   mfcrr3
  +   mfspr   r4, SPRN_CSRR1
  +   andi.   r4, r4, MSR_PR
  +   bne 1f
 
 
  +   /* debug interrupt happened in enter/exit path */
  +   mfspr   r4, SPRN_CSRR1
  +   rlwinm  r4, r4, 0, ~MSR_DE
  +   mtspr   SPRN_CSRR1, r4
  +   lis r4, 0x
  +   ori r4, r4, 0x
  +   mtspr   SPRN_DBSR, r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   mtcrr3
  +   lwz r3, VCPU_CRIT_SAVE(r4)
  +   mfspr   r4, \scratch
  +   rfci
 
  What is this part doing? Try to ignore the debug exit?
 
  As BOOKE doesn't have hardware support for virtualization,
  hardware never know
  current pc is in guest or in host.
  So when enable hardware single step for guest, it cannot be
  disabled at the
  time guest exit. Thus, we'll see that an single step interrupt
  happens at the beginning of guest exit path.
 
  With the above code we recognize this kind of single step
  interrupt disable
  single step and rfci.
 
  Why would we have MSR_DE
  enabled in the first place when we can't handle it?
 
  When QEMU is using hardware debug resource then we always set
  MSR_DE during
  guest is running.
 
  Right, but why is MSR_DE enabled during the exit path? If MSR_DE
  wasn't set, you wouldn't get a single step exit.
 
  We always set MSR_DE in hw MSR when qemu using the debug resource.
 
  In the _guest_ MSR, yes. But once we exit the guest, it shouldn't
  be set anymore, because we're in an interrupt handler, no? Or is
  MSR_DE kept alive on interrupts?
 
 
  During the exit code path, you could then swap DBSR back to what
  the host expects (which means no single step). Only after that
  enable MSR_DE again.
 
  We do not support deferred debug

Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-02-07 Thread Alexander Graf

On 07.02.2013, at 15:48, Bhushan Bharat-R65777 wrote:

 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:13 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan
 Bharat-R65777
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
 From: Bharat Bhushan bharat.bhus...@freescale.com
 
 Installed debug handler will be used for guest debug support
 and debug facility emulation features (patches for these
 features will follow this patch).
 
 Signed-off-by: Liu Yu yu@freescale.com
 [bharat.bhus...@freescale.com: Substantial changes]
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 arch/powerpc/include/asm/kvm_host.h |1 +
 arch/powerpc/kernel/asm-offsets.c   |1 +
 arch/powerpc/kvm/booke_interrupts.S |   49
 ++-
 --
 --
 3 files changed, 44 insertions(+), 7 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_host.h
 b/arch/powerpc/include/asm/kvm_host.h
 index 8a72d59..f4ba881 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
 u32 tlbcfg[4];
 u32 mmucfg;
 u32 epr;
 +   u32 crit_save;
 struct kvmppc_booke_debug_reg dbg_reg; #endif
 gpa_t paddr_accessed;
 diff --git a/arch/powerpc/kernel/asm-offsets.c
 b/arch/powerpc/kernel/asm-offsets.c
 index 46f6afd..02048f3 100644
 --- a/arch/powerpc/kernel/asm-offsets.c
 +++ b/arch/powerpc/kernel/asm-offsets.c
 @@ -562,6 +562,7 @@ int main(void)
 DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, 
 arch.last_inst));
 DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu,
 arch.fault_dear));
 DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu,
 arch.fault_esr));
 +   DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu,
 +arch.crit_save));
 #endif /* CONFIG_PPC_BOOK3S */ #endif /* CONFIG_KVM */
 
 diff --git a/arch/powerpc/kvm/booke_interrupts.S
 b/arch/powerpc/kvm/booke_interrupts.S
 index eae8483..dd9c5d4 100644
 --- a/arch/powerpc/kvm/booke_interrupts.S
 +++ b/arch/powerpc/kvm/booke_interrupts.S
 @@ -52,12 +52,7 @@
   (1BOOKE_INTERRUPT_PROGRAM) | \
   (1BOOKE_INTERRUPT_DTLB_MISS))
 
 -.macro KVM_HANDLER ivor_nr scratch srr0
 -_GLOBAL(kvmppc_handler_\ivor_nr)
 -   /* Get pointer to vcpu and record exit number. */
 -   mtspr   \scratch , r4
 -   mfspr   r4, SPRN_SPRG_THREAD
 -   lwz r4, THREAD_KVM_VCPU(r4)
 +.macro __KVM_HANDLER ivor_nr scratch srr0
 stw r3, VCPU_GPR(R3)(r4)
 stw r5, VCPU_GPR(R5)(r4)
 stw r6, VCPU_GPR(R6)(r4)
 @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
 bctr
 .endm
 
 +.macro KVM_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +   /* Get pointer to vcpu and record exit number. */
 +   mtspr   \scratch , r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
 +
 +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +   mtspr   \scratch, r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   stw r3, VCPU_CRIT_SAVE(r4)
 +   mfcrr3
 +   mfspr   r4, SPRN_CSRR1
 +   andi.   r4, r4, MSR_PR
 +   bne 1f
 
 
 +   /* debug interrupt happened in enter/exit path */
 +   mfspr   r4, SPRN_CSRR1
 +   rlwinm  r4, r4, 0, ~MSR_DE
 +   mtspr   SPRN_CSRR1, r4
 +   lis r4, 0x
 +   ori r4, r4, 0x
 +   mtspr   SPRN_DBSR, r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   mtcrr3
 +   lwz r3, VCPU_CRIT_SAVE(r4)
 +   mfspr   r4, \scratch
 +   rfci
 
 What is this part doing? Try to ignore the debug exit?
 
 As BOOKE doesn't have hardware support for virtualization,
 hardware never know
 current pc is in guest or in host.
 So when enable hardware single step for guest, it cannot be
 disabled at the
 time guest exit. Thus, we'll see that an single step interrupt
 happens at the beginning of guest exit path.
 
 With the above code we recognize this kind of single step
 interrupt disable
 single step and rfci.
 
 Why would we have MSR_DE
 enabled in the first place when we can't handle it?
 
 When QEMU is using hardware debug resource then we always set
 MSR_DE during
 guest is running.
 
 Right, but why is MSR_DE enabled during the exit path? If MSR_DE
 wasn't set, you wouldn't get a single step exit.
 
 We always set MSR_DE in hw MSR when qemu using the debug resource.
 
 In the _guest_ MSR, yes. But once we exit the guest, it shouldn't
 be set anymore, because we're in an interrupt handler, no? Or is
 MSR_DE kept alive on interrupts?
 
 
 During the exit code path, you could then swap DBSR back to what
 the host expects (which means no single step). Only after that
 enable MSR_DE again.
 
 We do not support deferred debug interrupt, so we do save restore dbsr.
 
 
 
 
 +1: /* debug interrupt happened

RE: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-02-07 Thread Bhushan Bharat-R65777
  -Original Message-
  From: Alexander Graf [mailto:ag...@suse.de]
  Sent: Friday, January 25, 2013 5:13 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan
  Bharat-R65777
  Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
  On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
  From: Bharat Bhushan bharat.bhus...@freescale.com
 
  Installed debug handler will be used for guest debug support
  and debug facility emulation features (patches for these
  features will follow this patch).
 
  Signed-off-by: Liu Yu yu@freescale.com
  [bharat.bhus...@freescale.com: Substantial changes]
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  arch/powerpc/include/asm/kvm_host.h |1 +
  arch/powerpc/kernel/asm-offsets.c   |1 +
  arch/powerpc/kvm/booke_interrupts.S |   49
  ++-
  --
  --
  3 files changed, 44 insertions(+), 7 deletions(-)
 
  diff --git a/arch/powerpc/include/asm/kvm_host.h
  b/arch/powerpc/include/asm/kvm_host.h
  index 8a72d59..f4ba881 100644
  --- a/arch/powerpc/include/asm/kvm_host.h
  +++ b/arch/powerpc/include/asm/kvm_host.h
  @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
  u32 tlbcfg[4];
  u32 mmucfg;
  u32 epr;
  +   u32 crit_save;
  struct kvmppc_booke_debug_reg dbg_reg; #endif
  gpa_t paddr_accessed;
  diff --git a/arch/powerpc/kernel/asm-offsets.c
  b/arch/powerpc/kernel/asm-offsets.c
  index 46f6afd..02048f3 100644
  --- a/arch/powerpc/kernel/asm-offsets.c
  +++ b/arch/powerpc/kernel/asm-offsets.c
  @@ -562,6 +562,7 @@ int main(void)
  DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, 
  arch.last_inst));
  DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu,
  arch.fault_dear));
  DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu,
  arch.fault_esr));
  +   DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu,
  +arch.crit_save));
  #endif /* CONFIG_PPC_BOOK3S */ #endif /* CONFIG_KVM */
 
  diff --git a/arch/powerpc/kvm/booke_interrupts.S
  b/arch/powerpc/kvm/booke_interrupts.S
  index eae8483..dd9c5d4 100644
  --- a/arch/powerpc/kvm/booke_interrupts.S
  +++ b/arch/powerpc/kvm/booke_interrupts.S
  @@ -52,12 +52,7 @@
 (1BOOKE_INTERRUPT_PROGRAM) | \
 (1BOOKE_INTERRUPT_DTLB_MISS))
 
  -.macro KVM_HANDLER ivor_nr scratch srr0
  -_GLOBAL(kvmppc_handler_\ivor_nr)
  -   /* Get pointer to vcpu and record exit number. */
  -   mtspr   \scratch , r4
  -   mfspr   r4, SPRN_SPRG_THREAD
  -   lwz r4, THREAD_KVM_VCPU(r4)
  +.macro __KVM_HANDLER ivor_nr scratch srr0
  stw r3, VCPU_GPR(R3)(r4)
  stw r5, VCPU_GPR(R5)(r4)
  stw r6, VCPU_GPR(R6)(r4)
  @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
  bctr
  .endm
 
  +.macro KVM_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  +   /* Get pointer to vcpu and record exit number. */
  +   mtspr   \scratch , r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
  +
  +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  +   mtspr   \scratch, r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   stw r3, VCPU_CRIT_SAVE(r4)
  +   mfcrr3
  +   mfspr   r4, SPRN_CSRR1
  +   andi.   r4, r4, MSR_PR
  +   bne 1f
 
 
  +   /* debug interrupt happened in enter/exit path */
  +   mfspr   r4, SPRN_CSRR1
  +   rlwinm  r4, r4, 0, ~MSR_DE
  +   mtspr   SPRN_CSRR1, r4
  +   lis r4, 0x
  +   ori r4, r4, 0x
  +   mtspr   SPRN_DBSR, r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   mtcrr3
  +   lwz r3, VCPU_CRIT_SAVE(r4)
  +   mfspr   r4, \scratch
  +   rfci
 
  What is this part doing? Try to ignore the debug exit?
 
  As BOOKE doesn't have hardware support for virtualization,
  hardware never know
  current pc is in guest or in host.
  So when enable hardware single step for guest, it cannot be
  disabled at the
  time guest exit. Thus, we'll see that an single step interrupt
  happens at the beginning of guest exit path.
 
  With the above code we recognize this kind of single step
  interrupt disable
  single step and rfci.
 
  Why would we have MSR_DE
  enabled in the first place when we can't handle it?
 
  When QEMU is using hardware debug resource then we always set
  MSR_DE during
  guest is running.
 
  Right, but why is MSR_DE enabled during the exit path? If MSR_DE
  wasn't set, you wouldn't get a single step exit.
 
  We always set MSR_DE in hw MSR when qemu using the debug resource.
 
  In the _guest_ MSR, yes. But once we exit the guest, it shouldn't
  be set anymore, because we're in an interrupt handler, no? Or is
  MSR_DE kept alive on interrupts?
 
 
  During the exit code path, you could then swap DBSR back to what
  the host expects (which means no single step). Only after that
  enable MSR_DE again.
 
  We do not support deferred debug

Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-02-01 Thread Alexander Graf

On 01.02.2013, at 06:04, Bhushan Bharat-R65777 wrote:

 
 
 -Original Message-
 From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On
 Behalf Of Alexander Graf
 Sent: Thursday, January 31, 2013 10:38 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, January 31, 2013 5:47 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:13 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan
 Bharat-R65777
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
 From: Bharat Bhushan bharat.bhus...@freescale.com
 
 Installed debug handler will be used for guest debug support and
 debug facility emulation features (patches for these features will
 follow this patch).
 
 Signed-off-by: Liu Yu yu@freescale.com
 [bharat.bhus...@freescale.com: Substantial changes]
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 arch/powerpc/include/asm/kvm_host.h |1 +
 arch/powerpc/kernel/asm-offsets.c   |1 +
 arch/powerpc/kvm/booke_interrupts.S |   49 
 ++-
 --
 --
 3 files changed, 44 insertions(+), 7 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_host.h
 b/arch/powerpc/include/asm/kvm_host.h
 index 8a72d59..f4ba881 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
 u32 tlbcfg[4];
 u32 mmucfg;
 u32 epr;
 +   u32 crit_save;
 struct kvmppc_booke_debug_reg dbg_reg; #endif
 gpa_t paddr_accessed;
 diff --git a/arch/powerpc/kernel/asm-offsets.c
 b/arch/powerpc/kernel/asm-offsets.c
 index 46f6afd..02048f3 100644
 --- a/arch/powerpc/kernel/asm-offsets.c
 +++ b/arch/powerpc/kernel/asm-offsets.c
 @@ -562,6 +562,7 @@ int main(void)
 DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, 
 arch.last_inst));
 DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, 
 arch.fault_dear));
 DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu,
 arch.fault_esr));
 +   DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu,
 +arch.crit_save));
 #endif /* CONFIG_PPC_BOOK3S */
 #endif /* CONFIG_KVM */
 
 diff --git a/arch/powerpc/kvm/booke_interrupts.S
 b/arch/powerpc/kvm/booke_interrupts.S
 index eae8483..dd9c5d4 100644
 --- a/arch/powerpc/kvm/booke_interrupts.S
 +++ b/arch/powerpc/kvm/booke_interrupts.S
 @@ -52,12 +52,7 @@
 (1BOOKE_INTERRUPT_PROGRAM) | \
 (1BOOKE_INTERRUPT_DTLB_MISS))
 
 -.macro KVM_HANDLER ivor_nr scratch srr0
 -_GLOBAL(kvmppc_handler_\ivor_nr)
 -   /* Get pointer to vcpu and record exit number. */
 -   mtspr   \scratch , r4
 -   mfspr   r4, SPRN_SPRG_THREAD
 -   lwz r4, THREAD_KVM_VCPU(r4)
 +.macro __KVM_HANDLER ivor_nr scratch srr0
 stw r3, VCPU_GPR(R3)(r4)
 stw r5, VCPU_GPR(R5)(r4)
 stw r6, VCPU_GPR(R6)(r4)
 @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
 bctr
 .endm
 
 +.macro KVM_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +   /* Get pointer to vcpu and record exit number. */
 +   mtspr   \scratch , r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
 +
 +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +   mtspr   \scratch, r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   stw r3, VCPU_CRIT_SAVE(r4)
 +   mfcrr3
 +   mfspr   r4, SPRN_CSRR1
 +   andi.   r4, r4, MSR_PR
 +   bne 1f
 
 
 +   /* debug interrupt happened in enter/exit path */
 +   mfspr   r4, SPRN_CSRR1
 +   rlwinm  r4, r4, 0, ~MSR_DE
 +   mtspr   SPRN_CSRR1, r4
 +   lis r4, 0x
 +   ori r4, r4, 0x
 +   mtspr   SPRN_DBSR, r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   mtcrr3
 +   lwz r3, VCPU_CRIT_SAVE(r4)
 +   mfspr   r4, \scratch
 +   rfci
 
 What is this part doing? Try to ignore the debug exit?
 
 As BOOKE doesn't have hardware support for virtualization, hardware
 never know
 current pc is in guest or in host.
 So when enable hardware single step for guest, it cannot be disabled
 at the
 time guest exit. Thus, we'll see that an single step interrupt
 happens at the beginning of guest exit path.
 
 With the above code we

RE: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-02-01 Thread Bhushan Bharat-R65777


 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, February 01, 2013 1:36 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 01.02.2013, at 06:04, Bhushan Bharat-R65777 wrote:
 
 
 
  -Original Message-
  From: kvm-ppc-ow...@vger.kernel.org
  [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf
  Sent: Thursday, January 31, 2013 10:38 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
  Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
  On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote:
 
 
 
  -Original Message-
  From: Alexander Graf [mailto:ag...@suse.de]
  Sent: Thursday, January 31, 2013 5:47 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
  Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
  On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:
 
 
 
  -Original Message-
  From: Alexander Graf [mailto:ag...@suse.de]
  Sent: Friday, January 25, 2013 5:13 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan
  Bharat-R65777
  Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
  On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
  From: Bharat Bhushan bharat.bhus...@freescale.com
 
  Installed debug handler will be used for guest debug support and
  debug facility emulation features (patches for these features
  will follow this patch).
 
  Signed-off-by: Liu Yu yu@freescale.com
  [bharat.bhus...@freescale.com: Substantial changes]
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  arch/powerpc/include/asm/kvm_host.h |1 +
  arch/powerpc/kernel/asm-offsets.c   |1 +
  arch/powerpc/kvm/booke_interrupts.S |   49
 ++-
  --
  --
  3 files changed, 44 insertions(+), 7 deletions(-)
 
  diff --git a/arch/powerpc/include/asm/kvm_host.h
  b/arch/powerpc/include/asm/kvm_host.h
  index 8a72d59..f4ba881 100644
  --- a/arch/powerpc/include/asm/kvm_host.h
  +++ b/arch/powerpc/include/asm/kvm_host.h
  @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
u32 tlbcfg[4];
u32 mmucfg;
u32 epr;
  + u32 crit_save;
struct kvmppc_booke_debug_reg dbg_reg; #endif
gpa_t paddr_accessed;
  diff --git a/arch/powerpc/kernel/asm-offsets.c
  b/arch/powerpc/kernel/asm-offsets.c
  index 46f6afd..02048f3 100644
  --- a/arch/powerpc/kernel/asm-offsets.c
  +++ b/arch/powerpc/kernel/asm-offsets.c
  @@ -562,6 +562,7 @@ int main(void)
DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, 
  arch.last_inst));
DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu,
 arch.fault_dear));
DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu,
  arch.fault_esr));
  + DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu,
  +arch.crit_save));
  #endif /* CONFIG_PPC_BOOK3S */
  #endif /* CONFIG_KVM */
 
  diff --git a/arch/powerpc/kvm/booke_interrupts.S
  b/arch/powerpc/kvm/booke_interrupts.S
  index eae8483..dd9c5d4 100644
  --- a/arch/powerpc/kvm/booke_interrupts.S
  +++ b/arch/powerpc/kvm/booke_interrupts.S
  @@ -52,12 +52,7 @@
  (1BOOKE_INTERRUPT_PROGRAM) | \
  (1BOOKE_INTERRUPT_DTLB_MISS))
 
  -.macro KVM_HANDLER ivor_nr scratch srr0
  -_GLOBAL(kvmppc_handler_\ivor_nr)
  - /* Get pointer to vcpu and record exit number. */
  - mtspr   \scratch , r4
  - mfspr   r4, SPRN_SPRG_THREAD
  - lwz r4, THREAD_KVM_VCPU(r4)
  +.macro __KVM_HANDLER ivor_nr scratch srr0
stw r3, VCPU_GPR(R3)(r4)
stw r5, VCPU_GPR(R5)(r4)
stw r6, VCPU_GPR(R6)(r4)
  @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
bctr
  .endm
 
  +.macro KVM_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  + /* Get pointer to vcpu and record exit number. */
  + mtspr   \scratch , r4
  + mfspr   r4, SPRN_SPRG_THREAD
  + lwz r4, THREAD_KVM_VCPU(r4)
  + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
  +
  +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  + mtspr   \scratch, r4
  + mfspr   r4, SPRN_SPRG_THREAD
  + lwz r4, THREAD_KVM_VCPU(r4)
  + stw r3, VCPU_CRIT_SAVE(r4)
  + mfcrr3
  + mfspr   r4, SPRN_CSRR1
  + andi.   r4, r4, MSR_PR
  + bne 1f
 
 
  + /* debug interrupt happened in enter/exit path */
  + mfspr   r4, SPRN_CSRR1
  + rlwinm  r4, r4, 0, ~MSR_DE
  + mtspr   SPRN_CSRR1, r4
  + lis r4, 0x
  + ori r4, r4, 0x
  + mtspr   SPRN_DBSR, r4
  + mfspr   r4, SPRN_SPRG_THREAD
  + lwz r4, THREAD_KVM_VCPU(r4)
  + mtcrr3
  + lwz r3, VCPU_CRIT_SAVE(r4)
  + mfspr   r4, \scratch
  + rfci
 
  What is this part doing? Try to ignore the debug exit?
 
  As BOOKE doesn't have hardware support

Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-02-01 Thread Alexander Graf

On 01.02.2013, at 06:04, Bhushan Bharat-R65777 wrote:

 
 
 -Original Message-
 From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On
 Behalf Of Alexander Graf
 Sent: Thursday, January 31, 2013 10:38 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, January 31, 2013 5:47 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:13 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan
 Bharat-R65777
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
 From: Bharat Bhushan bharat.bhus...@freescale.com
 
 Installed debug handler will be used for guest debug support and
 debug facility emulation features (patches for these features will
 follow this patch).
 
 Signed-off-by: Liu Yu yu@freescale.com
 [bharat.bhus...@freescale.com: Substantial changes]
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 arch/powerpc/include/asm/kvm_host.h |1 +
 arch/powerpc/kernel/asm-offsets.c   |1 +
 arch/powerpc/kvm/booke_interrupts.S |   49 
 ++-
 --
 --
 3 files changed, 44 insertions(+), 7 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_host.h
 b/arch/powerpc/include/asm/kvm_host.h
 index 8a72d59..f4ba881 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
 u32 tlbcfg[4];
 u32 mmucfg;
 u32 epr;
 +   u32 crit_save;
 struct kvmppc_booke_debug_reg dbg_reg; #endif
 gpa_t paddr_accessed;
 diff --git a/arch/powerpc/kernel/asm-offsets.c
 b/arch/powerpc/kernel/asm-offsets.c
 index 46f6afd..02048f3 100644
 --- a/arch/powerpc/kernel/asm-offsets.c
 +++ b/arch/powerpc/kernel/asm-offsets.c
 @@ -562,6 +562,7 @@ int main(void)
 DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, 
 arch.last_inst));
 DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, 
 arch.fault_dear));
 DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu,
 arch.fault_esr));
 +   DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu,
 +arch.crit_save));
 #endif /* CONFIG_PPC_BOOK3S */
 #endif /* CONFIG_KVM */
 
 diff --git a/arch/powerpc/kvm/booke_interrupts.S
 b/arch/powerpc/kvm/booke_interrupts.S
 index eae8483..dd9c5d4 100644
 --- a/arch/powerpc/kvm/booke_interrupts.S
 +++ b/arch/powerpc/kvm/booke_interrupts.S
 @@ -52,12 +52,7 @@
 (1BOOKE_INTERRUPT_PROGRAM) | \
 (1BOOKE_INTERRUPT_DTLB_MISS))
 
 -.macro KVM_HANDLER ivor_nr scratch srr0
 -_GLOBAL(kvmppc_handler_\ivor_nr)
 -   /* Get pointer to vcpu and record exit number. */
 -   mtspr   \scratch , r4
 -   mfspr   r4, SPRN_SPRG_THREAD
 -   lwz r4, THREAD_KVM_VCPU(r4)
 +.macro __KVM_HANDLER ivor_nr scratch srr0
 stw r3, VCPU_GPR(R3)(r4)
 stw r5, VCPU_GPR(R5)(r4)
 stw r6, VCPU_GPR(R6)(r4)
 @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
 bctr
 .endm
 
 +.macro KVM_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +   /* Get pointer to vcpu and record exit number. */
 +   mtspr   \scratch , r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
 +
 +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +   mtspr   \scratch, r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   stw r3, VCPU_CRIT_SAVE(r4)
 +   mfcrr3
 +   mfspr   r4, SPRN_CSRR1
 +   andi.   r4, r4, MSR_PR
 +   bne 1f
 
 
 +   /* debug interrupt happened in enter/exit path */
 +   mfspr   r4, SPRN_CSRR1
 +   rlwinm  r4, r4, 0, ~MSR_DE
 +   mtspr   SPRN_CSRR1, r4
 +   lis r4, 0x
 +   ori r4, r4, 0x
 +   mtspr   SPRN_DBSR, r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   mtcrr3
 +   lwz r3, VCPU_CRIT_SAVE(r4)
 +   mfspr   r4, \scratch
 +   rfci
 
 What is this part doing? Try to ignore the debug exit?
 
 As BOOKE doesn't have hardware support for virtualization, hardware
 never know
 current pc is in guest or in host.
 So when enable hardware single step for guest, it cannot be disabled
 at the
 time guest exit. Thus, we'll see that an single step interrupt
 happens at the beginning of guest exit path.
 
 With the above code

RE: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-02-01 Thread Bhushan Bharat-R65777


 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, February 01, 2013 1:36 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 01.02.2013, at 06:04, Bhushan Bharat-R65777 wrote:
 
 
 
  -Original Message-
  From: kvm-ppc-ow...@vger.kernel.org
  [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf
  Sent: Thursday, January 31, 2013 10:38 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org
  Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
  On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote:
 
 
 
  -Original Message-
  From: Alexander Graf [mailto:ag...@suse.de]
  Sent: Thursday, January 31, 2013 5:47 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org
  Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
  On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:
 
 
 
  -Original Message-
  From: Alexander Graf [mailto:ag...@suse.de]
  Sent: Friday, January 25, 2013 5:13 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan
  Bharat-R65777
  Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
  On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
  From: Bharat Bhushan bharat.bhus...@freescale.com
 
  Installed debug handler will be used for guest debug support and
  debug facility emulation features (patches for these features
  will follow this patch).
 
  Signed-off-by: Liu Yu yu@freescale.com
  [bharat.bhus...@freescale.com: Substantial changes]
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  arch/powerpc/include/asm/kvm_host.h |1 +
  arch/powerpc/kernel/asm-offsets.c   |1 +
  arch/powerpc/kvm/booke_interrupts.S |   49
 ++-
  --
  --
  3 files changed, 44 insertions(+), 7 deletions(-)
 
  diff --git a/arch/powerpc/include/asm/kvm_host.h
  b/arch/powerpc/include/asm/kvm_host.h
  index 8a72d59..f4ba881 100644
  --- a/arch/powerpc/include/asm/kvm_host.h
  +++ b/arch/powerpc/include/asm/kvm_host.h
  @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
u32 tlbcfg[4];
u32 mmucfg;
u32 epr;
  + u32 crit_save;
struct kvmppc_booke_debug_reg dbg_reg; #endif
gpa_t paddr_accessed;
  diff --git a/arch/powerpc/kernel/asm-offsets.c
  b/arch/powerpc/kernel/asm-offsets.c
  index 46f6afd..02048f3 100644
  --- a/arch/powerpc/kernel/asm-offsets.c
  +++ b/arch/powerpc/kernel/asm-offsets.c
  @@ -562,6 +562,7 @@ int main(void)
DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, 
  arch.last_inst));
DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu,
 arch.fault_dear));
DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu,
  arch.fault_esr));
  + DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu,
  +arch.crit_save));
  #endif /* CONFIG_PPC_BOOK3S */
  #endif /* CONFIG_KVM */
 
  diff --git a/arch/powerpc/kvm/booke_interrupts.S
  b/arch/powerpc/kvm/booke_interrupts.S
  index eae8483..dd9c5d4 100644
  --- a/arch/powerpc/kvm/booke_interrupts.S
  +++ b/arch/powerpc/kvm/booke_interrupts.S
  @@ -52,12 +52,7 @@
  (1BOOKE_INTERRUPT_PROGRAM) | \
  (1BOOKE_INTERRUPT_DTLB_MISS))
 
  -.macro KVM_HANDLER ivor_nr scratch srr0
  -_GLOBAL(kvmppc_handler_\ivor_nr)
  - /* Get pointer to vcpu and record exit number. */
  - mtspr   \scratch , r4
  - mfspr   r4, SPRN_SPRG_THREAD
  - lwz r4, THREAD_KVM_VCPU(r4)
  +.macro __KVM_HANDLER ivor_nr scratch srr0
stw r3, VCPU_GPR(R3)(r4)
stw r5, VCPU_GPR(R5)(r4)
stw r6, VCPU_GPR(R6)(r4)
  @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
bctr
  .endm
 
  +.macro KVM_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  + /* Get pointer to vcpu and record exit number. */
  + mtspr   \scratch , r4
  + mfspr   r4, SPRN_SPRG_THREAD
  + lwz r4, THREAD_KVM_VCPU(r4)
  + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
  +
  +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  + mtspr   \scratch, r4
  + mfspr   r4, SPRN_SPRG_THREAD
  + lwz r4, THREAD_KVM_VCPU(r4)
  + stw r3, VCPU_CRIT_SAVE(r4)
  + mfcrr3
  + mfspr   r4, SPRN_CSRR1
  + andi.   r4, r4, MSR_PR
  + bne 1f
 
 
  + /* debug interrupt happened in enter/exit path */
  + mfspr   r4, SPRN_CSRR1
  + rlwinm  r4, r4, 0, ~MSR_DE
  + mtspr   SPRN_CSRR1, r4
  + lis r4, 0x
  + ori r4, r4, 0x
  + mtspr   SPRN_DBSR, r4
  + mfspr   r4, SPRN_SPRG_THREAD
  + lwz r4, THREAD_KVM_VCPU(r4)
  + mtcrr3
  + lwz r3, VCPU_CRIT_SAVE(r4)
  + mfspr   r4, \scratch
  + rfci
 
  What is this part doing? Try to ignore the debug exit?
 
  As BOOKE doesn't have hardware

Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-01-31 Thread Alexander Graf

On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:

 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:13 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
 From: Bharat Bhushan bharat.bhus...@freescale.com
 
 Installed debug handler will be used for guest debug support and debug
 facility emulation features (patches for these features will follow
 this patch).
 
 Signed-off-by: Liu Yu yu@freescale.com
 [bharat.bhus...@freescale.com: Substantial changes]
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 arch/powerpc/include/asm/kvm_host.h |1 +
 arch/powerpc/kernel/asm-offsets.c   |1 +
 arch/powerpc/kvm/booke_interrupts.S |   49 
 ++-
 3 files changed, 44 insertions(+), 7 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_host.h
 b/arch/powerpc/include/asm/kvm_host.h
 index 8a72d59..f4ba881 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
 u32 tlbcfg[4];
 u32 mmucfg;
 u32 epr;
 +   u32 crit_save;
 struct kvmppc_booke_debug_reg dbg_reg; #endif
 gpa_t paddr_accessed;
 diff --git a/arch/powerpc/kernel/asm-offsets.c
 b/arch/powerpc/kernel/asm-offsets.c
 index 46f6afd..02048f3 100644
 --- a/arch/powerpc/kernel/asm-offsets.c
 +++ b/arch/powerpc/kernel/asm-offsets.c
 @@ -562,6 +562,7 @@ int main(void)
 DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst));
 DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear));
 DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr));
 +   DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save));
 #endif /* CONFIG_PPC_BOOK3S */
 #endif /* CONFIG_KVM */
 
 diff --git a/arch/powerpc/kvm/booke_interrupts.S
 b/arch/powerpc/kvm/booke_interrupts.S
 index eae8483..dd9c5d4 100644
 --- a/arch/powerpc/kvm/booke_interrupts.S
 +++ b/arch/powerpc/kvm/booke_interrupts.S
 @@ -52,12 +52,7 @@
   (1BOOKE_INTERRUPT_PROGRAM) | \
   (1BOOKE_INTERRUPT_DTLB_MISS))
 
 -.macro KVM_HANDLER ivor_nr scratch srr0
 -_GLOBAL(kvmppc_handler_\ivor_nr)
 -   /* Get pointer to vcpu and record exit number. */
 -   mtspr   \scratch , r4
 -   mfspr   r4, SPRN_SPRG_THREAD
 -   lwz r4, THREAD_KVM_VCPU(r4)
 +.macro __KVM_HANDLER ivor_nr scratch srr0
 stw r3, VCPU_GPR(R3)(r4)
 stw r5, VCPU_GPR(R5)(r4)
 stw r6, VCPU_GPR(R6)(r4)
 @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
 bctr
 .endm
 
 +.macro KVM_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +   /* Get pointer to vcpu and record exit number. */
 +   mtspr   \scratch , r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
 +
 +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +   mtspr   \scratch, r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   stw r3, VCPU_CRIT_SAVE(r4)
 +   mfcrr3
 +   mfspr   r4, SPRN_CSRR1
 +   andi.   r4, r4, MSR_PR
 +   bne 1f
 
 
 +   /* debug interrupt happened in enter/exit path */
 +   mfspr   r4, SPRN_CSRR1
 +   rlwinm  r4, r4, 0, ~MSR_DE
 +   mtspr   SPRN_CSRR1, r4
 +   lis r4, 0x
 +   ori r4, r4, 0x
 +   mtspr   SPRN_DBSR, r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   mtcrr3
 +   lwz r3, VCPU_CRIT_SAVE(r4)
 +   mfspr   r4, \scratch
 +   rfci
 
 What is this part doing? Try to ignore the debug exit?
 
 As BOOKE doesn't have hardware support for virtualization, hardware never 
 know current pc is in guest or in host.
 So when enable hardware single step for guest, it cannot be disabled at the 
 time guest exit. Thus, we'll see that an single step interrupt happens at the 
 beginning of guest exit path.
 
 With the above code we recognize this kind of single step interrupt disable 
 single step and rfci.
 
 Why would we have MSR_DE
 enabled in the first place when we can't handle it?
 
 When QEMU is using hardware debug resource then we always set MSR_DE during 
 guest is running.

Right, but why is MSR_DE enabled during the exit path? If MSR_DE wasn't set, 
you wouldn't get a single step exit. During the exit code path, you could then 
swap DBSR back to what the host expects (which means no single step). Only 
after that enable MSR_DE again.

 
 
 +1: /* debug interrupt happened in guest */
 +   mtcrr3
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   lwz r3, VCPU_CRIT_SAVE(r4)
 +   __KVM_HANDLER \ivor_nr \scratch \srr0
 
 I don't think you need the __KVM_HANDLER split. This should be quite easily
 refactorable into a simple DBG prolog.
 
 Can you please

RE: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-01-31 Thread Bhushan Bharat-R65777


 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, January 31, 2013 5:47 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:
 
 
 
  -Original Message-
  From: Alexander Graf [mailto:ag...@suse.de]
  Sent: Friday, January 25, 2013 5:13 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan
  Bharat-R65777
  Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
  On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
  From: Bharat Bhushan bharat.bhus...@freescale.com
 
  Installed debug handler will be used for guest debug support and
  debug facility emulation features (patches for these features will
  follow this patch).
 
  Signed-off-by: Liu Yu yu@freescale.com
  [bharat.bhus...@freescale.com: Substantial changes]
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  arch/powerpc/include/asm/kvm_host.h |1 +
  arch/powerpc/kernel/asm-offsets.c   |1 +
  arch/powerpc/kvm/booke_interrupts.S |   49 
  ++---
 --
  3 files changed, 44 insertions(+), 7 deletions(-)
 
  diff --git a/arch/powerpc/include/asm/kvm_host.h
  b/arch/powerpc/include/asm/kvm_host.h
  index 8a72d59..f4ba881 100644
  --- a/arch/powerpc/include/asm/kvm_host.h
  +++ b/arch/powerpc/include/asm/kvm_host.h
  @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
u32 tlbcfg[4];
u32 mmucfg;
u32 epr;
  + u32 crit_save;
struct kvmppc_booke_debug_reg dbg_reg; #endif
gpa_t paddr_accessed;
  diff --git a/arch/powerpc/kernel/asm-offsets.c
  b/arch/powerpc/kernel/asm-offsets.c
  index 46f6afd..02048f3 100644
  --- a/arch/powerpc/kernel/asm-offsets.c
  +++ b/arch/powerpc/kernel/asm-offsets.c
  @@ -562,6 +562,7 @@ int main(void)
DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst));
DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear));
DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr));
  + DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save));
  #endif /* CONFIG_PPC_BOOK3S */
  #endif /* CONFIG_KVM */
 
  diff --git a/arch/powerpc/kvm/booke_interrupts.S
  b/arch/powerpc/kvm/booke_interrupts.S
  index eae8483..dd9c5d4 100644
  --- a/arch/powerpc/kvm/booke_interrupts.S
  +++ b/arch/powerpc/kvm/booke_interrupts.S
  @@ -52,12 +52,7 @@
(1BOOKE_INTERRUPT_PROGRAM) | \
(1BOOKE_INTERRUPT_DTLB_MISS))
 
  -.macro KVM_HANDLER ivor_nr scratch srr0
  -_GLOBAL(kvmppc_handler_\ivor_nr)
  - /* Get pointer to vcpu and record exit number. */
  - mtspr   \scratch , r4
  - mfspr   r4, SPRN_SPRG_THREAD
  - lwz r4, THREAD_KVM_VCPU(r4)
  +.macro __KVM_HANDLER ivor_nr scratch srr0
stw r3, VCPU_GPR(R3)(r4)
stw r5, VCPU_GPR(R5)(r4)
stw r6, VCPU_GPR(R6)(r4)
  @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
bctr
  .endm
 
  +.macro KVM_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  + /* Get pointer to vcpu and record exit number. */
  + mtspr   \scratch , r4
  + mfspr   r4, SPRN_SPRG_THREAD
  + lwz r4, THREAD_KVM_VCPU(r4)
  + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
  +
  +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  + mtspr   \scratch, r4
  + mfspr   r4, SPRN_SPRG_THREAD
  + lwz r4, THREAD_KVM_VCPU(r4)
  + stw r3, VCPU_CRIT_SAVE(r4)
  + mfcrr3
  + mfspr   r4, SPRN_CSRR1
  + andi.   r4, r4, MSR_PR
  + bne 1f
 
 
  + /* debug interrupt happened in enter/exit path */
  + mfspr   r4, SPRN_CSRR1
  + rlwinm  r4, r4, 0, ~MSR_DE
  + mtspr   SPRN_CSRR1, r4
  + lis r4, 0x
  + ori r4, r4, 0x
  + mtspr   SPRN_DBSR, r4
  + mfspr   r4, SPRN_SPRG_THREAD
  + lwz r4, THREAD_KVM_VCPU(r4)
  + mtcrr3
  + lwz r3, VCPU_CRIT_SAVE(r4)
  + mfspr   r4, \scratch
  + rfci
 
  What is this part doing? Try to ignore the debug exit?
 
  As BOOKE doesn't have hardware support for virtualization, hardware never 
  know
 current pc is in guest or in host.
  So when enable hardware single step for guest, it cannot be disabled at the
 time guest exit. Thus, we'll see that an single step interrupt happens at the
 beginning of guest exit path.
 
  With the above code we recognize this kind of single step interrupt disable
 single step and rfci.
 
  Why would we have MSR_DE
  enabled in the first place when we can't handle it?
 
  When QEMU is using hardware debug resource then we always set MSR_DE during
 guest is running.
 
 Right, but why is MSR_DE enabled during the exit path? If MSR_DE wasn't set, 
 you
 wouldn't get a single step exit.

We always set MSR_DE in hw MSR when qemu using the debug resource.

 During the exit code path, you could then swap
 DBSR back to what the host expects (which means no single step). Only after 
 that
 enable MSR_DE again.

We

Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-01-31 Thread Alexander Graf

On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote:

 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, January 31, 2013 5:47 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:13 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan
 Bharat-R65777
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
 From: Bharat Bhushan bharat.bhus...@freescale.com
 
 Installed debug handler will be used for guest debug support and
 debug facility emulation features (patches for these features will
 follow this patch).
 
 Signed-off-by: Liu Yu yu@freescale.com
 [bharat.bhus...@freescale.com: Substantial changes]
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 arch/powerpc/include/asm/kvm_host.h |1 +
 arch/powerpc/kernel/asm-offsets.c   |1 +
 arch/powerpc/kvm/booke_interrupts.S |   49 
 ++---
 --
 3 files changed, 44 insertions(+), 7 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_host.h
 b/arch/powerpc/include/asm/kvm_host.h
 index 8a72d59..f4ba881 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
   u32 tlbcfg[4];
   u32 mmucfg;
   u32 epr;
 + u32 crit_save;
   struct kvmppc_booke_debug_reg dbg_reg; #endif
   gpa_t paddr_accessed;
 diff --git a/arch/powerpc/kernel/asm-offsets.c
 b/arch/powerpc/kernel/asm-offsets.c
 index 46f6afd..02048f3 100644
 --- a/arch/powerpc/kernel/asm-offsets.c
 +++ b/arch/powerpc/kernel/asm-offsets.c
 @@ -562,6 +562,7 @@ int main(void)
   DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst));
   DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear));
   DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr));
 + DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save));
 #endif /* CONFIG_PPC_BOOK3S */
 #endif /* CONFIG_KVM */
 
 diff --git a/arch/powerpc/kvm/booke_interrupts.S
 b/arch/powerpc/kvm/booke_interrupts.S
 index eae8483..dd9c5d4 100644
 --- a/arch/powerpc/kvm/booke_interrupts.S
 +++ b/arch/powerpc/kvm/booke_interrupts.S
 @@ -52,12 +52,7 @@
  (1BOOKE_INTERRUPT_PROGRAM) | \
  (1BOOKE_INTERRUPT_DTLB_MISS))
 
 -.macro KVM_HANDLER ivor_nr scratch srr0
 -_GLOBAL(kvmppc_handler_\ivor_nr)
 - /* Get pointer to vcpu and record exit number. */
 - mtspr   \scratch , r4
 - mfspr   r4, SPRN_SPRG_THREAD
 - lwz r4, THREAD_KVM_VCPU(r4)
 +.macro __KVM_HANDLER ivor_nr scratch srr0
   stw r3, VCPU_GPR(R3)(r4)
   stw r5, VCPU_GPR(R5)(r4)
   stw r6, VCPU_GPR(R6)(r4)
 @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
   bctr
 .endm
 
 +.macro KVM_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 + /* Get pointer to vcpu and record exit number. */
 + mtspr   \scratch , r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
 +
 +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 + mtspr   \scratch, r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + stw r3, VCPU_CRIT_SAVE(r4)
 + mfcrr3
 + mfspr   r4, SPRN_CSRR1
 + andi.   r4, r4, MSR_PR
 + bne 1f
 
 
 + /* debug interrupt happened in enter/exit path */
 + mfspr   r4, SPRN_CSRR1
 + rlwinm  r4, r4, 0, ~MSR_DE
 + mtspr   SPRN_CSRR1, r4
 + lis r4, 0x
 + ori r4, r4, 0x
 + mtspr   SPRN_DBSR, r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + mtcrr3
 + lwz r3, VCPU_CRIT_SAVE(r4)
 + mfspr   r4, \scratch
 + rfci
 
 What is this part doing? Try to ignore the debug exit?
 
 As BOOKE doesn't have hardware support for virtualization, hardware never 
 know
 current pc is in guest or in host.
 So when enable hardware single step for guest, it cannot be disabled at the
 time guest exit. Thus, we'll see that an single step interrupt happens at the
 beginning of guest exit path.
 
 With the above code we recognize this kind of single step interrupt disable
 single step and rfci.
 
 Why would we have MSR_DE
 enabled in the first place when we can't handle it?
 
 When QEMU is using hardware debug resource then we always set MSR_DE during
 guest is running.
 
 Right, but why is MSR_DE enabled during the exit path? If MSR_DE wasn't set, 
 you
 wouldn't get a single step exit.
 
 We always set MSR_DE in hw MSR when qemu using the debug resource.

In the _guest_ MSR, yes. But once we exit the guest, it shouldn't be set 
anymore, because we're in an interrupt handler, no? Or is MSR_DE kept alive on 
interrupts?

 
 During the exit code path, you

Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-01-31 Thread Alexander Graf

On 31.01.2013, at 18:08, Alexander Graf wrote:

 
 On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, January 31, 2013 5:47 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:13 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan
 Bharat-R65777
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
 From: Bharat Bhushan bharat.bhus...@freescale.com
 
 Installed debug handler will be used for guest debug support and
 debug facility emulation features (patches for these features will
 follow this patch).
 
 Signed-off-by: Liu Yu yu@freescale.com
 [bharat.bhus...@freescale.com: Substantial changes]
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 arch/powerpc/include/asm/kvm_host.h |1 +
 arch/powerpc/kernel/asm-offsets.c   |1 +
 arch/powerpc/kvm/booke_interrupts.S |   49 
 ++---
 --
 3 files changed, 44 insertions(+), 7 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_host.h
 b/arch/powerpc/include/asm/kvm_host.h
 index 8a72d59..f4ba881 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
  u32 tlbcfg[4];
  u32 mmucfg;
  u32 epr;
 +u32 crit_save;
  struct kvmppc_booke_debug_reg dbg_reg; #endif
  gpa_t paddr_accessed;
 diff --git a/arch/powerpc/kernel/asm-offsets.c
 b/arch/powerpc/kernel/asm-offsets.c
 index 46f6afd..02048f3 100644
 --- a/arch/powerpc/kernel/asm-offsets.c
 +++ b/arch/powerpc/kernel/asm-offsets.c
 @@ -562,6 +562,7 @@ int main(void)
  DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst));
  DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear));
  DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr));
 +DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, 
 arch.crit_save));
 #endif /* CONFIG_PPC_BOOK3S */
 #endif /* CONFIG_KVM */
 
 diff --git a/arch/powerpc/kvm/booke_interrupts.S
 b/arch/powerpc/kvm/booke_interrupts.S
 index eae8483..dd9c5d4 100644
 --- a/arch/powerpc/kvm/booke_interrupts.S
 +++ b/arch/powerpc/kvm/booke_interrupts.S
 @@ -52,12 +52,7 @@
 (1BOOKE_INTERRUPT_PROGRAM) | \
 (1BOOKE_INTERRUPT_DTLB_MISS))
 
 -.macro KVM_HANDLER ivor_nr scratch srr0
 -_GLOBAL(kvmppc_handler_\ivor_nr)
 -/* Get pointer to vcpu and record exit number. */
 -mtspr   \scratch , r4
 -mfspr   r4, SPRN_SPRG_THREAD
 -lwz r4, THREAD_KVM_VCPU(r4)
 +.macro __KVM_HANDLER ivor_nr scratch srr0
  stw r3, VCPU_GPR(R3)(r4)
  stw r5, VCPU_GPR(R5)(r4)
  stw r6, VCPU_GPR(R6)(r4)
 @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
  bctr
 .endm
 
 +.macro KVM_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +/* Get pointer to vcpu and record exit number. */
 +mtspr   \scratch , r4
 +mfspr   r4, SPRN_SPRG_THREAD
 +lwz r4, THREAD_KVM_VCPU(r4)
 +__KVM_HANDLER \ivor_nr \scratch \srr0 .endm
 +
 +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +mtspr   \scratch, r4
 +mfspr   r4, SPRN_SPRG_THREAD
 +lwz r4, THREAD_KVM_VCPU(r4)
 +stw r3, VCPU_CRIT_SAVE(r4)
 +mfcrr3
 +mfspr   r4, SPRN_CSRR1
 +andi.   r4, r4, MSR_PR
 +bne 1f
 
 
 +/* debug interrupt happened in enter/exit path */
 +mfspr   r4, SPRN_CSRR1
 +rlwinm  r4, r4, 0, ~MSR_DE
 +mtspr   SPRN_CSRR1, r4
 +lis r4, 0x
 +ori r4, r4, 0x
 +mtspr   SPRN_DBSR, r4
 +mfspr   r4, SPRN_SPRG_THREAD
 +lwz r4, THREAD_KVM_VCPU(r4)
 +mtcrr3
 +lwz r3, VCPU_CRIT_SAVE(r4)
 +mfspr   r4, \scratch
 +rfci
 
 What is this part doing? Try to ignore the debug exit?
 
 As BOOKE doesn't have hardware support for virtualization, hardware never 
 know
 current pc is in guest or in host.
 So when enable hardware single step for guest, it cannot be disabled at the
 time guest exit. Thus, we'll see that an single step interrupt happens at 
 the
 beginning of guest exit path.
 
 With the above code we recognize this kind of single step interrupt disable
 single step and rfci.
 
 Why would we have MSR_DE
 enabled in the first place when we can't handle it?
 
 When QEMU is using hardware debug resource then we always set MSR_DE during
 guest is running.
 
 Right, but why is MSR_DE enabled during the exit path? If MSR_DE wasn't 
 set, you
 wouldn't get a single step exit.
 
 We

RE: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-01-31 Thread Bhushan Bharat-R65777


 -Original Message-
 From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On
 Behalf Of Alexander Graf
 Sent: Thursday, January 31, 2013 10:38 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote:
 
 
 
  -Original Message-
  From: Alexander Graf [mailto:ag...@suse.de]
  Sent: Thursday, January 31, 2013 5:47 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org
  Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
  On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:
 
 
 
  -Original Message-
  From: Alexander Graf [mailto:ag...@suse.de]
  Sent: Friday, January 25, 2013 5:13 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan
  Bharat-R65777
  Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
  On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
  From: Bharat Bhushan bharat.bhus...@freescale.com
 
  Installed debug handler will be used for guest debug support and
  debug facility emulation features (patches for these features will
  follow this patch).
 
  Signed-off-by: Liu Yu yu@freescale.com
  [bharat.bhus...@freescale.com: Substantial changes]
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  arch/powerpc/include/asm/kvm_host.h |1 +
  arch/powerpc/kernel/asm-offsets.c   |1 +
  arch/powerpc/kvm/booke_interrupts.S |   49 
  ++-
 --
  --
  3 files changed, 44 insertions(+), 7 deletions(-)
 
  diff --git a/arch/powerpc/include/asm/kvm_host.h
  b/arch/powerpc/include/asm/kvm_host.h
  index 8a72d59..f4ba881 100644
  --- a/arch/powerpc/include/asm/kvm_host.h
  +++ b/arch/powerpc/include/asm/kvm_host.h
  @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
  u32 tlbcfg[4];
  u32 mmucfg;
  u32 epr;
  +   u32 crit_save;
  struct kvmppc_booke_debug_reg dbg_reg; #endif
  gpa_t paddr_accessed;
  diff --git a/arch/powerpc/kernel/asm-offsets.c
  b/arch/powerpc/kernel/asm-offsets.c
  index 46f6afd..02048f3 100644
  --- a/arch/powerpc/kernel/asm-offsets.c
  +++ b/arch/powerpc/kernel/asm-offsets.c
  @@ -562,6 +562,7 @@ int main(void)
  DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, 
  arch.last_inst));
  DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, 
  arch.fault_dear));
  DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu,
  arch.fault_esr));
  +   DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu,
  +arch.crit_save));
  #endif /* CONFIG_PPC_BOOK3S */
  #endif /* CONFIG_KVM */
 
  diff --git a/arch/powerpc/kvm/booke_interrupts.S
  b/arch/powerpc/kvm/booke_interrupts.S
  index eae8483..dd9c5d4 100644
  --- a/arch/powerpc/kvm/booke_interrupts.S
  +++ b/arch/powerpc/kvm/booke_interrupts.S
  @@ -52,12 +52,7 @@
   (1BOOKE_INTERRUPT_PROGRAM) | \
   (1BOOKE_INTERRUPT_DTLB_MISS))
 
  -.macro KVM_HANDLER ivor_nr scratch srr0
  -_GLOBAL(kvmppc_handler_\ivor_nr)
  -   /* Get pointer to vcpu and record exit number. */
  -   mtspr   \scratch , r4
  -   mfspr   r4, SPRN_SPRG_THREAD
  -   lwz r4, THREAD_KVM_VCPU(r4)
  +.macro __KVM_HANDLER ivor_nr scratch srr0
  stw r3, VCPU_GPR(R3)(r4)
  stw r5, VCPU_GPR(R5)(r4)
  stw r6, VCPU_GPR(R6)(r4)
  @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
  bctr
  .endm
 
  +.macro KVM_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  +   /* Get pointer to vcpu and record exit number. */
  +   mtspr   \scratch , r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
  +
  +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  +   mtspr   \scratch, r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   stw r3, VCPU_CRIT_SAVE(r4)
  +   mfcrr3
  +   mfspr   r4, SPRN_CSRR1
  +   andi.   r4, r4, MSR_PR
  +   bne 1f
 
 
  +   /* debug interrupt happened in enter/exit path */
  +   mfspr   r4, SPRN_CSRR1
  +   rlwinm  r4, r4, 0, ~MSR_DE
  +   mtspr   SPRN_CSRR1, r4
  +   lis r4, 0x
  +   ori r4, r4, 0x
  +   mtspr   SPRN_DBSR, r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   mtcrr3
  +   lwz r3, VCPU_CRIT_SAVE(r4)
  +   mfspr   r4, \scratch
  +   rfci
 
  What is this part doing? Try to ignore the debug exit?
 
  As BOOKE doesn't have hardware support for virtualization, hardware
  never know
  current pc is in guest or in host.
  So when enable hardware single step for guest, it cannot be disabled
  at the
  time guest exit. Thus, we'll see that an single step interrupt
  happens

Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-01-31 Thread Alexander Graf

On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:

 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:13 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
 From: Bharat Bhushan bharat.bhus...@freescale.com
 
 Installed debug handler will be used for guest debug support and debug
 facility emulation features (patches for these features will follow
 this patch).
 
 Signed-off-by: Liu Yu yu@freescale.com
 [bharat.bhus...@freescale.com: Substantial changes]
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 arch/powerpc/include/asm/kvm_host.h |1 +
 arch/powerpc/kernel/asm-offsets.c   |1 +
 arch/powerpc/kvm/booke_interrupts.S |   49 
 ++-
 3 files changed, 44 insertions(+), 7 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_host.h
 b/arch/powerpc/include/asm/kvm_host.h
 index 8a72d59..f4ba881 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
 u32 tlbcfg[4];
 u32 mmucfg;
 u32 epr;
 +   u32 crit_save;
 struct kvmppc_booke_debug_reg dbg_reg; #endif
 gpa_t paddr_accessed;
 diff --git a/arch/powerpc/kernel/asm-offsets.c
 b/arch/powerpc/kernel/asm-offsets.c
 index 46f6afd..02048f3 100644
 --- a/arch/powerpc/kernel/asm-offsets.c
 +++ b/arch/powerpc/kernel/asm-offsets.c
 @@ -562,6 +562,7 @@ int main(void)
 DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst));
 DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear));
 DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr));
 +   DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save));
 #endif /* CONFIG_PPC_BOOK3S */
 #endif /* CONFIG_KVM */
 
 diff --git a/arch/powerpc/kvm/booke_interrupts.S
 b/arch/powerpc/kvm/booke_interrupts.S
 index eae8483..dd9c5d4 100644
 --- a/arch/powerpc/kvm/booke_interrupts.S
 +++ b/arch/powerpc/kvm/booke_interrupts.S
 @@ -52,12 +52,7 @@
   (1BOOKE_INTERRUPT_PROGRAM) | \
   (1BOOKE_INTERRUPT_DTLB_MISS))
 
 -.macro KVM_HANDLER ivor_nr scratch srr0
 -_GLOBAL(kvmppc_handler_\ivor_nr)
 -   /* Get pointer to vcpu and record exit number. */
 -   mtspr   \scratch , r4
 -   mfspr   r4, SPRN_SPRG_THREAD
 -   lwz r4, THREAD_KVM_VCPU(r4)
 +.macro __KVM_HANDLER ivor_nr scratch srr0
 stw r3, VCPU_GPR(R3)(r4)
 stw r5, VCPU_GPR(R5)(r4)
 stw r6, VCPU_GPR(R6)(r4)
 @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
 bctr
 .endm
 
 +.macro KVM_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +   /* Get pointer to vcpu and record exit number. */
 +   mtspr   \scratch , r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
 +
 +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +   mtspr   \scratch, r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   stw r3, VCPU_CRIT_SAVE(r4)
 +   mfcrr3
 +   mfspr   r4, SPRN_CSRR1
 +   andi.   r4, r4, MSR_PR
 +   bne 1f
 
 
 +   /* debug interrupt happened in enter/exit path */
 +   mfspr   r4, SPRN_CSRR1
 +   rlwinm  r4, r4, 0, ~MSR_DE
 +   mtspr   SPRN_CSRR1, r4
 +   lis r4, 0x
 +   ori r4, r4, 0x
 +   mtspr   SPRN_DBSR, r4
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   mtcrr3
 +   lwz r3, VCPU_CRIT_SAVE(r4)
 +   mfspr   r4, \scratch
 +   rfci
 
 What is this part doing? Try to ignore the debug exit?
 
 As BOOKE doesn't have hardware support for virtualization, hardware never 
 know current pc is in guest or in host.
 So when enable hardware single step for guest, it cannot be disabled at the 
 time guest exit. Thus, we'll see that an single step interrupt happens at the 
 beginning of guest exit path.
 
 With the above code we recognize this kind of single step interrupt disable 
 single step and rfci.
 
 Why would we have MSR_DE
 enabled in the first place when we can't handle it?
 
 When QEMU is using hardware debug resource then we always set MSR_DE during 
 guest is running.

Right, but why is MSR_DE enabled during the exit path? If MSR_DE wasn't set, 
you wouldn't get a single step exit. During the exit code path, you could then 
swap DBSR back to what the host expects (which means no single step). Only 
after that enable MSR_DE again.

 
 
 +1: /* debug interrupt happened in guest */
 +   mtcrr3
 +   mfspr   r4, SPRN_SPRG_THREAD
 +   lwz r4, THREAD_KVM_VCPU(r4)
 +   lwz r3, VCPU_CRIT_SAVE(r4)
 +   __KVM_HANDLER \ivor_nr \scratch \srr0
 
 I don't think you need the __KVM_HANDLER split. This should be quite easily
 refactorable into a simple DBG prolog.
 
 Can you please

Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-01-31 Thread Alexander Graf

On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote:

 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, January 31, 2013 5:47 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:13 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan
 Bharat-R65777
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
 From: Bharat Bhushan bharat.bhus...@freescale.com
 
 Installed debug handler will be used for guest debug support and
 debug facility emulation features (patches for these features will
 follow this patch).
 
 Signed-off-by: Liu Yu yu@freescale.com
 [bharat.bhus...@freescale.com: Substantial changes]
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 arch/powerpc/include/asm/kvm_host.h |1 +
 arch/powerpc/kernel/asm-offsets.c   |1 +
 arch/powerpc/kvm/booke_interrupts.S |   49 
 ++---
 --
 3 files changed, 44 insertions(+), 7 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_host.h
 b/arch/powerpc/include/asm/kvm_host.h
 index 8a72d59..f4ba881 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
   u32 tlbcfg[4];
   u32 mmucfg;
   u32 epr;
 + u32 crit_save;
   struct kvmppc_booke_debug_reg dbg_reg; #endif
   gpa_t paddr_accessed;
 diff --git a/arch/powerpc/kernel/asm-offsets.c
 b/arch/powerpc/kernel/asm-offsets.c
 index 46f6afd..02048f3 100644
 --- a/arch/powerpc/kernel/asm-offsets.c
 +++ b/arch/powerpc/kernel/asm-offsets.c
 @@ -562,6 +562,7 @@ int main(void)
   DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst));
   DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear));
   DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr));
 + DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save));
 #endif /* CONFIG_PPC_BOOK3S */
 #endif /* CONFIG_KVM */
 
 diff --git a/arch/powerpc/kvm/booke_interrupts.S
 b/arch/powerpc/kvm/booke_interrupts.S
 index eae8483..dd9c5d4 100644
 --- a/arch/powerpc/kvm/booke_interrupts.S
 +++ b/arch/powerpc/kvm/booke_interrupts.S
 @@ -52,12 +52,7 @@
  (1BOOKE_INTERRUPT_PROGRAM) | \
  (1BOOKE_INTERRUPT_DTLB_MISS))
 
 -.macro KVM_HANDLER ivor_nr scratch srr0
 -_GLOBAL(kvmppc_handler_\ivor_nr)
 - /* Get pointer to vcpu and record exit number. */
 - mtspr   \scratch , r4
 - mfspr   r4, SPRN_SPRG_THREAD
 - lwz r4, THREAD_KVM_VCPU(r4)
 +.macro __KVM_HANDLER ivor_nr scratch srr0
   stw r3, VCPU_GPR(R3)(r4)
   stw r5, VCPU_GPR(R5)(r4)
   stw r6, VCPU_GPR(R6)(r4)
 @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
   bctr
 .endm
 
 +.macro KVM_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 + /* Get pointer to vcpu and record exit number. */
 + mtspr   \scratch , r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
 +
 +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 + mtspr   \scratch, r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + stw r3, VCPU_CRIT_SAVE(r4)
 + mfcrr3
 + mfspr   r4, SPRN_CSRR1
 + andi.   r4, r4, MSR_PR
 + bne 1f
 
 
 + /* debug interrupt happened in enter/exit path */
 + mfspr   r4, SPRN_CSRR1
 + rlwinm  r4, r4, 0, ~MSR_DE
 + mtspr   SPRN_CSRR1, r4
 + lis r4, 0x
 + ori r4, r4, 0x
 + mtspr   SPRN_DBSR, r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + mtcrr3
 + lwz r3, VCPU_CRIT_SAVE(r4)
 + mfspr   r4, \scratch
 + rfci
 
 What is this part doing? Try to ignore the debug exit?
 
 As BOOKE doesn't have hardware support for virtualization, hardware never 
 know
 current pc is in guest or in host.
 So when enable hardware single step for guest, it cannot be disabled at the
 time guest exit. Thus, we'll see that an single step interrupt happens at the
 beginning of guest exit path.
 
 With the above code we recognize this kind of single step interrupt disable
 single step and rfci.
 
 Why would we have MSR_DE
 enabled in the first place when we can't handle it?
 
 When QEMU is using hardware debug resource then we always set MSR_DE during
 guest is running.
 
 Right, but why is MSR_DE enabled during the exit path? If MSR_DE wasn't set, 
 you
 wouldn't get a single step exit.
 
 We always set MSR_DE in hw MSR when qemu using the debug resource.

In the _guest_ MSR, yes. But once we exit the guest, it shouldn't be set 
anymore, because we're in an interrupt handler, no? Or is MSR_DE kept alive on 
interrupts?

 
 During the exit code path, you

Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-01-31 Thread Alexander Graf

On 31.01.2013, at 18:08, Alexander Graf wrote:

 
 On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Thursday, January 31, 2013 5:47 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:
 
 
 
 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:13 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan
 Bharat-R65777
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
 From: Bharat Bhushan bharat.bhus...@freescale.com
 
 Installed debug handler will be used for guest debug support and
 debug facility emulation features (patches for these features will
 follow this patch).
 
 Signed-off-by: Liu Yu yu@freescale.com
 [bharat.bhus...@freescale.com: Substantial changes]
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 arch/powerpc/include/asm/kvm_host.h |1 +
 arch/powerpc/kernel/asm-offsets.c   |1 +
 arch/powerpc/kvm/booke_interrupts.S |   49 
 ++---
 --
 3 files changed, 44 insertions(+), 7 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_host.h
 b/arch/powerpc/include/asm/kvm_host.h
 index 8a72d59..f4ba881 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
  u32 tlbcfg[4];
  u32 mmucfg;
  u32 epr;
 +u32 crit_save;
  struct kvmppc_booke_debug_reg dbg_reg; #endif
  gpa_t paddr_accessed;
 diff --git a/arch/powerpc/kernel/asm-offsets.c
 b/arch/powerpc/kernel/asm-offsets.c
 index 46f6afd..02048f3 100644
 --- a/arch/powerpc/kernel/asm-offsets.c
 +++ b/arch/powerpc/kernel/asm-offsets.c
 @@ -562,6 +562,7 @@ int main(void)
  DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst));
  DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear));
  DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr));
 +DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, 
 arch.crit_save));
 #endif /* CONFIG_PPC_BOOK3S */
 #endif /* CONFIG_KVM */
 
 diff --git a/arch/powerpc/kvm/booke_interrupts.S
 b/arch/powerpc/kvm/booke_interrupts.S
 index eae8483..dd9c5d4 100644
 --- a/arch/powerpc/kvm/booke_interrupts.S
 +++ b/arch/powerpc/kvm/booke_interrupts.S
 @@ -52,12 +52,7 @@
 (1BOOKE_INTERRUPT_PROGRAM) | \
 (1BOOKE_INTERRUPT_DTLB_MISS))
 
 -.macro KVM_HANDLER ivor_nr scratch srr0
 -_GLOBAL(kvmppc_handler_\ivor_nr)
 -/* Get pointer to vcpu and record exit number. */
 -mtspr   \scratch , r4
 -mfspr   r4, SPRN_SPRG_THREAD
 -lwz r4, THREAD_KVM_VCPU(r4)
 +.macro __KVM_HANDLER ivor_nr scratch srr0
  stw r3, VCPU_GPR(R3)(r4)
  stw r5, VCPU_GPR(R5)(r4)
  stw r6, VCPU_GPR(R6)(r4)
 @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
  bctr
 .endm
 
 +.macro KVM_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +/* Get pointer to vcpu and record exit number. */
 +mtspr   \scratch , r4
 +mfspr   r4, SPRN_SPRG_THREAD
 +lwz r4, THREAD_KVM_VCPU(r4)
 +__KVM_HANDLER \ivor_nr \scratch \srr0 .endm
 +
 +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 +mtspr   \scratch, r4
 +mfspr   r4, SPRN_SPRG_THREAD
 +lwz r4, THREAD_KVM_VCPU(r4)
 +stw r3, VCPU_CRIT_SAVE(r4)
 +mfcrr3
 +mfspr   r4, SPRN_CSRR1
 +andi.   r4, r4, MSR_PR
 +bne 1f
 
 
 +/* debug interrupt happened in enter/exit path */
 +mfspr   r4, SPRN_CSRR1
 +rlwinm  r4, r4, 0, ~MSR_DE
 +mtspr   SPRN_CSRR1, r4
 +lis r4, 0x
 +ori r4, r4, 0x
 +mtspr   SPRN_DBSR, r4
 +mfspr   r4, SPRN_SPRG_THREAD
 +lwz r4, THREAD_KVM_VCPU(r4)
 +mtcrr3
 +lwz r3, VCPU_CRIT_SAVE(r4)
 +mfspr   r4, \scratch
 +rfci
 
 What is this part doing? Try to ignore the debug exit?
 
 As BOOKE doesn't have hardware support for virtualization, hardware never 
 know
 current pc is in guest or in host.
 So when enable hardware single step for guest, it cannot be disabled at the
 time guest exit. Thus, we'll see that an single step interrupt happens at 
 the
 beginning of guest exit path.
 
 With the above code we recognize this kind of single step interrupt disable
 single step and rfci.
 
 Why would we have MSR_DE
 enabled in the first place when we can't handle it?
 
 When QEMU is using hardware debug resource then we always set MSR_DE during
 guest is running.
 
 Right, but why is MSR_DE enabled during the exit path? If MSR_DE wasn't 
 set, you
 wouldn't get a single step exit.
 
 We

RE: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-01-31 Thread Bhushan Bharat-R65777


 -Original Message-
 From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] On
 Behalf Of Alexander Graf
 Sent: Thursday, January 31, 2013 10:38 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 31.01.2013, at 17:58, Bhushan Bharat-R65777 wrote:
 
 
 
  -Original Message-
  From: Alexander Graf [mailto:ag...@suse.de]
  Sent: Thursday, January 31, 2013 5:47 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org
  Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
  On 30.01.2013, at 12:30, Bhushan Bharat-R65777 wrote:
 
 
 
  -Original Message-
  From: Alexander Graf [mailto:ag...@suse.de]
  Sent: Friday, January 25, 2013 5:13 PM
  To: Bhushan Bharat-R65777
  Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan
  Bharat-R65777
  Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
  On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
  From: Bharat Bhushan bharat.bhus...@freescale.com
 
  Installed debug handler will be used for guest debug support and
  debug facility emulation features (patches for these features will
  follow this patch).
 
  Signed-off-by: Liu Yu yu@freescale.com
  [bharat.bhus...@freescale.com: Substantial changes]
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  arch/powerpc/include/asm/kvm_host.h |1 +
  arch/powerpc/kernel/asm-offsets.c   |1 +
  arch/powerpc/kvm/booke_interrupts.S |   49 
  ++-
 --
  --
  3 files changed, 44 insertions(+), 7 deletions(-)
 
  diff --git a/arch/powerpc/include/asm/kvm_host.h
  b/arch/powerpc/include/asm/kvm_host.h
  index 8a72d59..f4ba881 100644
  --- a/arch/powerpc/include/asm/kvm_host.h
  +++ b/arch/powerpc/include/asm/kvm_host.h
  @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
  u32 tlbcfg[4];
  u32 mmucfg;
  u32 epr;
  +   u32 crit_save;
  struct kvmppc_booke_debug_reg dbg_reg; #endif
  gpa_t paddr_accessed;
  diff --git a/arch/powerpc/kernel/asm-offsets.c
  b/arch/powerpc/kernel/asm-offsets.c
  index 46f6afd..02048f3 100644
  --- a/arch/powerpc/kernel/asm-offsets.c
  +++ b/arch/powerpc/kernel/asm-offsets.c
  @@ -562,6 +562,7 @@ int main(void)
  DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, 
  arch.last_inst));
  DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, 
  arch.fault_dear));
  DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu,
  arch.fault_esr));
  +   DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu,
  +arch.crit_save));
  #endif /* CONFIG_PPC_BOOK3S */
  #endif /* CONFIG_KVM */
 
  diff --git a/arch/powerpc/kvm/booke_interrupts.S
  b/arch/powerpc/kvm/booke_interrupts.S
  index eae8483..dd9c5d4 100644
  --- a/arch/powerpc/kvm/booke_interrupts.S
  +++ b/arch/powerpc/kvm/booke_interrupts.S
  @@ -52,12 +52,7 @@
   (1BOOKE_INTERRUPT_PROGRAM) | \
   (1BOOKE_INTERRUPT_DTLB_MISS))
 
  -.macro KVM_HANDLER ivor_nr scratch srr0
  -_GLOBAL(kvmppc_handler_\ivor_nr)
  -   /* Get pointer to vcpu and record exit number. */
  -   mtspr   \scratch , r4
  -   mfspr   r4, SPRN_SPRG_THREAD
  -   lwz r4, THREAD_KVM_VCPU(r4)
  +.macro __KVM_HANDLER ivor_nr scratch srr0
  stw r3, VCPU_GPR(R3)(r4)
  stw r5, VCPU_GPR(R5)(r4)
  stw r6, VCPU_GPR(R6)(r4)
  @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
  bctr
  .endm
 
  +.macro KVM_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  +   /* Get pointer to vcpu and record exit number. */
  +   mtspr   \scratch , r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
  +
  +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  +   mtspr   \scratch, r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   stw r3, VCPU_CRIT_SAVE(r4)
  +   mfcrr3
  +   mfspr   r4, SPRN_CSRR1
  +   andi.   r4, r4, MSR_PR
  +   bne 1f
 
 
  +   /* debug interrupt happened in enter/exit path */
  +   mfspr   r4, SPRN_CSRR1
  +   rlwinm  r4, r4, 0, ~MSR_DE
  +   mtspr   SPRN_CSRR1, r4
  +   lis r4, 0x
  +   ori r4, r4, 0x
  +   mtspr   SPRN_DBSR, r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   mtcrr3
  +   lwz r3, VCPU_CRIT_SAVE(r4)
  +   mfspr   r4, \scratch
  +   rfci
 
  What is this part doing? Try to ignore the debug exit?
 
  As BOOKE doesn't have hardware support for virtualization, hardware
  never know
  current pc is in guest or in host.
  So when enable hardware single step for guest, it cannot be disabled
  at the
  time guest exit. Thus, we'll see that an single step interrupt

RE: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-01-30 Thread Bhushan Bharat-R65777


 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:13 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; Bhushan Bharat-R65777
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
  From: Bharat Bhushan bharat.bhus...@freescale.com
 
  Installed debug handler will be used for guest debug support and debug
  facility emulation features (patches for these features will follow
  this patch).
 
  Signed-off-by: Liu Yu yu@freescale.com
  [bharat.bhus...@freescale.com: Substantial changes]
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  arch/powerpc/include/asm/kvm_host.h |1 +
  arch/powerpc/kernel/asm-offsets.c   |1 +
  arch/powerpc/kvm/booke_interrupts.S |   49 
  ++-
  3 files changed, 44 insertions(+), 7 deletions(-)
 
  diff --git a/arch/powerpc/include/asm/kvm_host.h
  b/arch/powerpc/include/asm/kvm_host.h
  index 8a72d59..f4ba881 100644
  --- a/arch/powerpc/include/asm/kvm_host.h
  +++ b/arch/powerpc/include/asm/kvm_host.h
  @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
  u32 tlbcfg[4];
  u32 mmucfg;
  u32 epr;
  +   u32 crit_save;
  struct kvmppc_booke_debug_reg dbg_reg; #endif
  gpa_t paddr_accessed;
  diff --git a/arch/powerpc/kernel/asm-offsets.c
  b/arch/powerpc/kernel/asm-offsets.c
  index 46f6afd..02048f3 100644
  --- a/arch/powerpc/kernel/asm-offsets.c
  +++ b/arch/powerpc/kernel/asm-offsets.c
  @@ -562,6 +562,7 @@ int main(void)
  DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst));
  DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear));
  DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr));
  +   DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save));
  #endif /* CONFIG_PPC_BOOK3S */
  #endif /* CONFIG_KVM */
 
  diff --git a/arch/powerpc/kvm/booke_interrupts.S
  b/arch/powerpc/kvm/booke_interrupts.S
  index eae8483..dd9c5d4 100644
  --- a/arch/powerpc/kvm/booke_interrupts.S
  +++ b/arch/powerpc/kvm/booke_interrupts.S
  @@ -52,12 +52,7 @@
 (1BOOKE_INTERRUPT_PROGRAM) | \
 (1BOOKE_INTERRUPT_DTLB_MISS))
 
  -.macro KVM_HANDLER ivor_nr scratch srr0
  -_GLOBAL(kvmppc_handler_\ivor_nr)
  -   /* Get pointer to vcpu and record exit number. */
  -   mtspr   \scratch , r4
  -   mfspr   r4, SPRN_SPRG_THREAD
  -   lwz r4, THREAD_KVM_VCPU(r4)
  +.macro __KVM_HANDLER ivor_nr scratch srr0
  stw r3, VCPU_GPR(R3)(r4)
  stw r5, VCPU_GPR(R5)(r4)
  stw r6, VCPU_GPR(R6)(r4)
  @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
  bctr
  .endm
 
  +.macro KVM_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  +   /* Get pointer to vcpu and record exit number. */
  +   mtspr   \scratch , r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
  +
  +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  +   mtspr   \scratch, r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   stw r3, VCPU_CRIT_SAVE(r4)
  +   mfcrr3
  +   mfspr   r4, SPRN_CSRR1
  +   andi.   r4, r4, MSR_PR
  +   bne 1f
 
 
  +   /* debug interrupt happened in enter/exit path */
  +   mfspr   r4, SPRN_CSRR1
  +   rlwinm  r4, r4, 0, ~MSR_DE
  +   mtspr   SPRN_CSRR1, r4
  +   lis r4, 0x
  +   ori r4, r4, 0x
  +   mtspr   SPRN_DBSR, r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   mtcrr3
  +   lwz r3, VCPU_CRIT_SAVE(r4)
  +   mfspr   r4, \scratch
  +   rfci
 
 What is this part doing? Try to ignore the debug exit?

As BOOKE doesn't have hardware support for virtualization, hardware never know 
current pc is in guest or in host.
So when enable hardware single step for guest, it cannot be disabled at the 
time guest exit. Thus, we'll see that an single step interrupt happens at the 
beginning of guest exit path.

With the above code we recognize this kind of single step interrupt disable 
single step and rfci.

 Why would we have MSR_DE
 enabled in the first place when we can't handle it?

When QEMU is using hardware debug resource then we always set MSR_DE during 
guest is running.

 
  +1: /* debug interrupt happened in guest */
  +   mtcrr3
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   lwz r3, VCPU_CRIT_SAVE(r4)
  +   __KVM_HANDLER \ivor_nr \scratch \srr0
 
 I don't think you need the __KVM_HANDLER split. This should be quite easily
 refactorable into a simple DBG prolog.

Can you please elaborate how you are envisioning this?

Thanks
-Bharat

 
 
 Alex
 
  +.endm
  +
  .macro KVM_HANDLER_ADDR ivor_nr
  .long   kvmppc_handler_\ivor_nr
  .endm
  @@ -98,7 +133,7 @@ KVM_HANDLER BOOKE_INTERRUPT_FIT SPRN_SPRG_RSCRATCH0
  SPRN_SRR0

RE: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-01-30 Thread Bhushan Bharat-R65777


 -Original Message-
 From: Alexander Graf [mailto:ag...@suse.de]
 Sent: Friday, January 25, 2013 5:13 PM
 To: Bhushan Bharat-R65777
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; Bhushan Bharat-R65777
 Subject: Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler
 
 
 On 16.01.2013, at 09:24, Bharat Bhushan wrote:
 
  From: Bharat Bhushan bharat.bhus...@freescale.com
 
  Installed debug handler will be used for guest debug support and debug
  facility emulation features (patches for these features will follow
  this patch).
 
  Signed-off-by: Liu Yu yu@freescale.com
  [bharat.bhus...@freescale.com: Substantial changes]
  Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
  ---
  arch/powerpc/include/asm/kvm_host.h |1 +
  arch/powerpc/kernel/asm-offsets.c   |1 +
  arch/powerpc/kvm/booke_interrupts.S |   49 
  ++-
  3 files changed, 44 insertions(+), 7 deletions(-)
 
  diff --git a/arch/powerpc/include/asm/kvm_host.h
  b/arch/powerpc/include/asm/kvm_host.h
  index 8a72d59..f4ba881 100644
  --- a/arch/powerpc/include/asm/kvm_host.h
  +++ b/arch/powerpc/include/asm/kvm_host.h
  @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
  u32 tlbcfg[4];
  u32 mmucfg;
  u32 epr;
  +   u32 crit_save;
  struct kvmppc_booke_debug_reg dbg_reg; #endif
  gpa_t paddr_accessed;
  diff --git a/arch/powerpc/kernel/asm-offsets.c
  b/arch/powerpc/kernel/asm-offsets.c
  index 46f6afd..02048f3 100644
  --- a/arch/powerpc/kernel/asm-offsets.c
  +++ b/arch/powerpc/kernel/asm-offsets.c
  @@ -562,6 +562,7 @@ int main(void)
  DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst));
  DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear));
  DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr));
  +   DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save));
  #endif /* CONFIG_PPC_BOOK3S */
  #endif /* CONFIG_KVM */
 
  diff --git a/arch/powerpc/kvm/booke_interrupts.S
  b/arch/powerpc/kvm/booke_interrupts.S
  index eae8483..dd9c5d4 100644
  --- a/arch/powerpc/kvm/booke_interrupts.S
  +++ b/arch/powerpc/kvm/booke_interrupts.S
  @@ -52,12 +52,7 @@
 (1BOOKE_INTERRUPT_PROGRAM) | \
 (1BOOKE_INTERRUPT_DTLB_MISS))
 
  -.macro KVM_HANDLER ivor_nr scratch srr0
  -_GLOBAL(kvmppc_handler_\ivor_nr)
  -   /* Get pointer to vcpu and record exit number. */
  -   mtspr   \scratch , r4
  -   mfspr   r4, SPRN_SPRG_THREAD
  -   lwz r4, THREAD_KVM_VCPU(r4)
  +.macro __KVM_HANDLER ivor_nr scratch srr0
  stw r3, VCPU_GPR(R3)(r4)
  stw r5, VCPU_GPR(R5)(r4)
  stw r6, VCPU_GPR(R6)(r4)
  @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
  bctr
  .endm
 
  +.macro KVM_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  +   /* Get pointer to vcpu and record exit number. */
  +   mtspr   \scratch , r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   __KVM_HANDLER \ivor_nr \scratch \srr0 .endm
  +
  +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
  +_GLOBAL(kvmppc_handler_\ivor_nr)
  +   mtspr   \scratch, r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   stw r3, VCPU_CRIT_SAVE(r4)
  +   mfcrr3
  +   mfspr   r4, SPRN_CSRR1
  +   andi.   r4, r4, MSR_PR
  +   bne 1f
 
 
  +   /* debug interrupt happened in enter/exit path */
  +   mfspr   r4, SPRN_CSRR1
  +   rlwinm  r4, r4, 0, ~MSR_DE
  +   mtspr   SPRN_CSRR1, r4
  +   lis r4, 0x
  +   ori r4, r4, 0x
  +   mtspr   SPRN_DBSR, r4
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   mtcrr3
  +   lwz r3, VCPU_CRIT_SAVE(r4)
  +   mfspr   r4, \scratch
  +   rfci
 
 What is this part doing? Try to ignore the debug exit?

As BOOKE doesn't have hardware support for virtualization, hardware never know 
current pc is in guest or in host.
So when enable hardware single step for guest, it cannot be disabled at the 
time guest exit. Thus, we'll see that an single step interrupt happens at the 
beginning of guest exit path.

With the above code we recognize this kind of single step interrupt disable 
single step and rfci.

 Why would we have MSR_DE
 enabled in the first place when we can't handle it?

When QEMU is using hardware debug resource then we always set MSR_DE during 
guest is running.

 
  +1: /* debug interrupt happened in guest */
  +   mtcrr3
  +   mfspr   r4, SPRN_SPRG_THREAD
  +   lwz r4, THREAD_KVM_VCPU(r4)
  +   lwz r3, VCPU_CRIT_SAVE(r4)
  +   __KVM_HANDLER \ivor_nr \scratch \srr0
 
 I don't think you need the __KVM_HANDLER split. This should be quite easily
 refactorable into a simple DBG prolog.

Can you please elaborate how you are envisioning this?

Thanks
-Bharat

 
 
 Alex
 
  +.endm
  +
  .macro KVM_HANDLER_ADDR ivor_nr
  .long   kvmppc_handler_\ivor_nr
  .endm
  @@ -98,7 +133,7 @@ KVM_HANDLER BOOKE_INTERRUPT_FIT SPRN_SPRG_RSCRATCH0
  SPRN_SRR0

Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-01-25 Thread Alexander Graf

On 16.01.2013, at 09:24, Bharat Bhushan wrote:

 From: Bharat Bhushan bharat.bhus...@freescale.com
 
 Installed debug handler will be used for guest debug support
 and debug facility emulation features (patches for these
 features will follow this patch).
 
 Signed-off-by: Liu Yu yu@freescale.com
 [bharat.bhus...@freescale.com: Substantial changes]
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 arch/powerpc/include/asm/kvm_host.h |1 +
 arch/powerpc/kernel/asm-offsets.c   |1 +
 arch/powerpc/kvm/booke_interrupts.S |   49 ++-
 3 files changed, 44 insertions(+), 7 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_host.h 
 b/arch/powerpc/include/asm/kvm_host.h
 index 8a72d59..f4ba881 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
   u32 tlbcfg[4];
   u32 mmucfg;
   u32 epr;
 + u32 crit_save;
   struct kvmppc_booke_debug_reg dbg_reg;
 #endif
   gpa_t paddr_accessed;
 diff --git a/arch/powerpc/kernel/asm-offsets.c 
 b/arch/powerpc/kernel/asm-offsets.c
 index 46f6afd..02048f3 100644
 --- a/arch/powerpc/kernel/asm-offsets.c
 +++ b/arch/powerpc/kernel/asm-offsets.c
 @@ -562,6 +562,7 @@ int main(void)
   DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst));
   DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear));
   DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr));
 + DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save));
 #endif /* CONFIG_PPC_BOOK3S */
 #endif /* CONFIG_KVM */
 
 diff --git a/arch/powerpc/kvm/booke_interrupts.S 
 b/arch/powerpc/kvm/booke_interrupts.S
 index eae8483..dd9c5d4 100644
 --- a/arch/powerpc/kvm/booke_interrupts.S
 +++ b/arch/powerpc/kvm/booke_interrupts.S
 @@ -52,12 +52,7 @@
(1BOOKE_INTERRUPT_PROGRAM) | \
(1BOOKE_INTERRUPT_DTLB_MISS))
 
 -.macro KVM_HANDLER ivor_nr scratch srr0
 -_GLOBAL(kvmppc_handler_\ivor_nr)
 - /* Get pointer to vcpu and record exit number. */
 - mtspr   \scratch , r4
 - mfspr   r4, SPRN_SPRG_THREAD
 - lwz r4, THREAD_KVM_VCPU(r4)
 +.macro __KVM_HANDLER ivor_nr scratch srr0
   stw r3, VCPU_GPR(R3)(r4)
   stw r5, VCPU_GPR(R5)(r4)
   stw r6, VCPU_GPR(R6)(r4)
 @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
   bctr
 .endm
 
 +.macro KVM_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 + /* Get pointer to vcpu and record exit number. */
 + mtspr   \scratch , r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + __KVM_HANDLER \ivor_nr \scratch \srr0
 +.endm
 +
 +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 + mtspr   \scratch, r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + stw r3, VCPU_CRIT_SAVE(r4)
 + mfcrr3
 + mfspr   r4, SPRN_CSRR1
 + andi.   r4, r4, MSR_PR
 + bne 1f


 + /* debug interrupt happened in enter/exit path */
 + mfspr   r4, SPRN_CSRR1
 + rlwinm  r4, r4, 0, ~MSR_DE
 + mtspr   SPRN_CSRR1, r4
 + lis r4, 0x
 + ori r4, r4, 0x
 + mtspr   SPRN_DBSR, r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + mtcrr3
 + lwz r3, VCPU_CRIT_SAVE(r4)
 + mfspr   r4, \scratch
 + rfci

What is this part doing? Try to ignore the debug exit? Why would we have MSR_DE 
enabled in the first place when we can't handle it?

 +1:   /* debug interrupt happened in guest */
 + mtcrr3
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + lwz r3, VCPU_CRIT_SAVE(r4)
 + __KVM_HANDLER \ivor_nr \scratch \srr0

I don't think you need the __KVM_HANDLER split. This should be quite easily 
refactorable into a simple DBG prolog.


Alex

 +.endm
 +
 .macro KVM_HANDLER_ADDR ivor_nr
   .long   kvmppc_handler_\ivor_nr
 .endm
 @@ -98,7 +133,7 @@ KVM_HANDLER BOOKE_INTERRUPT_FIT SPRN_SPRG_RSCRATCH0 
 SPRN_SRR0
 KVM_HANDLER BOOKE_INTERRUPT_WATCHDOG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0
 KVM_HANDLER BOOKE_INTERRUPT_DTLB_MISS SPRN_SPRG_RSCRATCH0 SPRN_SRR0
 KVM_HANDLER BOOKE_INTERRUPT_ITLB_MISS SPRN_SPRG_RSCRATCH0 SPRN_SRR0
 -KVM_HANDLER BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0
 +KVM_DBG_HANDLER BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0
 KVM_HANDLER BOOKE_INTERRUPT_SPE_UNAVAIL SPRN_SPRG_RSCRATCH0 SPRN_SRR0
 KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_DATA SPRN_SPRG_RSCRATCH0 SPRN_SRR0
 KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_ROUND SPRN_SPRG_RSCRATCH0 SPRN_SRR0
 -- 
 1.7.0.4
 
 

--
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


Re: [PATCH 3/8] KVM: PPC: booke: Added debug handler

2013-01-25 Thread Alexander Graf

On 16.01.2013, at 09:24, Bharat Bhushan wrote:

 From: Bharat Bhushan bharat.bhus...@freescale.com
 
 Installed debug handler will be used for guest debug support
 and debug facility emulation features (patches for these
 features will follow this patch).
 
 Signed-off-by: Liu Yu yu@freescale.com
 [bharat.bhus...@freescale.com: Substantial changes]
 Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
 ---
 arch/powerpc/include/asm/kvm_host.h |1 +
 arch/powerpc/kernel/asm-offsets.c   |1 +
 arch/powerpc/kvm/booke_interrupts.S |   49 ++-
 3 files changed, 44 insertions(+), 7 deletions(-)
 
 diff --git a/arch/powerpc/include/asm/kvm_host.h 
 b/arch/powerpc/include/asm/kvm_host.h
 index 8a72d59..f4ba881 100644
 --- a/arch/powerpc/include/asm/kvm_host.h
 +++ b/arch/powerpc/include/asm/kvm_host.h
 @@ -503,6 +503,7 @@ struct kvm_vcpu_arch {
   u32 tlbcfg[4];
   u32 mmucfg;
   u32 epr;
 + u32 crit_save;
   struct kvmppc_booke_debug_reg dbg_reg;
 #endif
   gpa_t paddr_accessed;
 diff --git a/arch/powerpc/kernel/asm-offsets.c 
 b/arch/powerpc/kernel/asm-offsets.c
 index 46f6afd..02048f3 100644
 --- a/arch/powerpc/kernel/asm-offsets.c
 +++ b/arch/powerpc/kernel/asm-offsets.c
 @@ -562,6 +562,7 @@ int main(void)
   DEFINE(VCPU_LAST_INST, offsetof(struct kvm_vcpu, arch.last_inst));
   DEFINE(VCPU_FAULT_DEAR, offsetof(struct kvm_vcpu, arch.fault_dear));
   DEFINE(VCPU_FAULT_ESR, offsetof(struct kvm_vcpu, arch.fault_esr));
 + DEFINE(VCPU_CRIT_SAVE, offsetof(struct kvm_vcpu, arch.crit_save));
 #endif /* CONFIG_PPC_BOOK3S */
 #endif /* CONFIG_KVM */
 
 diff --git a/arch/powerpc/kvm/booke_interrupts.S 
 b/arch/powerpc/kvm/booke_interrupts.S
 index eae8483..dd9c5d4 100644
 --- a/arch/powerpc/kvm/booke_interrupts.S
 +++ b/arch/powerpc/kvm/booke_interrupts.S
 @@ -52,12 +52,7 @@
(1BOOKE_INTERRUPT_PROGRAM) | \
(1BOOKE_INTERRUPT_DTLB_MISS))
 
 -.macro KVM_HANDLER ivor_nr scratch srr0
 -_GLOBAL(kvmppc_handler_\ivor_nr)
 - /* Get pointer to vcpu and record exit number. */
 - mtspr   \scratch , r4
 - mfspr   r4, SPRN_SPRG_THREAD
 - lwz r4, THREAD_KVM_VCPU(r4)
 +.macro __KVM_HANDLER ivor_nr scratch srr0
   stw r3, VCPU_GPR(R3)(r4)
   stw r5, VCPU_GPR(R5)(r4)
   stw r6, VCPU_GPR(R6)(r4)
 @@ -74,6 +69,46 @@ _GLOBAL(kvmppc_handler_\ivor_nr)
   bctr
 .endm
 
 +.macro KVM_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 + /* Get pointer to vcpu and record exit number. */
 + mtspr   \scratch , r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + __KVM_HANDLER \ivor_nr \scratch \srr0
 +.endm
 +
 +.macro KVM_DBG_HANDLER ivor_nr scratch srr0
 +_GLOBAL(kvmppc_handler_\ivor_nr)
 + mtspr   \scratch, r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + stw r3, VCPU_CRIT_SAVE(r4)
 + mfcrr3
 + mfspr   r4, SPRN_CSRR1
 + andi.   r4, r4, MSR_PR
 + bne 1f


 + /* debug interrupt happened in enter/exit path */
 + mfspr   r4, SPRN_CSRR1
 + rlwinm  r4, r4, 0, ~MSR_DE
 + mtspr   SPRN_CSRR1, r4
 + lis r4, 0x
 + ori r4, r4, 0x
 + mtspr   SPRN_DBSR, r4
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + mtcrr3
 + lwz r3, VCPU_CRIT_SAVE(r4)
 + mfspr   r4, \scratch
 + rfci

What is this part doing? Try to ignore the debug exit? Why would we have MSR_DE 
enabled in the first place when we can't handle it?

 +1:   /* debug interrupt happened in guest */
 + mtcrr3
 + mfspr   r4, SPRN_SPRG_THREAD
 + lwz r4, THREAD_KVM_VCPU(r4)
 + lwz r3, VCPU_CRIT_SAVE(r4)
 + __KVM_HANDLER \ivor_nr \scratch \srr0

I don't think you need the __KVM_HANDLER split. This should be quite easily 
refactorable into a simple DBG prolog.


Alex

 +.endm
 +
 .macro KVM_HANDLER_ADDR ivor_nr
   .long   kvmppc_handler_\ivor_nr
 .endm
 @@ -98,7 +133,7 @@ KVM_HANDLER BOOKE_INTERRUPT_FIT SPRN_SPRG_RSCRATCH0 
 SPRN_SRR0
 KVM_HANDLER BOOKE_INTERRUPT_WATCHDOG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0
 KVM_HANDLER BOOKE_INTERRUPT_DTLB_MISS SPRN_SPRG_RSCRATCH0 SPRN_SRR0
 KVM_HANDLER BOOKE_INTERRUPT_ITLB_MISS SPRN_SPRG_RSCRATCH0 SPRN_SRR0
 -KVM_HANDLER BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0
 +KVM_DBG_HANDLER BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0
 KVM_HANDLER BOOKE_INTERRUPT_SPE_UNAVAIL SPRN_SPRG_RSCRATCH0 SPRN_SRR0
 KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_DATA SPRN_SPRG_RSCRATCH0 SPRN_SRR0
 KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_ROUND SPRN_SPRG_RSCRATCH0 SPRN_SRR0
 -- 
 1.7.0.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