From patchwork Tue Aug 12 12:15:46 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Graf X-Patchwork-Id: 379317 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 40D4F1400B7 for ; Tue, 12 Aug 2014 22:15:50 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751893AbaHLMPt (ORCPT ); Tue, 12 Aug 2014 08:15:49 -0400 Received: from cantor2.suse.de ([195.135.220.15]:34633 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751890AbaHLMPs (ORCPT ); Tue, 12 Aug 2014 08:15:48 -0400 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 66B73AD6A; Tue, 12 Aug 2014 12:15:47 +0000 (UTC) Message-ID: <53EA0572.1070806@suse.de> Date: Tue, 12 Aug 2014 14:15:46 +0200 From: Alexander Graf User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:31.0) Gecko/20100101 Thunderbird/31.0 MIME-Version: 1.0 To: Madhavan Srinivasan , Benjamin Herrenschmidt CC: paulus@samba.org, linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org, kvm-ppc@vger.kernel.org Subject: Re: [PATCH v3] powerpc/kvm: support to handle sw breakpoint References: <1406868643-26291-1-git-send-email-maddy@linux.vnet.ibm.com> <53E87023.6080200@suse.de> <1407747061.4508.75.camel@pasglop> <53E889BA.40108@suse.de> <53E9A383.5060009@linux.vnet.ibm.com> <53E9F824.9080906@suse.de> <53E9FBF6.5010008@linux.vnet.ibm.com> In-Reply-To: <53E9FBF6.5010008@linux.vnet.ibm.com> Sender: kvm-ppc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm-ppc@vger.kernel.org On 12.08.14 13:35, Madhavan Srinivasan wrote: > On Tuesday 12 August 2014 04:49 PM, Alexander Graf wrote: >> On 12.08.14 07:17, Madhavan Srinivasan wrote: >>> On Monday 11 August 2014 02:45 PM, Alexander Graf wrote: >>>> On 11.08.14 10:51, Benjamin Herrenschmidt wrote: >>>>> On Mon, 2014-08-11 at 09:26 +0200, Alexander Graf wrote: >>>>>>> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c >>>>>>> index da86d9b..d95014e 100644 >>>>>>> --- a/arch/powerpc/kvm/emulate.c >>>>>>> +++ b/arch/powerpc/kvm/emulate.c >>>>>> This should be book3s_emulate.c. >>>>> Any reason we can't make that 00dddd00 opcode as breakpoint common to >>>>> all powerpc variants ? >>>> I can't think of a good reason. We use a hypercall on booke (which traps >>>> into an illegal instruction for pr) today, but I don't think it has to >>>> be that way. >>>> >>>> Given that the user space API allows us to change it dynamically, there >>>> should be nothing blocking us from going with 00dddd00 always. >>>> >>> Kindly correct me if i am wrong. So we can still have a common code in >>> emulate.c to set the env for both HV and pr incase of illegal >>> instruction (i will rebase latest src). But suggestion here to use >>> 00dddd00, in that case current path in embed is kvmppc_handle_exit >>> (booke.c) -> BOOKE_INTERRUPT_HV_PRIV -> emulation_exit -> >>> kvmppc_emulate_instruction, will change to kvmppc_handle_exit (booke.c) >>> -> BOOKE_INTERRUPT_PROGRAM -> if debug instr call emulation_exit else >>> send to guest? >> I can't follow your description above. >> > My bad. > >> With the latest git version HV KVM does not include emulate.c anymore. >> >> Also, it would make a lot of sense of have the same soft breakpoint >> instruction across all ppc targets, so it would make sense to change it >> to 0x00dddd00 for booke as well. >> > Got it. Was describing the current control flow with respect to booke > and where changes needed (for same software breakpoint inst). This is > for my understanding and wanted verify. > > kvmppc_handle_exit(booke.c) > -> BOOKE_INTERRUPT_HV_PRIV > -> emulation_exit > ->kvmppc_emulate_instruction > > Incase of using the same software breakpoint instruction (0x00dddd00), > then we need to add code in booke something like this > > kvmppc_handle_exit (booke.c) > -> BOOKE_INTERRUPT_PROGRAM > -> if debug instr > ->emulation_exit > else > ->send to guest? Bleks. I see your point. I guess you need something like this for booke: } @@ -953,7 +958,8 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; case BOOKE_INTERRUPT_PROGRAM: - if (vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) { + if ((vcpu->arch.shared->msr & (MSR_PR | MSR_GS)) && + (last_inst != KVMPPC_INST_SOFT_BREAKPOINT)) { /* * Program traps generated by user-level software must * be handled by the guest kernel. > >> Basically you would have handling code in emulate.c and book3s_hv.c at >> the end of the day. >> > Yes. Will resend the patch with updated code. Thanks, Alex --- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 074b7fc..1fdeee0 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -876,6 +876,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, case BOOKE_INTERRUPT_HV_PRIV: emulated = kvmppc_get_last_inst(vcpu, false, &last_inst); break; + case BOOKE_INTERRUPT_PROGRAM: + /* SW breakpoints arrive as illegal instructions on HV */ + if (vcpu->guest_debug & KVM_GUESTDBG_USE_SW_BP) + emulated = kvmppc_get_last_inst(vcpu, false, &last_inst); + break; default: break;