[Qemu-devel] [PATCH 0/6 v6] ppc: Add debug stub support

[Qemu-devel] [PATCH 0/6 v6] ppc: Add debug stub support

[Bharat Bhushan]

This patchset add support for
 - software breakpoint
 - h/w breakpoint
 - h/w watchpoint

Please find description in individual patch.

v5->v6
 - Added a new patch to synchronize excp_vectors.
 - Inject program exception rather than debug exception if
   guest is not able to handle debug exception. why? detail
   in respective patch.

v4->v5
 - cleanup in kvmppc_hw_debug_points_init()
 - replaced assert in kvm_arch_insert_hw_breakpoint() with return.
   This allows gdb to through warning "You may have requested too many
   hardware breakpoints/watchpoints.". Now user can remove extra breakpint
   and continue.

v3->v4
 - remove hardcoding for size of instruction in software breakpoint
 - remove unnecessary assert in debug handler
 - Corrected assert comparison in insert_breakpoint()

v2->v3
 - Sharing of code for book3s support (which may come in future)
 - Initializing number of hw breakpoint/watchpoints from KVM world
 - Other minor cleanup/fixes

v1->v2:
 - use kvm_get_one_reg() for getting trap instruction
 - factored out e500 specific code based on exception model POWERPC_EXCP_BOOKE.
 - Not supporting ppc440

Bharat Bhushan (6):
  ppc: debug stub: Get trap instruction opcode from KVM
  ppc: Add interface to inject interrupt to guest
  ppc: synchronize excp_vectors for injecting exception
  ppc: Add program exception injection handler
  ppc: Add software breakpoint support
  ppc: Add hw breakpoint watchpoint support

 target-ppc/cpu.h |   1 +
 target-ppc/kvm.c | 390 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 363 insertions(+), 28 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/6 v6] ppc: debug stub: Get trap instruction opcode from KVM

[Bharat Bhushan]

Get trap instruction opcode from KVM and this opcode will
be used for setting software breakpoint in following patch

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v5->v6
 - no change

 target-ppc/kvm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 2d87108..4df23dd 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -72,6 +72,8 @@ static int cap_papr;
 static int cap_htab_fd;
 static int cap_fixup_hcalls;
 
+static uint32_t debug_inst_opcode;
+
 /* XXX We have a race condition where we actually have a level triggered
  *     interrupt, but the infrastructure can't expose that yet, so the guest
  *     takes but ignores it, goes to sleep and never gets notified that there's
@@ -436,6 +438,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
         break;
     }
 
+    kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode);
+
     return ret;
 }
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH 1/6 v6] ppc: debug stub: Get trap instruction opcode from KVM

[Peter Maydell]

On 10 July 2014 11:57, Bharat Bhushan <Bharat.Bhushan@freescale.com> wrote:
> Get trap instruction opcode from KVM and this opcode will
> be used for setting software breakpoint in following patch
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> ---
> v5->v6
>  - no change
>
>  target-ppc/kvm.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 2d87108..4df23dd 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -72,6 +72,8 @@ static int cap_papr;
>  static int cap_htab_fd;
>  static int cap_fixup_hcalls;
>
> +static uint32_t debug_inst_opcode;
> +
>  /* XXX We have a race condition where we actually have a level triggered
>   *     interrupt, but the infrastructure can't expose that yet, so the guest
>   *     takes but ignores it, goes to sleep and never gets notified that there's
> @@ -436,6 +438,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
>          break;
>      }
>
> +    kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode);
> +

Presumably the kernel guarantees that this is always the
same value for every CPU in the system?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/6 v6] ppc: debug stub: Get trap instruction opcode from KVM

[Alexander Graf]

On 10.07.14 16:07, Peter Maydell wrote:
> On 10 July 2014 11:57, Bharat Bhushan <Bharat.Bhushan@freescale.com> wrote:
>> Get trap instruction opcode from KVM and this opcode will
>> be used for setting software breakpoint in following patch
>>
>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
>> ---
>> v5->v6
>>   - no change
>>
>>   target-ppc/kvm.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index 2d87108..4df23dd 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -72,6 +72,8 @@ static int cap_papr;
>>   static int cap_htab_fd;
>>   static int cap_fixup_hcalls;
>>
>> +static uint32_t debug_inst_opcode;
>> +
>>   /* XXX We have a race condition where we actually have a level triggered
>>    *     interrupt, but the infrastructure can't expose that yet, so the guest
>>    *     takes but ignores it, goes to sleep and never gets notified that there's
>> @@ -436,6 +438,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>           break;
>>       }
>>
>> +    kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode);
>> +
> Presumably the kernel guarantees that this is always the
> same value for every CPU in the system?

Yes :)


Alex

[Qemu-devel] [PATCH 2/6 v6] ppc: Add interface to inject interrupt to guest

[Bharat Bhushan]

This patch adds interface to inject interrupt to guest.
Currently a void program check exception function added.
Follow up patch will use this interface to inject program
check exception to guest

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v5->v6
 - replace debug with program interrupt

 target-ppc/cpu.h |  1 +
 target-ppc/kvm.c | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index b64c652..e60fc40 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -2144,6 +2144,7 @@ enum {
     PPC_INTERRUPT_CDOORBELL,      /* Critical doorbell interrupt          */
     PPC_INTERRUPT_DOORBELL,       /* Doorbell interrupt                   */
     PPC_INTERRUPT_PERFM,          /* Performance monitor interrupt        */
+    PPC_INTERRUPT_PROGRAM,        /* Program check exception              */
 };
 
 /* Processor Compatibility mask (PCR) */
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 4df23dd..673c369 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -761,6 +761,25 @@ static int kvm_put_vpa(CPUState *cs)
 }
 #endif /* TARGET_PPC64 */
 
