From patchwork Thu Apr 7 10:53:58 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julien Grall X-Patchwork-Id: 65285 Delivered-To: patch@linaro.org Received: by 10.112.43.237 with SMTP id z13csp778lbl; Thu, 7 Apr 2016 03:55:36 -0700 (PDT) X-Received: by 10.55.50.136 with SMTP id y130mr2461353qky.79.1460026536455; Thu, 07 Apr 2016 03:55:36 -0700 (PDT) Return-Path: Received: from lists.xenproject.org (lists.xenproject.org. [192.237.175.120]) by mx.google.com with ESMTPS id r79si5440864qkl.62.2016.04.07.03.55.36 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 07 Apr 2016 03:55:36 -0700 (PDT) Received-SPF: neutral (google.com: 192.237.175.120 is neither permitted nor denied by best guess record for domain of xen-devel-bounces@lists.xen.org) client-ip=192.237.175.120; Authentication-Results: mx.google.com; spf=neutral (google.com: 192.237.175.120 is neither permitted nor denied by best guess record for domain of xen-devel-bounces@lists.xen.org) smtp.mailfrom=xen-devel-bounces@lists.xen.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ao7Zl-00039z-Bl; Thu, 07 Apr 2016 10:54:13 +0000 Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ao7Zj-00039p-Li for xen-devel@lists.xen.org; Thu, 07 Apr 2016 10:54:11 +0000 Received: from [85.158.137.68] by server-8.bemta-3.messagelabs.com id 1C/79-04050-25C36075; Thu, 07 Apr 2016 10:54:10 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrPLMWRWlGSWpSXmKPExsVysyfVTTfIhi3 c4LuixZKPi1kcGD2O7v7NFMAYxZqZl5RfkcCacebAK+aClTsZK47vuMLWwLi5i7GLkZNDSGAj o8TyXyVdjFxA9mlGiaOXVoAl2AQ0Je58/sQEYosISEtc+3yZEaSIWeAuo0Tr09dgCWEBN4nLa 1+BNbAIqEpcvHGXBcTmFXCROPPuOhuILSEgJ3Hy2GTWCYycCxgZVjGqF6cWlaUW6RrqJRVlpm eU5CZm5ugaGhjr5aYWFyemp+YkJhXrJefnbmIEeowBCHYwLv/odIhRkoNJSZT3rBRbuBBfUn5 KZUZicUZ8UWlOavEhRhkODiUJ3lhroJxgUWp6akVaZg4wdGDSEhw8SiK80SBp3uKCxNzizHSI 1ClGRSlx3tUgCQGQREZpHlwbLFwvMcpKCfMyAh0ixFOQWpSbWYIq/4pRnINRSZh3PsgUnsy8E rjpr4AWMwEtvsAPtrgkESEl1cDI6LvDUcA+//CHA8cOsijqOb8RDHoXf/Scw4zoh57pU+/6GE 64JqGqxH3z/Ic/aR8WT493f7vu9bcr3w++L3tp1lTA/E3q2+7r9ZPP624Jmc9/u8BjRdMxVTV j+63Xll/knjxfsaziFfsWD6ZXqZ07JVxObm+34v2j5rzU6jGvG3/OxXnZAqKPlFiKMxINtZiL ihMBxpzn01ICAAA= X-Env-Sender: julien.grall@arm.com X-Msg-Ref: server-3.tower-31.messagelabs.com!1460026449!33455955!1 X-Originating-IP: [217.140.101.70] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 8.28; banners=-,-,- X-VirusChecked: Checked Received: (qmail 53654 invoked from network); 7 Apr 2016 10:54:09 -0000 Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by server-3.tower-31.messagelabs.com with SMTP; 7 Apr 2016 10:54:09 -0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3783528; Thu, 7 Apr 2016 03:52:57 -0700 (PDT) Received: from e108454-lin.cambridge.arm.com (e108454-lin.cambridge.arm.com [10.1.215.28]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 201363F25E; Thu, 7 Apr 2016 03:54:06 -0700 (PDT) From: Julien Grall To: xen-devel@lists.xen.org Date: Thu, 7 Apr 2016 11:53:58 +0100 Message-Id: <1460026438-10372-1-git-send-email-julien.grall@arm.com> X-Mailer: git-send-email 1.9.1 Cc: sstabellini@kernel.org, wei.liu2@citrix.com, Ian Campbell , Stefano Stabellini , Marc Zyngier , Julien Grall Subject: [Xen-devel] [for-4.7] xen/arm64: correctly emulate the {w, x}zr registers X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" On AArch64, encoding 31 for an R in the HSR is used to represent either {w,x}sp or {w,x}zr (See C1.2.4 in ARM DDI 0486A.d) depending on how the register field is interpreted by the instruction. All the instructions trapped by Xen (either via a sysreg access or data abort) interpret encoding 31 as {w,x}zr. Therefore we don't have to worry about the possibility that a trap could refer to sp or about decoding the instruction. For example AArch64 LDR and STR can have zr in the source/target register , but never sp. sp can be present in the destination pointer( i.e. "[sp]"), but that would be represented by the value of FAR_EL2, not in the HSR. For AArch32 it is possible for a LDR to target the PC, but this would not result in a valid ISS in the HSR register. However this could only occur if loading or storing the PC to MMIO, which we simply choose not to support for now. Finally, features such as xenaccess can lead to us trapping on arbitrary instructions accessing RAM and not just for MMIO. However in many such cases HSR.ISS is not valid and in general features such as xenaccess do not rely on the nature of the specific instruction, they resolve the fault (via information found elsewhere e.g. FAR_EL2) without needing to know anything about the instruction which triggered the trap. The register zr represents the zero register, i.e it will always return 0 and write to it is ignored. To properly handle this property, 2 new helpers have been introduced {get,set}_user_reg to read/write a value from/to a register. All the calls to select_user_reg have been replaced by these 2 helpers. Furthermore, the code to emulate encoding 31 in select_user_reg has been dropped because it was invalid. For Aarch64 context, the encoding is used for sp or zr. For AArch32 context, the ISS won't be valid for data abort from AArch32 using r15 (i.e pc) as source/destination (See D7-1881 ARM DDI 0487A.d, note the validity is more restrictive than on ARMv7). It's also not possible to use r15 in co-processor instructions. This patch fixes setting MMIO register and sysreg to a random value (actually PC) instead of zero by something like: *((volatile int*)reg) = 0; compilers tend to generate "str wzr, [xx]" here. [ian: added BUG_ON to select_user_reg and clarified bits of the commit message] Reported-by: Marc Zyngier Signed-off-by: Julien Grall Signed-off-by: Ian Campbell Reviewed-by: Stefano Stabellini --- Stefano, let me know the new helper corresponds to change you requested (see [1]) This patch is a bug fix for Xen 4.7. Without it, a MMIO register and sysreg will be set to a random value (actually PC) when the zero register is used. I'm not sure if we should consider this patch to be backported to Xen 4.6 and Xen 4.5. It depends on other patches and it would require some rework to backport it alone. [1] http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg03100.html --- xen/arch/arm/io.c | 34 ++++++++---- xen/arch/arm/traps.c | 126 ++++++++++++++++++++++++++++++--------------- xen/arch/arm/vgic-v3.c | 3 +- xen/arch/arm/vtimer.c | 59 ++++++++++++++++----- xen/include/asm-arm/regs.h | 7 +-- 5 files changed, 158 insertions(+), 71 deletions(-) diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c index 7e29943..0156755 100644 --- a/xen/arch/arm/io.c +++ b/xen/arch/arm/io.c @@ -24,12 +24,19 @@ #include static int handle_read(const struct mmio_handler *handler, struct vcpu *v, - mmio_info_t *info, register_t *r) + mmio_info_t *info) { const struct hsr_dabt dabt = info->dabt; + struct cpu_user_regs *regs = guest_cpu_user_regs(); + /* + * Initialize to zero to avoid leaking data if there is an + * implementation error in the emulation (such as not correctly + * setting r). + */ + register_t r = 0; uint8_t size = (1 << dabt.size) * 8; - if ( !handler->ops->read(v, info, r, handler->priv) ) + if ( !handler->ops->read(v, info, &r, handler->priv) ) return 0; /* @@ -37,7 +44,7 @@ static int handle_read(const struct mmio_handler *handler, struct vcpu *v, * Note that we expect the read handler to have zeroed the bits * outside the requested access size. */ - if ( dabt.sign && (*r & (1UL << (size - 1))) ) + if ( dabt.sign && (r & (1UL << (size - 1))) ) { /* * We are relying on register_t using the same as @@ -45,21 +52,30 @@ static int handle_read(const struct mmio_handler *handler, struct vcpu *v, * code smaller. */ BUILD_BUG_ON(sizeof(register_t) != sizeof(unsigned long)); - *r |= (~0UL) << size; + r |= (~0UL) << size; } + set_user_reg(regs, dabt.reg, r); + return 1; } +static int handle_write(const struct mmio_handler *handler, struct vcpu *v, + mmio_info_t *info) +{ + const struct hsr_dabt dabt = info->dabt; + struct cpu_user_regs *regs = guest_cpu_user_regs(); + + return handler->ops->write(v, info, get_user_reg(regs, dabt.reg), + handler->priv); +} + int handle_mmio(mmio_info_t *info) { struct vcpu *v = current; int i; const struct mmio_handler *handler = NULL; const struct vmmio *vmmio = &v->domain->arch.vmmio; - struct hsr_dabt dabt = info->dabt; - struct cpu_user_regs *regs = guest_cpu_user_regs(); - register_t *r = select_user_reg(regs, dabt.reg); for ( i = 0; i < vmmio->num_entries; i++ ) { @@ -74,9 +90,9 @@ int handle_mmio(mmio_info_t *info) return 0; if ( info->dabt.write ) - return handler->ops->write(v, info, *r, handler->priv); + return handle_write(handler, v, info); else - return handle_read(handler, v, info, r); + return handle_read(handler, v, info); } void register_mmio_handler(struct domain *d, diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 31d2115..1516abd 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -149,7 +149,31 @@ static void print_xen_info(void) debug_build() ? 'y' : 'n', print_tainted(taint_str)); } -register_t *select_user_reg(struct cpu_user_regs *regs, int reg) +#ifdef CONFIG_ARM_32 +static inline bool_t is_zero_register(int reg) +{ + /* There is no zero register for ARM32 */ + return 0; +} +#else +static inline bool_t is_zero_register(int reg) +{ + /* + * For store/load and sysreg instruction, the encoding 31 always + * corresponds to {w,x}zr which is the zero register. + */ + return (reg == 31); +} +#endif + +/* + * Returns a pointer to the given register value in regs, taking the + * processor mode (CPSR) into account. + * + * Note that this function should not be used directly but via + * {get,set}_user_reg. + */ +static register_t *select_user_reg(struct cpu_user_regs *regs, int reg) { BUG_ON( !guest_mode(regs) ); @@ -207,16 +231,31 @@ register_t *select_user_reg(struct cpu_user_regs *regs, int reg) } #undef REGOFFS #else - /* In 64 bit the syndrome register contains the AArch64 register - * number even if the trap was from AArch32 mode. Except that - * AArch32 R15 (PC) is encoded as 0b11111. + /* + * On 64-bit the syndrome register contains the register index as + * viewed in AArch64 state even if the trap was from AArch32 mode. */ - if ( reg == 0x1f /* && is aarch32 guest */) - return ®s->pc; + BUG_ON(is_zero_register(reg)); /* Cannot be {w,x}zr */ return ®s->x0 + reg; #endif } +register_t get_user_reg(struct cpu_user_regs *regs, int reg) +{ + if ( is_zero_register(reg) ) + return 0; + + return *select_user_reg(regs, reg); +} + +void set_user_reg(struct cpu_user_regs *regs, int reg, register_t value) +{ + if ( is_zero_register(reg) ) + return; + + *select_user_reg(regs, reg) = value; +} + static const char *decode_fsc(uint32_t fsc, int *level) { const char *msg = NULL; @@ -1241,22 +1280,19 @@ static arm_hypercall_t arm_hypercall_table[] = { #ifndef NDEBUG static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code) { - register_t *r; uint32_t reg; uint32_t domid = current->domain->domain_id; switch ( code ) { case 0xe0 ... 0xef: reg = code - 0xe0; - r = select_user_reg(regs, reg); printk("DOM%d: R%d = 0x%"PRIregister" at 0x%"PRIvaddr"\n", - domid, reg, *r, regs->pc); + domid, reg, get_user_reg(regs, reg), regs->pc); break; case 0xfd: printk("DOM%d: Reached %"PRIvaddr"\n", domid, regs->pc); break; case 0xfe: - r = select_user_reg(regs, 0); - printk("%c", (char)(*r & 0xff)); + printk("%c", (char)(get_user_reg(regs, 0) & 0xff)); break; case 0xff: printk("DOM%d: DEBUG\n", domid); @@ -1614,7 +1650,7 @@ static void advance_pc(struct cpu_user_regs *regs, const union hsr hsr) /* Read as zero and write ignore */ static void handle_raz_wi(struct cpu_user_regs *regs, - register_t *reg, + int regidx, bool_t read, const union hsr hsr, int min_el) @@ -1625,7 +1661,7 @@ static void handle_raz_wi(struct cpu_user_regs *regs, return inject_undef_exception(regs, hsr); if ( read ) - *reg = 0; + set_user_reg(regs, regidx, 0); /* else: write ignored */ advance_pc(regs, hsr); @@ -1633,7 +1669,7 @@ static void handle_raz_wi(struct cpu_user_regs *regs, /* Write only as write ignore */ static void handle_wo_wi(struct cpu_user_regs *regs, - register_t *reg, + int regidx, bool_t read, const union hsr hsr, int min_el) @@ -1652,7 +1688,7 @@ static void handle_wo_wi(struct cpu_user_regs *regs, /* Read only as read as zero */ static void handle_ro_raz(struct cpu_user_regs *regs, - register_t *reg, + int regidx, bool_t read, const union hsr hsr, int min_el) @@ -1666,7 +1702,7 @@ static void handle_ro_raz(struct cpu_user_regs *regs, return inject_undef_exception(regs, hsr); /* else: raz */ - *reg = 0; + set_user_reg(regs, regidx, 0); advance_pc(regs, hsr); } @@ -1675,7 +1711,7 @@ static void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr) { const struct hsr_cp32 cp32 = hsr.cp32; - register_t *r = select_user_reg(regs, cp32.reg); + int regidx = cp32.reg; struct vcpu *v = current; if ( !check_conditional_instr(regs, hsr) ) @@ -1708,7 +1744,7 @@ static void do_cp15_32(struct cpu_user_regs *regs, if ( psr_mode_is_user(regs) ) return inject_undef_exception(regs, hsr); if ( cp32.read ) - *r = v->arch.actlr; + set_user_reg(regs, regidx, v->arch.actlr); break; /* @@ -1740,13 +1776,13 @@ static void do_cp15_32(struct cpu_user_regs *regs, case HSR_CPREG32(PMUSERENR): /* RO at EL0. RAZ/WI at EL1 */ if ( psr_mode_is_user(regs) ) - return handle_ro_raz(regs, r, cp32.read, hsr, 0); + return handle_ro_raz(regs, regidx, cp32.read, hsr, 0); else - return handle_raz_wi(regs, r, cp32.read, hsr, 1); + return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); case HSR_CPREG32(PMINTENSET): case HSR_CPREG32(PMINTENCLR): /* EL1 only, however MDCR_EL2.TPM==1 means EL0 may trap here also. */ - return handle_raz_wi(regs, r, cp32.read, hsr, 1); + return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); case HSR_CPREG32(PMCR): case HSR_CPREG32(PMCNTENSET): case HSR_CPREG32(PMCNTENCLR): @@ -1763,7 +1799,7 @@ static void do_cp15_32(struct cpu_user_regs *regs, * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We * emulate that register as 0 above. */ - return handle_raz_wi(regs, r, cp32.read, hsr, 1); + return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); /* * HCR_EL2.TIDCP @@ -1866,7 +1902,7 @@ static void do_cp15_64(struct cpu_user_regs *regs, static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) { const struct hsr_cp32 cp32 = hsr.cp32; - register_t *r = select_user_reg(regs, cp32.reg); + int regidx = cp32.reg; struct domain *d = current->domain; if ( !check_conditional_instr(regs, hsr) ) @@ -1888,9 +1924,9 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) * DBGPRCR */ case HSR_CPREG32(DBGOSLAR): - return handle_wo_wi(regs, r, cp32.read, hsr, 1); + return handle_wo_wi(regs, regidx, cp32.read, hsr, 1); case HSR_CPREG32(DBGOSDLR): - return handle_raz_wi(regs, r, cp32.read, hsr, 1); + return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); /* * MDCR_EL2.TDA @@ -1915,6 +1951,9 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) * DBGOSECCR */ case HSR_CPREG32(DBGDIDR): + { + uint32_t val; + /* * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis * is set to 0, which we emulated below. @@ -1928,23 +1967,26 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) * - Version: ARMv7 v7.1 * - Variant and Revision bits match MDIR */ - *r = (1 << 24) | (5 << 16); - *r |= ((d->arch.vpidr >> 20) & 0xf) | (d->arch.vpidr & 0xf); + val = (1 << 24) | (5 << 16); + val |= ((d->arch.vpidr >> 20) & 0xf) | (d->arch.vpidr & 0xf); + set_user_reg(regs, regidx, val); + break; + } case HSR_CPREG32(DBGDSCRINT): /* * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis * is set to 0, which we emulated below. */ - return handle_ro_raz(regs, r, cp32.read, hsr, 1); + return handle_ro_raz(regs, regidx, cp32.read, hsr, 1); case HSR_CPREG32(DBGDSCREXT): /* * Implement debug status and control register as RAZ/WI. * The OS won't use Hardware debug if MDBGen not set. */ - return handle_raz_wi(regs, r, cp32.read, hsr, 1); + return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); case HSR_CPREG32(DBGVCR): case HSR_CPREG32(DBGBVR0): @@ -1953,7 +1995,7 @@ static void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) case HSR_CPREG32(DBGWCR0): case HSR_CPREG32(DBGBVR1): case HSR_CPREG32(DBGBCR1): - return handle_raz_wi(regs, r, cp32.read, hsr, 1); + return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); /* * CPTR_EL2.TTA @@ -2077,7 +2119,7 @@ static void do_cp(struct cpu_user_regs *regs, const union hsr hsr) static void do_sysreg(struct cpu_user_regs *regs, const union hsr hsr) { - register_t *x = select_user_reg(regs, hsr.sysreg.reg); + int regidx = hsr.sysreg.reg; struct vcpu *v = current; switch ( hsr.bits & HSR_SYSREG_REGS_MASK ) @@ -2091,7 +2133,7 @@ static void do_sysreg(struct cpu_user_regs *regs, if ( psr_mode_is_user(regs) ) return inject_undef_exception(regs, hsr); if ( hsr.sysreg.read ) - *x = v->arch.actlr; + set_user_reg(regs, regidx, v->arch.actlr); break; /* @@ -2100,7 +2142,7 @@ static void do_sysreg(struct cpu_user_regs *regs, * ARMv8 (DDI 0487A.d): D1-1508 Table D1-57 */ case HSR_SYSREG_MDRAR_EL1: - return handle_ro_raz(regs, x, hsr.sysreg.read, hsr, 1); + return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 1); /* * MDCR_EL2.TDOSA @@ -2112,9 +2154,9 @@ static void do_sysreg(struct cpu_user_regs *regs, * DBGPRCR_EL1 */ case HSR_SYSREG_OSLAR_EL1: - return handle_wo_wi(regs, x, hsr.sysreg.read, hsr, 1); + return handle_wo_wi(regs, regidx, hsr.sysreg.read, hsr, 1); case HSR_SYSREG_OSDLR_EL1: - return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1); + return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1); /* * MDCR_EL2.TDA @@ -2134,18 +2176,18 @@ static void do_sysreg(struct cpu_user_regs *regs, * DBGAUTHSTATUS_EL1 */ case HSR_SYSREG_MDSCR_EL1: - return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1); + return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1); case HSR_SYSREG_MDCCSR_EL0: /* * Accessible at EL0 only if MDSCR_EL1.TDCC is set to 0. We emulate that * register as RAZ/WI above. So RO at both EL0 and EL1. */ - return handle_ro_raz(regs, x, hsr.sysreg.read, hsr, 0); + return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 0); HSR_SYSREG_DBG_CASES(DBGBVR): HSR_SYSREG_DBG_CASES(DBGBCR): HSR_SYSREG_DBG_CASES(DBGWVR): HSR_SYSREG_DBG_CASES(DBGWCR): - return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1); + return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1); /* * MDCR_EL2.TPM @@ -2169,13 +2211,13 @@ static void do_sysreg(struct cpu_user_regs *regs, * Accessible from EL1 only, but if EL0 trap happens handle as * undef. */ - return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1); + return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1); case HSR_SYSREG_PMUSERENR_EL0: /* RO at EL0. RAZ/WI at EL1 */ if ( psr_mode_is_user(regs) ) - return handle_ro_raz(regs, x, hsr.sysreg.read, hsr, 0); + return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 0); else - return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1); + return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1); case HSR_SYSREG_PMCR_EL0: case HSR_SYSREG_PMCNTENSET_EL0: case HSR_SYSREG_PMCNTENCLR_EL0: @@ -2192,7 +2234,7 @@ static void do_sysreg(struct cpu_user_regs *regs, * Accessible at EL0 only if PMUSERENR_EL0.EN is set. We * emulate that register as 0 above. */ - return handle_raz_wi(regs, x, hsr.sysreg.read, hsr, 1); + return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1); /* * !CNTHCTL_EL2.EL1PCEN diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c index dba2449..b37a7c0 100644 --- a/xen/arch/arm/vgic-v3.c +++ b/xen/arch/arm/vgic-v3.c @@ -1304,7 +1304,6 @@ static int vgic_v3_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr) { struct vcpu *v = current; struct hsr_sysreg sysreg = hsr.sysreg; - register_t *r = select_user_reg(regs, sysreg.reg); ASSERT (hsr.ec == HSR_EC_SYSREG); @@ -1318,7 +1317,7 @@ static int vgic_v3_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr) case HSR_SYSREG_ICC_SGI1R_EL1: /* WO */ if ( !sysreg.read ) - return vgic_v3_to_sgi(v, *r); + return vgic_v3_to_sgi(v, get_user_reg(regs, sysreg.reg)); else { gprintk(XENLOG_WARNING, "Reading SGI1R_EL1 - WO register\n"); diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c index b9c0b41..f636705 100644 --- a/xen/arch/arm/vtimer.c +++ b/xen/arch/arm/vtimer.c @@ -249,32 +249,49 @@ static int vtimer_cntp_cval(struct cpu_user_regs *regs, uint64_t *r, int read) static int vtimer_emulate_cp32(struct cpu_user_regs *regs, union hsr hsr) { struct hsr_cp32 cp32 = hsr.cp32; - uint32_t *r = (uint32_t *)select_user_reg(regs, cp32.reg); + /* + * Initialize to zero to avoid leaking data if there is an + * implementation error in the emulation (such as not correctly + * setting r). + */ + uint32_t r = 0; + int res; + if ( cp32.read ) perfc_incr(vtimer_cp32_reads); else perfc_incr(vtimer_cp32_writes); + if ( !cp32.read ) + r = get_user_reg(regs, cp32.reg); + switch ( hsr.bits & HSR_CP32_REGS_MASK ) { case HSR_CPREG32(CNTP_CTL): - return vtimer_cntp_ctl(regs, r, cp32.read); + res = vtimer_cntp_ctl(regs, &r, cp32.read); + break; case HSR_CPREG32(CNTP_TVAL): - return vtimer_cntp_tval(regs, r, cp32.read); + res = vtimer_cntp_tval(regs, &r, cp32.read); + break; default: return 0; } + + if ( res && cp32.read ) + set_user_reg(regs, cp32.reg, r); + + return res; } static int vtimer_emulate_cp64(struct cpu_user_regs *regs, union hsr hsr) { struct hsr_cp64 cp64 = hsr.cp64; - uint32_t *r1 = (uint32_t *)select_user_reg(regs, cp64.reg1); - uint32_t *r2 = (uint32_t *)select_user_reg(regs, cp64.reg2); - uint64_t x = (uint64_t)(*r1) | ((uint64_t)(*r2) << 32); + uint32_t r1 = get_user_reg(regs, cp64.reg1); + uint32_t r2 = get_user_reg(regs, cp64.reg2); + uint64_t x = (uint64_t)r1 | ((uint64_t)r2 << 32); if ( cp64.read ) perfc_incr(vtimer_cp64_reads); @@ -294,8 +311,8 @@ static int vtimer_emulate_cp64(struct cpu_user_regs *regs, union hsr hsr) if ( cp64.read ) { - *r1 = (uint32_t)(x & 0xffffffff); - *r2 = (uint32_t)(x >> 32); + set_user_reg(regs, cp64.reg1, x & 0xffffffff); + set_user_reg(regs, cp64.reg2, x >> 32); } return 1; @@ -311,14 +328,16 @@ static int vtimer_emulate_sysreg32(struct cpu_user_regs *regs, union hsr hsr, vtimer_sysreg32_fn_t fn) { struct hsr_sysreg sysreg = hsr.sysreg; - register_t *x = select_user_reg(regs, sysreg.reg); - uint32_t r = *x; + uint32_t r = 0; int ret; + if ( !sysreg.read ) + r = get_user_reg(regs, sysreg.reg); + ret = fn(regs, &r, sysreg.read); if ( ret && sysreg.read ) - *x = r; + set_user_reg(regs, sysreg.reg, r); return ret; } @@ -327,9 +346,23 @@ static int vtimer_emulate_sysreg64(struct cpu_user_regs *regs, union hsr hsr, vtimer_sysreg64_fn_t fn) { struct hsr_sysreg sysreg = hsr.sysreg; - uint64_t *x = select_user_reg(regs, sysreg.reg); + /* + * Initialize to zero to avoid leaking data if there is an + * implementation error in the emulation (such as not correctly + * setting x). + */ + uint64_t x = 0; + int ret; + + if ( !sysreg.read ) + x = get_user_reg(regs, sysreg.reg); - return fn(regs, x, sysreg.read); + ret = fn(regs, &x, sysreg.read); + + if ( ret && sysreg.read ) + set_user_reg(regs, sysreg.reg, x); + + return ret; } static int vtimer_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr) diff --git a/xen/include/asm-arm/regs.h b/xen/include/asm-arm/regs.h index 56d53d6..2440edb 100644 --- a/xen/include/asm-arm/regs.h +++ b/xen/include/asm-arm/regs.h @@ -50,11 +50,8 @@ #define return_reg(v) ((v)->arch.cpu_info->guest_cpu_user_regs.r0) -/* - * Returns a pointer to the given register value in regs, taking the - * processor mode (CPSR) into account. - */ -extern register_t *select_user_reg(struct cpu_user_regs *regs, int reg); +register_t get_user_reg(struct cpu_user_regs *regs, int reg); +void set_user_reg(struct cpu_user_regs *regs, int reg, register_t val); #endif