On 26.06.2013, at 11:27, Bhushan Bharat-R65777 wrote: > > >> -----Original Message----- >> From: tiejun.chen [mailto:tiejun.c...@windriver.com] >> Sent: Wednesday, June 26, 2013 2:47 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 >> 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.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". >> >> 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 ;-).
Seconded. On x86 we also just use int3 for soft breakpoints IIRC. Alex _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