+static int kvmppc_inject_program_exception(CPUState *cs)
+{
+    return 0;
+}
+
+static void kvmppc_inject_exception(CPUState *cs)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+
+    if (env->pending_interrupts & (1 << PPC_INTERRUPT_PROGRAM)) {
+        if (kvmppc_inject_program_exception(cs)) {
+            fprintf(stderr, "%s: PROGRAM exception injection failed\n", __func__);
+            return;
+        }
+        env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PROGRAM);
+    }
+}
+
 int kvm_arch_put_registers(CPUState *cs, int level)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
@@ -774,6 +793,10 @@ int kvm_arch_put_registers(CPUState *cs, int level)
         return ret;
     }
 
+    if (env->pending_interrupts) {
+        kvmppc_inject_exception(cs);
+    }
+
     regs.ctr = env->ctr;
     regs.lr  = env->lr;
     regs.xer = cpu_read_xer(env);
-- 
1.9.3

Re: [Qemu-devel] [PATCH 2/6 v6] ppc: Add interface to inject interrupt to guest

[Alexander Graf]

On 10.07.14 12:57, Bharat Bhushan wrote:
> This patch adds interface to inject interrupt to guest.
> Currently a void program check exception function added.
> Follow up patch will use this interface to inject program
> check exception to guest
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> ---
> v5->v6
>   - replace debug with program interrupt
>
>   target-ppc/cpu.h |  1 +
>   target-ppc/kvm.c | 23 +++++++++++++++++++++++
>   2 files changed, 24 insertions(+)
>
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index b64c652..e60fc40 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -2144,6 +2144,7 @@ enum {
>       PPC_INTERRUPT_CDOORBELL,      /* Critical doorbell interrupt          */
>       PPC_INTERRUPT_DOORBELL,       /* Doorbell interrupt                   */
>       PPC_INTERRUPT_PERFM,          /* Performance monitor interrupt        */
> +    PPC_INTERRUPT_PROGRAM,        /* Program check exception              */
>   };
>   
>   /* Processor Compatibility mask (PCR) */
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 4df23dd..673c369 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -761,6 +761,25 @@ static int kvm_put_vpa(CPUState *cs)
>   }
>   #endif /* TARGET_PPC64 */
>   
> +static int kvmppc_inject_program_exception(CPUState *cs)
> +{
> +    return 0;
> +}
> +
> +static void kvmppc_inject_exception(CPUState *cs)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +
> +    if (env->pending_interrupts & (1 << PPC_INTERRUPT_PROGRAM)) {
> +        if (kvmppc_inject_program_exception(cs)) {
> +            fprintf(stderr, "%s: PROGRAM exception injection failed\n", __func__);
> +            return;
> +        }
> +        env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PROGRAM);

Do we really need to jump through these hoops? We could just directly 
call the injection, no?


Alex

> +    }
> +}
> +
>   int kvm_arch_put_registers(CPUState *cs, int level)
>   {
>       PowerPCCPU *cpu = POWERPC_CPU(cs);
> @@ -774,6 +793,10 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>           return ret;
>       }
>   
> +    if (env->pending_interrupts) {
> +        kvmppc_inject_exception(cs);
> +    }
> +
>       regs.ctr = env->ctr;
>       regs.lr  = env->lr;
>       regs.xer = cpu_read_xer(env);

[Qemu-devel] [PATCH 3/6 v6] ppc: synchronize excp_vectors for injecting exception

[Bharat Bhushan]

This patch synchronizes env->excp_vectors[] with env->iovr[].
This is required for using the existing interrupt injection mechanism
for kvm.

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v5->v6
 - new patch

 target-ppc/kvm.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 673c369..329a38b 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1008,35 +1008,79 @@ int kvm_arch_get_registers(CPUState *cs)
 
         if (sregs.u.e.features & KVM_SREGS_E_IVOR) {
             env->spr[SPR_BOOKE_IVOR0] = sregs.u.e.ivor_low[0];
+            env->excp_vectors[POWERPC_EXCP_CRITICAL] =
+                 env->spr[SPR_BOOKE_IVOR0] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR1] = sregs.u.e.ivor_low[1];
+            env->excp_vectors[POWERPC_EXCP_MCHECK] =
+                 env->spr[SPR_BOOKE_IVOR1] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR2] = sregs.u.e.ivor_low[2];
+            env->excp_vectors[POWERPC_EXCP_DSI] =
+                 env->spr[SPR_BOOKE_IVOR2] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR3] = sregs.u.e.ivor_low[3];
+            env->excp_vectors[POWERPC_EXCP_ISI] =
+                 env->spr[SPR_BOOKE_IVOR3] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR4] = sregs.u.e.ivor_low[4];
+            env->excp_vectors[POWERPC_EXCP_EXTERNAL] =
+                 env->spr[SPR_BOOKE_IVOR4] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR5] = sregs.u.e.ivor_low[5];
+            env->excp_vectors[POWERPC_EXCP_ALIGN] =
+                 env->spr[SPR_BOOKE_IVOR5] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR6] = sregs.u.e.ivor_low[6];
+            env->excp_vectors[POWERPC_EXCP_PROGRAM] =
+                 env->spr[SPR_BOOKE_IVOR6] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR7] = sregs.u.e.ivor_low[7];
+            env->excp_vectors[POWERPC_EXCP_FPU] =
+                 env->spr[SPR_BOOKE_IVOR7] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR8] = sregs.u.e.ivor_low[8];
+            env->excp_vectors[POWERPC_EXCP_SYSCALL] =
+                 env->spr[SPR_BOOKE_IVOR8] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR9] = sregs.u.e.ivor_low[9];
+            env->excp_vectors[POWERPC_EXCP_APU] =
+                 env->spr[SPR_BOOKE_IVOR9] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR10] = sregs.u.e.ivor_low[10];
+            env->excp_vectors[POWERPC_EXCP_DECR] =
+                 env->spr[SPR_BOOKE_IVOR10] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR11] = sregs.u.e.ivor_low[11];
+            env->excp_vectors[POWERPC_EXCP_FIT] =
+                 env->spr[SPR_BOOKE_IVOR11] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR12] = sregs.u.e.ivor_low[12];
+            env->excp_vectors[POWERPC_EXCP_WDT] =
+                 env->spr[SPR_BOOKE_IVOR12] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR13] = sregs.u.e.ivor_low[13];
+            env->excp_vectors[POWERPC_EXCP_DTLB] =
+                 env->spr[SPR_BOOKE_IVOR13] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR14] = sregs.u.e.ivor_low[14];
+            env->excp_vectors[POWERPC_EXCP_ITLB] =
+                 env->spr[SPR_BOOKE_IVOR14] + env->spr[SPR_BOOKE_IVPR];
             env->spr[SPR_BOOKE_IVOR15] = sregs.u.e.ivor_low[15];
