RE: [PATCH 4/8] Added ONE_REG interface for debug instruction

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Alexander Graf [mailto:agraf@xxxxxxx]
> Sent: Friday, January 25, 2013 5:18 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; Bhushan Bharat-R65777
> Subject: Re: [PATCH 4/8] Added ONE_REG interface for debug instruction
> 
> 
> On 16.01.2013, at 09:24, Bharat Bhushan wrote:
> 
> > This patch adds the one_reg interface to get the special instruction
> > to be used for setting software breakpoint from userspace.
> >
> > Signed-off-by: Bharat Bhushan <bharat.bhushan@xxxxxxxxxxxxx>
> > ---
> > Documentation/virtual/kvm/api.txt   |    1 +
> > arch/powerpc/include/asm/kvm_ppc.h  |    1 +
> > arch/powerpc/include/uapi/asm/kvm.h |    3 +++
> > arch/powerpc/kvm/44x.c              |    5 +++++
> > arch/powerpc/kvm/booke.c            |   10 ++++++++++
> > arch/powerpc/kvm/e500.c             |    5 +++++
> > arch/powerpc/kvm/e500.h             |    9 +++++++++
> > arch/powerpc/kvm/e500mc.c           |    5 +++++
> > 8 files changed, 39 insertions(+), 0 deletions(-)
> >
> > diff --git a/Documentation/virtual/kvm/api.txt
> > b/Documentation/virtual/kvm/api.txt
> > index 09905cb..7e8be9e 100644
> > --- a/Documentation/virtual/kvm/api.txt
> > +++ b/Documentation/virtual/kvm/api.txt
> > @@ -1775,6 +1775,7 @@ registers, find a list below:
> >   PPC   | KVM_REG_PPC_VPA_DTL   | 128
> >   PPC   | KVM_REG_PPC_EPCR	| 32
> >   PPC   | KVM_REG_PPC_EPR	| 32
> > +  PPC   | KVM_REG_PPC_DEBUG_INST| 32
> >
> > 4.69 KVM_GET_ONE_REG
> >
> > diff --git a/arch/powerpc/include/asm/kvm_ppc.h
> > b/arch/powerpc/include/asm/kvm_ppc.h
> > index 44a657a..b3c481e 100644
> > --- a/arch/powerpc/include/asm/kvm_ppc.h
> > +++ b/arch/powerpc/include/asm/kvm_ppc.h
> > @@ -235,6 +235,7 @@ union kvmppc_one_reg {
> >
> > void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
> > *sregs); int kvmppc_core_set_sregs(struct kvm_vcpu *vcpu, struct
> > kvm_sregs *sregs);
> > +u32 kvmppc_core_debug_inst_op(void);
> >
> > void kvmppc_get_sregs_ivor(struct kvm_vcpu *vcpu, struct kvm_sregs
> > *sregs); int kvmppc_set_sregs_ivor(struct kvm_vcpu *vcpu, struct
> > kvm_sregs *sregs); diff --git a/arch/powerpc/include/uapi/asm/kvm.h
> > b/arch/powerpc/include/uapi/asm/kvm.h
> > index 16064d0..e81ae5b 100644
> > --- a/arch/powerpc/include/uapi/asm/kvm.h
> > +++ b/arch/powerpc/include/uapi/asm/kvm.h
> > @@ -417,4 +417,7 @@ struct kvm_get_htab_header {
> > #define KVM_REG_PPC_EPCR	(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85)
> > #define KVM_REG_PPC_EPR		(KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x86)
> >
> > +/* Debugging: Special instruction for software breakpoint */ #define
> > +KVM_REG_PPC_DEBUG_INST (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x87)
> > +
> > #endif /* __LINUX_KVM_POWERPC_H */
> > diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c index
> > 3d7fd21..41501be 100644
> > --- a/arch/powerpc/kvm/44x.c
> > +++ b/arch/powerpc/kvm/44x.c
> > @@ -114,6 +114,11 @@ int kvmppc_core_vcpu_translate(struct kvm_vcpu *vcpu,
> > 	return 0;
> > }
> >
> > +u32 kvmppc_core_debug_inst_op(void)
> > +{
> > +	return -1;
> > +}
> > +
> > void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
> > *sregs) {
> > 	kvmppc_get_sregs_ivor(vcpu, sregs);
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
> > d2f502d..453a10f 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> 
> Please provide the DEBUG_INST on a more global level - across all ppc subarchs.

Do you mean defining in powerpc.c ?

We are using one_reg for DEBUG_INST and one_reg_ioctl and defined in respective subarchs (booke and books have their separate handler). So how you want this to be defined in more common way for all subarchs?

Thanks
-Bharat

> 
> > @@ -1424,6 +1424,12 @@ int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
> struct kvm_one_reg *reg)
> > 		r = put_user(vcpu->arch.epcr, (u32 __user *)(long)reg->addr);
> > 		break;
> > #endif
> > +	case KVM_REG_PPC_DEBUG_INST: {
> > +		u32 opcode = kvmppc_core_debug_inst_op();
> > +		r = copy_to_user((u32 __user *)(long)reg->addr,
> > +				 &opcode, sizeof(u32));
> > +		break;
> > +	}
> > 	default:
> > 		break;
> > 	}
> > @@ -1467,6 +1473,10 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
> struct kvm_one_reg *reg)
> > 		break;
> > 	}
> > #endif
> > +	case KVM_REG_PPC_DEBUG_INST:
> > +		/* This is read only, so write to this is nop*/
> > +		r = 0;
> > +		break;
> 
> Just don't support set_one_reg on this reg.
> 
> > 	default:
> > 		break;
> > 	}
> > diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c index
> > 6dd4de7..d8a5e8e 100644
> > --- a/arch/powerpc/kvm/e500.c
> > +++ b/arch/powerpc/kvm/e500.c
> > @@ -367,6 +367,11 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu)
> > 	return 0;
> > }
> >
> > +u32 kvmppc_core_debug_inst_op(void)
> > +{
> > +	return KVMPPC_INST_GUEST_GDB;
> > +}
> > +
> > void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
> > *sregs) {
> > 	struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu); diff --git
> > a/arch/powerpc/kvm/e500.h b/arch/powerpc/kvm/e500.h index
> > c70d37e..17942d2 100644
> > --- a/arch/powerpc/kvm/e500.h
> > +++ b/arch/powerpc/kvm/e500.h
> > @@ -302,4 +302,13 @@ static inline unsigned int get_tlbmiss_tid(struct
> kvm_vcpu *vcpu)
> > #define get_tlb_sts(gtlbe)              (MAS1_TS)
> > #endif /* !BOOKE_HV */
> >
> > +/* When setting software breakpoint, Change the software breakpoint
> > + * instruction to special trap/invalid instruction and set
> > + * KVM_GUESTDBG_USE_SW_BP flag in kvm_guest_debug->control. KVM does
> > + * keep track of software breakpoints. So when KVM_GUESTDBG_USE_SW_BP
> > + * flag is set and special trap instruction is executed by guest then
> > + * exit to userspace.
> 
> This comment chunk no apply to define. Also please fix English ;).
> 
> 
> Alex
> 
> > + */
> > +#define KVMPPC_INST_GUEST_GDB		0x7C00021C	/* ehpriv OC=0 */
> > +
> > #endif /* KVM_E500_H */
> > diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
> > index 1f89d26..dead142 100644
> > --- a/arch/powerpc/kvm/e500mc.c
> > +++ b/arch/powerpc/kvm/e500mc.c
> > @@ -199,6 +199,11 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu)
> > 	return 0;
> > }
> >
> > +u32 kvmppc_core_debug_inst_op(void)
> > +{
> > +	return KVMPPC_INST_GUEST_GDB;
> > +}
> > +
> > void kvmppc_core_get_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs
> > *sregs) {
> > 	struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
> > --
> > 1.7.0.4
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info
> > at  http://vger.kernel.org/majordomo-info.html
> 


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


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux