Re: [PATCH v3] kvmppc/booke: Set ESR and DEAR when inject interrupt to guest

2010-02-03 Thread Hollis Blanchard
On Tue, Feb 2, 2010 at 3:44 AM, Liu Yu  wrote:
> Old method prematurely sets ESR and DEAR.
> Move this part after we decide to inject interrupt,
> which is more like hardware behave.
>
> Signed-off-by: Liu Yu 

Acked-by: Hollis Blanchard 
--
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


we have lists of Lawyers, New Businesses, Financial Planners and consumers

2010-02-03 Thread bestow Byrd


Many of these lists are on sale too. Just email me here:  
garrett.st...@leadsparadise.com

  




this is where to go to stop the mailing please send an email to 
disapp...@leadsparadise.com
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/4] kvmppc/booke: exit_nr fixup for guest debug single step

2010-02-03 Thread Alexander Graf
Liu Yu-B13201 wrote:
>  
>
>   
>> -Original Message-
>> From: Alexander Graf [mailto:ag...@suse.de] 
>> Sent: Wednesday, February 03, 2010 6:14 PM
>> To: Liu Yu-B13201
>> Cc: hol...@penguinppc.org; kvm-ppc@vger.kernel.org; 
>> k...@vger.kernel.org
>> Subject: Re: [PATCH 4/4] kvmppc/booke: exit_nr fixup for 
>> guest debug single step
>>
>> Liu Yu-B13201 wrote:
>> 
>>>  
>>>
>>>   
>>>   
 -Original Message-
 From: kvm-ppc-ow...@vger.kernel.org 
 [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf
 Sent: Wednesday, February 03, 2010 5:03 PM
 To: Liu Yu-B13201
 Cc: hol...@penguinppc.org; kvm-ppc@vger.kernel.org; 
 k...@vger.kernel.org; Liu Yu-B13201
 Subject: Re: [PATCH 4/4] kvmppc/booke: exit_nr fixup for 
 guest debug single step


 Am 03.02.2010 um 08:53 schrieb Liu Yu :

 
 
> As BOOKE doesn't have hardware support for virtualization,
> hardware never know who's guest and host.
>
> When enable hardware single step in guest,
> we cannot disabled it at the point we switch back to host.
>   
>   
 Why not? We directly arrive in our code. So we can just 
 disable it, no?

 Or does that break when you'd try to debug the guest 
 interrupt handlers?
 
 
>>> That's the hardware limitition.
>>> Assume received itlb miss interrupt, but it doesn't clear 
>>>   
>> MSR_DE in MSR,
>> 
>>> so on the exit path single step still work and then debug 
>>>   
>> interrupt is
>> 
>>> triggled.
>>>   
>>>   
>> MSRDE is set to 0 by critical class interrupts
>> unless Category E.ED is supported, by Debug   
>> interrupts, and by Machine Check interrupts,  
>> and is left unchanged by all other interrupts.
>>
>> Great.
>>
>> So when single stepping is enabled, you jump into the guest, 
>> get an itlb
>> miss, get out, still have DE set, get in KVM's own DE handler and can
>> process things from there.
>>
>> Could you check if the debug instruction was on PR=0? If so, you can
>> just rfi and be good, right?
>>
>> 
>
> Hr?
> The moment we found this happen we've already saved the guest and loaded host 
> on exit path
> Rfi will make exit path again which means save guest again.
>   

Well the guest saving code is in our hands. So we can just modify the
debug interrupt handler in booke_interrupts.S to check for PR=0 first
thing and then decide whether to save to guest state or return to the
host kernel.

I think that'd make it a lot cleaner.

Alex

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


RE: [PATCH 4/4] kvmppc/booke: exit_nr fixup for guest debug single step

2010-02-03 Thread Liu Yu-B13201
 

> -Original Message-
> From: Alexander Graf [mailto:ag...@suse.de] 
> Sent: Wednesday, February 03, 2010 6:14 PM
> To: Liu Yu-B13201
> Cc: hol...@penguinppc.org; kvm-ppc@vger.kernel.org; 
> k...@vger.kernel.org
> Subject: Re: [PATCH 4/4] kvmppc/booke: exit_nr fixup for 
> guest debug single step
> 
> Liu Yu-B13201 wrote:
> >  
> >
> >   
> >> -Original Message-
> >> From: kvm-ppc-ow...@vger.kernel.org 
> >> [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf
> >> Sent: Wednesday, February 03, 2010 5:03 PM
> >> To: Liu Yu-B13201
> >> Cc: hol...@penguinppc.org; kvm-ppc@vger.kernel.org; 
> >> k...@vger.kernel.org; Liu Yu-B13201
> >> Subject: Re: [PATCH 4/4] kvmppc/booke: exit_nr fixup for 
> >> guest debug single step
> >>
> >>
> >> Am 03.02.2010 um 08:53 schrieb Liu Yu :
> >>
> >> 
> >>> As BOOKE doesn't have hardware support for virtualization,
> >>> hardware never know who's guest and host.
> >>>
> >>> When enable hardware single step in guest,
> >>> we cannot disabled it at the point we switch back to host.
> >>>   
> >> Why not? We directly arrive in our code. So we can just 
> >> disable it, no?
> >>
> >> Or does that break when you'd try to debug the guest 
> >> interrupt handlers?
> >> 
> >
> > That's the hardware limitition.
> > Assume received itlb miss interrupt, but it doesn't clear 
> MSR_DE in MSR,
> > so on the exit path single step still work and then debug 
> interrupt is
> > triggled.
> >   
> 
> MSRDE is set to 0 by critical class interrupts
> unless Category E.ED is supported, by Debug   
> interrupts, and by Machine Check interrupts,  
> and is left unchanged by all other interrupts.
> 
> Great.
> 
> So when single stepping is enabled, you jump into the guest, 
> get an itlb
> miss, get out, still have DE set, get in KVM's own DE handler and can
> process things from there.
> 
> Could you check if the debug instruction was on PR=0? If so, you can
> just rfi and be good, right?
> 

Hr?
The moment we found this happen we've already saved the guest and loaded host 
on exit path
Rfi will make exit path again which means save guest again.

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


Re: [PATCH 3/4] kvmppc/booke: guest debug support

2010-02-03 Thread Jan Kiszka
Liu Yu wrote:
> According to user's gdb command,
> we set the corresponding debug control bits in shadow.
> 
> Signed-off-by: Liu Yu 
> ---
>  arch/powerpc/include/asm/kvm_ppc.h |3 +
>  arch/powerpc/kvm/booke.c   |   93 
> ++--
>  arch/powerpc/kvm/e500.c|8 ---
>  arch/powerpc/kvm/powerpc.c |2 +-
>  4 files changed, 93 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h 
> b/arch/powerpc/include/asm/kvm_ppc.h
> index e264282..8918aac 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -94,6 +94,9 @@ extern int kvmppc_core_emulate_op(struct kvm_run *run, 
> struct kvm_vcpu *vcpu,
>  extern int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int 
> rs);
>  extern int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int 
> rt);
>  
> +extern int kvmppc_core_set_guest_debug(struct kvm_vcpu *vcpu,
> +   struct kvm_guest_debug *dbg);
> +
>  extern int kvmppc_booke_init(void);
>  extern void kvmppc_booke_exit(void);
>  
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 4d686cc..ec2722d 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -267,6 +267,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
> kvm_vcpu *vcpu,
>   break;
>   }
>  
> + if (unlikely(vcpu->guest_debug & KVM_GUESTDBG_ENABLE) &&

This should better check for KVM_GUESTDBG_USE_SW_BP.

> +  (vcpu->arch.last_inst == KVM_INST_GUESTGDB)) {
> + run->exit_reason = KVM_EXIT_DEBUG;
> + run->debug.arch.pc = vcpu->arch.pc;
> + run->debug.arch.exception = exit_nr;
> + kvmppc_account_exit(vcpu, DEBUG_EXITS);
> + r = RESUME_HOST;
> + break;
> + }
> +
>   er = kvmppc_emulate_instruction(run, vcpu);
>   switch (er) {
>   case EMULATE_DONE:
> @@ -293,6 +303,12 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
> kvm_vcpu *vcpu,
>   default:
>   BUG();
>   }
> +
> + if (unlikely(vcpu->guest_debug & KVM_GUESTDBG_ENABLE) &&
> + (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)) {

Checking for KVM_GUESTDBG_ENABLE is redundant as you enforce guest_debug
= 0 in kvmppc_core_set_guest_debug if KVM_GUESTDBG_ENABLE is not set.

> + run->exit_reason = KVM_EXIT_DEBUG;
> + r = RESUME_HOST;
> + }
>   break;
>  
>   case BOOKE_INTERRUPT_FP_UNAVAIL:
> @@ -421,12 +437,27 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
> kvm_vcpu *vcpu,
>   u32 dbsr;
>  
>   vcpu->arch.pc = mfspr(SPRN_CSRR0);
> -
> - /* clear IAC events in DBSR register */
>   dbsr = mfspr(SPRN_DBSR);
> - dbsr &= DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4;
> - mtspr(SPRN_DBSR, dbsr);
> + run->debug.arch.pc = vcpu->arch.pc;
> + run->debug.arch.status = 0;
> +
> + if (dbsr & (DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4)) {
> + run->debug.arch.status |= KVMPPC_DEBUG_BREAKPOINT;
> + } else {
> + if (dbsr & (DBSR_DAC1W | DBSR_DAC2W))
> + run->debug.arch.status |= 
> KVMPPC_DEBUG_WATCH_WRITE;
> + else if (dbsr & (DBSR_DAC1R | DBSR_DAC2R))
> + run->debug.arch.status |= 
> KVMPPC_DEBUG_WATCH_READ;
> + if (dbsr & (DBSR_DAC1R | DBSR_DAC1W))
> + run->debug.arch.pc = 
> vcpu->arch.shadow_dbg_reg.dac1;
> + else if (dbsr & (DBSR_DAC2R | DBSR_DAC2W))
> + run->debug.arch.pc = 
> vcpu->arch.shadow_dbg_reg.dac2;
> + }
>  
> + /* clear events in DBSR register */
> + mtspr(SPRN_DBSR, ~0);
> +
> + run->debug.arch.exception = exit_nr;
>   run->exit_reason = KVM_EXIT_DEBUG;
>   kvmppc_account_exit(vcpu, DEBUG_EXITS);
>   r = RESUME_HOST;
> @@ -560,6 +591,60 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct 
> kvm_dirty_log *log)
>   return -ENOTSUPP;
>  }
>  
> +int kvmppc_core_set_guest_debug(struct kvm_vcpu *vcpu,
> +struct kvm_guest_debug *dbg)
> +{
> + if (!(dbg->control & KVM_GUESTDBG_ENABLE)) {
> + vcpu->guest_debug = 0;
> + return 0;
> + }
> +
> + vcpu->guest_debug = dbg->control;
> + vcpu->arch.shadow_dbg_reg.dbcr0 = 0;
> +
> + if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
> + vcpu->arch.shadow_dbg_reg.dbcr0 |= DBCR0_IDM | DBCR0_IC;
> +
> + if (vcpu->guest_debug & KVM_GUESTDB

Re: [PATCH 1/4] kvmppc: guest debug definitions

2010-02-03 Thread Alexander Graf
Liu Yu-B13201 wrote:
>  
>
>   
>> -Original Message-
>> From: Alexander Graf [mailto:ag...@suse.de] 
>> Sent: Wednesday, February 03, 2010 5:51 PM
>> To: Liu Yu-B13201
>> Cc: hol...@penguinppc.org; kvm-ppc@vger.kernel.org; 
>> k...@vger.kernel.org
>> Subject: Re: [PATCH 1/4] kvmppc: guest debug definitions
>>
>> Liu Yu-B13201 wrote:
>> 
>>>  
>>>
>>>   
>>>   
 -Original Message-
 From: kvm-ppc-ow...@vger.kernel.org 
 [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf
 Sent: Wednesday, February 03, 2010 4:57 PM
 To: Liu Yu-B13201
 Cc: hol...@penguinppc.org; kvm-ppc@vger.kernel.org; 
 k...@vger.kernel.org; Liu Yu-B13201
 Subject: Re: [PATCH 1/4] kvmppc: guest debug definitions


 Am 03.02.2010 um 08:53 schrieb Liu Yu :

 
 
> Signed-off-by: Liu Yu 
> ---
> arch/powerpc/include/asm/kvm.h  |   20 
> arch/powerpc/include/asm/kvm_host.h |   16 
> 2 files changed, 36 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm.h 
>   
>> b/arch/powerpc/include/ 
>> 
> asm/kvm.h
> index 81f3b0b..b7f7861 100644
> --- a/arch/powerpc/include/asm/kvm.h
> +++ b/arch/powerpc/include/asm/kvm.h
> @@ -22,6 +22,9 @@
>
> #include 
>
> +/* Select powerpc specific features in  */
> +#define __KVM_HAVE_GUEST_DEBUG
> +
> struct kvm_regs {
>__u64 pc;
>__u64 cr;
> @@ -71,10 +74,27 @@ struct kvm_fpu {
> };
>
> struct kvm_debug_exit_arch {
> +__u32 exception;
> +__u32 pc;
> +__u32 status;
> };
>
> +#define KVM_INST_GUESTGDB   0x4422
>   
>   
 What instruction is this again? :) Is it something reserved for  
 purposes like this?

 
 
>>> Just an invalid instruction which can generate program interrupt...
>>> I'm open to it's value btw.
>>>   
>>>   
>> Well this definitely doesn't generate a program interrupt. Or at least
>> it shouldn't :-).
>> I just remembered where I've seen an opcode like this before. 
>> This is a
>> part of a dump of arch/powerpc/boot/ps3-hvcall.o
>>
>>  :
>>0:   7c 08 02 a6 mflrr0
>>4:   90 01 00 04 stw r0,4(r1)
>>8:   94 21 ff f0 stwur1,-16(r1)
>>c:   90 61 00 08 stw r3,8(r1)
>>   10:   39 60 00 45 li  r11,69
>>   14:   44 00 00 22 sc  1
>>
>> So as you can see, this is the hypercall instruction for lv1. 
>> IIRC beat
>> uses the same. I don't think we want to reuse that opcode for 
>> ourselves.
>> Maybe one day someone figures it's a good idea to implement a 
>> beat-style
>> ABI in KVM.
>>
>> But IIRC sc can take a lot of values, so we can just take sc 0x1234 or
>> so :-).
>>
>> 
> +
> +#define KVM_GUESTDBG_USE_SW_BP  0x0001
> +#define KVM_GUESTDBG_USE_HW_BP  0x0002
> +
> +#define KVMPPC_DEBUG_NOTYPE 0x0
> +#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
> +#define KVMPPC_DEBUG_WATCH_WRITE(1UL << 2)
> +#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
> +
> /* for KVM_SET_GUEST_DEBUG */
> struct kvm_guest_debug_arch {
> +struct {
> +__u32 addr;
> +__u32 type;
> +} bp[6];
>   
>   
 I can't look up the sources right now. Is this a struct that 
 1:1 maps  
 to an ioctl struct? If so, we should add padding for a 
 possible future  
 extension of debug registers.
 
 
>>> Yes it's used by ioctl.
>>> What's the usually pad size?
>>>   
>>>   
>> I don't think there's a default. I just tend to pad it to something
>> reasonable. I guess in this case we can even just extend bp to 128
>> entries, add a reasonable amount of churn to the debug info 
>> and be good:
>>
>> struct kvm_guest_debug_arch {
>> struct {
>> __u64 addr;
>> __u32 type;
>> __u32 pad1;
>> __u64 pad2;
>> } bp[128];
>> }
>>
>> 
>
> Software breakpoint is maintained by qemu.
> Here it's only used by hardware breakpoint/watchpoint
> Is 128 much too large?
>   

Well, it's only 3kb. And that way we're _really_ future-proof ;-).

Remember, this is only the interface to userspace. The data we keep
around in the kernel can be much smaller.

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


RE: [PATCH 1/4] kvmppc: guest debug definitions

2010-02-03 Thread Liu Yu-B13201
 

> -Original Message-
> From: Alexander Graf [mailto:ag...@suse.de] 
> Sent: Wednesday, February 03, 2010 5:51 PM
> To: Liu Yu-B13201
> Cc: hol...@penguinppc.org; kvm-ppc@vger.kernel.org; 
> k...@vger.kernel.org
> Subject: Re: [PATCH 1/4] kvmppc: guest debug definitions
> 
> Liu Yu-B13201 wrote:
> >  
> >
> >   
> >> -Original Message-
> >> From: kvm-ppc-ow...@vger.kernel.org 
> >> [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf
> >> Sent: Wednesday, February 03, 2010 4:57 PM
> >> To: Liu Yu-B13201
> >> Cc: hol...@penguinppc.org; kvm-ppc@vger.kernel.org; 
> >> k...@vger.kernel.org; Liu Yu-B13201
> >> Subject: Re: [PATCH 1/4] kvmppc: guest debug definitions
> >>
> >>
> >> Am 03.02.2010 um 08:53 schrieb Liu Yu :
> >>
> >> 
> >>> Signed-off-by: Liu Yu 
> >>> ---
> >>> arch/powerpc/include/asm/kvm.h  |   20 
> >>> arch/powerpc/include/asm/kvm_host.h |   16 
> >>> 2 files changed, 36 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/include/asm/kvm.h 
> b/arch/powerpc/include/ 
> >>> asm/kvm.h
> >>> index 81f3b0b..b7f7861 100644
> >>> --- a/arch/powerpc/include/asm/kvm.h
> >>> +++ b/arch/powerpc/include/asm/kvm.h
> >>> @@ -22,6 +22,9 @@
> >>>
> >>> #include 
> >>>
> >>> +/* Select powerpc specific features in  */
> >>> +#define __KVM_HAVE_GUEST_DEBUG
> >>> +
> >>> struct kvm_regs {
> >>>__u64 pc;
> >>>__u64 cr;
> >>> @@ -71,10 +74,27 @@ struct kvm_fpu {
> >>> };
> >>>
> >>> struct kvm_debug_exit_arch {
> >>> +__u32 exception;
> >>> +__u32 pc;
> >>> +__u32 status;
> >>> };
> >>>
> >>> +#define KVM_INST_GUESTGDB   0x4422
> >>>   
> >> What instruction is this again? :) Is it something reserved for  
> >> purposes like this?
> >>
> >> 
> >
> > Just an invalid instruction which can generate program interrupt...
> > I'm open to it's value btw.
> >   
> 
> Well this definitely doesn't generate a program interrupt. Or at least
> it shouldn't :-).
> I just remembered where I've seen an opcode like this before. 
> This is a
> part of a dump of arch/powerpc/boot/ps3-hvcall.o
> 
>  :
>0:   7c 08 02 a6 mflrr0
>4:   90 01 00 04 stw r0,4(r1)
>8:   94 21 ff f0 stwur1,-16(r1)
>c:   90 61 00 08 stw r3,8(r1)
>   10:   39 60 00 45 li  r11,69
>   14:   44 00 00 22 sc  1
> 
> So as you can see, this is the hypercall instruction for lv1. 
> IIRC beat
> uses the same. I don't think we want to reuse that opcode for 
> ourselves.
> Maybe one day someone figures it's a good idea to implement a 
> beat-style
> ABI in KVM.
> 
> But IIRC sc can take a lot of values, so we can just take sc 0x1234 or
> so :-).
> 
> >>> +
> >>> +#define KVM_GUESTDBG_USE_SW_BP  0x0001
> >>> +#define KVM_GUESTDBG_USE_HW_BP  0x0002
> >>> +
> >>> +#define KVMPPC_DEBUG_NOTYPE 0x0
> >>> +#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
> >>> +#define KVMPPC_DEBUG_WATCH_WRITE(1UL << 2)
> >>> +#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
> >>> +
> >>> /* for KVM_SET_GUEST_DEBUG */
> >>> struct kvm_guest_debug_arch {
> >>> +struct {
> >>> +__u32 addr;
> >>> +__u32 type;
> >>> +} bp[6];
> >>>   
> >> I can't look up the sources right now. Is this a struct that 
> >> 1:1 maps  
> >> to an ioctl struct? If so, we should add padding for a 
> >> possible future  
> >> extension of debug registers.
> >> 
> >
> > Yes it's used by ioctl.
> > What's the usually pad size?
> >   
> 
> I don't think there's a default. I just tend to pad it to something
> reasonable. I guess in this case we can even just extend bp to 128
> entries, add a reasonable amount of churn to the debug info 
> and be good:
> 
> struct kvm_guest_debug_arch {
> struct {
> __u64 addr;
> __u32 type;
> __u32 pad1;
> __u64 pad2;
> } bp[128];
> }
> 

Software breakpoint is maintained by qemu.
Here it's only used by hardware breakpoint/watchpoint
Is 128 much too large?

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


Re: [PATCH 4/4] kvmppc/booke: exit_nr fixup for guest debug single step

2010-02-03 Thread Alexander Graf
Liu Yu-B13201 wrote:
>  
>
>   
>> -Original Message-
>> From: kvm-ppc-ow...@vger.kernel.org 
>> [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf
>> Sent: Wednesday, February 03, 2010 5:03 PM
>> To: Liu Yu-B13201
>> Cc: hol...@penguinppc.org; kvm-ppc@vger.kernel.org; 
>> k...@vger.kernel.org; Liu Yu-B13201
>> Subject: Re: [PATCH 4/4] kvmppc/booke: exit_nr fixup for 
>> guest debug single step
>>
>>
>> Am 03.02.2010 um 08:53 schrieb Liu Yu :
>>
>> 
>>> As BOOKE doesn't have hardware support for virtualization,
>>> hardware never know who's guest and host.
>>>
>>> When enable hardware single step in guest,
>>> we cannot disabled it at the point we switch back to host.
>>>   
>> Why not? We directly arrive in our code. So we can just 
>> disable it, no?
>>
>> Or does that break when you'd try to debug the guest 
>> interrupt handlers?
>> 
>
> That's the hardware limitition.
> Assume received itlb miss interrupt, but it doesn't clear MSR_DE in MSR,
> so on the exit path single step still work and then debug interrupt is
> triggled.
>   

MSRDE is set to 0 by critical class interrupts
unless Category E.ED is supported, by Debug   
interrupts, and by Machine Check interrupts,  
and is left unchanged by all other interrupts.

Great.

So when single stepping is enabled, you jump into the guest, get an itlb
miss, get out, still have DE set, get in KVM's own DE handler and can
process things from there.

Could you check if the debug instruction was on PR=0? If so, you can
just rfi and be good, right?


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


RE: [PATCH 4/4] kvmppc/booke: exit_nr fixup for guest debug single step

2010-02-03 Thread Liu Yu-B13201
 

> -Original Message-
> From: kvm-ppc-ow...@vger.kernel.org 
> [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf
> Sent: Wednesday, February 03, 2010 5:03 PM
> To: Liu Yu-B13201
> Cc: hol...@penguinppc.org; kvm-ppc@vger.kernel.org; 
> k...@vger.kernel.org; Liu Yu-B13201
> Subject: Re: [PATCH 4/4] kvmppc/booke: exit_nr fixup for 
> guest debug single step
> 
> 
> Am 03.02.2010 um 08:53 schrieb Liu Yu :
> 
> > As BOOKE doesn't have hardware support for virtualization,
> > hardware never know who's guest and host.
> >
> > When enable hardware single step in guest,
> > we cannot disabled it at the point we switch back to host.
> 
> Why not? We directly arrive in our code. So we can just 
> disable it, no?
> 
> Or does that break when you'd try to debug the guest 
> interrupt handlers?

That's the hardware limitition.
Assume received itlb miss interrupt, but it doesn't clear MSR_DE in MSR,
so on the exit path single step still work and then debug interrupt is
triggled.


> 
> > Thus, we'll see that an single step interrupt happens at
> > the beginning of guest exit path.
> >
> > Then we need to recognize this kind of single step interrupt
> > and fix the exit_nr to the original value.
> > So that everything looks like normal.
> >
> > Signed-off-by: Liu Yu 
> > ---
> > arch/powerpc/kvm/booke.c|   82 
> ++ 
> > +
> > arch/powerpc/kvm/booke_interrupts.S |9 ++--
> > 2 files changed, 87 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> > index ec2722d..9056708 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -24,6 +24,7 @@
> > #include 
> > #include 
> > #include 
> > +#include 
> >
> > #include 
> > #include 
> > @@ -34,6 +35,8 @@
> > #include "booke.h"
> >
> > unsigned long kvmppc_booke_handlers;
> > +unsigned long kvmppc_booke_handler_addr[16];
> > +#define handler_vector_num 
> (sizeof(kvmppc_booke_handler_addr)/sizeof 
> > (kvmppc_booke_handler_addr[0]))
> >
> > #define VM_STAT(x) offsetof(struct kvm, stat.x), KVM_STAT_VM
> > #define VCPU_STAT(x) offsetof(struct kvm_vcpu, stat.x), 
> KVM_STAT_VCPU
> > @@ -214,6 +217,80 @@ void kvmppc_core_deliver_interrupts(struct  
> > kvm_vcpu *vcpu)
> >}
> > }
> >
> > +int kvmppc_read_guest(struct kvm_vcpu *vcpu, unsigned long geaddr,
> > +  void *data, int len)
> 
> Ah, nice. I have something similar in book3s.c. IIRC it's called  
> kvmppc_ld.
> 
> I think we should make the semantics identical and declare it as  
> common kvmppc_core function.
> 
Cool.
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] kvmppc: guest debug definitions

2010-02-03 Thread Alexander Graf
Liu Yu-B13201 wrote:
>  
>
>   
>> -Original Message-
>> From: kvm-ppc-ow...@vger.kernel.org 
>> [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf
>> Sent: Wednesday, February 03, 2010 4:57 PM
>> To: Liu Yu-B13201
>> Cc: hol...@penguinppc.org; kvm-ppc@vger.kernel.org; 
>> k...@vger.kernel.org; Liu Yu-B13201
>> Subject: Re: [PATCH 1/4] kvmppc: guest debug definitions
>>
>>
>> Am 03.02.2010 um 08:53 schrieb Liu Yu :
>>
>> 
>>> Signed-off-by: Liu Yu 
>>> ---
>>> arch/powerpc/include/asm/kvm.h  |   20 
>>> arch/powerpc/include/asm/kvm_host.h |   16 
>>> 2 files changed, 36 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/ 
>>> asm/kvm.h
>>> index 81f3b0b..b7f7861 100644
>>> --- a/arch/powerpc/include/asm/kvm.h
>>> +++ b/arch/powerpc/include/asm/kvm.h
>>> @@ -22,6 +22,9 @@
>>>
>>> #include 
>>>
>>> +/* Select powerpc specific features in  */
>>> +#define __KVM_HAVE_GUEST_DEBUG
>>> +
>>> struct kvm_regs {
>>>__u64 pc;
>>>__u64 cr;
>>> @@ -71,10 +74,27 @@ struct kvm_fpu {
>>> };
>>>
>>> struct kvm_debug_exit_arch {
>>> +__u32 exception;
>>> +__u32 pc;
>>> +__u32 status;
>>> };
>>>
>>> +#define KVM_INST_GUESTGDB   0x4422
>>>   
>> What instruction is this again? :) Is it something reserved for  
>> purposes like this?
>>
>> 
>
> Just an invalid instruction which can generate program interrupt...
> I'm open to it's value btw.
>   

Well this definitely doesn't generate a program interrupt. Or at least
it shouldn't :-).
I just remembered where I've seen an opcode like this before. This is a
part of a dump of arch/powerpc/boot/ps3-hvcall.o

 :
   0:   7c 08 02 a6 mflrr0
   4:   90 01 00 04 stw r0,4(r1)
   8:   94 21 ff f0 stwur1,-16(r1)
   c:   90 61 00 08 stw r3,8(r1)
  10:   39 60 00 45 li  r11,69
  14:   44 00 00 22 sc  1

So as you can see, this is the hypercall instruction for lv1. IIRC beat
uses the same. I don't think we want to reuse that opcode for ourselves.
Maybe one day someone figures it's a good idea to implement a beat-style
ABI in KVM.

But IIRC sc can take a lot of values, so we can just take sc 0x1234 or
so :-).

>>> +
>>> +#define KVM_GUESTDBG_USE_SW_BP  0x0001
>>> +#define KVM_GUESTDBG_USE_HW_BP  0x0002
>>> +
>>> +#define KVMPPC_DEBUG_NOTYPE 0x0
>>> +#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
>>> +#define KVMPPC_DEBUG_WATCH_WRITE(1UL << 2)
>>> +#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
>>> +
>>> /* for KVM_SET_GUEST_DEBUG */
>>> struct kvm_guest_debug_arch {
>>> +struct {
>>> +__u32 addr;
>>> +__u32 type;
>>> +} bp[6];
>>>   
>> I can't look up the sources right now. Is this a struct that 
>> 1:1 maps  
>> to an ioctl struct? If so, we should add padding for a 
>> possible future  
>> extension of debug registers.
>> 
>
> Yes it's used by ioctl.
> What's the usually pad size?
>   

I don't think there's a default. I just tend to pad it to something
reasonable. I guess in this case we can even just extend bp to 128
entries, add a reasonable amount of churn to the debug info and be good:

struct kvm_guest_debug_arch {
struct {
__u64 addr;
__u32 type;
__u32 pad1;
__u64 pad2;
} bp[128];
}

This should be enough to even leverage performance monitoring stuff later on 
that would be able to check if r1 == 0x1234 and then stop :-).


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


RE: [PATCH 1/4] kvmppc: guest debug definitions

2010-02-03 Thread Liu Yu-B13201
 

> -Original Message-
> From: kvm-ppc-ow...@vger.kernel.org 
> [mailto:kvm-ppc-ow...@vger.kernel.org] On Behalf Of Alexander Graf
> Sent: Wednesday, February 03, 2010 4:57 PM
> To: Liu Yu-B13201
> Cc: hol...@penguinppc.org; kvm-ppc@vger.kernel.org; 
> k...@vger.kernel.org; Liu Yu-B13201
> Subject: Re: [PATCH 1/4] kvmppc: guest debug definitions
> 
> 
> Am 03.02.2010 um 08:53 schrieb Liu Yu :
> 
> > Signed-off-by: Liu Yu 
> > ---
> > arch/powerpc/include/asm/kvm.h  |   20 
> > arch/powerpc/include/asm/kvm_host.h |   16 
> > 2 files changed, 36 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/ 
> > asm/kvm.h
> > index 81f3b0b..b7f7861 100644
> > --- a/arch/powerpc/include/asm/kvm.h
> > +++ b/arch/powerpc/include/asm/kvm.h
> > @@ -22,6 +22,9 @@
> >
> > #include 
> >
> > +/* Select powerpc specific features in  */
> > +#define __KVM_HAVE_GUEST_DEBUG
> > +
> > struct kvm_regs {
> >__u64 pc;
> >__u64 cr;
> > @@ -71,10 +74,27 @@ struct kvm_fpu {
> > };
> >
> > struct kvm_debug_exit_arch {
> > +__u32 exception;
> > +__u32 pc;
> > +__u32 status;
> > };
> >
> > +#define KVM_INST_GUESTGDB   0x4422
> 
> What instruction is this again? :) Is it something reserved for  
> purposes like this?
> 

Just an invalid instruction which can generate program interrupt...
I'm open to it's value btw.

> 
> > +
> > +#define KVM_GUESTDBG_USE_SW_BP  0x0001
> > +#define KVM_GUESTDBG_USE_HW_BP  0x0002
> > +
> > +#define KVMPPC_DEBUG_NOTYPE 0x0
> > +#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
> > +#define KVMPPC_DEBUG_WATCH_WRITE(1UL << 2)
> > +#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
> > +
> > /* for KVM_SET_GUEST_DEBUG */
> > struct kvm_guest_debug_arch {
> > +struct {
> > +__u32 addr;
> > +__u32 type;
> > +} bp[6];
> 
> I can't look up the sources right now. Is this a struct that 
> 1:1 maps  
> to an ioctl struct? If so, we should add padding for a 
> possible future  
> extension of debug registers.

Yes it's used by ioctl.
What's the usually pad size?

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


Re: [PATCH 4/4] kvmppc/booke: exit_nr fixup for guest debug single step

2010-02-03 Thread Alexander Graf


Am 03.02.2010 um 08:53 schrieb Liu Yu :


As BOOKE doesn't have hardware support for virtualization,
hardware never know who's guest and host.

When enable hardware single step in guest,
we cannot disabled it at the point we switch back to host.


Why not? We directly arrive in our code. So we can just disable it, no?

Or does that break when you'd try to debug the guest interrupt handlers?


Thus, we'll see that an single step interrupt happens at
the beginning of guest exit path.

Then we need to recognize this kind of single step interrupt
and fix the exit_nr to the original value.
So that everything looks like normal.

Signed-off-by: Liu Yu 
---
arch/powerpc/kvm/booke.c|   82 ++ 
+

arch/powerpc/kvm/booke_interrupts.S |9 ++--
2 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index ec2722d..9056708 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -24,6 +24,7 @@
#include 
#include 
#include 
+#include 

#include 
#include 
@@ -34,6 +35,8 @@
#include "booke.h"

unsigned long kvmppc_booke_handlers;
+unsigned long kvmppc_booke_handler_addr[16];
+#define handler_vector_num (sizeof(kvmppc_booke_handler_addr)/sizeof 
(kvmppc_booke_handler_addr[0]))


#define VM_STAT(x) offsetof(struct kvm, stat.x), KVM_STAT_VM
#define VCPU_STAT(x) offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU
@@ -214,6 +217,80 @@ void kvmppc_core_deliver_interrupts(struct  
kvm_vcpu *vcpu)

   }
}

+int kvmppc_read_guest(struct kvm_vcpu *vcpu, unsigned long geaddr,
+  void *data, int len)


Ah, nice. I have something similar in book3s.c. IIRC it's called  
kvmppc_ld.


I think we should make the semantics identical and declare it as  
common kvmppc_core function.


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


Re: [PATCH 1/4] kvmppc: guest debug definitions

2010-02-03 Thread Alexander Graf


Am 03.02.2010 um 08:53 schrieb Liu Yu :


Signed-off-by: Liu Yu 
---
arch/powerpc/include/asm/kvm.h  |   20 
arch/powerpc/include/asm/kvm_host.h |   16 
2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/ 
asm/kvm.h

index 81f3b0b..b7f7861 100644
--- a/arch/powerpc/include/asm/kvm.h
+++ b/arch/powerpc/include/asm/kvm.h
@@ -22,6 +22,9 @@

#include 

+/* Select powerpc specific features in  */
+#define __KVM_HAVE_GUEST_DEBUG
+
struct kvm_regs {
   __u64 pc;
   __u64 cr;
@@ -71,10 +74,27 @@ struct kvm_fpu {
};

struct kvm_debug_exit_arch {
+__u32 exception;
+__u32 pc;
+__u32 status;
};

+#define KVM_INST_GUESTGDB   0x4422


What instruction is this again? :) Is it something reserved for  
purposes like this?


