From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Subject: [PATCH v4 4/5] target-arm: kvm64 fix save/restore of SPSR regs Date: Mon, 16 Mar 2015 11:01:55 +0000 Message-ID: <1426503716-13931-5-git-send-email-alex.bennee@linaro.org> References: <1426503716-13931-1-git-send-email-alex.bennee@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org, marc.zyngier@arm.com, =?UTF-8?q?Alex=20Benn=C3=A9e?= , Peter Maydell To: qemu-devel@nongnu.org Return-path: Received: from static.88-198-71-155.clients.your-server.de ([88.198.71.155]:55625 "EHLO socrates.bennee.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752580AbbCPLGk (ORCPT ); Mon, 16 Mar 2015 07:06:40 -0400 In-Reply-To: <1426503716-13931-1-git-send-email-alex.bennee@linaro.org> Sender: kvm-owner@vger.kernel.org List-ID: =46rom: Christoffer Dall The current code was negatively indexing the cpu state array and not synchronizing banked spsr register state with the current mode's spsr state, causing occasional failures with migration. Some munging is done to take care of the aarch64 mapping and also to ensure the most current value of the spsr is updated to the banked registers (relevant for KVM<->TCG migration). Signed-off-by: Christoffer Dall Signed-off-by: Alex Benn=C3=A9e --- v2 (ajb) - minor tweaks and clarifications v3 - Use the correct bank index function for setting/getting env->spsr - only deal with spsrs in elevated exception levels v4 - try and make commentary clearer - ensure env->banked_spsr[0] =3D env->spsr before we sync diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c index 8fd0c8d..7ddb1b1 100644 --- a/target-arm/kvm64.c +++ b/target-arm/kvm64.c @@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level) uint64_t val; int i; int ret; + unsigned int el; =20 ARMCPU *cpu =3D ARM_CPU(cs); CPUARMState *env =3D &cpu->env; @@ -206,9 +207,29 @@ int kvm_arch_put_registers(CPUState *cs, int level= ) return ret; } =20 + /* Saved Program State Registers + * + * Before we restore from the banked_spsr[] array we need to + * ensure that any modifications to env->spsr are correctly + * reflected and map aarch64 exception levels if required. + */ + el =3D arm_current_el(env); + if (el > 0) { + if (is_a64(env)) { + g_assert(el =3D=3D 1); + env->banked_spsr[0] =3D env->spsr; + /* QEMUs AARCH64 EL1 SPSR is in bank 0, so map it to + * KVM_SPSR_SVC for syncing to KVM */ + env->banked_spsr[1] =3D env->banked_spsr[0]; + } else { + i =3D bank_number(env->uncached_cpsr & CPSR_M); + env->banked_spsr[i] =3D env->spsr; + } + } + for (i =3D 0; i < KVM_NR_SPSR; i++) { reg.id =3D AARCH64_CORE_REG(spsr[i]); - reg.addr =3D (uintptr_t) &env->banked_spsr[i - 1]; + reg.addr =3D (uintptr_t) &env->banked_spsr[i+1]; ret =3D kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); if (ret) { return ret; @@ -254,6 +275,7 @@ int kvm_arch_get_registers(CPUState *cs) struct kvm_one_reg reg; uint64_t val; uint32_t fpr; + unsigned int el; int i; int ret; =20 @@ -326,15 +348,34 @@ int kvm_arch_get_registers(CPUState *cs) return ret; } =20 + /* Fetch the SPSR registers + * + * KVM has an array of state indexed for all the possible aarch32 + * privilege levels. These map onto QEMUs aarch32 banks 1 - 4. + */ for (i =3D 0; i < KVM_NR_SPSR; i++) { reg.id =3D AARCH64_CORE_REG(spsr[i]); - reg.addr =3D (uintptr_t) &env->banked_spsr[i - 1]; + reg.addr =3D (uintptr_t) &env->banked_spsr[i+1]; ret =3D kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); if (ret) { return ret; } } =20 + el =3D arm_current_el(env); + if (el > 0) { + if (is_a64(env)) { + g_assert(el =3D=3D 1); + /* KVM_SPSR_SVC holds the AARCH64 EL1 SPSR which QEMU + * keeps in bank 0 so copy it across. */ + env->banked_spsr[0] =3D env->banked_spsr[1]; + i =3D aarch64_banked_spsr_index(el); + } else { + i =3D bank_number(env->uncached_cpsr & CPSR_M); + } + env->spsr =3D env->banked_spsr[i]; + } + /* Advanced SIMD and FP registers */ for (i =3D 0; i < 32; i++) { reg.id =3D AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]); --=20 2.3.2 From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50930) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YXSr8-0007Nz-65 for qemu-devel@nongnu.org; Mon, 16 Mar 2015 07:06:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YXSr1-00079c-Fs for qemu-devel@nongnu.org; Mon, 16 Mar 2015 07:06:46 -0400 Received: from static.88-198-71-155.clients.your-server.de ([88.198.71.155]:33330 helo=socrates.bennee.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YXSr1-00078o-8z for qemu-devel@nongnu.org; Mon, 16 Mar 2015 07:06:39 -0400 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Mon, 16 Mar 2015 11:01:55 +0000 Message-Id: <1426503716-13931-5-git-send-email-alex.bennee@linaro.org> In-Reply-To: <1426503716-13931-1-git-send-email-alex.bennee@linaro.org> References: <1426503716-13931-1-git-send-email-alex.bennee@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: [Qemu-devel] [PATCH v4 4/5] target-arm: kvm64 fix save/restore of SPSR regs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Peter Maydell , kvm@vger.kernel.org, marc.zyngier@arm.com, linux-arm-kernel@lists.infradead.org, =?UTF-8?q?Alex=20Benn=C3=A9e?= , kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org From: Christoffer Dall The current code was negatively indexing the cpu state array and not synchronizing banked spsr register state with the current mode's spsr state, causing occasional failures with migration. Some munging is done to take care of the aarch64 mapping and also to ensure the most current value of the spsr is updated to the banked registers (relevant for KVM<->TCG migration). Signed-off-by: Christoffer Dall Signed-off-by: Alex BennĂ©e --- v2 (ajb) - minor tweaks and clarifications v3 - Use the correct bank index function for setting/getting env->spsr - only deal with spsrs in elevated exception levels v4 - try and make commentary clearer - ensure env->banked_spsr[0] = env->spsr before we sync diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c index 8fd0c8d..7ddb1b1 100644 --- a/target-arm/kvm64.c +++ b/target-arm/kvm64.c @@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level) uint64_t val; int i; int ret; + unsigned int el; ARMCPU *cpu = ARM_CPU(cs); CPUARMState *env = &cpu->env; @@ -206,9 +207,29 @@ int kvm_arch_put_registers(CPUState *cs, int level) return ret; } + /* Saved Program State Registers + * + * Before we restore from the banked_spsr[] array we need to + * ensure that any modifications to env->spsr are correctly + * reflected and map aarch64 exception levels if required. + */ + el = arm_current_el(env); + if (el > 0) { + if (is_a64(env)) { + g_assert(el == 1); + env->banked_spsr[0] = env->spsr; + /* QEMUs AARCH64 EL1 SPSR is in bank 0, so map it to + * KVM_SPSR_SVC for syncing to KVM */ + env->banked_spsr[1] = env->banked_spsr[0]; + } else { + i = bank_number(env->uncached_cpsr & CPSR_M); + env->banked_spsr[i] = env->spsr; + } + } + for (i = 0; i < KVM_NR_SPSR; i++) { reg.id = AARCH64_CORE_REG(spsr[i]); - reg.addr = (uintptr_t) &env->banked_spsr[i - 1]; + reg.addr = (uintptr_t) &env->banked_spsr[i+1]; ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); if (ret) { return ret; @@ -254,6 +275,7 @@ int kvm_arch_get_registers(CPUState *cs) struct kvm_one_reg reg; uint64_t val; uint32_t fpr; + unsigned int el; int i; int ret; @@ -326,15 +348,34 @@ int kvm_arch_get_registers(CPUState *cs) return ret; } + /* Fetch the SPSR registers + * + * KVM has an array of state indexed for all the possible aarch32 + * privilege levels. These map onto QEMUs aarch32 banks 1 - 4. + */ for (i = 0; i < KVM_NR_SPSR; i++) { reg.id = AARCH64_CORE_REG(spsr[i]); - reg.addr = (uintptr_t) &env->banked_spsr[i - 1]; + reg.addr = (uintptr_t) &env->banked_spsr[i+1]; ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); if (ret) { return ret; } } + el = arm_current_el(env); + if (el > 0) { + if (is_a64(env)) { + g_assert(el == 1); + /* KVM_SPSR_SVC holds the AARCH64 EL1 SPSR which QEMU + * keeps in bank 0 so copy it across. */ + env->banked_spsr[0] = env->banked_spsr[1]; + i = aarch64_banked_spsr_index(el); + } else { + i = bank_number(env->uncached_cpsr & CPSR_M); + } + env->spsr = env->banked_spsr[i]; + } + /* Advanced SIMD and FP registers */ for (i = 0; i < 32; i++) { reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]); -- 2.3.2 From mboxrd@z Thu Jan 1 00:00:00 1970 From: alex.bennee@linaro.org (=?UTF-8?q?Alex=20Benn=C3=A9e?=) Date: Mon, 16 Mar 2015 11:01:55 +0000 Subject: [PATCH v4 4/5] target-arm: kvm64 fix save/restore of SPSR regs In-Reply-To: <1426503716-13931-1-git-send-email-alex.bennee@linaro.org> References: <1426503716-13931-1-git-send-email-alex.bennee@linaro.org> Message-ID: <1426503716-13931-5-git-send-email-alex.bennee@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org From: Christoffer Dall The current code was negatively indexing the cpu state array and not synchronizing banked spsr register state with the current mode's spsr state, causing occasional failures with migration. Some munging is done to take care of the aarch64 mapping and also to ensure the most current value of the spsr is updated to the banked registers (relevant for KVM<->TCG migration). Signed-off-by: Christoffer Dall Signed-off-by: Alex Benn?e --- v2 (ajb) - minor tweaks and clarifications v3 - Use the correct bank index function for setting/getting env->spsr - only deal with spsrs in elevated exception levels v4 - try and make commentary clearer - ensure env->banked_spsr[0] = env->spsr before we sync diff --git a/target-arm/kvm64.c b/target-arm/kvm64.c index 8fd0c8d..7ddb1b1 100644 --- a/target-arm/kvm64.c +++ b/target-arm/kvm64.c @@ -140,6 +140,7 @@ int kvm_arch_put_registers(CPUState *cs, int level) uint64_t val; int i; int ret; + unsigned int el; ARMCPU *cpu = ARM_CPU(cs); CPUARMState *env = &cpu->env; @@ -206,9 +207,29 @@ int kvm_arch_put_registers(CPUState *cs, int level) return ret; } + /* Saved Program State Registers + * + * Before we restore from the banked_spsr[] array we need to + * ensure that any modifications to env->spsr are correctly + * reflected and map aarch64 exception levels if required. + */ + el = arm_current_el(env); + if (el > 0) { + if (is_a64(env)) { + g_assert(el == 1); + env->banked_spsr[0] = env->spsr; + /* QEMUs AARCH64 EL1 SPSR is in bank 0, so map it to + * KVM_SPSR_SVC for syncing to KVM */ + env->banked_spsr[1] = env->banked_spsr[0]; + } else { + i = bank_number(env->uncached_cpsr & CPSR_M); + env->banked_spsr[i] = env->spsr; + } + } + for (i = 0; i < KVM_NR_SPSR; i++) { reg.id = AARCH64_CORE_REG(spsr[i]); - reg.addr = (uintptr_t) &env->banked_spsr[i - 1]; + reg.addr = (uintptr_t) &env->banked_spsr[i+1]; ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, ®); if (ret) { return ret; @@ -254,6 +275,7 @@ int kvm_arch_get_registers(CPUState *cs) struct kvm_one_reg reg; uint64_t val; uint32_t fpr; + unsigned int el; int i; int ret; @@ -326,15 +348,34 @@ int kvm_arch_get_registers(CPUState *cs) return ret; } + /* Fetch the SPSR registers + * + * KVM has an array of state indexed for all the possible aarch32 + * privilege levels. These map onto QEMUs aarch32 banks 1 - 4. + */ for (i = 0; i < KVM_NR_SPSR; i++) { reg.id = AARCH64_CORE_REG(spsr[i]); - reg.addr = (uintptr_t) &env->banked_spsr[i - 1]; + reg.addr = (uintptr_t) &env->banked_spsr[i+1]; ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, ®); if (ret) { return ret; } } + el = arm_current_el(env); + if (el > 0) { + if (is_a64(env)) { + g_assert(el == 1); + /* KVM_SPSR_SVC holds the AARCH64 EL1 SPSR which QEMU + * keeps in bank 0 so copy it across. */ + env->banked_spsr[0] = env->banked_spsr[1]; + i = aarch64_banked_spsr_index(el); + } else { + i = bank_number(env->uncached_cpsr & CPSR_M); + } + env->spsr = env->banked_spsr[i]; + } + /* Advanced SIMD and FP registers */ for (i = 0; i < 32; i++) { reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]); -- 2.3.2