> -----Original Message-----
> From: tiejun.chen [mailto:tiejun.c...@windriver.com]
> Sent: Wednesday, June 26, 2013 12:25 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-...@vger.kernel.org; k...@vger.kernel.org; ag...@suse.de; Wood Scott-
> B07421; b...@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org; linux-
> ker...@vger.kernel.org; mi...@neuling.org; Bhushan Bharat-R65777
> Subject: Re: [PATCH 4/6 v5] KVM: PPC: exit to user space on "ehpriv" 
> instruction
> 
> On 06/26/2013 01:42 PM, Bharat Bhushan wrote:
> > "ehpriv" instruction is used for setting software breakpoints
> > by user space. This patch adds support to exit to user space
> > with "run->debug" have relevant information.
> >
> > As this is the first point we are using run->debug, also defined
> > the run->debug structure.
> >
> > Signed-off-by: Bharat Bhushan <bharat.bhus...@freescale.com>
> > ---
> >   arch/powerpc/include/asm/disassemble.h |    4 ++++
> >   arch/powerpc/include/uapi/asm/kvm.h    |   21 +++++++++++++++++----
> >   arch/powerpc/kvm/e500_emulate.c        |   27 +++++++++++++++++++++++++++
> >   3 files changed, 48 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/disassemble.h
> b/arch/powerpc/include/asm/disassemble.h
> > index 9b198d1..856f8de 100644
> > --- a/arch/powerpc/include/asm/disassemble.h
> > +++ b/arch/powerpc/include/asm/disassemble.h
> > @@ -77,4 +77,8 @@ static inline unsigned int get_d(u32 inst)
> >     return inst & 0xffff;
> >   }
> >
> > +static inline unsigned int get_oc(u32 inst)
> > +{
> > +   return (inst >> 11) & 0x7fff;
> > +}
> >   #endif /* __ASM_PPC_DISASSEMBLE_H__ */
> > diff --git a/arch/powerpc/include/uapi/asm/kvm.h
> b/arch/powerpc/include/uapi/asm/kvm.h
> > index 0fb1a6e..ded0607 100644
> > --- a/arch/powerpc/include/uapi/asm/kvm.h
> > +++ b/arch/powerpc/include/uapi/asm/kvm.h
> > @@ -269,7 +269,24 @@ struct kvm_fpu {
> >     __u64 fpr[32];
> >   };
> >
> > +/*
> > + * Defines for h/w breakpoint, watchpoint (read, write or both) and
> > + * software breakpoint.
> > + * These are used as "type" in KVM_SET_GUEST_DEBUG ioctl and "status"
> > + * for KVM_DEBUG_EXIT.
> > + */
> > +#define KVMPPC_DEBUG_NONE          0x0
> > +#define KVMPPC_DEBUG_BREAKPOINT            (1UL << 1)
> > +#define KVMPPC_DEBUG_WATCH_WRITE   (1UL << 2)
> > +#define KVMPPC_DEBUG_WATCH_READ            (1UL << 3)
> >   struct kvm_debug_exit_arch {
> > +   __u64 address;
> > +   /*
> > +    * exiting to userspace because of h/w breakpoint, watchpoint
> > +    * (read, write or both) and software breakpoint.
> > +    */
> > +   __u32 status;
> > +   __u32 reserved;
> >   };
> >
> >   /* for KVM_SET_GUEST_DEBUG */
> > @@ -281,10 +298,6 @@ struct kvm_guest_debug_arch {
> >              * Type denotes h/w breakpoint, read watchpoint, write
> >              * watchpoint or watchpoint (both read and write).
> >              */
> > -#define KVMPPC_DEBUG_NONE          0x0
> > -#define KVMPPC_DEBUG_BREAKPOINT            (1UL << 1)
> > -#define KVMPPC_DEBUG_WATCH_WRITE   (1UL << 2)
> > -#define KVMPPC_DEBUG_WATCH_READ            (1UL << 3)
> >             __u32 type;
> >             __u32 reserved;
> >     } bp[16];
> > diff --git a/arch/powerpc/kvm/e500_emulate.c 
> > b/arch/powerpc/kvm/e500_emulate.c
> > index b10a012..dab9d07 100644
> > --- a/arch/powerpc/kvm/e500_emulate.c
> > +++ b/arch/powerpc/kvm/e500_emulate.c
> > @@ -26,6 +26,8 @@
> >   #define XOP_TLBRE   946
> >   #define XOP_TLBWE   978
> >   #define XOP_TLBILX  18
> > +#define XOP_EHPRIV  270
> > +#define EHPRIV_OC_DEBUG 0
> 
> As I think the case, "OC = 0", is a bit specific since IIRC, if the OC
> operand is omitted, its equal 0 by default. So I think we should start this OC
> value from 1 or other magic number.

ehpriv instruction is defined to be used as:
        ehpriv OC // where OC can be 0,1, ... n
and in extended for it can be used as
        ehpriv // With no OC, and here it assumes OC = 0
So OC = 0 is not specific but "ehpriv" is same as "ehpriv 0".

I do not think of any special reason to reserve "ehpriv" and "ehpriv 0".

Thanks
-Bharat

> 
> And if possible, we'd better add some comments to describe this to make the OC
> definition readable.
> 
> Tiejun
> 
> >
> >   #ifdef CONFIG_KVM_E500MC
> >   static int dbell2prio(ulong param)
> > @@ -82,6 +84,26 @@ static int kvmppc_e500_emul_msgsnd(struct kvm_vcpu *vcpu,
> int rb)
> >   }
> >   #endif
> >
> > +static int kvmppc_e500_emul_ehpriv(struct kvm_run *run, struct kvm_vcpu
> *vcpu,
> > +                              unsigned int inst, int *advance)
> > +{
> > +   int emulated = EMULATE_DONE;
> > +
> > +   switch (get_oc(inst)) {
> > +   case EHPRIV_OC_DEBUG:
> > +           run->exit_reason = KVM_EXIT_DEBUG;
> > +           run->debug.arch.address = vcpu->arch.pc;
> > +           run->debug.arch.status = 0;
> > +           kvmppc_account_exit(vcpu, DEBUG_EXITS);
> > +           emulated = EMULATE_EXIT_USER;
> > +           *advance = 0;
> > +           break;
> > +   default:
> > +           emulated = EMULATE_FAIL;
> > +   }
> > +   return emulated;
> > +}
> > +
> >   int kvmppc_core_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu,
> >                              unsigned int inst, int *advance)
> >   {
> > @@ -130,6 +152,11 @@ int kvmppc_core_emulate_op(struct kvm_run *run, struct
> kvm_vcpu *vcpu,
> >                     emulated = kvmppc_e500_emul_tlbivax(vcpu, ea);
> >                     break;
> >
> > +           case XOP_EHPRIV:
> > +                   emulated = kvmppc_e500_emul_ehpriv(run, vcpu, inst,
> > +                                                      advance);
> > +                   break;
> > +
> >             default:
> >                     emulated = EMULATE_FAIL;
> >             }
> >
> 

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to