+            env->excp_vectors[POWERPC_EXCP_DEBUG] =
+                 env->spr[SPR_BOOKE_IVOR15] + env->spr[SPR_BOOKE_IVPR];
 
             if (sregs.u.e.features & KVM_SREGS_E_SPE) {
                 env->spr[SPR_BOOKE_IVOR32] = sregs.u.e.ivor_high[0];
+                env->excp_vectors[POWERPC_EXCP_SPEU] =
+                     env->spr[SPR_BOOKE_IVOR32] + env->spr[SPR_BOOKE_IVPR];
                 env->spr[SPR_BOOKE_IVOR33] = sregs.u.e.ivor_high[1];
+                env->excp_vectors[POWERPC_EXCP_EFPDI] =
+                     env->spr[SPR_BOOKE_IVOR33] + env->spr[SPR_BOOKE_IVPR];
                 env->spr[SPR_BOOKE_IVOR34] = sregs.u.e.ivor_high[2];
+                env->excp_vectors[POWERPC_EXCP_EFPRI] =
+                     env->spr[SPR_BOOKE_IVOR34] + env->spr[SPR_BOOKE_IVPR];
             }
 
             if (sregs.u.e.features & KVM_SREGS_E_PM) {
                 env->spr[SPR_BOOKE_IVOR35] = sregs.u.e.ivor_high[3];
+                env->excp_vectors[POWERPC_EXCP_EPERFM] =
+                     env->spr[SPR_BOOKE_IVOR35] + env->spr[SPR_BOOKE_IVPR];
             }
 
             if (sregs.u.e.features & KVM_SREGS_E_PC) {
                 env->spr[SPR_BOOKE_IVOR36] = sregs.u.e.ivor_high[4];
+                env->excp_vectors[POWERPC_EXCP_DOORI] =
+                     env->spr[SPR_BOOKE_IVOR36] + env->spr[SPR_BOOKE_IVPR];
                 env->spr[SPR_BOOKE_IVOR37] = sregs.u.e.ivor_high[5];
+                env->excp_vectors[POWERPC_EXCP_DOORCI] =
+                     env->spr[SPR_BOOKE_IVOR37] + env->spr[SPR_BOOKE_IVPR];
             }
         }
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH 3/6 v6] ppc: synchronize excp_vectors for injecting exception

[Alexander Graf]

