From patchwork Wed Jul 2 11:37:09 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bharat Bhushan X-Patchwork-Id: 4464971 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id F24789F26C for ; Wed, 2 Jul 2014 11:37:27 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E66B520397 for ; Wed, 2 Jul 2014 11:37:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C47A5202E5 for ; Wed, 2 Jul 2014 11:37:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751297AbaGBLhU (ORCPT ); Wed, 2 Jul 2014 07:37:20 -0400 Received: from mail-by2lp0235.outbound.protection.outlook.com ([207.46.163.235]:24677 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750955AbaGBLhS (ORCPT ); Wed, 2 Jul 2014 07:37:18 -0400 Received: from BLUPR03MB566.namprd03.prod.outlook.com (10.141.77.155) by BLUPR03MB392.namprd03.prod.outlook.com (10.141.78.28) with Microsoft SMTP Server (TLS) id 15.0.959.24; Wed, 2 Jul 2014 11:37:16 +0000 Received: from BLUPR03MB566.namprd03.prod.outlook.com ([10.141.77.155]) by BLUPR03MB566.namprd03.prod.outlook.com ([10.141.77.155]) with mapi id 15.00.0949.001; Wed, 2 Jul 2014 11:37:10 +0000 From: "Bharat.Bhushan@freescale.com" To: Scott Wood , Alexander Graf CC: "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org" Subject: RE: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest Thread-Topic: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to guest Thread-Index: AQHPkdC5yLvMW2lPQESQPdmRbP8NOJuFRhoAgAPIF6CAAREhgIAAptwAgACP6YCAAAHdgIAACLWAgAANFoCAAAUJAIAABSJg Date: Wed, 2 Jul 2014 11:37:09 +0000 Message-ID: <65676863af53426ab5993d1b45b360e3@BLUPR03MB566.namprd03.prod.outlook.com> References: <1403850306-12394-1-git-send-email-Bharat.Bhushan@freescale.com> <1403850306-12394-3-git-send-email-Bharat.Bhushan@freescale.com> <1403893398.2435.111.camel@snotra.buserror.net> <120d75cc52bf4c2faa9b12523887f1c4@DM2PR03MB574.namprd03.prod.outlook.com> <1404159947.2435.170.camel@snotra.buserror.net> <819D42C0-A555-4B92-A951-2CF32F2AD170@suse.de> <1404226685.2435.220.camel@snotra.buserror.net> <53B2CE0D.600@suse.de> <1404228955.2435.241.camel@snotra.buserror.net> <53B2E055.80905@suse.de> <1404232846.2435.268.camel@snotra.buserror.net> In-Reply-To: <1404232846.2435.268.camel@snotra.buserror.net> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [192.88.169.1] x-microsoft-antispam: BCL:0;PCL:0;RULEID: x-forefront-prvs: 0260457E99 x-forefront-antispam-report: SFV:NSPM; SFS:(6009001)(51704005)(189002)(199002)(377454003)(377424004)(13464003)(24454002)(77982001)(76176999)(99396002)(79102001)(33646001)(66066001)(46102001)(19580395003)(64706001)(85852003)(83322001)(19580405001)(83072002)(54356999)(4396001)(20776003)(50986999)(81542001)(2656002)(93886003)(76482001)(80022001)(31966008)(76576001)(86362001)(87936001)(21056001)(106356001)(101416001)(95666004)(105586002)(74662001)(74502001)(85306003)(107046002)(99286002)(81342001)(74316001)(92566001)(106116001)(108616002)(24736002); DIR:OUT; SFP:; SCL:1; SRVR:BLUPR03MB392; H:BLUPR03MB566.namprd03.prod.outlook.com; FPR:; MLV:sfv; PTR:InfoNoRecords; MX:1; A:1; LANG:en; MIME-Version: 1.0 X-OriginatorOrg: freescale.com Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP > -----Original Message----- > From: Wood Scott-B07421 > Sent: Tuesday, July 01, 2014 10:11 PM > To: Alexander Graf > Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org > Subject: Re: [PATCH 2/2] KVM : powerpc/booke: Allow debug interrupt injection to > guest > > On Tue, 2014-07-01 at 18:22 +0200, Alexander Graf wrote: > > On 01.07.14 17:35, Scott Wood wrote: > > > On Tue, 2014-07-01 at 17:04 +0200, Alexander Graf wrote: > > >> On 01.07.14 16:58, Scott Wood wrote: > > >>> On Tue, 2014-07-01 at 08:23 +0200, Alexander Graf wrote: > > >>>> I don't think QEMU should be aware of these limitations. > > >>> OK, but we should at least have some idea of how the whole thing > > >>> is supposed to work, in order to determine if this is the correct > > >>> behavior for QEMU. I thought the model was that debug resources > > >>> are either owned by QEMU or by the guest, and in the latter case, > > >>> QEMU would never see the debug exception to begin with. > > >> That's bad for a number of reasons. For starters it's different > > >> from how > > >> x86 handles debug registers - and I hate to be different just for > > >> the sake of being different. > > > How does it work on x86? > > > > It overwrites more-or-less random breakpoints with its own ones, but > > leaves the others intact ;). > > Are you talking about software breakpoints or management of hardware debug > registers? > > > >> So if we do want to declare that debug registers are owned by > > >> either QEMU or the guest, we should change the semantics for all > > >> architectures. > > > If we want to say that ownership of the registers is shared, we need > > > a plan for how that would actually work. > > > > I think you're overengineering here :). When do people actually use > > gdbstub? Usually when they want to debug a broken guest. We can either > > > > * overengineer heavily and reduce the number of registers available > > to the guest to always have spares > > * overengineer a bit and turn off guest debugging completely when > > we use gdbstub > > * just leave as much alive as we can, hoping that it helps with the > > debugging > > > > Option 3 is what x86 does - and I think it's a reasonable approach. > > This is not an interface that needs to be 100% consistent and bullet > > proof, it's a best effort to enable you to debug as much as possible. > > I'm not insisting on 100% -- just hoping for some explanation/discussion about > how it's intended to work for the cases where it can. > > How will MSR[DE] and MSRP[DEP] be handled? > > How would I go about telling QEMU/KVM that I don't want this shared mode, > because I don't want guest to interfere with the debugging I'm trying to do from > QEMU? > > Will guest accesses to debug registers cause a userspace exit when guest_debug > is enabled? > > > >> I think we're in a path that is slow enough already to not worry > > >> about performance. > > > It's not just about performance, but simplicity of use, and > > > consistency of API. > > > > > > Oh, and it looks like there already exist one reg definitions and > > > implementations for most of the debug registers. > > > > For BookE? Where? > > arch/powerpc/kvm/booke.c: KVM_REG_PPC_IACn, KVM_REG_PPC_DACn I tried to quickly prototype what I think we want to do (this is not tested) ----------------------------------------------------------------------- ------------------ Thanks -Bharat > > -Scott > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h index e8b3982..746b5c6 100644 --- a/arch/powerpc/include/asm/kvm_ppc.h +++ b/arch/powerpc/include/asm/kvm_ppc.h @@ -179,6 +179,9 @@ extern int kvmppc_xics_get_xive(struct kvm *kvm, u32 irq, u32 *server, extern int kvmppc_xics_int_on(struct kvm *kvm, u32 irq); extern int kvmppc_xics_int_off(struct kvm *kvm, u32 irq); +void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu); +void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu); + /* * Cuts out inst bits with ordering according to spec. * That means the leftmost bit is zero. All given bits are included. diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 9f13056..0b7e4e4 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -235,6 +235,16 @@ void kvmppc_core_dequeue_dec(struct kvm_vcpu *vcpu) clear_bit(BOOKE_IRQPRIO_DECREMENTER, &vcpu->arch.pending_exceptions); } +void kvmppc_core_queue_debug(struct kvm_vcpu *vcpu) +{ + kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_DEBUG); +} + +void kvmppc_core_dequeue_debug(struct kvm_vcpu *vcpu) +{ + clear_bit(BOOKE_IRQPRIO_DEBUG, &vcpu->arch.pending_exceptions); +} + void kvmppc_core_queue_external(struct kvm_vcpu *vcpu, struct kvm_interrupt *irq) { @@ -841,6 +851,20 @@ static int kvmppc_handle_debug(struct kvm_run *run, struct kvm_vcpu *vcpu) struct debug_reg *dbg_reg = &(vcpu->arch.shadow_dbg_reg); u32 dbsr = vcpu->arch.dbsr; + /* Userspace (QEMU) is not using debug resource, so inject debug interrupt + * directly to guest debug. + */ + if (vcpu->guest_debug == 0) { + if (dbsr && (vcpu->arch.shared->msr & MSR_DE)) + kvmppc_core_queue_debug(vcpu); + + /* Inject a program interrupt if trap debug is not allowed */ + if ((dbsr & DBSR_TIE) && !(vcpu->arch.shared->msr & MSR_DE)) + kvmppc_core_queue_program(vcpu, ESR_PTR); + + return RESUME_GUEST; + } + run->debug.arch.status = 0; run->debug.arch.address = vcpu->arch.pc; @@ -1920,7 +1944,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu, vcpu->guest_debug = dbg->control; vcpu->arch.shadow_dbg_reg.dbcr0 = 0; /* Set DBCR0_EDM in guest visible DBCR0 register. */ - vcpu->arch.dbg_reg.dbcr0 = DBCR0_EDM; +// vcpu->arch.dbg_reg.dbcr0 = DBCR0_EDM; if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) vcpu->arch.shadow_dbg_reg.dbcr0 |= DBCR0_IDM | DBCR0_IC; diff --git a/arch/powerpc/kvm/booke_emulate.c b/arch/powerpc/kvm/booke_emulate.c index aaff1b7..ef8de7f 100644 --- a/arch/powerpc/kvm/booke_emulate.c +++ b/arch/powerpc/kvm/booke_emulate.c @@ -26,6 +26,7 @@ #define OP_19_XOP_RFMCI 38 #define OP_19_XOP_RFI 50 #define OP_19_XOP_RFCI 51 +#define OP_19_XOP_RFDI 39 #define OP_31_XOP_MFMSR 83 #define OP_31_XOP_WRTEE 131 @@ -38,10 +39,24 @@ static void kvmppc_emul_rfi(struct kvm_vcpu *vcpu) kvmppc_set_msr(vcpu, vcpu->arch.shared->srr1); } +static void kvmppc_emul_rfdi(struct kvm_vcpu *vcpu) +{ + vcpu->arch.pc = vcpu->arch.dsrr0; + /* Force MSR_DE when guest does not own debug facilities */ + if (vcpu->guest_debug) + kvmppc_set_msr(vcpu, vcpu->arch.dsrr1 | MSR_DE); + else + kvmppc_set_msr(vcpu, vcpu->arch.dsrr1); +} + static void kvmppc_emul_rfci(struct kvm_vcpu *vcpu) { vcpu->arch.pc = vcpu->arch.csrr0; - kvmppc_set_msr(vcpu, vcpu->arch.csrr1); + /* Force MSR_DE when guest does not own debug facilities */ + if (vcpu->guest_debug) + kvmppc_set_msr(vcpu, vcpu->arch.csrr1 | MSR_DE); + else + kvmppc_set_msr(vcpu, vcpu->arch.csrr1); } static void kvmppc_emul_rfmci(struct kvm_vcpu *vcpu) @@ -78,6 +93,12 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu, *advance = 0; break; + case OP_19_XOP_RFDI: + kvmppc_emul_rfdi(vcpu); +// kvmppc_set_exit_type(vcpu, EMULATED_RFDI_EXITS); + *advance = 0; + break; + default: emulated = EMULATE_FAIL; break; @@ -131,6 +152,7 @@ int kvmppc_booke_emulate_op(struct kvm_run *run, struct kvm_vcpu *vcpu, int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val) { int emulated = EMULATE_DONE; + bool debug_inst = false; switch (sprn) { case SPRN_DEAR: @@ -145,21 +167,74 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val) case SPRN_CSRR1: vcpu->arch.csrr1 = spr_val; break; - case SPRN_DBCR0: - vcpu->arch.dbg_reg.dbcr0 = spr_val; + case SPRN_DSRR0: + vcpu->arch.dsrr0 = spr_val; break; - case SPRN_DBCR1: - vcpu->arch.dbg_reg.dbcr1 = spr_val; + case SPRN_DSRR1: + vcpu->arch.dsrr1 = spr_val; + break; + case SPRN_IAC1: + debug_inst = true; + vcpu->arch.dbg_reg.iac1 = spr_val; + vcpu->arch.shadow_dbg_reg.iac1 = spr_val; + break; + case SPRN_IAC2: + debug_inst = true; + vcpu->arch.dbg_reg.iac2 = spr_val; + vcpu->arch.shadow_dbg_reg.iac2 = spr_val; + break; +#ifndef CONFIG_PPC_FSL_BOOK3E + case SPRN_IAC3: + debug_inst = true; + vcpu->arch.dbg_reg.iac3 = spr_val; + vcpu->arch.shadow_dbg_reg.iac3 = spr_val; break; + case SPRN_IAC4: + debug_inst = true; + vcpu->arch.dbg_reg.iac4 = spr_val; + vcpu->arch.shadow_dbg_reg.iac4 = spr_val; + break; +#endif + case SPRN_DAC1: + debug_inst = true; + vcpu->arch.dbg_reg.dac1 = spr_val; + vcpu->arch.shadow_dbg_reg.dac1 = spr_val; + break; + case SPRN_DAC2: + debug_inst = true; + vcpu->arch.dbg_reg.dac2 = spr_val; + vcpu->arch.shadow_dbg_reg.dac2 = spr_val; + break; + case SPRN_DBCR0: + debug_inst = true; + spr_val &= (DBCR0_IDM | DBCR0_IC | DBCR0_BT | DBCR0_TIE | + DBCR0_IAC1 | DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4 | + DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W); + + vcpu->arch.dbg_reg.dbcr0 = spr_val; + vcpu->arch.shadow_dbg_reg.dbcr0 = spr_val; + break; + case SPRN_DBCR1: + debug_inst = true; + vcpu->arch.dbg_reg.dbcr1 = spr_val; + vcpu->arch.shadow_dbg_reg.dbcr1 = spr_val; + break; + case SPRN_DBCR2: + debug_inst = true; + vcpu->arch.dbg_reg.dbcr2 = spr_val; + vcpu->arch.shadow_dbg_reg.dbcr2 = spr_val; + break; + case SPRN_DBSR: + vcpu->arch.dbsr &= ~spr_val; + if (vcpu->arch.dbsr == 0) + kvmppc_core_dequeue_debug(vcpu); + break; case SPRN_MCSRR0: vcpu->arch.mcsrr0 = spr_val; break; case SPRN_MCSRR1: vcpu->arch.mcsrr1 = spr_val; break; - case SPRN_DBSR: - vcpu->arch.dbsr &= ~spr_val; - break; case SPRN_TSR: kvmppc_clr_tsr_bits(vcpu, spr_val); break; @@ -271,6 +346,10 @@ int kvmppc_booke_emulate_mtspr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val) emulated = EMULATE_FAIL; } + if (debug_inst) { + switch_booke_debug_regs(&vcpu->arch.shadow_dbg_reg); + current->thread.debug = vcpu->arch.shadow_dbg_reg; + } return emulated; } @@ -297,12 +376,41 @@ int kvmppc_booke_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val) case SPRN_CSRR1: *spr_val = vcpu->arch.csrr1; break; + case SPRN_DSRR0: + *spr_val = vcpu->arch.dsrr0; + break; + case SPRN_DSRR1: + *spr_val = vcpu->arch.dsrr1; + break; + case SPRN_IAC1: + *spr_val = vcpu->arch.dbg_reg.iac1; + break; + case SPRN_IAC2: + *spr_val = vcpu->arch.dbg_reg.iac2; + break; +#ifndef CONFIG_PPC_FSL_BOOK3E + case SPRN_IAC3: + *spr_val = vcpu->arch.dbg_reg.iac3; + break; + case SPRN_IAC4: + *spr_val = vcpu->arch.dbg_reg.iac4; + break; +#endif + case SPRN_DAC1: + *spr_val = vcpu->arch.dbg_reg.dac1; + break; + case SPRN_DAC2: + *spr_val = vcpu->arch.dbg_reg.dac2; + break; case SPRN_DBCR0: *spr_val = vcpu->arch.dbg_reg.dbcr0; break; case SPRN_DBCR1: *spr_val = vcpu->arch.dbg_reg.dbcr1; break; + case SPRN_DBCR2: + *spr_val = vcpu->arch.dbg_reg.dbcr2; + break; case SPRN_MCSRR0: *spr_val = vcpu->arch.mcsrr0; break; diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c index bbf7cdd..560b576 100644 --- a/arch/powerpc/kvm/e500mc.c +++ b/arch/powerpc/kvm/e500mc.c @@ -249,7 +249,7 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu) #ifdef CONFIG_64BIT vcpu->arch.shadow_epcr |= SPRN_EPCR_ICM; #endif - vcpu->arch.shadow_msrp = MSRP_UCLEP | MSRP_DEP | MSRP_PMMP; + vcpu->arch.shadow_msrp = MSRP_UCLEP | MSRP_PMMP; vcpu->arch.pvr = mfspr(SPRN_PVR); vcpu_e500->svr = mfspr(SPRN_SVR);