From patchwork Tue May 13 16:13:59 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Victor Kamensky X-Patchwork-Id: 4169501 Return-Path: X-Original-To: patchwork-linux-arm@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 AF57B9F1C0 for ; Tue, 13 May 2014 16:19:18 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 85B2220221 for ; Tue, 13 May 2014 16:19:17 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 452552020A for ; Tue, 13 May 2014 16:19:16 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WkFNR-0003Af-Ol; Tue, 13 May 2014 16:16:25 +0000 Received: from mail-pa0-f46.google.com ([209.85.220.46]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WkFMR-0001of-SQ for linux-arm-kernel@lists.infradead.org; Tue, 13 May 2014 16:15:24 +0000 Received: by mail-pa0-f46.google.com with SMTP id kq14so458758pab.5 for ; Tue, 13 May 2014 09:15:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=Dy3BV2RGvux2tq+7HuL5/XUU9MO0oRXn87kv80AtLG4=; b=f0brTZbPiqMU9BDQG6uwosNjwO4TcLwE/pMtVInU2xssklsg5mQCZ1FfVDOSrI7bTF KXslmo6uCT24un2oKTQ5TKi97M7MyF+ShaIwj6z3Y+2TwYMwAXFICzAGA8H7jcy08lvd C4RO0QAzkkcK01tofs9zojRFctXlsYjiJHe+GxpzH6/OzoXCblY/3V6Uc1/R/isywE+X dhuSIeyeFTz7RrbQpCGCLNDYRescZ4ZEFyhIooeKgd1LwMeH4LFN1bKSuNZ2hZo8qGsC p64DjENIeY3UEzHYqF7K17ZN0wLA0G2wpNQNIOjZ9eTqa6YdLwokI3chYR2mIVak2BYy 5fuw== X-Gm-Message-State: ALoCoQlv5GNzqkaojZkMwqMPrzSFS9tO3pPCCq6/e9G3XJZ7UHD6AB/KgbIZmD8CFZR/mQSv7iXV X-Received: by 10.68.241.68 with SMTP id wg4mr6276317pbc.66.1399997703013; Tue, 13 May 2014 09:15:03 -0700 (PDT) Received: from kamensky-w530.hsd1.ca.comcast.net (c-24-6-79-41.hsd1.ca.comcast.net. [24.6.79.41]) by mx.google.com with ESMTPSA id dd5sm28958360pbc.85.2014.05.13.09.15.01 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 13 May 2014 09:15:02 -0700 (PDT) From: Victor Kamensky To: kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org, marc.zyngier@arm.com, agraf@suse.de Subject: [PATCH v3 07/14] ARM: KVM: one_reg coproc set and get BE fixes Date: Tue, 13 May 2014 09:13:59 -0700 Message-Id: <1399997646-4716-8-git-send-email-victor.kamensky@linaro.org> X-Mailer: git-send-email 1.8.1.4 In-Reply-To: <1399997646-4716-1-git-send-email-victor.kamensky@linaro.org> References: <1399997646-4716-1-git-send-email-victor.kamensky@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140513_091523_965606_4E34BEC0 X-CRM114-Status: GOOD ( 20.87 ) X-Spam-Score: -0.7 (/) Cc: taras.kondratiuk@linaro.org, linaro-kernel@lists.linaro.org, linux-arm-kernel@lists.infradead.org, Victor Kamensky 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: , MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,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 Fix code that handles KVM_SET_ONE_REG, KVM_GET_ONE_REG ioctls to work in BE image. Before this fix get/set_one_reg functions worked correctly only in LE case - reg_from_user was taking 'void *' kernel address that actually could be target/source memory of either 4 bytes size or 8 bytes size, and code copied from/to user memory that could hold either 4 bytes register, 8 byte register or pair of 4 bytes registers. For example note that there was a case when 4 bytes register was read from user-land to kernel target address of 8 bytes value. Because it was working in LE, least significant word was memcpy(ied) and it just worked. In BE code with 'void *' as target/source 'val' type it is impossible to tell whether 4 bytes register from user-land should be copied to 'val' address itself (4 bytes target) or it should be copied to 'val' + 4 (least significant word of 8 bytes value). So first change was to introduce strongly typed functions, where type of target/source 'val' is strongly defined: reg_from_user_to_u64 - reads register from user-land to kernel 'u64 *val' address; register size could be 4 or 8 bytes reg_from_user_to_u32 - reads register(s) from user-land to kernel 'u32 *val' address; note it could be one or two 4 bytes registers reg_to_user_from_u64 - writes register from kernel 'u64 *val' address to user-land register memory; register size could be 4 or 8 bytes ret_to_user_from_u32 - writes register(s) from kernel 'u32 *val' address to user-land register(s) memory; note it could be one or two 4 bytes registers All places where reg_from_user, reg_to_user functions were used, were changed to use either corresponding u64 or u32 variants of functions depending on type of source/target kernel memory variable. In case of 'u64 *val' and register size equals 4 bytes, reg_from_user_to_u64 and reg_to_user_from_u64 work only with least siginificant word of target/source kernel value. It would be nice to deal only with u32 kernel values in _u32 functions and with u64 kernel values in _u64 functions, however it is not always possible because functions like set_invariant_cp15 get_invariant_cp15 store values in u64 types but registers are 32bit. Signed-off-by: Victor Kamensky --- arch/arm/kvm/coproc.c | 118 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 92 insertions(+), 26 deletions(-) diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c index c58a351..5ca6582 100644 --- a/arch/arm/kvm/coproc.c +++ b/arch/arm/kvm/coproc.c @@ -682,17 +682,83 @@ static struct coproc_reg invariant_cp15[] = { { CRn( 0), CRm( 0), Op1( 1), Op2( 7), is32, NULL, get_AIDR }, }; -static int reg_from_user(void *val, const void __user *uaddr, u64 id) +/* + * Reads register value from user-land uaddr address into kernel u64 value + * given by val address. Note register size could be 4 bytes, so user-land + * 4 bytes value will be copied into least significant word. Or register + * size could be 8 bytes, so function works as regular copy. + */ +static int reg_from_user_to_u64(u64 *val, const void __user *uaddr, u64 id) +{ + unsigned long regsize = KVM_REG_SIZE(id); + union { + u32 word; + u64 dword; + } tmp = {0}; + + if (copy_from_user(&tmp, uaddr, regsize) != 0) + return -EFAULT; + + switch (regsize) { + case 4: + *val = tmp.word; + break; + case 8: + *val = tmp.dword; + break; + } + return 0; +} + +/* + * Reads register value from user-land uaddr address to kernel u32 value + * or array of two u32 values. I.e. it may really copy two u32 registers + * when used with register which size is 8 bytes (for example V7 64 + * bit registers like TTBR0/TTBR1). + */ +static int reg_from_user_to_u32(u32 *val, const void __user *uaddr, u64 id) { - /* This Just Works because we are little endian. */ if (copy_from_user(val, uaddr, KVM_REG_SIZE(id)) != 0) return -EFAULT; return 0; } -static int reg_to_user(void __user *uaddr, const void *val, u64 id) +/* + * Writes register value to user-land uaddr address from kernel u64 value + * given by val address. Note register size could be 4 bytes, so only 4 + * bytes from least significant word will by copied into uaddr address. + * In case of 8 bytes sized register it works as regular copy. + */ +static int reg_to_user_from_u64(void __user *uaddr, const u64 *val, u64 id) +{ + unsigned long regsize = KVM_REG_SIZE(id); + union { + u32 word; + u64 dword; + } tmp; + + switch (regsize) { + case 4: + tmp.word = *val; + break; + case 8: + tmp.dword = *val; + break; + } + + if (copy_to_user(uaddr, &tmp, regsize) != 0) + return -EFAULT; + return 0; +} + +/* + * Writes register value to user-land uaddr address from kernel u32 value + * or arra of two u32 values. I.e it may really copy two u32 registers + * when used with register which size is 8 bytes (for example V7 64 + * bit registers like TTBR0/TTBR1). + */ +static int reg_to_user_from_u32(void __user *uaddr, const u32 *val, u64 id) { - /* This Just Works because we are little endian. */ if (copy_to_user(uaddr, val, KVM_REG_SIZE(id)) != 0) return -EFAULT; return 0; @@ -710,7 +776,7 @@ static int get_invariant_cp15(u64 id, void __user *uaddr) if (!r) return -ENOENT; - return reg_to_user(uaddr, &r->val, id); + return reg_to_user_from_u64(uaddr, &r->val, id); } static int set_invariant_cp15(u64 id, void __user *uaddr) @@ -726,7 +792,7 @@ static int set_invariant_cp15(u64 id, void __user *uaddr) if (!r) return -ENOENT; - err = reg_from_user(&val, uaddr, id); + err = reg_from_user_to_u64(&val, uaddr, id); if (err) return err; @@ -894,8 +960,8 @@ static int vfp_get_reg(const struct kvm_vcpu *vcpu, u64 id, void __user *uaddr) if (vfpid < num_fp_regs()) { if (KVM_REG_SIZE(id) != 8) return -ENOENT; - return reg_to_user(uaddr, &vcpu->arch.vfp_guest.fpregs[vfpid], - id); + return reg_to_user_from_u64(uaddr, &vcpu->arch.vfp_guest.fpregs[vfpid], + id); } /* FP control registers are all 32 bit. */ @@ -904,22 +970,22 @@ static int vfp_get_reg(const struct kvm_vcpu *vcpu, u64 id, void __user *uaddr) switch (vfpid) { case KVM_REG_ARM_VFP_FPEXC: - return reg_to_user(uaddr, &vcpu->arch.vfp_guest.fpexc, id); + return reg_to_user_from_u32(uaddr, &vcpu->arch.vfp_guest.fpexc, id); case KVM_REG_ARM_VFP_FPSCR: - return reg_to_user(uaddr, &vcpu->arch.vfp_guest.fpscr, id); + return reg_to_user_from_u32(uaddr, &vcpu->arch.vfp_guest.fpscr, id); case KVM_REG_ARM_VFP_FPINST: - return reg_to_user(uaddr, &vcpu->arch.vfp_guest.fpinst, id); + return reg_to_user_from_u32(uaddr, &vcpu->arch.vfp_guest.fpinst, id); case KVM_REG_ARM_VFP_FPINST2: - return reg_to_user(uaddr, &vcpu->arch.vfp_guest.fpinst2, id); + return reg_to_user_from_u32(uaddr, &vcpu->arch.vfp_guest.fpinst2, id); case KVM_REG_ARM_VFP_MVFR0: val = fmrx(MVFR0); - return reg_to_user(uaddr, &val, id); + return reg_to_user_from_u32(uaddr, &val, id); case KVM_REG_ARM_VFP_MVFR1: val = fmrx(MVFR1); - return reg_to_user(uaddr, &val, id); + return reg_to_user_from_u32(uaddr, &val, id); case KVM_REG_ARM_VFP_FPSID: val = fmrx(FPSID); - return reg_to_user(uaddr, &val, id); + return reg_to_user_from_u32(uaddr, &val, id); default: return -ENOENT; } @@ -938,8 +1004,8 @@ static int vfp_set_reg(struct kvm_vcpu *vcpu, u64 id, const void __user *uaddr) if (vfpid < num_fp_regs()) { if (KVM_REG_SIZE(id) != 8) return -ENOENT; - return reg_from_user(&vcpu->arch.vfp_guest.fpregs[vfpid], - uaddr, id); + return reg_from_user_to_u64(&vcpu->arch.vfp_guest.fpregs[vfpid], + uaddr, id); } /* FP control registers are all 32 bit. */ @@ -948,28 +1014,28 @@ static int vfp_set_reg(struct kvm_vcpu *vcpu, u64 id, const void __user *uaddr) switch (vfpid) { case KVM_REG_ARM_VFP_FPEXC: - return reg_from_user(&vcpu->arch.vfp_guest.fpexc, uaddr, id); + return reg_from_user_to_u32(&vcpu->arch.vfp_guest.fpexc, uaddr, id); case KVM_REG_ARM_VFP_FPSCR: - return reg_from_user(&vcpu->arch.vfp_guest.fpscr, uaddr, id); + return reg_from_user_to_u32(&vcpu->arch.vfp_guest.fpscr, uaddr, id); case KVM_REG_ARM_VFP_FPINST: - return reg_from_user(&vcpu->arch.vfp_guest.fpinst, uaddr, id); + return reg_from_user_to_u32(&vcpu->arch.vfp_guest.fpinst, uaddr, id); case KVM_REG_ARM_VFP_FPINST2: - return reg_from_user(&vcpu->arch.vfp_guest.fpinst2, uaddr, id); + return reg_from_user_to_u32(&vcpu->arch.vfp_guest.fpinst2, uaddr, id); /* These are invariant. */ case KVM_REG_ARM_VFP_MVFR0: - if (reg_from_user(&val, uaddr, id)) + if (reg_from_user_to_u32(&val, uaddr, id)) return -EFAULT; if (val != fmrx(MVFR0)) return -EINVAL; return 0; case KVM_REG_ARM_VFP_MVFR1: - if (reg_from_user(&val, uaddr, id)) + if (reg_from_user_to_u32(&val, uaddr, id)) return -EFAULT; if (val != fmrx(MVFR1)) return -EINVAL; return 0; case KVM_REG_ARM_VFP_FPSID: - if (reg_from_user(&val, uaddr, id)) + if (reg_from_user_to_u32(&val, uaddr, id)) return -EFAULT; if (val != fmrx(FPSID)) return -EINVAL; @@ -1016,7 +1082,7 @@ int kvm_arm_coproc_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) return get_invariant_cp15(reg->id, uaddr); /* Note: copies two regs if size is 64 bit. */ - return reg_to_user(uaddr, &vcpu->arch.cp15[r->reg], reg->id); + return reg_to_user_from_u32(uaddr, &vcpu->arch.cp15[r->reg], reg->id); } int kvm_arm_coproc_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) @@ -1035,7 +1101,7 @@ int kvm_arm_coproc_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) return set_invariant_cp15(reg->id, uaddr); /* Note: copies two regs if size is 64 bit */ - return reg_from_user(&vcpu->arch.cp15[r->reg], uaddr, reg->id); + return reg_from_user_to_u32(&vcpu->arch.cp15[r->reg], uaddr, reg->id); } static unsigned int num_demux_regs(void)