On 10.07.14 12:57, Bharat Bhushan wrote:
> This patch synchronizes env->excp_vectors[] with env->iovr[].
> This is required for using the existing interrupt injection mechanism
> for kvm.
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> ---
> v5->v6
>   - new patch
>
>   target-ppc/kvm.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 44 insertions(+)
>
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 673c369..329a38b 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1008,35 +1008,79 @@ int kvm_arch_get_registers(CPUState *cs)
>   
>           if (sregs.u.e.features & KVM_SREGS_E_IVOR) {
>               env->spr[SPR_BOOKE_IVOR0] = sregs.u.e.ivor_low[0];
> +            env->excp_vectors[POWERPC_EXCP_CRITICAL] =
> +                 env->spr[SPR_BOOKE_IVOR0] + env->spr[SPR_BOOKE_IVPR];

This is quite hard to read. How about

static void kvm_sync_excp(CPUPPCState *env, int vector, int ivor)
{
     env->excp_vectors[vector] = env->spr[ivor] + env->spr[SPR_BOOKE_IVPR];
}

kvm_sync_excp(env, POWERPC_EXCP_CRITICAL,  SPR_BOOKE_IVOR0);


Alex

>               env->spr[SPR_BOOKE_IVOR1] = sregs.u.e.ivor_low[1];
> +            env->excp_vectors[POWERPC_EXCP_MCHECK] =
> +                 env->spr[SPR_BOOKE_IVOR1] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR2] = sregs.u.e.ivor_low[2];
> +            env->excp_vectors[POWERPC_EXCP_DSI] =
> +                 env->spr[SPR_BOOKE_IVOR2] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR3] = sregs.u.e.ivor_low[3];
> +            env->excp_vectors[POWERPC_EXCP_ISI] =
> +                 env->spr[SPR_BOOKE_IVOR3] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR4] = sregs.u.e.ivor_low[4];
> +            env->excp_vectors[POWERPC_EXCP_EXTERNAL] =
> +                 env->spr[SPR_BOOKE_IVOR4] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR5] = sregs.u.e.ivor_low[5];
> +            env->excp_vectors[POWERPC_EXCP_ALIGN] =
> +                 env->spr[SPR_BOOKE_IVOR5] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR6] = sregs.u.e.ivor_low[6];
> +            env->excp_vectors[POWERPC_EXCP_PROGRAM] =
> +                 env->spr[SPR_BOOKE_IVOR6] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR7] = sregs.u.e.ivor_low[7];
> +            env->excp_vectors[POWERPC_EXCP_FPU] =
> +                 env->spr[SPR_BOOKE_IVOR7] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR8] = sregs.u.e.ivor_low[8];
> +            env->excp_vectors[POWERPC_EXCP_SYSCALL] =
> +                 env->spr[SPR_BOOKE_IVOR8] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR9] = sregs.u.e.ivor_low[9];
> +            env->excp_vectors[POWERPC_EXCP_APU] =
> +                 env->spr[SPR_BOOKE_IVOR9] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR10] = sregs.u.e.ivor_low[10];
> +            env->excp_vectors[POWERPC_EXCP_DECR] =
> +                 env->spr[SPR_BOOKE_IVOR10] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR11] = sregs.u.e.ivor_low[11];
> +            env->excp_vectors[POWERPC_EXCP_FIT] =
> +                 env->spr[SPR_BOOKE_IVOR11] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR12] = sregs.u.e.ivor_low[12];
> +            env->excp_vectors[POWERPC_EXCP_WDT] =
> +                 env->spr[SPR_BOOKE_IVOR12] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR13] = sregs.u.e.ivor_low[13];
> +            env->excp_vectors[POWERPC_EXCP_DTLB] =
> +                 env->spr[SPR_BOOKE_IVOR13] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR14] = sregs.u.e.ivor_low[14];
> +            env->excp_vectors[POWERPC_EXCP_ITLB] =
> +                 env->spr[SPR_BOOKE_IVOR14] + env->spr[SPR_BOOKE_IVPR];
>               env->spr[SPR_BOOKE_IVOR15] = sregs.u.e.ivor_low[15];
> +            env->excp_vectors[POWERPC_EXCP_DEBUG] =
> +                 env->spr[SPR_BOOKE_IVOR15] + env->spr[SPR_BOOKE_IVPR];
>   
>               if (sregs.u.e.features & KVM_SREGS_E_SPE) {
>                   env->spr[SPR_BOOKE_IVOR32] = sregs.u.e.ivor_high[0];
> +                env->excp_vectors[POWERPC_EXCP_SPEU] =
> +                     env->spr[SPR_BOOKE_IVOR32] + env->spr[SPR_BOOKE_IVPR];
>                   env->spr[SPR_BOOKE_IVOR33] = sregs.u.e.ivor_high[1];
> +                env->excp_vectors[POWERPC_EXCP_EFPDI] =
> +                     env->spr[SPR_BOOKE_IVOR33] + env->spr[SPR_BOOKE_IVPR];
>                   env->spr[SPR_BOOKE_IVOR34] = sregs.u.e.ivor_high[2];
> +                env->excp_vectors[POWERPC_EXCP_EFPRI] =
> +                     env->spr[SPR_BOOKE_IVOR34] + env->spr[SPR_BOOKE_IVPR];
>               }
>   
>               if (sregs.u.e.features & KVM_SREGS_E_PM) {
>                   env->spr[SPR_BOOKE_IVOR35] = sregs.u.e.ivor_high[3];
> +                env->excp_vectors[POWERPC_EXCP_EPERFM] =
> +                     env->spr[SPR_BOOKE_IVOR35] + env->spr[SPR_BOOKE_IVPR];
>               }
>   
>               if (sregs.u.e.features & KVM_SREGS_E_PC) {
>                   env->spr[SPR_BOOKE_IVOR36] = sregs.u.e.ivor_high[4];
> +                env->excp_vectors[POWERPC_EXCP_DOORI] =
> +                     env->spr[SPR_BOOKE_IVOR36] + env->spr[SPR_BOOKE_IVPR];
>                   env->spr[SPR_BOOKE_IVOR37] = sregs.u.e.ivor_high[5];
> +                env->excp_vectors[POWERPC_EXCP_DOORCI] =
> +                     env->spr[SPR_BOOKE_IVOR37] + env->spr[SPR_BOOKE_IVPR];
>               }
>           }
>   

[Qemu-devel] [PATCH 4/6 v6] ppc: Add program exception injection handler

[Bharat Bhushan]

With this patch a program check exception can be injected
to guest. Follow up patch will use this interface to inject
program exception to guest.

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v5->v6
 - new patch (infact replace debug interrupt injection)

 target-ppc/kvm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 329a38b..d1239b4 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -763,6 +763,11 @@ static int kvm_put_vpa(CPUState *cs)
 
 static int kvmppc_inject_program_exception(CPUState *cs)
 {
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+    cs->exception_index = POWERPC_EXCP_PROGRAM;
+    env->error_code = POWERPC_EXCP_INVAL;
+    ppc_cpu_do_interrupt(cs);
     return 0;
 }
 
-- 
1.9.3

[Qemu-devel] [PATCH 5/6 v6] ppc: Add software breakpoint support

[Bharat Bhushan]

This patch allow insert/remove software breakpoint

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v5->v6
 - No change

 target-ppc/kvm.c | 73 +++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 59 insertions(+), 14 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index d1239b4..c4a1fa5 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1320,6 +1320,55 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env, uint32_t dcrn, uint32_t dat
     return 0;
 }
 
+int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+{
+    /* Mixed endian case is not handled */
+    uint32_t sc = debug_inst_opcode;
+
+    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
+                            sizeof(sc), 0) ||
+        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, sizeof(sc), 1)) {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+{
+    uint32_t sc;
+
+    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, sizeof(sc), 0) ||
+        sc != debug_inst_opcode ||
+        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
+                            sizeof(sc), 1)) {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
+{
+    /* Software Breakpoint updates */
+    if (kvm_sw_breakpoints_active(cs)) {
+        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
+    }
+}
+
+static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run)
+{
+    CPUState *cs = CPU(cpu);
+    struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
+    int handle = 0;
+
+    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
+        handle = 1;
+    }
+
+    return handle;
+}
+
 int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
@@ -1360,6 +1409,16 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
         ret = 0;
         break;
 
+    case KVM_EXIT_DEBUG:
+        DPRINTF("handle debug exception\n");
+        if (kvm_handle_debug(cpu, run)) {
+            ret = EXCP_DEBUG;
+            break;
+        }
+        /* re-enter, this exception was guest-internal */
+        ret = 0;
+        break;
+
     default:
         fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
         ret = -1;
@@ -2048,16 +2107,6 @@ void kvm_arch_init_irq_routing(KVMState *s)
 {
 }
 
-int kvm_arch_insert_sw_breakpoint(CPUState *cpu, struct kvm_sw_breakpoint *bp)
-{
-    return -EINVAL;
-}
-
-int kvm_arch_remove_sw_breakpoint(CPUState *cpu, struct kvm_sw_breakpoint *bp)
-{
-    return -EINVAL;
-}
-
 int kvm_arch_insert_hw_breakpoint(target_ulong addr, target_ulong len, int type)
 {
     return -EINVAL;
@@ -2072,10 +2121,6 @@ void kvm_arch_remove_all_hw_breakpoints(void)
 {
 }
 
-void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
-{
-}
-
 struct kvm_get_htab_buf {
     struct kvm_get_htab_header header;
     /*
-- 
1.9.3

Re: [Qemu-devel] [PATCH 5/6 v6] ppc: Add software breakpoint support

[Alexander Graf]

On 10.07.14 12:58, Bharat Bhushan wrote:
> This patch allow insert/remove software breakpoint
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> ---
> v5->v6
>   - No change
>
>   target-ppc/kvm.c | 73 +++++++++++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 59 insertions(+), 14 deletions(-)
>
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index d1239b4..c4a1fa5 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1320,6 +1320,55 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env, uint32_t dcrn, uint32_t dat
>       return 0;
>   }
>   
> +int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +{
> +    /* Mixed endian case is not handled */
> +    uint32_t sc = debug_inst_opcode;
> +
> +    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
> +                            sizeof(sc), 0) ||
> +        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, sizeof(sc), 1)) {
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
> +int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
> +{
> +    uint32_t sc;
> +
> +    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, sizeof(sc), 0) ||
> +        sc != debug_inst_opcode ||
> +        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
> +                            sizeof(sc), 1)) {
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
> +void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
> +{
> +    /* Software Breakpoint updates */
> +    if (kvm_sw_breakpoints_active(cs)) {
> +        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
> +    }
> +}
> +
> +static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run)
> +{
> +    CPUState *cs = CPU(cpu);
> +    struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
> +    int handle = 0;
> +
> +    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
> +        handle = 1;
> +    }
> +
> +    return handle;
> +}
> +
>   int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
>   {
>       PowerPCCPU *cpu = POWERPC_CPU(cs);
> @@ -1360,6 +1409,16 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
>           ret = 0;
>           break;
>   
> +    case KVM_EXIT_DEBUG:
> +        DPRINTF("handle debug exception\n");
> +        if (kvm_handle_debug(cpu, run)) {
> +            ret = EXCP_DEBUG;
> +            break;
> +        }
> +        /* re-enter, this exception was guest-internal */
> +        ret = 0;

Where do we inject the program interrupt into our guest here?


Alex

[Qemu-devel] [PATCH 6/6 v6] ppc: Add hw breakpoint watchpoint support

[Bharat Bhushan]

This patch adds hardware breakpoint and hardware watchpoint support
for ppc.

On BOOKE architecture we cannot share debug resources between QEMU
and guest because:
    When QEMU is using debug resources then debug exception must
    be always enabled. To achieve this we set MSR_DE and also set
    MSRP_DEP so guest cannot change MSR_DE.

    When emulating debug resource for guest we want guest
    to control MSR_DE (enable/disable debug interrupt on need).

    So above mentioned two configuration cannot be supported
    at the same time. So the result is that we cannot share
    debug resources between QEMU and Guest on BOOKE architecture.

In the current design QEMU gets priority over guest,
this means that if QEMU is using debug resources then guest
cannot use them and if guest is using debug resource then
qemu can overwrite them.

When QEMU is not able to handle debug exception then we inject program
exception to guest. Yes program exception NOT debug exception and the
reason is:
 1) QEMU and guest not sharing debug resources
 2) For software breakpoint QEMU uses a ehpriv-1 instruction;

 So there cannot be any reason that we are in qemu with exit reason
 KVM_EXIT_DEBUG  for guest set debug exception, only possibility is
 guest executed ehpriv-1 privilege instruction and that's why we are
 injecting program exception.

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v5->v6
 - Inject program exception rather than debug exception if
   guest is not able to handle debug exception. why? detail
   in respective patch.

 target-ppc/kvm.c | 245 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 229 insertions(+), 16 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index c4a1fa5..6156f09 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -38,6 +38,7 @@
 #include "hw/ppc/ppc.h"
 #include "sysemu/watchdog.h"
 #include "trace.h"
+#include "exec/gdbstub.h"
 
 //#define DEBUG_KVM
 
@@ -412,6 +413,38 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu)
     return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
 }
 
+/* e500 supports 2 h/w breakpoint and 2 watchpoint.
+ * book3s supports only 1 watchpoint, so array size
+ * of 4 is sufficient for now.
+ */
+#define MAX_HW_BKPTS 4
+
+static struct HWBreakpoint {
+    target_ulong addr;
+    int type;
+} hw_debug_points[MAX_HW_BKPTS];
+
+static CPUWatchpoint hw_watchpoint;
+
+/* Default there is no breakpoint and watchpoint supported */
+static int max_hw_breakpoint;
+static int max_hw_watchpoint;
+static int nb_hw_breakpoint;
+static int nb_hw_watchpoint;
+
+static void kvmppc_hw_debug_points_init(CPUPPCState *cenv)
+{
+    if (cenv->excp_model == POWERPC_EXCP_BOOKE) {
+        max_hw_breakpoint = 2;
+        max_hw_watchpoint = 2;
+    }
+
+    if ((max_hw_breakpoint + max_hw_watchpoint) > MAX_HW_BKPTS) {
+        fprintf(stderr, "Error initializing h/w breakpoints\n");
+        return;
+    }
+}
+
 int kvm_arch_init_vcpu(CPUState *cs)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
@@ -439,6 +472,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
     }
 
     kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode);
+    kvmppc_hw_debug_points_init(cenv);
 
     return ret;
 }
@@ -1348,24 +1382,217 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
     return 0;
 }
 
+static int find_hw_breakpoint(target_ulong addr, int type)
+{
+    int n;
+
+    assert((nb_hw_breakpoint + nb_hw_watchpoint)
+           <= ARRAY_SIZE(hw_debug_points));
+
+    for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) {
+        if (hw_debug_points[n].addr == addr && hw_debug_points[n].type == type) {
+            return n;
+        }
+    }
+
+    return -1;
+}
+
+static int find_hw_watchpoint(target_ulong addr, int *flag)
+{
+    int n;
+
+    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_ACCESS);
+    if (n >= 0) {
+        *flag = BP_MEM_ACCESS;
+        return n;
+    }
+
+    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_WRITE);
+    if (n >= 0) {
+        *flag = BP_MEM_WRITE;
+        return n;
+    }
+
+    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_READ);
+    if (n >= 0) {
+        *flag = BP_MEM_READ;
+        return n;
+    }
+
+    return -1;
+}
+
+int kvm_arch_insert_hw_breakpoint(target_ulong addr,
+                                  target_ulong len, int type)
+{
+    if ((nb_hw_breakpoint + nb_hw_watchpoint) >= ARRAY_SIZE(hw_debug_points))
+	return -ENOBUFS;
+
+    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].addr = addr;
+    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].type = type;
+
+    switch (type) {
+    case GDB_BREAKPOINT_HW:
+        if (nb_hw_breakpoint >= max_hw_breakpoint) {
+            return -ENOBUFS;
+        }
+
+        if (find_hw_breakpoint(addr, type) >= 0) {
+            return -EEXIST;
+        }
+
+        nb_hw_breakpoint++;
+        break;
+
+    case GDB_WATCHPOINT_WRITE:
+    case GDB_WATCHPOINT_READ:
+    case GDB_WATCHPOINT_ACCESS:
+        if (nb_hw_watchpoint >= max_hw_watchpoint) {
+            return -ENOBUFS;
+        }
+
+        if (find_hw_breakpoint(addr, type) >= 0) {
+            return -EEXIST;
+        }
+
+        nb_hw_watchpoint++;
+        break;
+
+    default:
+        return -ENOSYS;
+    }
+
+    return 0;
+}
+
+int kvm_arch_remove_hw_breakpoint(target_ulong addr,
+                                  target_ulong len, int type)
+{
+    int n;
+
+    n = find_hw_breakpoint(addr, type);
+    if (n < 0) {
+        return -ENOENT;
+    }
+
+    switch (type) {
+    case GDB_BREAKPOINT_HW:
+        nb_hw_breakpoint--;
+        break;
+
+    case GDB_WATCHPOINT_WRITE:
+    case GDB_WATCHPOINT_READ:
+    case GDB_WATCHPOINT_ACCESS:
+        nb_hw_watchpoint--;
+        break;
+
+    default:
+        return -ENOSYS;
+    }
+    hw_debug_points[n] = hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint];
+
+    return 0;
+}
+
+void kvm_arch_remove_all_hw_breakpoints(void)
+{
+    nb_hw_breakpoint = nb_hw_watchpoint = 0;
+}
+
 void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
 {
+    int n;
+
     /* Software Breakpoint updates */
     if (kvm_sw_breakpoints_active(cs)) {
         dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
     }
+
+    assert((nb_hw_breakpoint + nb_hw_watchpoint)
+           <= ARRAY_SIZE(hw_debug_points));
+    assert((nb_hw_breakpoint + nb_hw_watchpoint) <= ARRAY_SIZE(dbg->arch.bp));
+
+    if (nb_hw_breakpoint + nb_hw_watchpoint > 0) {
+        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP;
+        memset(dbg->arch.bp, 0, sizeof(dbg->arch.bp));
+        for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) {
+            switch (hw_debug_points[n].type) {
+            case GDB_BREAKPOINT_HW:
+                dbg->arch.bp[n].type = KVMPPC_DEBUG_BREAKPOINT;
+                break;
+            case GDB_WATCHPOINT_WRITE:
+                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE;
+                break;
+            case GDB_WATCHPOINT_READ:
+                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_READ;
+                break;
+            case GDB_WATCHPOINT_ACCESS:
+                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE |
+                                        KVMPPC_DEBUG_WATCH_READ;
+                break;
+            default:
+                cpu_abort(cs, "Unsupported breakpoint type\n");
+            }
+            dbg->arch.bp[n].addr = hw_debug_points[n].addr;
+        }
+    }
 }
 
 static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run)
 {
     CPUState *cs = CPU(cpu);
+    CPUPPCState *env = &cpu->env;
     struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
     int handle = 0;
+    int n;
+    int flag = 0;
 
-    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
+    if (cs->singlestep_enabled) {
         handle = 1;
+    } else if (arch_info->status) {
+        if (nb_hw_breakpoint + nb_hw_watchpoint > 0) {
+            if (arch_info->status & KVMPPC_DEBUG_BREAKPOINT) {
+                n = find_hw_breakpoint(arch_info->address, GDB_BREAKPOINT_HW);
+                if (n >= 0) {
+                    handle = 1;
+                }
+            } else if (arch_info->status & (KVMPPC_DEBUG_WATCH_READ |
+                                            KVMPPC_DEBUG_WATCH_WRITE)) {
+                n = find_hw_watchpoint(arch_info->address,  &flag);
+                if (n >= 0) {
+                    handle = 1;
+                    cs->watchpoint_hit = &hw_watchpoint;
+                    hw_watchpoint.vaddr = hw_debug_points[n].addr;
+                    hw_watchpoint.flags = flag;
+                }
+            }
+        }
+    } else if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
+        handle = 1;
+    } else {
+        /* QEMU is not able to handle debug exception, so inject
+         * program exception to guest;
+         * Yes program exception NOT debug exception !!
+         * When QEMU is using debug resources then debug exception must
+         * be always set. To achieve this we set MSR_DE and also set
+         * MSRP_DEP so guest cannot change MSR_DE.
+         * When emulating debug resource for guest we want guest
+         * to control MSR_DE (enable/disable debug interrupt on need).
+         * Supporting both configurations are NOT possible.
+         * So the result is that we cannot share debug resources
+         * between QEMU and Guest on BOOKE architecture.
+         * In the current design QEMU gets the priority over guest,
+         * this means that if QEMU is using debug resources then guest
+         * cannot use them;
+         * For software breakpoint QEMU uses a ehpriv-1 instruction;
+         * So there cannot be any reason that we are here for guest
+         * set debug exception, only possibility is guest executed a
+         * privilege instruction and that's why we are injecting
+         * program exception.
+         */
+        env->pending_interrupts |=  1 << PPC_INTERRUPT_DEBUG;
     }