Alex


+
+#define KVM_GUESTDBG_USE_SW_BP  0x0001
+#define KVM_GUESTDBG_USE_HW_BP  0x0002
+
+#define KVMPPC_DEBUG_NOTYPE 0x0
+#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
+#define KVMPPC_DEBUG_WATCH_WRITE(1UL << 2)
+#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
+
/* for KVM_SET_GUEST_DEBUG */
struct kvm_guest_debug_arch {
+struct {
+__u32 addr;
+__u32 type;
+} bp[6];


I can't look up the sources right now. Is this a struct that 1:1 maps  
to an ioctl struct? If so, we should add padding for a possible future  
extension of debug registers.


I'd also prefer to see addr be u64. On 32 bit targets we can just use  
the lower 32 bits only.


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


Re: [PATCH RESEND 0/4] kvmppc/booke: add guest debug support

2010-02-03 Thread Alexander Graf


Am 03.02.2010 um 08:53 schrieb Liu Yu :


This patchset add guest debug support for booke.


I'd like to see an ack from Jan here. Some code looks like it uses  
generic interfaces.


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


[PATCH 4/4] kvmppc/booke: exit_nr fixup for guest debug single step

2010-02-03 Thread Liu Yu
As BOOKE doesn't have hardware support for virtualization,
hardware never know who's guest and host.

When enable hardware single step in guest,
we cannot disabled it at the point we switch back to host.
Thus, we'll see that an single step interrupt happens at
the beginning of guest exit path.

Then we need to recognize this kind of single step interrupt
and fix the exit_nr to the original value.
So that everything looks like normal.

Signed-off-by: Liu Yu 
---
 arch/powerpc/kvm/booke.c|   82 +++
 arch/powerpc/kvm/booke_interrupts.S |9 ++--
 2 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index ec2722d..9056708 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -34,6 +35,8 @@
 #include "booke.h"
 
 unsigned long kvmppc_booke_handlers;
+unsigned long kvmppc_booke_handler_addr[16];
+#define handler_vector_num 
(sizeof(kvmppc_booke_handler_addr)/sizeof(kvmppc_booke_handler_addr[0]))
 
 #define VM_STAT(x) offsetof(struct kvm, stat.x), KVM_STAT_VM
 #define VCPU_STAT(x) offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU
@@ -214,6 +217,80 @@ void kvmppc_core_deliver_interrupts(struct kvm_vcpu *vcpu)
}
 }
 
+int kvmppc_read_guest(struct kvm_vcpu *vcpu, unsigned long geaddr,
+ void *data, int len)
+{
+   int gtlb_index;
+   gpa_t gpa;
+   gfn_t gfn;
+   struct page *page;
+   void *headdr, *from;
+
+   /* Check the guest TLB. */
+   gtlb_index = kvmppc_mmu_itlb_index(vcpu, geaddr);
+   if (gtlb_index < 0)
+   return -EFAULT;
+
+   gpa = kvmppc_mmu_xlate(vcpu, gtlb_index, geaddr);
+   gfn = gpa >> PAGE_SHIFT;
+
+   page = gfn_to_page(vcpu->kvm, gfn);
+   if (page == bad_page)
+   return -EFAULT;
+
+   headdr = kmap_atomic(page, KM_USER0);
+   if (!headdr)
+   return -EFAULT;
+   from = headdr + (geaddr & (PAGE_SIZE - 1));
+   memcpy(data, from, len);
+   kunmap_atomic(headdr, KM_USER0);
+
+   return 0;
+}
+
+static unsigned int kvmppc_guest_debug_exitnr_fixup(struct kvm_vcpu *vcpu,
+  unsigned int exit_nr)
+{
+   unsigned int ret = exit_nr;
+
+   u32 csrr0 = mfspr(SPRN_CSRR0);
+   u32 dbsr = mfspr(SPRN_DBSR);
+
+   if ((dbsr | DBSR_IC) &&
+   csrr0 >= kvmppc_booke_handlers &&
+   csrr0 < kvmppc_booke_handlers + (PAGE_SIZE << VCPU_SIZE_ORDER)) {
+   int i = 0;
+
+   for (i = 0; i < handler_vector_num; i++) {
+   if (kvmppc_booke_handler_addr[i] &&
+   csrr0 == kvmppc_booke_handler_addr[i] + 4) {
+   mtspr(SPRN_DBSR, ~0);
+   ret = i;
+   break;
+   }
+   }
+
+   }
+
+   switch (ret) {
+   case BOOKE_INTERRUPT_DEBUG:
+   case BOOKE_INTERRUPT_ITLB_MISS:
+   case BOOKE_INTERRUPT_EXTERNAL:
+   case BOOKE_INTERRUPT_DECREMENTER:
+   break;
+
+   case BOOKE_INTERRUPT_PROGRAM:
+   case BOOKE_INTERRUPT_DTLB_MISS:
+   /* Need to save the last instruction */
+   kvmppc_read_guest(vcpu, vcpu->arch.pc, &vcpu->arch.last_inst, 
4);
+   break;
+   default:
+   printk("Unhandled debug after interrupt:%d\n", ret);
+   }
+
+   return ret;
+}
+
 /**
  * kvmppc_handle_exit
  *
@@ -233,6 +310,9 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu 
*vcpu,
run->exit_reason = KVM_EXIT_UNKNOWN;
run->ready_for_interrupt_injection = 1;
 
+   if (unlikely(exit_nr == BOOKE_INTERRUPT_DEBUG))
+   exit_nr = kvmppc_guest_debug_exitnr_fixup(vcpu, exit_nr);
+
switch (exit_nr) {
case BOOKE_INTERRUPT_MACHINE_CHECK:
printk("MACHINE CHECK: %lx\n", mfspr(SPRN_MCSR));
@@ -686,6 +766,8 @@ int __init kvmppc_booke_init(void)
memcpy((void *)kvmppc_booke_handlers + ivor[i],
   kvmppc_handlers_start + i * kvmppc_handler_len,
   kvmppc_handler_len);
+   kvmppc_booke_handler_addr[i] =
+   (unsigned long)kvmppc_booke_handlers + ivor[i];
}
flush_icache_range(kvmppc_booke_handlers,
   kvmppc_booke_handlers + max_ivor + 
kvmppc_handler_len);
diff --git a/arch/powerpc/kvm/booke_interrupts.S 
b/arch/powerpc/kvm/booke_interrupts.S
index 644ff1d..fdc48c1 100644
--- a/arch/powerpc/kvm/booke_interrupts.S
+++ b/arch/powerpc/kvm/booke_interrupts.S
@@ -42,16 +42,17 @@
 #define HOST_STACK_LR   (HOST_STACK_SIZE + 4) /* In caller stack frame. */
 
 #define NEED_INST_MASK ((1

