[PATCH 4/6 v5] KVM: PPC: exit to user space on "ehpriv" instruction

Bhushan Bharat-R65777 R65777 at freescale.com
Wed Jun 26 19:27:09 EST 2013



> -----Original Message-----
> From: tiejun.chen [mailto:tiejun.chen at windriver.com]
> Sent: Wednesday, June 26, 2013 2:47 PM
> To: Bhushan Bharat-R65777
> Cc: kvm-ppc at vger.kernel.org; kvm at vger.kernel.org; agraf at suse.de; Wood Scott-
> B07421; benh at kernel.crashing.org; linuxppc-dev at lists.ozlabs.org; linux-
> kernel at vger.kernel.org; mikey at neuling.org
> Subject: Re: [PATCH 4/6 v5] KVM: PPC: exit to user space on "ehpriv" instruction
> 
> On 06/26/2013 04:44 PM, Bhushan Bharat-R65777 wrote:
> >
> >
> >> -----Original Message-----
> >> From: tiejun.chen [mailto:tiejun.chen at windriver.com]
> >> Sent: Wednesday, June 26, 2013 12:25 PM
> >> To: Bhushan Bharat-R65777
> >> Cc: kvm-ppc at vger.kernel.org; kvm at vger.kernel.org; agraf at suse.de; Wood
> >> Scott- B07421; benh at kernel.crashing.org;
> >> linuxppc-dev at lists.ozlabs.org; linux- kernel at vger.kernel.org;
> >> mikey at 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.bhushan at 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".
> 
> Yes, this is just what I mean.
> 
> >
> > I do not think of any special reason to reserve "ehpriv" and "ehpriv 0".
> 
> So I still prefer we can reserve the 'ehpriv' without OC operand as one simple
> approach to test or develop something for KVM quickly because its really
> convenient to trap into the hypervisor only with one 'ehpriv' instruction
> easily.
> 
> But I have no further objection if you guys are fine to this ;-)

I can see the using "ehpriv" can be a default choice. But all ehvpriv trap is handled at one place (in a single function) so the accidently overlap with debug should not be an issue.

I too do not have any strong opinion to keep either way, so want to leave as is ;-).

-Bharat

> 
> Tiejun
> 
> >
> > 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;
> >>>    		}
> >>>
> >>
> >
> 



More information about the Linuxppc-dev mailing list