-
     return handle;
 }
 
@@ -2107,20 +2334,6 @@ void kvm_arch_init_irq_routing(KVMState *s)
 {
 }
 
-int kvm_arch_insert_hw_breakpoint(target_ulong addr, target_ulong len, int type)
-{
-    return -EINVAL;
-}
-
-int kvm_arch_remove_hw_breakpoint(target_ulong addr, target_ulong len, int type)
-{
-    return -EINVAL;
-}
-
-void kvm_arch_remove_all_hw_breakpoints(void)
-{
-}
-
 struct kvm_get_htab_buf {
     struct kvm_get_htab_header header;
     /*
-- 
1.9.3

Re: [Qemu-devel] [PATCH 6/6 v6] ppc: Add hw breakpoint watchpoint support

[Alexander Graf]

On 10.07.14 12:58, Bharat Bhushan wrote:
> This patch adds hardware breakpoint and hardware watchpoint support
> for ppc.
>
> On BOOKE architecture we cannot share debug resources between QEMU
> and guest because:
>      When QEMU is using debug resources then debug exception must
>      be always enabled. To achieve this we set MSR_DE and also set
>      MSRP_DEP so guest cannot change MSR_DE.
>
>      When emulating debug resource for guest we want guest
>      to control MSR_DE (enable/disable debug interrupt on need).
>
>      So above mentioned two configuration cannot be supported
>      at the same time. So the result is that we cannot share
>      debug resources between QEMU and Guest on BOOKE architecture.
>
> In the current design QEMU gets priority over guest,
> this means that if QEMU is using debug resources then guest
> cannot use them and if guest is using debug resource then
> qemu can overwrite them.
>
> When QEMU is not able to handle debug exception then we inject program
> exception to guest. Yes program exception NOT debug exception and the
> reason is:
>   1) QEMU and guest not sharing debug resources
>   2) For software breakpoint QEMU uses a ehpriv-1 instruction;
>
>   So there cannot be any reason that we are in qemu with exit reason
>   KVM_EXIT_DEBUG  for guest set debug exception, only possibility is
>   guest executed ehpriv-1 privilege instruction and that's why we are
>   injecting program exception.
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> ---
> v5->v6
>   - Inject program exception rather than debug exception if
>     guest is not able to handle debug exception. why? detail
>     in respective patch.
>
>   target-ppc/kvm.c | 245 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 229 insertions(+), 16 deletions(-)
>
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index c4a1fa5..6156f09 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -38,6 +38,7 @@
>   #include "hw/ppc/ppc.h"
>   #include "sysemu/watchdog.h"
>   #include "trace.h"
> +#include "exec/gdbstub.h"
>   
>   //#define DEBUG_KVM
>   
> @@ -412,6 +413,38 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu)
>       return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>   }
>   
> +/* e500 supports 2 h/w breakpoint and 2 watchpoint.
> + * book3s supports only 1 watchpoint, so array size
> + * of 4 is sufficient for now.
> + */
> +#define MAX_HW_BKPTS 4
> +
> +static struct HWBreakpoint {
> +    target_ulong addr;
> +    int type;
> +} hw_debug_points[MAX_HW_BKPTS];
> +
> +static CPUWatchpoint hw_watchpoint;
> +
> +/* Default there is no breakpoint and watchpoint supported */
> +static int max_hw_breakpoint;
> +static int max_hw_watchpoint;
> +static int nb_hw_breakpoint;
> +static int nb_hw_watchpoint;
> +
> +static void kvmppc_hw_debug_points_init(CPUPPCState *cenv)
> +{
> +    if (cenv->excp_model == POWERPC_EXCP_BOOKE) {
> +        max_hw_breakpoint = 2;
> +        max_hw_watchpoint = 2;
> +    }
> +
> +    if ((max_hw_breakpoint + max_hw_watchpoint) > MAX_HW_BKPTS) {
> +        fprintf(stderr, "Error initializing h/w breakpoints\n");
> +        return;
> +    }
> +}
> +
>   int kvm_arch_init_vcpu(CPUState *cs)
>   {
>       PowerPCCPU *cpu = POWERPC_CPU(cs);
> @@ -439,6 +472,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>       }
>   
>       kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode);
> +    kvmppc_hw_debug_points_init(cenv);
>   
>       return ret;
>   }
> @@ -1348,24 +1382,217 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
>       return 0;
>   }
>   
> +static int find_hw_breakpoint(target_ulong addr, int type)
> +{
> +    int n;
> +
> +    assert((nb_hw_breakpoint + nb_hw_watchpoint)
> +           <= ARRAY_SIZE(hw_debug_points));
> +
> +    for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) {
> +        if (hw_debug_points[n].addr == addr && hw_debug_points[n].type == type) {
> +            return n;
> +        }
> +    }
> +
> +    return -1;
> +}
> +
> +static int find_hw_watchpoint(target_ulong addr, int *flag)
> +{
> +    int n;
> +
> +    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_ACCESS);
> +    if (n >= 0) {
> +        *flag = BP_MEM_ACCESS;
> +        return n;
> +    }
> +
> +    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_WRITE);
> +    if (n >= 0) {
> +        *flag = BP_MEM_WRITE;
> +        return n;
> +    }
> +
> +    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_READ);
> +    if (n >= 0) {
> +        *flag = BP_MEM_READ;
> +        return n;
> +    }
> +
> +    return -1;
> +}
> +
> +int kvm_arch_insert_hw_breakpoint(target_ulong addr,
> +                                  target_ulong len, int type)
> +{
> +    if ((nb_hw_breakpoint + nb_hw_watchpoint) >= ARRAY_SIZE(hw_debug_points))
> +	return -ENOBUFS;
> +
> +    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].addr = addr;
> +    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].type = type;
> +
> +    switch (type) {
> +    case GDB_BREAKPOINT_HW:
> +        if (nb_hw_breakpoint >= max_hw_breakpoint) {
> +            return -ENOBUFS;
> +        }
> +
> +        if (find_hw_breakpoint(addr, type) >= 0) {
> +            return -EEXIST;
> +        }
> +
> +        nb_hw_breakpoint++;
> +        break;
> +
> +    case GDB_WATCHPOINT_WRITE:
> +    case GDB_WATCHPOINT_READ:
> +    case GDB_WATCHPOINT_ACCESS:
> +        if (nb_hw_watchpoint >= max_hw_watchpoint) {
> +            return -ENOBUFS;
> +        }
> +
> +        if (find_hw_breakpoint(addr, type) >= 0) {
> +            return -EEXIST;
> +        }
> +
> +        nb_hw_watchpoint++;
> +        break;
> +
> +    default:
> +        return -ENOSYS;
> +    }
> +
> +    return 0;
> +}
> +
> +int kvm_arch_remove_hw_breakpoint(target_ulong addr,
> +                                  target_ulong len, int type)
> +{
> +    int n;
> +
> +    n = find_hw_breakpoint(addr, type);
> +    if (n < 0) {
> +        return -ENOENT;
> +    }
> +
> +    switch (type) {
> +    case GDB_BREAKPOINT_HW:
> +        nb_hw_breakpoint--;
> +        break;
> +
> +    case GDB_WATCHPOINT_WRITE:
> +    case GDB_WATCHPOINT_READ:
> +    case GDB_WATCHPOINT_ACCESS:
> +        nb_hw_watchpoint--;
> +        break;
> +
> +    default:
> +        return -ENOSYS;
> +    }
> +    hw_debug_points[n] = hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint];
> +
> +    return 0;
> +}
> +
> +void kvm_arch_remove_all_hw_breakpoints(void)
> +{
> +    nb_hw_breakpoint = nb_hw_watchpoint = 0;
> +}
> +
>   void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
>   {
> +    int n;
> +
>       /* Software Breakpoint updates */
>       if (kvm_sw_breakpoints_active(cs)) {
>           dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
>       }
> +
> +    assert((nb_hw_breakpoint + nb_hw_watchpoint)
> +           <= ARRAY_SIZE(hw_debug_points));
> +    assert((nb_hw_breakpoint + nb_hw_watchpoint) <= ARRAY_SIZE(dbg->arch.bp));
> +
> +    if (nb_hw_breakpoint + nb_hw_watchpoint > 0) {
> +        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP;
> +        memset(dbg->arch.bp, 0, sizeof(dbg->arch.bp));
> +        for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) {
> +            switch (hw_debug_points[n].type) {
> +            case GDB_BREAKPOINT_HW:
> +                dbg->arch.bp[n].type = KVMPPC_DEBUG_BREAKPOINT;
> +                break;
> +            case GDB_WATCHPOINT_WRITE:
> +                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE;
> +                break;
> +            case GDB_WATCHPOINT_READ:
> +                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_READ;
> +                break;
> +            case GDB_WATCHPOINT_ACCESS:
> +                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE |
> +                                        KVMPPC_DEBUG_WATCH_READ;
> +                break;
> +            default:
> +                cpu_abort(cs, "Unsupported breakpoint type\n");
> +            }
> +            dbg->arch.bp[n].addr = hw_debug_points[n].addr;
> +        }
> +    }
>   }
>   
>   static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run)
>   {
>       CPUState *cs = CPU(cpu);
> +    CPUPPCState *env = &cpu->env;
>       struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
>       int handle = 0;
> +    int n;
> +    int flag = 0;
>   
> -    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
> +    if (cs->singlestep_enabled) {
>           handle = 1;
> +    } else if (arch_info->status) {
> +        if (nb_hw_breakpoint + nb_hw_watchpoint > 0) {
> +            if (arch_info->status & KVMPPC_DEBUG_BREAKPOINT) {
> +                n = find_hw_breakpoint(arch_info->address, GDB_BREAKPOINT_HW);
> +                if (n >= 0) {
> +                    handle = 1;
> +                }
> +            } else if (arch_info->status & (KVMPPC_DEBUG_WATCH_READ |
> +                                            KVMPPC_DEBUG_WATCH_WRITE)) {
> +                n = find_hw_watchpoint(arch_info->address,  &flag);
> +                if (n >= 0) {
> +                    handle = 1;
> +                    cs->watchpoint_hit = &hw_watchpoint;
> +                    hw_watchpoint.vaddr = hw_debug_points[n].addr;
> +                    hw_watchpoint.flags = flag;
> +                }
> +            }
> +        }
> +    } else if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
> +        handle = 1;
> +    } else {
> +        /* QEMU is not able to handle debug exception, so inject
> +         * program exception to guest;
> +         * Yes program exception NOT debug exception !!
> +         * When QEMU is using debug resources then debug exception must
> +         * be always set. To achieve this we set MSR_DE and also set
> +         * MSRP_DEP so guest cannot change MSR_DE.
> +         * When emulating debug resource for guest we want guest
> +         * to control MSR_DE (enable/disable debug interrupt on need).
> +         * Supporting both configurations are NOT possible.
> +         * So the result is that we cannot share debug resources
> +         * between QEMU and Guest on BOOKE architecture.
> +         * In the current design QEMU gets the priority over guest,
> +         * this means that if QEMU is using debug resources then guest
> +         * cannot use them;
> +         * For software breakpoint QEMU uses a ehpriv-1 instruction;
> +         * So there cannot be any reason that we are here for guest
> +         * set debug exception, only possibility is guest executed a
> +         * privilege instruction and that's why we are injecting
> +         * program exception.
> +         */
> +        env->pending_interrupts |=  1 << PPC_INTERRUPT_DEBUG;

This should be part of patch 5/6. Also you don't have to mess with 
env->pending_interrupts at all here - just inject the fault straight away.


Alex