[PATCH 3/4] kvmppc/booke: guest debug support

2010-02-03 Thread Liu Yu
According to user's gdb command,
we set the corresponding debug control bits in shadow.

Signed-off-by: Liu Yu 
---
 arch/powerpc/include/asm/kvm_ppc.h |3 +
 arch/powerpc/kvm/booke.c   |   93 ++--
 arch/powerpc/kvm/e500.c|8 ---
 arch/powerpc/kvm/powerpc.c |2 +-
 4 files changed, 93 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_ppc.h 
b/arch/powerpc/include/asm/kvm_ppc.h
index e264282..8918aac 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -94,6 +94,9 @@ extern int kvmppc_core_emulate_op(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
 extern int kvmppc_core_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, int rs);
 extern int kvmppc_core_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt);
 
+extern int kvmppc_core_set_guest_debug(struct kvm_vcpu *vcpu,
+   struct kvm_guest_debug *dbg);
+
 extern int kvmppc_booke_init(void);
 extern void kvmppc_booke_exit(void);
 
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 4d686cc..ec2722d 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -267,6 +267,16 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
break;
}
 
+   if (unlikely(vcpu->guest_debug & KVM_GUESTDBG_ENABLE) &&
+(vcpu->arch.last_inst == KVM_INST_GUESTGDB)) {
+   run->exit_reason = KVM_EXIT_DEBUG;
+   run->debug.arch.pc = vcpu->arch.pc;
+   run->debug.arch.exception = exit_nr;
+   kvmppc_account_exit(vcpu, DEBUG_EXITS);
+   r = RESUME_HOST;
+   break;
+   }
+
er = kvmppc_emulate_instruction(run, vcpu);
switch (er) {
case EMULATE_DONE:
@@ -293,6 +303,12 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
default:
BUG();
}
+
+   if (unlikely(vcpu->guest_debug & KVM_GUESTDBG_ENABLE) &&
+   (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)) {
+   run->exit_reason = KVM_EXIT_DEBUG;
+   r = RESUME_HOST;
+   }
break;
 
