From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from e28smtp03.in.ibm.com (e28smtp03.in.ibm.com [122.248.162.3]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id EB4C41A0296 for ; Tue, 17 Jun 2014 21:07:38 +1000 (EST) Received: from /spool/local by e28smtp03.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 17 Jun 2014 16:37:34 +0530 Received: from d28relay01.in.ibm.com (d28relay01.in.ibm.com [9.184.220.58]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id E40A01258055 for ; Tue, 17 Jun 2014 16:36:56 +0530 (IST) Received: from d28av02.in.ibm.com (d28av02.in.ibm.com [9.184.220.64]) by d28relay01.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s5HB8UVc39649350 for ; Tue, 17 Jun 2014 16:38:30 +0530 Received: from d28av02.in.ibm.com (localhost [127.0.0.1]) by d28av02.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s5HB7SXH021249 for ; Tue, 17 Jun 2014 16:37:28 +0530 Message-ID: <53A0216F.9060504@linux.vnet.ibm.com> Date: Tue, 17 Jun 2014 16:37:27 +0530 From: Madhavan Srinivasan MIME-Version: 1.0 To: Alexander Graf , benh@kernel.crashing.org, paulus@samba.org Subject: Re: [PATCH] powerpc/kvm: support to handle sw breakpoint References: <1402780097-28827-1-git-send-email-maddy@linux.vnet.ibm.com> <53A0022D.5020108@suse.de> In-Reply-To: <53A0022D.5020108@suse.de> Content-Type: text/plain; charset=ISO-8859-1 Cc: linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org, kvm@vger.kernel.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Tuesday 17 June 2014 02:24 PM, Alexander Graf wrote: > > On 14.06.14 23:08, Madhavan Srinivasan wrote: >> This patch adds kernel side support for software breakpoint. >> Design is that, by using an illegal instruction, we trap to hypervisor >> via Emulation Assistance interrupt, where we check for the illegal >> instruction >> and accordingly we return to Host or Guest. Patch mandates use of >> "abs" instruction >> (primary opcode 31 and extended opcode 360) as sw breakpoint instruction. >> Based on PowerISA v2.01, ABS instruction has been dropped from the >> architecture >> and treated an illegal instruction. >> >> Signed-off-by: Madhavan Srinivasan >> --- >> arch/powerpc/kvm/book3s.c | 3 ++- >> arch/powerpc/kvm/book3s_hv.c | 23 +++++++++++++++++++---- >> 2 files changed, 21 insertions(+), 5 deletions(-) >> >> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c >> index c254c27..b40fe5d 100644 >> --- a/arch/powerpc/kvm/book3s.c >> +++ b/arch/powerpc/kvm/book3s.c >> @@ -789,7 +789,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu >> *vcpu, >> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, >> struct kvm_guest_debug *dbg) >> { >> - return -EINVAL; >> + vcpu->guest_debug = dbg->control; >> + return 0; >> } >> void kvmppc_decrementer_func(unsigned long data) >> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c >> index 7a12edb..688421d 100644 >> --- a/arch/powerpc/kvm/book3s_hv.c >> +++ b/arch/powerpc/kvm/book3s_hv.c >> @@ -67,6 +67,14 @@ >> /* Used as a "null" value for timebase values */ >> #define TB_NIL (~(u64)0) >> +/* >> + * SW_BRK_DBG_INT is debug Instruction for supporting Software >> Breakpoint. >> + * Instruction mnemonic is ABS, primary opcode is 31 and extended >> opcode is 360. >> + * Based on PowerISA v2.01, ABS instruction has been dropped from the >> architecture >> + * and treated an illegal instruction. >> + */ >> +#define SW_BRK_DBG_INT 0x7c0002d0 > > The instruction we use to trap needs to get exposed to user space via a > ONE_REG property. > Yes. I got to know about that from Bharat (patchset "ppc debug: Add debug stub support"). I will change it. > Also, why don't we use twi always or something else that actually is > defined as illegal instruction? I would like to see this shared with > book3s_32 PR. > >> + >> static void kvmppc_end_cede(struct kvm_vcpu *vcpu); >> static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu); >> @@ -721,12 +729,19 @@ static int kvmppc_handle_exit_hv(struct >> kvm_run *run, struct kvm_vcpu *vcpu, >> break; >> /* >> * This occurs if the guest executes an illegal instruction. >> - * We just generate a program interrupt to the guest, since >> - * we don't emulate any guest instructions at this stage. >> + * To support software breakpoint, we check for the sw breakpoint >> + * instruction to return back to host, if not we just generate a >> + * program interrupt to the guest. >> */ >> case BOOK3S_INTERRUPT_H_EMUL_ASSIST: >> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL); >> - r = RESUME_GUEST; >> + if (vcpu->arch.last_inst == SW_BRK_DBG_INT) { > > Don't access last_inst directly. Instead use the provided helpers. > Ok. Will look and replace it. >> + run->exit_reason = KVM_EXIT_DEBUG; >> + run->debug.arch.address = vcpu->arch.pc; >> + r = RESUME_HOST; >> + } else { >> + kvmppc_core_queue_program(vcpu, 0x80000); > > magic numbers ^^^^^ I did not understand this? >> + r = RESUME_GUEST; >> + } >> break; >> /* >> * This occurs if the guest (kernel or userspace), does >> something that > > Please enable PR KVM as well while you're at it. > My bad, I did not try the PR KVM. I will try it out. > > Alex > Thanks for review Regards Maddy