From patchwork Fri Jun 21 18:47:48 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 2763971 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 304BAC0AB1 for ; Fri, 21 Jun 2013 18:46:36 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 27FE2201FE for ; Fri, 21 Jun 2013 18:46:35 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CFF8C20203 for ; Fri, 21 Jun 2013 18:46:33 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Uq6Lb-0006Hk-AN; Fri, 21 Jun 2013 18:46:11 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Uq6LO-0002xF-Mo; Fri, 21 Jun 2013 18:45:58 +0000 Received: from mail-pa0-x22d.google.com ([2607:f8b0:400e:c03::22d]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Uq6LL-0002wg-HH for linux-arm-kernel@lists.infradead.org; Fri, 21 Jun 2013 18:45:57 +0000 Received: by mail-pa0-f45.google.com with SMTP id bi5so8245795pad.4 for ; Fri, 21 Jun 2013 11:45:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent :x-gm-message-state; bh=+FH72YyQvYrWBGSFkyfcTKXy2g/Vkur+CZKFum50unE=; b=IHoKBkgLdEY/vkuwb9ZHYWAx5cOCBHZ+VOCOIRIzi46iEmyh9XFO6kYZJMwhcqJbJC hQ9g9L5ud2nAwC83QjrWjJWUzL6CVgZN0+Q0+P2LPGiVbTHQ+GqOrMBByo3VDsU7plYR bK6D/vN4ZqpuZcmrHr4zhQE+jZ+pjEu23kd5KXJbBNGSMBsheXuylWV7jd6ABCjEmT6l iE8ojV2lgAQYftxz9IrcJSxYfUhV7SHW7NFGCEbi5r2hKNxvthZ5yRKlWxyOCKzMBgAc 9GbZuaQvdLPTwp97HBhoXLfVSWgMaWC4N61+CtBmoQE/3wwgJ58v7KkCng79z/n+Kj8C Mg6w== X-Received: by 10.66.221.163 with SMTP id qf3mr17464688pac.38.1371840332121; Fri, 21 Jun 2013 11:45:32 -0700 (PDT) Received: from localhost (c-67-169-183-77.hsd1.ca.comcast.net. [67.169.183.77]) by mx.google.com with ESMTPSA id sq5sm6761678pab.11.2013.06.21.11.45.30 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 21 Jun 2013 11:45:31 -0700 (PDT) Date: Fri, 21 Jun 2013 11:47:48 -0700 From: Christoffer Dall To: Marc Zyngier Subject: Re: [PATCH v2 1/3] ARM: KVM: perform save/restore of PAR Message-ID: <20130621184748.GA2816@lvm> References: <1371816528-25078-1-git-send-email-marc.zyngier@arm.com> <1371816528-25078-2-git-send-email-marc.zyngier@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1371816528-25078-2-git-send-email-marc.zyngier@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Gm-Message-State: ALoCoQma2yhJDepdIjQ1OeELLnMreOMtIsg/w/H3Dr+aPugTvDTFfV6P/S75U/xJ05WrRb/Q417P X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130621_144555_916204_EFF71CC4 X-CRM114-Status: GOOD ( 21.52 ) X-Spam-Score: -1.9 (-) Cc: kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable 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 On Fri, Jun 21, 2013 at 01:08:46PM +0100, Marc Zyngier wrote: > Not saving PAR is an unfortunate oversight. If the guest performs > an AT* operation and gets scheduled out before reading the result > of the translation from PAR, it could become corrupted by another > guest or the host. > > Saving this register is made slightly more complicated as KVM also > uses it on the permission fault handling path, leading to an ugly > "stash and restore" sequence. Fortunately, this is already a slow > path so we don't really care. Also, Linux doesn't do any AT* > operation, so Linux guests are not impacted by this bug. > > Signed-off-by: Marc Zyngier > --- > arch/arm/include/asm/kvm_asm.h | 22 ++++++++++++---------- > arch/arm/kvm/coproc.c | 4 ++++ > arch/arm/kvm/interrupts.S | 12 +++++++++++- > arch/arm/kvm/interrupts_head.S | 10 ++++++++-- > 4 files changed, 35 insertions(+), 13 deletions(-) > > diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h > index 18d5032..4bb08e3 100644 > --- a/arch/arm/include/asm/kvm_asm.h > +++ b/arch/arm/include/asm/kvm_asm.h > @@ -37,16 +37,18 @@ > #define c5_AIFSR 15 /* Auxilary Instrunction Fault Status R */ > #define c6_DFAR 16 /* Data Fault Address Register */ > #define c6_IFAR 17 /* Instruction Fault Address Register */ > -#define c9_L2CTLR 18 /* Cortex A15 L2 Control Register */ > -#define c10_PRRR 19 /* Primary Region Remap Register */ > -#define c10_NMRR 20 /* Normal Memory Remap Register */ > -#define c12_VBAR 21 /* Vector Base Address Register */ > -#define c13_CID 22 /* Context ID Register */ > -#define c13_TID_URW 23 /* Thread ID, User R/W */ > -#define c13_TID_URO 24 /* Thread ID, User R/O */ > -#define c13_TID_PRIV 25 /* Thread ID, Privileged */ > -#define c14_CNTKCTL 26 /* Timer Control Register (PL1) */ > -#define NR_CP15_REGS 27 /* Number of regs (incl. invalid) */ > +#define c7_PAR 18 /* Physical Address Register */ > +#define c7_PAR_high 19 /* PAR top 32 bits */ > +#define c9_L2CTLR 20 /* Cortex A15 L2 Control Register */ > +#define c10_PRRR 21 /* Primary Region Remap Register */ > +#define c10_NMRR 22 /* Normal Memory Remap Register */ > +#define c12_VBAR 23 /* Vector Base Address Register */ > +#define c13_CID 24 /* Context ID Register */ > +#define c13_TID_URW 25 /* Thread ID, User R/W */ > +#define c13_TID_URO 26 /* Thread ID, User R/O */ > +#define c13_TID_PRIV 27 /* Thread ID, Privileged */ > +#define c14_CNTKCTL 28 /* Timer Control Register (PL1) */ > +#define NR_CP15_REGS 29 /* Number of regs (incl. invalid) */ > > #define ARM_EXCEPTION_RESET 0 > #define ARM_EXCEPTION_UNDEFINED 1 > diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c > index 8eea97b..4a51990 100644 > --- a/arch/arm/kvm/coproc.c > +++ b/arch/arm/kvm/coproc.c > @@ -180,6 +180,10 @@ static const struct coproc_reg cp15_regs[] = { > NULL, reset_unknown, c6_DFAR }, > { CRn( 6), CRm( 0), Op1( 0), Op2( 2), is32, > NULL, reset_unknown, c6_IFAR }, > + > + /* PAR swapped by interrupt.S */ > + { CRn( 7), Op1( 0), is64, NULL, reset_unknown64, c7_PAR }, > + > /* > * DC{C,I,CI}SW operations: > */ > diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S > index f7793df..d0a8fa3 100644 > --- a/arch/arm/kvm/interrupts.S > +++ b/arch/arm/kvm/interrupts.S > @@ -414,6 +414,10 @@ guest_trap: > mrcne p15, 4, r2, c6, c0, 4 @ HPFAR > bne 3f > > + /* Preserve PAR */ > + mrrc p15, 0, r0, r1, c7 @ PAR > + push {r0, r1} > + > /* Resolve IPA using the xFAR */ > mcr p15, 0, r2, c7, c8, 0 @ ATS1CPR > isb > @@ -424,13 +428,19 @@ guest_trap: > lsl r2, r2, #4 > orr r2, r2, r1, lsl #24 > > + /* Restore PAR */ > + pop {r0, r1} > + mcrr p15, 0, r0, r1, c7 @ PAR > + > 3: load_vcpu @ Load VCPU pointer to r0 > str r2, [r0, #VCPU_HPFAR] > > 1: mov r1, #ARM_EXCEPTION_HVC > b __kvm_vcpu_return > > -4: pop {r0, r1, r2} @ Failed translation, return to guest > +4: pop {r0, r1} @ Failed translation, return to guest > + mcrr p15, 0, r0, r1, c7 @ PAR > + pop {r0, r1, r2} > eret > > /* > diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S > index 3c8f2f0..2478af1 100644 > --- a/arch/arm/kvm/interrupts_head.S > +++ b/arch/arm/kvm/interrupts_head.S > @@ -302,11 +302,14 @@ vcpu .req r0 @ vcpu pointer always in r0 > .endif > > mrc p15, 0, r2, c14, c1, 0 @ CNTKCTL > + mrrc p15, 0, r3, r4, c7 @ PAR > > .if \store_to_vcpu == 0 > - push {r2} > + push {r2-r4} > .else > str r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)] > + add r12, vcpu, #CP15_OFFSET(c7_PAR) > + strd r3, r4, [r12] my compiler complains that the load should start with an even register, so I've changed this with the patch below. > .endif > .endm > > @@ -319,12 +322,15 @@ vcpu .req r0 @ vcpu pointer always in r0 > */ > .macro write_cp15_state read_from_vcpu > .if \read_from_vcpu == 0 > - pop {r2} > + pop {r2-r4} > .else > ldr r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)] > + add r12, vcpu, #CP15_OFFSET(c7_PAR) > + ldrd r3, r4, [r12] ditto > .endif > > mcr p15, 0, r2, c14, c1, 0 @ CNTKCTL > + mcrr p15, 0, r3, r4, c7 @ PAR > > .if \read_from_vcpu == 0 > pop {r2-r12} > -- > 1.8.2.3 > Here's the fixup patch: -Christoffer diff --git a/arch/arm/kvm/interrupts_head.S b/arch/arm/kvm/interrupts_head.S index 2478af1..2b44b95 100644 --- a/arch/arm/kvm/interrupts_head.S +++ b/arch/arm/kvm/interrupts_head.S @@ -302,14 +302,14 @@ vcpu .req r0 @ vcpu pointer always in r0 .endif mrc p15, 0, r2, c14, c1, 0 @ CNTKCTL - mrrc p15, 0, r3, r4, c7 @ PAR + mrrc p15, 0, r4, r5, c7 @ PAR .if \store_to_vcpu == 0 - push {r2-r4} + push {r2,r4-r5} .else str r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)] add r12, vcpu, #CP15_OFFSET(c7_PAR) - strd r3, r4, [r12] + strd r4, r5, [r12] .endif .endm @@ -322,15 +322,15 @@ vcpu .req r0 @ vcpu pointer always in r0 */ .macro write_cp15_state read_from_vcpu .if \read_from_vcpu == 0 - pop {r2-r4} + pop {r2,r4-r5} .else ldr r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)] add r12, vcpu, #CP15_OFFSET(c7_PAR) - ldrd r3, r4, [r12] + ldrd r4, r5, [r12] .endif mcr p15, 0, r2, c14, c1, 0 @ CNTKCTL - mcrr p15, 0, r3, r4, c7 @ PAR + mcrr p15, 0, r4, r5, c7 @ PAR .if \read_from_vcpu == 0 pop {r2-r12}