case BOOKE_INTERRUPT_FP_UNAVAIL:
@@ -421,12 +437,27 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
u32 dbsr;
 
vcpu->arch.pc = mfspr(SPRN_CSRR0);
-
-   /* clear IAC events in DBSR register */
dbsr = mfspr(SPRN_DBSR);
-   dbsr &= DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4;
-   mtspr(SPRN_DBSR, dbsr);
+   run->debug.arch.pc = vcpu->arch.pc;
+   run->debug.arch.status = 0;
+
+   if (dbsr & (DBSR_IAC1 | DBSR_IAC2 | DBSR_IAC3 | DBSR_IAC4)) {
+   run->debug.arch.status |= KVMPPC_DEBUG_BREAKPOINT;
+   } else {
+   if (dbsr & (DBSR_DAC1W | DBSR_DAC2W))
+   run->debug.arch.status |= 
KVMPPC_DEBUG_WATCH_WRITE;
+   else if (dbsr & (DBSR_DAC1R | DBSR_DAC2R))
+   run->debug.arch.status |= 
KVMPPC_DEBUG_WATCH_READ;
+   if (dbsr & (DBSR_DAC1R | DBSR_DAC1W))
+   run->debug.arch.pc = 
vcpu->arch.shadow_dbg_reg.dac1;
+   else if (dbsr & (DBSR_DAC2R | DBSR_DAC2W))
+   run->debug.arch.pc = 
vcpu->arch.shadow_dbg_reg.dac2;
+   }
 
+   /* clear events in DBSR register */
+   mtspr(SPRN_DBSR, ~0);
+
+   run->debug.arch.exception = exit_nr;
run->exit_reason = KVM_EXIT_DEBUG;
kvmppc_account_exit(vcpu, DEBUG_EXITS);
r = RESUME_HOST;
@@ -560,6 +591,60 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct 
kvm_dirty_log *log)
return -ENOTSUPP;
 }
 
+int kvmppc_core_set_guest_debug(struct kvm_vcpu *vcpu,
+struct kvm_guest_debug *dbg)
+{
+   if (!(dbg->control & KVM_GUESTDBG_ENABLE)) {
+   vcpu->guest_debug = 0;
+   return 0;
+   }
+
+   vcpu->guest_debug = dbg->control;
+   vcpu->arch.shadow_dbg_reg.dbcr0 = 0;
+
+   if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
+   vcpu->arch.shadow_dbg_reg.dbcr0 |= DBCR0_IDM | DBCR0_IC;
+
+   if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
+   struct kvmppc_debug_reg *gdbgr = &(vcpu->arch.shadow_dbg_reg);
+   int n, b = 0, w = 0;
+   const u32 bp_code[] = {
+   DBCR0_IAC1 | DBCR0_IDM,
+   DBCR0_IAC2 | DBCR0_IDM,
+   DBCR0_IAC3 | DBCR0_IDM,
+   DBCR

[PATCH 2/4] kvmppc/booke: switch shadow/host debug registers on guest enter/exit path

2010-02-03 Thread Liu Yu
This provide a precise way to avoid confounding settings of guest and host.
Also the guest hardware emulation about debug can be implemented based on this.

Signed-off-by: Liu Yu 
---
 arch/powerpc/kernel/asm-offsets.c   |3 ++
 arch/powerpc/kvm/booke_interrupts.S |   58 +++
 2 files changed, 61 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/asm-offsets.c 
b/arch/powerpc/kernel/asm-offsets.c
index 957ceb7..67e978d 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -425,6 +425,9 @@ 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_SHADOW_DBG, offsetof(struct kvm_vcpu, arch.shadow_dbg_reg));
+   DEFINE(VCPU_HOST_DBG, offsetof(struct kvm_vcpu, arch.host_dbg_reg));
+   DEFINE(VCPU_GUEST_DEBUG, offsetof(struct kvm_vcpu, guest_debug));
 
/* book3s_64 */
 #ifdef CONFIG_PPC64
diff --git a/arch/powerpc/kvm/booke_interrupts.S 
b/arch/powerpc/kvm/booke_interrupts.S
index 380a78c..644ff1d 100644
--- a/arch/powerpc/kvm/booke_interrupts.S
+++ b/arch/powerpc/kvm/booke_interrupts.S
@@ -168,6 +168,26 @@ _GLOBAL(kvmppc_resume_host)
stw r9, VCPU_FAULT_ESR(r4)
 ..skip_esr:
 
+   lwz r6, VCPU_GUEST_DEBUG(r4)
+   or. r6, r6, r6
+   beq ..skip_load_host_debug
+   addir7, r4, VCPU_HOST_DBG - 4
+   lwzur9, 4(r7)
+   mtspr   SPRN_DBCR0, r9
+   lwzur9, 4(r7)
+   mtspr   SPRN_IAC1, r9
+   lwzur9, 4(r7)
+   mtspr   SPRN_IAC2, r9
+   lwzur9, 4(r7)
+   mtspr   SPRN_IAC3, r9
+   lwzur9, 4(r7)
+   mtspr   SPRN_IAC4, r9
+   lwzur9, 4(r7)
+   mtspr   SPRN_DAC1, r9
+   lwzur9, 4(r7)
+   mtspr   SPRN_DAC2, r9
+..skip_load_host_debug:
+
/* Save remaining volatile guest register state to vcpu. */
stw r0, VCPU_GPR(r0)(r4)
stw r1, VCPU_GPR(r1)(r4)
@@ -392,6 +412,44 @@ lightweight_exit:
lwz r3, VCPU_SPRG7(r4)
mtspr   SPRN_SPRG7W, r3
 
+   lwz r6, VCPU_GUEST_DEBUG(r4)
+   or. r6, r6, r6
+   beq ..skip_load_guest_debug
+   mfmsr   r7
+   rlwinm  r7, r7, 0, ~MSR_DE
+   mtmsr   r7
+   addir7, r4, VCPU_HOST_DBG - 4
+   mfspr   r8, SPRN_DBCR0
+   stwur8, 4(r7)
+   mfspr   r8, SPRN_IAC1
+   stwur8, 4(r7)
+   mfspr   r8, SPRN_IAC2
+   stwur8, 4(r7)
+   mfspr   r8, SPRN_IAC3
+   stwur8, 4(r7)
+   mfspr   r8, SPRN_IAC4
+   stwur8, 4(r7)
+   mfspr   r8, SPRN_DAC1
+   stwur8, 4(r7)
+   mfspr   r8, SPRN_DAC2
+   stwur8, 4(r7)
+   addir7, r4, VCPU_SHADOW_DBG - 4
+   lwzur8, 4(r7)
+   mtspr   SPRN_DBCR0, r8
+   lwzur8, 4(r7)
+   mtspr   SPRN_IAC1, r8
+   lwzur8, 4(r7)
+   mtspr   SPRN_IAC2, r8
+   lwzur8, 4(r7)
+   mtspr   SPRN_IAC3, r8
+   lwzur8, 4(r7)
+   mtspr   SPRN_IAC4, r8
+   lwzur8, 4(r7)
+   mtspr   SPRN_DAC1, r8
+   lwzur8, 4(r7)
+   mtspr   SPRN_DAC2, r8
+..skip_load_guest_debug:
+
 #ifdef CONFIG_KVM_EXIT_TIMING
/* save enter time */
 1:
-- 
1.6.4

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


[PATCH RESEND 0/4] kvmppc/booke: add guest debug support

2010-02-03 Thread Liu Yu
This patchset add guest debug support for booke.

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


[PATCH 1/4] kvmppc: guest debug definitions

2010-02-03 Thread Liu Yu
Signed-off-by: Liu Yu 
---
 arch/powerpc/include/asm/kvm.h  |   20 
 arch/powerpc/include/asm/kvm_host.h |   16 
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm.h b/arch/powerpc/include/asm/kvm.h
index 81f3b0b..b7f7861 100644
--- a/arch/powerpc/include/asm/kvm.h
+++ b/arch/powerpc/include/asm/kvm.h
@@ -22,6 +22,9 @@
 
 #include 
 
+/* Select powerpc specific features in  */
+#define __KVM_HAVE_GUEST_DEBUG
+
 struct kvm_regs {
__u64 pc;
__u64 cr;
@@ -71,10 +74,27 @@ struct kvm_fpu {
 };
 
 struct kvm_debug_exit_arch {
+   __u32 exception;
+   __u32 pc;
+   __u32 status;
 };
 
+#define KVM_INST_GUESTGDB   0x4422
+
+#define KVM_GUESTDBG_USE_SW_BP  0x0001
+#define KVM_GUESTDBG_USE_HW_BP  0x0002
+
+#define KVMPPC_DEBUG_NOTYPE 0x0
+#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
+#define KVMPPC_DEBUG_WATCH_WRITE(1UL << 2)
+#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
+
 /* for KVM_SET_GUEST_DEBUG */
 struct kvm_guest_debug_arch {
+   struct {
+   __u32 addr;
+   __u32 type;
+   } bp[6];
 };
 
 #endif /* __LINUX_KVM_POWERPC_H */
diff --git a/arch/powerpc/include/asm/kvm_host.h 
b/arch/powerpc/include/asm/kvm_host.h
index 5e5bae7..a364832 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -157,6 +157,18 @@ struct hpte_cache {
struct kvmppc_pte pte;
 };
 
+struct kvmppc_debug_reg {
+   u32 dbcr0;
+   u32 iac[0];
+   u32 iac1;
+   u32 iac2;
+   u32 iac3;
+   u32 iac4;
+   u32 dac[0];
+   u32 dac1;
+   u32 dac2;
+};
+
 struct kvm_vcpu_arch {
ulong host_stack;
u32 host_pid;
@@ -240,6 +252,9 @@ struct kvm_vcpu_arch {
u32 dbcr1;
u32 dbsr;
 
+   struct kvmppc_debug_reg shadow_dbg_reg;
+   struct kvmppc_debug_reg host_dbg_reg;
+
 #ifdef CONFIG_KVM_EXIT_TIMING
struct kvmppc_exit_timing timing_exit;
struct kvmppc_exit_timing timing_last_enter;
@@ -274,6 +289,7 @@ struct kvm_vcpu_arch {
struct tasklet_struct tasklet;
u64 dec_jiffies;
unsigned long pending_exceptions;
+   struct kvm_guest_debug_arch dbg;
 
 #ifdef CONFIG_PPC64
struct hpte_cache hpte_cache[HPTEG_CACHE_NUM];
-- 
1.6.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