* [PATCH v4 01/17] target/ppc: Move helper_regs.h functions out-of-line
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
@ 2021-03-15 18:45 ` Richard Henderson
2021-03-16 8:12 ` Cédric Le Goater
2021-03-22 3:25 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 02/17] target/ppc: Move 601 hflags adjustment to hreg_compute_hflags Richard Henderson
` (16 subsequent siblings)
17 siblings, 2 replies; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:45 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
Move the functions to a new file, helper_regs.c.
Note int_helper.c was relying on helper_regs.h to
indirectly include qemu/log.h.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/helper_regs.h | 184 ++----------------------------------
target/ppc/helper_regs.c | 197 +++++++++++++++++++++++++++++++++++++++
target/ppc/int_helper.c | 1 +
target/ppc/meson.build | 1 +
4 files changed, 207 insertions(+), 176 deletions(-)
create mode 100644 target/ppc/helper_regs.c
diff --git a/target/ppc/helper_regs.h b/target/ppc/helper_regs.h
index efcc903427..4148a442b3 100644
--- a/target/ppc/helper_regs.h
+++ b/target/ppc/helper_regs.h
@@ -20,184 +20,16 @@
#ifndef HELPER_REGS_H
#define HELPER_REGS_H
-#include "qemu/main-loop.h"
-#include "exec/exec-all.h"
-#include "sysemu/kvm.h"
+void hreg_swap_gpr_tgpr(CPUPPCState *env);
+void hreg_compute_mem_idx(CPUPPCState *env);
+void hreg_compute_hflags(CPUPPCState *env);
+void cpu_interrupt_exittb(CPUState *cs);
+int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv);
-/* Swap temporary saved registers with GPRs */
-static inline void hreg_swap_gpr_tgpr(CPUPPCState *env)
-{
- target_ulong tmp;
-
- tmp = env->gpr[0];
- env->gpr[0] = env->tgpr[0];
- env->tgpr[0] = tmp;
- tmp = env->gpr[1];
- env->gpr[1] = env->tgpr[1];
- env->tgpr[1] = tmp;
- tmp = env->gpr[2];
- env->gpr[2] = env->tgpr[2];
- env->tgpr[2] = tmp;
- tmp = env->gpr[3];
- env->gpr[3] = env->tgpr[3];
- env->tgpr[3] = tmp;
-}
-
-static inline void hreg_compute_mem_idx(CPUPPCState *env)
-{
- /*
- * This is our encoding for server processors. The architecture
- * specifies that there is no such thing as userspace with
- * translation off, however it appears that MacOS does it and some
- * 32-bit CPUs support it. Weird...
- *
- * 0 = Guest User space virtual mode
- * 1 = Guest Kernel space virtual mode
- * 2 = Guest User space real mode
- * 3 = Guest Kernel space real mode
- * 4 = HV User space virtual mode
- * 5 = HV Kernel space virtual mode
- * 6 = HV User space real mode
- * 7 = HV Kernel space real mode
- *
- * For BookE, we need 8 MMU modes as follow:
- *
- * 0 = AS 0 HV User space
- * 1 = AS 0 HV Kernel space
- * 2 = AS 1 HV User space
- * 3 = AS 1 HV Kernel space
- * 4 = AS 0 Guest User space
- * 5 = AS 0 Guest Kernel space
- * 6 = AS 1 Guest User space
- * 7 = AS 1 Guest Kernel space
- */
- if (env->mmu_model & POWERPC_MMU_BOOKE) {
- env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
- env->immu_idx += msr_is ? 2 : 0;
- env->dmmu_idx += msr_ds ? 2 : 0;
- env->immu_idx += msr_gs ? 4 : 0;
- env->dmmu_idx += msr_gs ? 4 : 0;
- } else {
- env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
- env->immu_idx += msr_ir ? 0 : 2;
- env->dmmu_idx += msr_dr ? 0 : 2;
- env->immu_idx += msr_hv ? 4 : 0;
- env->dmmu_idx += msr_hv ? 4 : 0;
- }
-}
-
-static inline void hreg_compute_hflags(CPUPPCState *env)
-{
- target_ulong hflags_mask;
-
- /* We 'forget' FE0 & FE1: we'll never generate imprecise exceptions */
- hflags_mask = (1 << MSR_VR) | (1 << MSR_AP) | (1 << MSR_SA) |
- (1 << MSR_PR) | (1 << MSR_FP) | (1 << MSR_SE) | (1 << MSR_BE) |
- (1 << MSR_LE) | (1 << MSR_VSX) | (1 << MSR_IR) | (1 << MSR_DR);
- hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
- hreg_compute_mem_idx(env);
- env->hflags = env->msr & hflags_mask;
- /* Merge with hflags coming from other registers */
- env->hflags |= env->hflags_nmsr;
-}
-
-static inline void cpu_interrupt_exittb(CPUState *cs)
-{
- if (!kvm_enabled()) {
- return;
- }
-
- if (!qemu_mutex_iothread_locked()) {
- qemu_mutex_lock_iothread();
- cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
- qemu_mutex_unlock_iothread();
- } else {
- cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
- }
-}
-
-static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
- int alter_hv)
-{
- int excp;
-#if !defined(CONFIG_USER_ONLY)
- CPUState *cs = env_cpu(env);
-#endif
-
- excp = 0;
- value &= env->msr_mask;
-#if !defined(CONFIG_USER_ONLY)
- /* Neither mtmsr nor guest state can alter HV */
- if (!alter_hv || !(env->msr & MSR_HVB)) {
- value &= ~MSR_HVB;
- value |= env->msr & MSR_HVB;
- }
- if (((value >> MSR_IR) & 1) != msr_ir ||
- ((value >> MSR_DR) & 1) != msr_dr) {
- cpu_interrupt_exittb(cs);
- }
- if ((env->mmu_model & POWERPC_MMU_BOOKE) &&
- ((value >> MSR_GS) & 1) != msr_gs) {
- cpu_interrupt_exittb(cs);
- }
- if (unlikely((env->flags & POWERPC_FLAG_TGPR) &&
- ((value ^ env->msr) & (1 << MSR_TGPR)))) {
- /* Swap temporary saved registers with GPRs */
- hreg_swap_gpr_tgpr(env);
- }
- if (unlikely((value >> MSR_EP) & 1) != msr_ep) {
- /* Change the exception prefix on PowerPC 601 */
- env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000;
- }
- /*
- * If PR=1 then EE, IR and DR must be 1
- *
- * Note: We only enforce this on 64-bit server processors.
- * It appears that:
- * - 32-bit implementations supports PR=1 and EE/DR/IR=0 and MacOS
- * exploits it.
- * - 64-bit embedded implementations do not need any operation to be
- * performed when PR is set.
- */
- if (is_book3s_arch2x(env) && ((value >> MSR_PR) & 1)) {
- value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
- }
-#endif
- env->msr = value;
- hreg_compute_hflags(env);
-#if !defined(CONFIG_USER_ONLY)
- if (unlikely(msr_pow == 1)) {
- if (!env->pending_interrupts && (*env->check_pow)(env)) {
- cs->halted = 1;
- excp = EXCP_HALTED;
- }
- }
-#endif
-
- return excp;
-}
-
-#if !defined(CONFIG_USER_ONLY)
-static inline void check_tlb_flush(CPUPPCState *env, bool global)
-{
- CPUState *cs = env_cpu(env);
-
- /* Handle global flushes first */
- if (global && (env->tlb_need_flush & TLB_NEED_GLOBAL_FLUSH)) {
- env->tlb_need_flush &= ~TLB_NEED_GLOBAL_FLUSH;
- env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
- tlb_flush_all_cpus_synced(cs);
- return;
- }
-
- /* Then handle local ones */
- if (env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) {
- env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
- tlb_flush(cs);
- }
-}
-#else
+#ifdef CONFIG_USER_ONLY
static inline void check_tlb_flush(CPUPPCState *env, bool global) { }
+#else
+void check_tlb_flush(CPUPPCState *env, bool global);
#endif
#endif /* HELPER_REGS_H */
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
new file mode 100644
index 0000000000..5e18232b84
--- /dev/null
+++ b/target/ppc/helper_regs.c
@@ -0,0 +1,197 @@
+/*
+ * PowerPC emulation special registers manipulation helpers for qemu.
+ *
+ * Copyright (c) 2003-2007 Jocelyn Mayer
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
+#include "exec/exec-all.h"
+#include "sysemu/kvm.h"
+#include "helper_regs.h"
+
+/* Swap temporary saved registers with GPRs */
+void hreg_swap_gpr_tgpr(CPUPPCState *env)
+{
+ target_ulong tmp;
+
+ tmp = env->gpr[0];
+ env->gpr[0] = env->tgpr[0];
+ env->tgpr[0] = tmp;
+ tmp = env->gpr[1];
+ env->gpr[1] = env->tgpr[1];
+ env->tgpr[1] = tmp;
+ tmp = env->gpr[2];
+ env->gpr[2] = env->tgpr[2];
+ env->tgpr[2] = tmp;
+ tmp = env->gpr[3];
+ env->gpr[3] = env->tgpr[3];
+ env->tgpr[3] = tmp;
+}
+
+void hreg_compute_mem_idx(CPUPPCState *env)
+{
+ /*
+ * This is our encoding for server processors. The architecture
+ * specifies that there is no such thing as userspace with
+ * translation off, however it appears that MacOS does it and some
+ * 32-bit CPUs support it. Weird...
+ *
+ * 0 = Guest User space virtual mode
+ * 1 = Guest Kernel space virtual mode
+ * 2 = Guest User space real mode
+ * 3 = Guest Kernel space real mode
+ * 4 = HV User space virtual mode
+ * 5 = HV Kernel space virtual mode
+ * 6 = HV User space real mode
+ * 7 = HV Kernel space real mode
+ *
+ * For BookE, we need 8 MMU modes as follow:
+ *
+ * 0 = AS 0 HV User space
+ * 1 = AS 0 HV Kernel space
+ * 2 = AS 1 HV User space
+ * 3 = AS 1 HV Kernel space
+ * 4 = AS 0 Guest User space
+ * 5 = AS 0 Guest Kernel space
+ * 6 = AS 1 Guest User space
+ * 7 = AS 1 Guest Kernel space
+ */
+ if (env->mmu_model & POWERPC_MMU_BOOKE) {
+ env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
+ env->immu_idx += msr_is ? 2 : 0;
+ env->dmmu_idx += msr_ds ? 2 : 0;
+ env->immu_idx += msr_gs ? 4 : 0;
+ env->dmmu_idx += msr_gs ? 4 : 0;
+ } else {
+ env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
+ env->immu_idx += msr_ir ? 0 : 2;
+ env->dmmu_idx += msr_dr ? 0 : 2;
+ env->immu_idx += msr_hv ? 4 : 0;
+ env->dmmu_idx += msr_hv ? 4 : 0;
+ }
+}
+
+void hreg_compute_hflags(CPUPPCState *env)
+{
+ target_ulong hflags_mask;
+
+ /* We 'forget' FE0 & FE1: we'll never generate imprecise exceptions */
+ hflags_mask = (1 << MSR_VR) | (1 << MSR_AP) | (1 << MSR_SA) |
+ (1 << MSR_PR) | (1 << MSR_FP) | (1 << MSR_SE) | (1 << MSR_BE) |
+ (1 << MSR_LE) | (1 << MSR_VSX) | (1 << MSR_IR) | (1 << MSR_DR);
+ hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
+ hreg_compute_mem_idx(env);
+ env->hflags = env->msr & hflags_mask;
+ /* Merge with hflags coming from other registers */
+ env->hflags |= env->hflags_nmsr;
+}
+
+void cpu_interrupt_exittb(CPUState *cs)
+{
+ if (!kvm_enabled()) {
+ return;
+ }
+
+ if (!qemu_mutex_iothread_locked()) {
+ qemu_mutex_lock_iothread();
+ cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
+ qemu_mutex_unlock_iothread();
+ } else {
+ cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
+ }
+}
+
+int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv)
+{
+ int excp;
+#if !defined(CONFIG_USER_ONLY)
+ CPUState *cs = env_cpu(env);
+#endif
+
+ excp = 0;
+ value &= env->msr_mask;
+#if !defined(CONFIG_USER_ONLY)
+ /* Neither mtmsr nor guest state can alter HV */
+ if (!alter_hv || !(env->msr & MSR_HVB)) {
+ value &= ~MSR_HVB;
+ value |= env->msr & MSR_HVB;
+ }
+ if (((value >> MSR_IR) & 1) != msr_ir ||
+ ((value >> MSR_DR) & 1) != msr_dr) {
+ cpu_interrupt_exittb(cs);
+ }
+ if ((env->mmu_model & POWERPC_MMU_BOOKE) &&
+ ((value >> MSR_GS) & 1) != msr_gs) {
+ cpu_interrupt_exittb(cs);
+ }
+ if (unlikely((env->flags & POWERPC_FLAG_TGPR) &&
+ ((value ^ env->msr) & (1 << MSR_TGPR)))) {
+ /* Swap temporary saved registers with GPRs */
+ hreg_swap_gpr_tgpr(env);
+ }
+ if (unlikely((value >> MSR_EP) & 1) != msr_ep) {
+ /* Change the exception prefix on PowerPC 601 */
+ env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000;
+ }
+ /*
+ * If PR=1 then EE, IR and DR must be 1
+ *
+ * Note: We only enforce this on 64-bit server processors.
+ * It appears that:
+ * - 32-bit implementations supports PR=1 and EE/DR/IR=0 and MacOS
+ * exploits it.
+ * - 64-bit embedded implementations do not need any operation to be
+ * performed when PR is set.
+ */
+ if (is_book3s_arch2x(env) && ((value >> MSR_PR) & 1)) {
+ value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
+ }
+#endif
+ env->msr = value;
+ hreg_compute_hflags(env);
+#if !defined(CONFIG_USER_ONLY)
+ if (unlikely(msr_pow == 1)) {
+ if (!env->pending_interrupts && (*env->check_pow)(env)) {
+ cs->halted = 1;
+ excp = EXCP_HALTED;
+ }
+ }
+#endif
+
+ return excp;
+}
+
+#ifndef CONFIG_USER_ONLY
+void check_tlb_flush(CPUPPCState *env, bool global)
+{
+ CPUState *cs = env_cpu(env);
+
+ /* Handle global flushes first */
+ if (global && (env->tlb_need_flush & TLB_NEED_GLOBAL_FLUSH)) {
+ env->tlb_need_flush &= ~TLB_NEED_GLOBAL_FLUSH;
+ env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
+ tlb_flush_all_cpus_synced(cs);
+ return;
+ }
+
+ /* Then handle local ones */
+ if (env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) {
+ env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
+ tlb_flush(cs);
+ }
+}
+#endif
diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index 429de28494..a44c2d90ea 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -22,6 +22,7 @@
#include "internal.h"
#include "qemu/host-utils.h"
#include "qemu/main-loop.h"
+#include "qemu/log.h"
#include "exec/helper-proto.h"
#include "crypto/aes.h"
#include "fpu/softfloat.h"
diff --git a/target/ppc/meson.build b/target/ppc/meson.build
index bbfef90e08..4079d01ee3 100644
--- a/target/ppc/meson.build
+++ b/target/ppc/meson.build
@@ -6,6 +6,7 @@ ppc_ss.add(files(
'excp_helper.c',
'fpu_helper.c',
'gdbstub.c',
+ 'helper_regs.c',
'int_helper.c',
'mem_helper.c',
'misc_helper.c',
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 01/17] target/ppc: Move helper_regs.h functions out-of-line
2021-03-15 18:45 ` [PATCH v4 01/17] target/ppc: Move helper_regs.h functions out-of-line Richard Henderson
@ 2021-03-16 8:12 ` Cédric Le Goater
2021-03-22 3:25 ` David Gibson
1 sibling, 0 replies; 51+ messages in thread
From: Cédric Le Goater @ 2021-03-16 8:12 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc, david
On 3/15/21 7:45 PM, Richard Henderson wrote:
> Move the functions to a new file, helper_regs.c.
>
> Note int_helper.c was relying on helper_regs.h to
> indirectly include qemu/log.h.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> target/ppc/helper_regs.h | 184 ++----------------------------------
> target/ppc/helper_regs.c | 197 +++++++++++++++++++++++++++++++++++++++
> target/ppc/int_helper.c | 1 +
> target/ppc/meson.build | 1 +
> 4 files changed, 207 insertions(+), 176 deletions(-)
> create mode 100644 target/ppc/helper_regs.c
>
> diff --git a/target/ppc/helper_regs.h b/target/ppc/helper_regs.h
> index efcc903427..4148a442b3 100644
> --- a/target/ppc/helper_regs.h
> +++ b/target/ppc/helper_regs.h
> @@ -20,184 +20,16 @@
> #ifndef HELPER_REGS_H
> #define HELPER_REGS_H
>
> -#include "qemu/main-loop.h"
> -#include "exec/exec-all.h"
> -#include "sysemu/kvm.h"
> +void hreg_swap_gpr_tgpr(CPUPPCState *env);
> +void hreg_compute_mem_idx(CPUPPCState *env);
> +void hreg_compute_hflags(CPUPPCState *env);
> +void cpu_interrupt_exittb(CPUState *cs);
> +int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv);
>
> -/* Swap temporary saved registers with GPRs */
> -static inline void hreg_swap_gpr_tgpr(CPUPPCState *env)
> -{
> - target_ulong tmp;
> -
> - tmp = env->gpr[0];
> - env->gpr[0] = env->tgpr[0];
> - env->tgpr[0] = tmp;
> - tmp = env->gpr[1];
> - env->gpr[1] = env->tgpr[1];
> - env->tgpr[1] = tmp;
> - tmp = env->gpr[2];
> - env->gpr[2] = env->tgpr[2];
> - env->tgpr[2] = tmp;
> - tmp = env->gpr[3];
> - env->gpr[3] = env->tgpr[3];
> - env->tgpr[3] = tmp;
> -}
> -
> -static inline void hreg_compute_mem_idx(CPUPPCState *env)
> -{
> - /*
> - * This is our encoding for server processors. The architecture
> - * specifies that there is no such thing as userspace with
> - * translation off, however it appears that MacOS does it and some
> - * 32-bit CPUs support it. Weird...
> - *
> - * 0 = Guest User space virtual mode
> - * 1 = Guest Kernel space virtual mode
> - * 2 = Guest User space real mode
> - * 3 = Guest Kernel space real mode
> - * 4 = HV User space virtual mode
> - * 5 = HV Kernel space virtual mode
> - * 6 = HV User space real mode
> - * 7 = HV Kernel space real mode
> - *
> - * For BookE, we need 8 MMU modes as follow:
> - *
> - * 0 = AS 0 HV User space
> - * 1 = AS 0 HV Kernel space
> - * 2 = AS 1 HV User space
> - * 3 = AS 1 HV Kernel space
> - * 4 = AS 0 Guest User space
> - * 5 = AS 0 Guest Kernel space
> - * 6 = AS 1 Guest User space
> - * 7 = AS 1 Guest Kernel space
> - */
> - if (env->mmu_model & POWERPC_MMU_BOOKE) {
> - env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
> - env->immu_idx += msr_is ? 2 : 0;
> - env->dmmu_idx += msr_ds ? 2 : 0;
> - env->immu_idx += msr_gs ? 4 : 0;
> - env->dmmu_idx += msr_gs ? 4 : 0;
> - } else {
> - env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
> - env->immu_idx += msr_ir ? 0 : 2;
> - env->dmmu_idx += msr_dr ? 0 : 2;
> - env->immu_idx += msr_hv ? 4 : 0;
> - env->dmmu_idx += msr_hv ? 4 : 0;
> - }
> -}
> -
> -static inline void hreg_compute_hflags(CPUPPCState *env)
> -{
> - target_ulong hflags_mask;
> -
> - /* We 'forget' FE0 & FE1: we'll never generate imprecise exceptions */
> - hflags_mask = (1 << MSR_VR) | (1 << MSR_AP) | (1 << MSR_SA) |
> - (1 << MSR_PR) | (1 << MSR_FP) | (1 << MSR_SE) | (1 << MSR_BE) |
> - (1 << MSR_LE) | (1 << MSR_VSX) | (1 << MSR_IR) | (1 << MSR_DR);
> - hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
> - hreg_compute_mem_idx(env);
> - env->hflags = env->msr & hflags_mask;
> - /* Merge with hflags coming from other registers */
> - env->hflags |= env->hflags_nmsr;
> -}
> -
> -static inline void cpu_interrupt_exittb(CPUState *cs)
> -{
> - if (!kvm_enabled()) {
> - return;
> - }
> -
> - if (!qemu_mutex_iothread_locked()) {
> - qemu_mutex_lock_iothread();
> - cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
> - qemu_mutex_unlock_iothread();
> - } else {
> - cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
> - }
> -}
> -
> -static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
> - int alter_hv)
> -{
> - int excp;
> -#if !defined(CONFIG_USER_ONLY)
> - CPUState *cs = env_cpu(env);
> -#endif
> -
> - excp = 0;
> - value &= env->msr_mask;
> -#if !defined(CONFIG_USER_ONLY)
> - /* Neither mtmsr nor guest state can alter HV */
> - if (!alter_hv || !(env->msr & MSR_HVB)) {
> - value &= ~MSR_HVB;
> - value |= env->msr & MSR_HVB;
> - }
> - if (((value >> MSR_IR) & 1) != msr_ir ||
> - ((value >> MSR_DR) & 1) != msr_dr) {
> - cpu_interrupt_exittb(cs);
> - }
> - if ((env->mmu_model & POWERPC_MMU_BOOKE) &&
> - ((value >> MSR_GS) & 1) != msr_gs) {
> - cpu_interrupt_exittb(cs);
> - }
> - if (unlikely((env->flags & POWERPC_FLAG_TGPR) &&
> - ((value ^ env->msr) & (1 << MSR_TGPR)))) {
> - /* Swap temporary saved registers with GPRs */
> - hreg_swap_gpr_tgpr(env);
> - }
> - if (unlikely((value >> MSR_EP) & 1) != msr_ep) {
> - /* Change the exception prefix on PowerPC 601 */
> - env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000;
> - }
> - /*
> - * If PR=1 then EE, IR and DR must be 1
> - *
> - * Note: We only enforce this on 64-bit server processors.
> - * It appears that:
> - * - 32-bit implementations supports PR=1 and EE/DR/IR=0 and MacOS
> - * exploits it.
> - * - 64-bit embedded implementations do not need any operation to be
> - * performed when PR is set.
> - */
> - if (is_book3s_arch2x(env) && ((value >> MSR_PR) & 1)) {
> - value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
> - }
> -#endif
> - env->msr = value;
> - hreg_compute_hflags(env);
> -#if !defined(CONFIG_USER_ONLY)
> - if (unlikely(msr_pow == 1)) {
> - if (!env->pending_interrupts && (*env->check_pow)(env)) {
> - cs->halted = 1;
> - excp = EXCP_HALTED;
> - }
> - }
> -#endif
> -
> - return excp;
> -}
> -
> -#if !defined(CONFIG_USER_ONLY)
> -static inline void check_tlb_flush(CPUPPCState *env, bool global)
> -{
> - CPUState *cs = env_cpu(env);
> -
> - /* Handle global flushes first */
> - if (global && (env->tlb_need_flush & TLB_NEED_GLOBAL_FLUSH)) {
> - env->tlb_need_flush &= ~TLB_NEED_GLOBAL_FLUSH;
> - env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
> - tlb_flush_all_cpus_synced(cs);
> - return;
> - }
> -
> - /* Then handle local ones */
> - if (env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) {
> - env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
> - tlb_flush(cs);
> - }
> -}
> -#else
> +#ifdef CONFIG_USER_ONLY
> static inline void check_tlb_flush(CPUPPCState *env, bool global) { }
> +#else
> +void check_tlb_flush(CPUPPCState *env, bool global);
> #endif
>
> #endif /* HELPER_REGS_H */
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> new file mode 100644
> index 0000000000..5e18232b84
> --- /dev/null
> +++ b/target/ppc/helper_regs.c
> @@ -0,0 +1,197 @@
> +/*
> + * PowerPC emulation special registers manipulation helpers for qemu.
> + *
> + * Copyright (c) 2003-2007 Jocelyn Mayer
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> +#include "exec/exec-all.h"
> +#include "sysemu/kvm.h"
> +#include "helper_regs.h"
> +
> +/* Swap temporary saved registers with GPRs */
> +void hreg_swap_gpr_tgpr(CPUPPCState *env)
> +{
> + target_ulong tmp;
> +
> + tmp = env->gpr[0];
> + env->gpr[0] = env->tgpr[0];
> + env->tgpr[0] = tmp;
> + tmp = env->gpr[1];
> + env->gpr[1] = env->tgpr[1];
> + env->tgpr[1] = tmp;
> + tmp = env->gpr[2];
> + env->gpr[2] = env->tgpr[2];
> + env->tgpr[2] = tmp;
> + tmp = env->gpr[3];
> + env->gpr[3] = env->tgpr[3];
> + env->tgpr[3] = tmp;
> +}
> +
> +void hreg_compute_mem_idx(CPUPPCState *env)
> +{
> + /*
> + * This is our encoding for server processors. The architecture
> + * specifies that there is no such thing as userspace with
> + * translation off, however it appears that MacOS does it and some
> + * 32-bit CPUs support it. Weird...
> + *
> + * 0 = Guest User space virtual mode
> + * 1 = Guest Kernel space virtual mode
> + * 2 = Guest User space real mode
> + * 3 = Guest Kernel space real mode
> + * 4 = HV User space virtual mode
> + * 5 = HV Kernel space virtual mode
> + * 6 = HV User space real mode
> + * 7 = HV Kernel space real mode
> + *
> + * For BookE, we need 8 MMU modes as follow:
> + *
> + * 0 = AS 0 HV User space
> + * 1 = AS 0 HV Kernel space
> + * 2 = AS 1 HV User space
> + * 3 = AS 1 HV Kernel space
> + * 4 = AS 0 Guest User space
> + * 5 = AS 0 Guest Kernel space
> + * 6 = AS 1 Guest User space
> + * 7 = AS 1 Guest Kernel space
> + */
> + if (env->mmu_model & POWERPC_MMU_BOOKE) {
> + env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
> + env->immu_idx += msr_is ? 2 : 0;
> + env->dmmu_idx += msr_ds ? 2 : 0;
> + env->immu_idx += msr_gs ? 4 : 0;
> + env->dmmu_idx += msr_gs ? 4 : 0;
> + } else {
> + env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
> + env->immu_idx += msr_ir ? 0 : 2;
> + env->dmmu_idx += msr_dr ? 0 : 2;
> + env->immu_idx += msr_hv ? 4 : 0;
> + env->dmmu_idx += msr_hv ? 4 : 0;
> + }
> +}
> +
> +void hreg_compute_hflags(CPUPPCState *env)
> +{
> + target_ulong hflags_mask;
> +
> + /* We 'forget' FE0 & FE1: we'll never generate imprecise exceptions */
> + hflags_mask = (1 << MSR_VR) | (1 << MSR_AP) | (1 << MSR_SA) |
> + (1 << MSR_PR) | (1 << MSR_FP) | (1 << MSR_SE) | (1 << MSR_BE) |
> + (1 << MSR_LE) | (1 << MSR_VSX) | (1 << MSR_IR) | (1 << MSR_DR);
> + hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
> + hreg_compute_mem_idx(env);
> + env->hflags = env->msr & hflags_mask;
> + /* Merge with hflags coming from other registers */
> + env->hflags |= env->hflags_nmsr;
> +}
> +
> +void cpu_interrupt_exittb(CPUState *cs)
> +{
> + if (!kvm_enabled()) {
> + return;
> + }
> +
> + if (!qemu_mutex_iothread_locked()) {
> + qemu_mutex_lock_iothread();
> + cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
> + qemu_mutex_unlock_iothread();
> + } else {
> + cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
> + }
> +}
> +
> +int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv)
> +{
> + int excp;
> +#if !defined(CONFIG_USER_ONLY)
> + CPUState *cs = env_cpu(env);
> +#endif
> +
> + excp = 0;
> + value &= env->msr_mask;
> +#if !defined(CONFIG_USER_ONLY)
> + /* Neither mtmsr nor guest state can alter HV */
> + if (!alter_hv || !(env->msr & MSR_HVB)) {
> + value &= ~MSR_HVB;
> + value |= env->msr & MSR_HVB;
> + }
> + if (((value >> MSR_IR) & 1) != msr_ir ||
> + ((value >> MSR_DR) & 1) != msr_dr) {
> + cpu_interrupt_exittb(cs);
> + }
> + if ((env->mmu_model & POWERPC_MMU_BOOKE) &&
> + ((value >> MSR_GS) & 1) != msr_gs) {
> + cpu_interrupt_exittb(cs);
> + }
> + if (unlikely((env->flags & POWERPC_FLAG_TGPR) &&
> + ((value ^ env->msr) & (1 << MSR_TGPR)))) {
> + /* Swap temporary saved registers with GPRs */
> + hreg_swap_gpr_tgpr(env);
> + }
> + if (unlikely((value >> MSR_EP) & 1) != msr_ep) {
> + /* Change the exception prefix on PowerPC 601 */
> + env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000;
> + }
> + /*
> + * If PR=1 then EE, IR and DR must be 1
> + *
> + * Note: We only enforce this on 64-bit server processors.
> + * It appears that:
> + * - 32-bit implementations supports PR=1 and EE/DR/IR=0 and MacOS
> + * exploits it.
> + * - 64-bit embedded implementations do not need any operation to be
> + * performed when PR is set.
> + */
> + if (is_book3s_arch2x(env) && ((value >> MSR_PR) & 1)) {
> + value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
> + }
> +#endif
> + env->msr = value;
> + hreg_compute_hflags(env);
> +#if !defined(CONFIG_USER_ONLY)
> + if (unlikely(msr_pow == 1)) {
> + if (!env->pending_interrupts && (*env->check_pow)(env)) {
> + cs->halted = 1;
> + excp = EXCP_HALTED;
> + }
> + }
> +#endif
> +
> + return excp;
> +}
> +
> +#ifndef CONFIG_USER_ONLY
> +void check_tlb_flush(CPUPPCState *env, bool global)
> +{
> + CPUState *cs = env_cpu(env);
> +
> + /* Handle global flushes first */
> + if (global && (env->tlb_need_flush & TLB_NEED_GLOBAL_FLUSH)) {
> + env->tlb_need_flush &= ~TLB_NEED_GLOBAL_FLUSH;
> + env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
> + tlb_flush_all_cpus_synced(cs);
> + return;
> + }
> +
> + /* Then handle local ones */
> + if (env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) {
> + env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
> + tlb_flush(cs);
> + }
> +}
> +#endif
> diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
> index 429de28494..a44c2d90ea 100644
> --- a/target/ppc/int_helper.c
> +++ b/target/ppc/int_helper.c
> @@ -22,6 +22,7 @@
> #include "internal.h"
> #include "qemu/host-utils.h"
> #include "qemu/main-loop.h"
> +#include "qemu/log.h"
> #include "exec/helper-proto.h"
> #include "crypto/aes.h"
> #include "fpu/softfloat.h"
> diff --git a/target/ppc/meson.build b/target/ppc/meson.build
> index bbfef90e08..4079d01ee3 100644
> --- a/target/ppc/meson.build
> +++ b/target/ppc/meson.build
> @@ -6,6 +6,7 @@ ppc_ss.add(files(
> 'excp_helper.c',
> 'fpu_helper.c',
> 'gdbstub.c',
> + 'helper_regs.c',
> 'int_helper.c',
> 'mem_helper.c',
> 'misc_helper.c',
>
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v4 01/17] target/ppc: Move helper_regs.h functions out-of-line
2021-03-15 18:45 ` [PATCH v4 01/17] target/ppc: Move helper_regs.h functions out-of-line Richard Henderson
2021-03-16 8:12 ` Cédric Le Goater
@ 2021-03-22 3:25 ` David Gibson
1 sibling, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 3:25 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 15628 bytes --]
On Mon, Mar 15, 2021 at 12:45:59PM -0600, Richard Henderson wrote:
> Move the functions to a new file, helper_regs.c.
>
> Note int_helper.c was relying on helper_regs.h to
> indirectly include qemu/log.h.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Applied to ppc-for-6.0, thanks.
> ---
> target/ppc/helper_regs.h | 184 ++----------------------------------
> target/ppc/helper_regs.c | 197 +++++++++++++++++++++++++++++++++++++++
> target/ppc/int_helper.c | 1 +
> target/ppc/meson.build | 1 +
> 4 files changed, 207 insertions(+), 176 deletions(-)
> create mode 100644 target/ppc/helper_regs.c
>
> diff --git a/target/ppc/helper_regs.h b/target/ppc/helper_regs.h
> index efcc903427..4148a442b3 100644
> --- a/target/ppc/helper_regs.h
> +++ b/target/ppc/helper_regs.h
> @@ -20,184 +20,16 @@
> #ifndef HELPER_REGS_H
> #define HELPER_REGS_H
>
> -#include "qemu/main-loop.h"
> -#include "exec/exec-all.h"
> -#include "sysemu/kvm.h"
> +void hreg_swap_gpr_tgpr(CPUPPCState *env);
> +void hreg_compute_mem_idx(CPUPPCState *env);
> +void hreg_compute_hflags(CPUPPCState *env);
> +void cpu_interrupt_exittb(CPUState *cs);
> +int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv);
>
> -/* Swap temporary saved registers with GPRs */
> -static inline void hreg_swap_gpr_tgpr(CPUPPCState *env)
> -{
> - target_ulong tmp;
> -
> - tmp = env->gpr[0];
> - env->gpr[0] = env->tgpr[0];
> - env->tgpr[0] = tmp;
> - tmp = env->gpr[1];
> - env->gpr[1] = env->tgpr[1];
> - env->tgpr[1] = tmp;
> - tmp = env->gpr[2];
> - env->gpr[2] = env->tgpr[2];
> - env->tgpr[2] = tmp;
> - tmp = env->gpr[3];
> - env->gpr[3] = env->tgpr[3];
> - env->tgpr[3] = tmp;
> -}
> -
> -static inline void hreg_compute_mem_idx(CPUPPCState *env)
> -{
> - /*
> - * This is our encoding for server processors. The architecture
> - * specifies that there is no such thing as userspace with
> - * translation off, however it appears that MacOS does it and some
> - * 32-bit CPUs support it. Weird...
> - *
> - * 0 = Guest User space virtual mode
> - * 1 = Guest Kernel space virtual mode
> - * 2 = Guest User space real mode
> - * 3 = Guest Kernel space real mode
> - * 4 = HV User space virtual mode
> - * 5 = HV Kernel space virtual mode
> - * 6 = HV User space real mode
> - * 7 = HV Kernel space real mode
> - *
> - * For BookE, we need 8 MMU modes as follow:
> - *
> - * 0 = AS 0 HV User space
> - * 1 = AS 0 HV Kernel space
> - * 2 = AS 1 HV User space
> - * 3 = AS 1 HV Kernel space
> - * 4 = AS 0 Guest User space
> - * 5 = AS 0 Guest Kernel space
> - * 6 = AS 1 Guest User space
> - * 7 = AS 1 Guest Kernel space
> - */
> - if (env->mmu_model & POWERPC_MMU_BOOKE) {
> - env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
> - env->immu_idx += msr_is ? 2 : 0;
> - env->dmmu_idx += msr_ds ? 2 : 0;
> - env->immu_idx += msr_gs ? 4 : 0;
> - env->dmmu_idx += msr_gs ? 4 : 0;
> - } else {
> - env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
> - env->immu_idx += msr_ir ? 0 : 2;
> - env->dmmu_idx += msr_dr ? 0 : 2;
> - env->immu_idx += msr_hv ? 4 : 0;
> - env->dmmu_idx += msr_hv ? 4 : 0;
> - }
> -}
> -
> -static inline void hreg_compute_hflags(CPUPPCState *env)
> -{
> - target_ulong hflags_mask;
> -
> - /* We 'forget' FE0 & FE1: we'll never generate imprecise exceptions */
> - hflags_mask = (1 << MSR_VR) | (1 << MSR_AP) | (1 << MSR_SA) |
> - (1 << MSR_PR) | (1 << MSR_FP) | (1 << MSR_SE) | (1 << MSR_BE) |
> - (1 << MSR_LE) | (1 << MSR_VSX) | (1 << MSR_IR) | (1 << MSR_DR);
> - hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
> - hreg_compute_mem_idx(env);
> - env->hflags = env->msr & hflags_mask;
> - /* Merge with hflags coming from other registers */
> - env->hflags |= env->hflags_nmsr;
> -}
> -
> -static inline void cpu_interrupt_exittb(CPUState *cs)
> -{
> - if (!kvm_enabled()) {
> - return;
> - }
> -
> - if (!qemu_mutex_iothread_locked()) {
> - qemu_mutex_lock_iothread();
> - cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
> - qemu_mutex_unlock_iothread();
> - } else {
> - cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
> - }
> -}
> -
> -static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
> - int alter_hv)
> -{
> - int excp;
> -#if !defined(CONFIG_USER_ONLY)
> - CPUState *cs = env_cpu(env);
> -#endif
> -
> - excp = 0;
> - value &= env->msr_mask;
> -#if !defined(CONFIG_USER_ONLY)
> - /* Neither mtmsr nor guest state can alter HV */
> - if (!alter_hv || !(env->msr & MSR_HVB)) {
> - value &= ~MSR_HVB;
> - value |= env->msr & MSR_HVB;
> - }
> - if (((value >> MSR_IR) & 1) != msr_ir ||
> - ((value >> MSR_DR) & 1) != msr_dr) {
> - cpu_interrupt_exittb(cs);
> - }
> - if ((env->mmu_model & POWERPC_MMU_BOOKE) &&
> - ((value >> MSR_GS) & 1) != msr_gs) {
> - cpu_interrupt_exittb(cs);
> - }
> - if (unlikely((env->flags & POWERPC_FLAG_TGPR) &&
> - ((value ^ env->msr) & (1 << MSR_TGPR)))) {
> - /* Swap temporary saved registers with GPRs */
> - hreg_swap_gpr_tgpr(env);
> - }
> - if (unlikely((value >> MSR_EP) & 1) != msr_ep) {
> - /* Change the exception prefix on PowerPC 601 */
> - env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000;
> - }
> - /*
> - * If PR=1 then EE, IR and DR must be 1
> - *
> - * Note: We only enforce this on 64-bit server processors.
> - * It appears that:
> - * - 32-bit implementations supports PR=1 and EE/DR/IR=0 and MacOS
> - * exploits it.
> - * - 64-bit embedded implementations do not need any operation to be
> - * performed when PR is set.
> - */
> - if (is_book3s_arch2x(env) && ((value >> MSR_PR) & 1)) {
> - value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
> - }
> -#endif
> - env->msr = value;
> - hreg_compute_hflags(env);
> -#if !defined(CONFIG_USER_ONLY)
> - if (unlikely(msr_pow == 1)) {
> - if (!env->pending_interrupts && (*env->check_pow)(env)) {
> - cs->halted = 1;
> - excp = EXCP_HALTED;
> - }
> - }
> -#endif
> -
> - return excp;
> -}
> -
> -#if !defined(CONFIG_USER_ONLY)
> -static inline void check_tlb_flush(CPUPPCState *env, bool global)
> -{
> - CPUState *cs = env_cpu(env);
> -
> - /* Handle global flushes first */
> - if (global && (env->tlb_need_flush & TLB_NEED_GLOBAL_FLUSH)) {
> - env->tlb_need_flush &= ~TLB_NEED_GLOBAL_FLUSH;
> - env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
> - tlb_flush_all_cpus_synced(cs);
> - return;
> - }
> -
> - /* Then handle local ones */
> - if (env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) {
> - env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
> - tlb_flush(cs);
> - }
> -}
> -#else
> +#ifdef CONFIG_USER_ONLY
> static inline void check_tlb_flush(CPUPPCState *env, bool global) { }
> +#else
> +void check_tlb_flush(CPUPPCState *env, bool global);
> #endif
>
> #endif /* HELPER_REGS_H */
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> new file mode 100644
> index 0000000000..5e18232b84
> --- /dev/null
> +++ b/target/ppc/helper_regs.c
> @@ -0,0 +1,197 @@
> +/*
> + * PowerPC emulation special registers manipulation helpers for qemu.
> + *
> + * Copyright (c) 2003-2007 Jocelyn Mayer
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> +#include "exec/exec-all.h"
> +#include "sysemu/kvm.h"
> +#include "helper_regs.h"
> +
> +/* Swap temporary saved registers with GPRs */
> +void hreg_swap_gpr_tgpr(CPUPPCState *env)
> +{
> + target_ulong tmp;
> +
> + tmp = env->gpr[0];
> + env->gpr[0] = env->tgpr[0];
> + env->tgpr[0] = tmp;
> + tmp = env->gpr[1];
> + env->gpr[1] = env->tgpr[1];
> + env->tgpr[1] = tmp;
> + tmp = env->gpr[2];
> + env->gpr[2] = env->tgpr[2];
> + env->tgpr[2] = tmp;
> + tmp = env->gpr[3];
> + env->gpr[3] = env->tgpr[3];
> + env->tgpr[3] = tmp;
> +}
> +
> +void hreg_compute_mem_idx(CPUPPCState *env)
> +{
> + /*
> + * This is our encoding for server processors. The architecture
> + * specifies that there is no such thing as userspace with
> + * translation off, however it appears that MacOS does it and some
> + * 32-bit CPUs support it. Weird...
> + *
> + * 0 = Guest User space virtual mode
> + * 1 = Guest Kernel space virtual mode
> + * 2 = Guest User space real mode
> + * 3 = Guest Kernel space real mode
> + * 4 = HV User space virtual mode
> + * 5 = HV Kernel space virtual mode
> + * 6 = HV User space real mode
> + * 7 = HV Kernel space real mode
> + *
> + * For BookE, we need 8 MMU modes as follow:
> + *
> + * 0 = AS 0 HV User space
> + * 1 = AS 0 HV Kernel space
> + * 2 = AS 1 HV User space
> + * 3 = AS 1 HV Kernel space
> + * 4 = AS 0 Guest User space
> + * 5 = AS 0 Guest Kernel space
> + * 6 = AS 1 Guest User space
> + * 7 = AS 1 Guest Kernel space
> + */
> + if (env->mmu_model & POWERPC_MMU_BOOKE) {
> + env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
> + env->immu_idx += msr_is ? 2 : 0;
> + env->dmmu_idx += msr_ds ? 2 : 0;
> + env->immu_idx += msr_gs ? 4 : 0;
> + env->dmmu_idx += msr_gs ? 4 : 0;
> + } else {
> + env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
> + env->immu_idx += msr_ir ? 0 : 2;
> + env->dmmu_idx += msr_dr ? 0 : 2;
> + env->immu_idx += msr_hv ? 4 : 0;
> + env->dmmu_idx += msr_hv ? 4 : 0;
> + }
> +}
> +
> +void hreg_compute_hflags(CPUPPCState *env)
> +{
> + target_ulong hflags_mask;
> +
> + /* We 'forget' FE0 & FE1: we'll never generate imprecise exceptions */
> + hflags_mask = (1 << MSR_VR) | (1 << MSR_AP) | (1 << MSR_SA) |
> + (1 << MSR_PR) | (1 << MSR_FP) | (1 << MSR_SE) | (1 << MSR_BE) |
> + (1 << MSR_LE) | (1 << MSR_VSX) | (1 << MSR_IR) | (1 << MSR_DR);
> + hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
> + hreg_compute_mem_idx(env);
> + env->hflags = env->msr & hflags_mask;
> + /* Merge with hflags coming from other registers */
> + env->hflags |= env->hflags_nmsr;
> +}
> +
> +void cpu_interrupt_exittb(CPUState *cs)
> +{
> + if (!kvm_enabled()) {
> + return;
> + }
> +
> + if (!qemu_mutex_iothread_locked()) {
> + qemu_mutex_lock_iothread();
> + cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
> + qemu_mutex_unlock_iothread();
> + } else {
> + cpu_interrupt(cs, CPU_INTERRUPT_EXITTB);
> + }
> +}
> +
> +int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv)
> +{
> + int excp;
> +#if !defined(CONFIG_USER_ONLY)
> + CPUState *cs = env_cpu(env);
> +#endif
> +
> + excp = 0;
> + value &= env->msr_mask;
> +#if !defined(CONFIG_USER_ONLY)
> + /* Neither mtmsr nor guest state can alter HV */
> + if (!alter_hv || !(env->msr & MSR_HVB)) {
> + value &= ~MSR_HVB;
> + value |= env->msr & MSR_HVB;
> + }
> + if (((value >> MSR_IR) & 1) != msr_ir ||
> + ((value >> MSR_DR) & 1) != msr_dr) {
> + cpu_interrupt_exittb(cs);
> + }
> + if ((env->mmu_model & POWERPC_MMU_BOOKE) &&
> + ((value >> MSR_GS) & 1) != msr_gs) {
> + cpu_interrupt_exittb(cs);
> + }
> + if (unlikely((env->flags & POWERPC_FLAG_TGPR) &&
> + ((value ^ env->msr) & (1 << MSR_TGPR)))) {
> + /* Swap temporary saved registers with GPRs */
> + hreg_swap_gpr_tgpr(env);
> + }
> + if (unlikely((value >> MSR_EP) & 1) != msr_ep) {
> + /* Change the exception prefix on PowerPC 601 */
> + env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF00000;
> + }
> + /*
> + * If PR=1 then EE, IR and DR must be 1
> + *
> + * Note: We only enforce this on 64-bit server processors.
> + * It appears that:
> + * - 32-bit implementations supports PR=1 and EE/DR/IR=0 and MacOS
> + * exploits it.
> + * - 64-bit embedded implementations do not need any operation to be
> + * performed when PR is set.
> + */
> + if (is_book3s_arch2x(env) && ((value >> MSR_PR) & 1)) {
> + value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
> + }
> +#endif
> + env->msr = value;
> + hreg_compute_hflags(env);
> +#if !defined(CONFIG_USER_ONLY)
> + if (unlikely(msr_pow == 1)) {
> + if (!env->pending_interrupts && (*env->check_pow)(env)) {
> + cs->halted = 1;
> + excp = EXCP_HALTED;
> + }
> + }
> +#endif
> +
> + return excp;
> +}
> +
> +#ifndef CONFIG_USER_ONLY
> +void check_tlb_flush(CPUPPCState *env, bool global)
> +{
> + CPUState *cs = env_cpu(env);
> +
> + /* Handle global flushes first */
> + if (global && (env->tlb_need_flush & TLB_NEED_GLOBAL_FLUSH)) {
> + env->tlb_need_flush &= ~TLB_NEED_GLOBAL_FLUSH;
> + env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
> + tlb_flush_all_cpus_synced(cs);
> + return;
> + }
> +
> + /* Then handle local ones */
> + if (env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) {
> + env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
> + tlb_flush(cs);
> + }
> +}
> +#endif
> diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
> index 429de28494..a44c2d90ea 100644
> --- a/target/ppc/int_helper.c
> +++ b/target/ppc/int_helper.c
> @@ -22,6 +22,7 @@
> #include "internal.h"
> #include "qemu/host-utils.h"
> #include "qemu/main-loop.h"
> +#include "qemu/log.h"
> #include "exec/helper-proto.h"
> #include "crypto/aes.h"
> #include "fpu/softfloat.h"
> diff --git a/target/ppc/meson.build b/target/ppc/meson.build
> index bbfef90e08..4079d01ee3 100644
> --- a/target/ppc/meson.build
> +++ b/target/ppc/meson.build
> @@ -6,6 +6,7 @@ ppc_ss.add(files(
> 'excp_helper.c',
> 'fpu_helper.c',
> 'gdbstub.c',
> + 'helper_regs.c',
> 'int_helper.c',
> 'mem_helper.c',
> 'misc_helper.c',
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 02/17] target/ppc: Move 601 hflags adjustment to hreg_compute_hflags
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
2021-03-15 18:45 ` [PATCH v4 01/17] target/ppc: Move helper_regs.h functions out-of-line Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-16 8:12 ` Cédric Le Goater
2021-03-22 3:35 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 03/17] target/ppc: Properly sync cpu state with new msr in cpu_load_old Richard Henderson
` (15 subsequent siblings)
17 siblings, 2 replies; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
Keep all hflags computation in one place, as this will be
especially important later.
Introduce a new POWERPC_FLAG_HID0_LE bit to indicate when
LE should be taken from HID0. This appears to be set if
and only if POWERPC_FLAG_RTC_CLK is set, but we're not
short of bits and having both names will avoid confusion.
Note that this was the only user of hflags_nmsr, so we can
perform a straight assignment rather than mask and set.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/cpu.h | 2 ++
target/ppc/helper_regs.c | 13 +++++++++++--
target/ppc/misc_helper.c | 8 +++-----
target/ppc/translate_init.c.inc | 4 ++--
4 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index e73416da68..061d2eed1b 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -581,6 +581,8 @@ enum {
POWERPC_FLAG_TM = 0x00100000,
/* Has SCV (ISA 3.00) */
POWERPC_FLAG_SCV = 0x00200000,
+ /* Has HID0 for LE bit (601) */
+ POWERPC_FLAG_HID0_LE = 0x00400000,
};
/*****************************************************************************/
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 5e18232b84..95b9aca61f 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -96,8 +96,17 @@ void hreg_compute_hflags(CPUPPCState *env)
hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
hreg_compute_mem_idx(env);
env->hflags = env->msr & hflags_mask;
- /* Merge with hflags coming from other registers */
- env->hflags |= env->hflags_nmsr;
+
+ if (env->flags & POWERPC_FLAG_HID0_LE) {
+ /*
+ * Note that MSR_LE is not set in env->msr_mask for this cpu,
+ * and so will never be set in msr or hflags at this point.
+ */
+ uint32_t le = extract32(env->spr[SPR_HID0], 3, 1);
+ env->hflags |= le << MSR_LE;
+ /* Retain for backward compatibility with migration. */
+ env->hflags_nmsr = le << MSR_LE;
+ }
}
void cpu_interrupt_exittb(CPUState *cs)
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index 5d6e0de396..63e3147eb4 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -194,16 +194,14 @@ void helper_store_hid0_601(CPUPPCState *env, target_ulong val)
target_ulong hid0;
hid0 = env->spr[SPR_HID0];
+ env->spr[SPR_HID0] = (uint32_t)val;
+
if ((val ^ hid0) & 0x00000008) {
/* Change current endianness */
- env->hflags &= ~(1 << MSR_LE);
- env->hflags_nmsr &= ~(1 << MSR_LE);
- env->hflags_nmsr |= (1 << MSR_LE) & (((val >> 3) & 1) << MSR_LE);
- env->hflags |= env->hflags_nmsr;
+ hreg_compute_hflags(env);
qemu_log("%s: set endianness to %c => " TARGET_FMT_lx "\n", __func__,
val & 0x8 ? 'l' : 'b', env->hflags);
}
- env->spr[SPR_HID0] = (uint32_t)val;
}
void helper_store_403_pbr(CPUPPCState *env, uint32_t num, target_ulong value)
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index c03a7c4f52..049d76cfd1 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -5441,7 +5441,7 @@ POWERPC_FAMILY(601)(ObjectClass *oc, void *data)
pcc->excp_model = POWERPC_EXCP_601;
pcc->bus_model = PPC_FLAGS_INPUT_6xx;
pcc->bfd_mach = bfd_mach_ppc_601;
- pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK;
+ pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK | POWERPC_FLAG_HID0_LE;
}
#define POWERPC_MSRR_601v (0x0000000000001040ULL)
@@ -5485,7 +5485,7 @@ POWERPC_FAMILY(601v)(ObjectClass *oc, void *data)
#endif
pcc->bus_model = PPC_FLAGS_INPUT_6xx;
pcc->bfd_mach = bfd_mach_ppc_601;
- pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK;
+ pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK | POWERPC_FLAG_HID0_LE;
}
static void init_proc_602(CPUPPCState *env)
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 02/17] target/ppc: Move 601 hflags adjustment to hreg_compute_hflags
2021-03-15 18:46 ` [PATCH v4 02/17] target/ppc: Move 601 hflags adjustment to hreg_compute_hflags Richard Henderson
@ 2021-03-16 8:12 ` Cédric Le Goater
2021-03-22 3:35 ` David Gibson
1 sibling, 0 replies; 51+ messages in thread
From: Cédric Le Goater @ 2021-03-16 8:12 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc, david
On 3/15/21 7:46 PM, Richard Henderson wrote:
> Keep all hflags computation in one place, as this will be
> especially important later.
>
> Introduce a new POWERPC_FLAG_HID0_LE bit to indicate when
> LE should be taken from HID0. This appears to be set if
> and only if POWERPC_FLAG_RTC_CLK is set, but we're not
> short of bits and having both names will avoid confusion.
>
> Note that this was the only user of hflags_nmsr, so we can
> perform a straight assignment rather than mask and set.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> target/ppc/cpu.h | 2 ++
> target/ppc/helper_regs.c | 13 +++++++++++--
> target/ppc/misc_helper.c | 8 +++-----
> target/ppc/translate_init.c.inc | 4 ++--
> 4 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index e73416da68..061d2eed1b 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -581,6 +581,8 @@ enum {
> POWERPC_FLAG_TM = 0x00100000,
> /* Has SCV (ISA 3.00) */
> POWERPC_FLAG_SCV = 0x00200000,
> + /* Has HID0 for LE bit (601) */
> + POWERPC_FLAG_HID0_LE = 0x00400000,
> };
>
> /*****************************************************************************/
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index 5e18232b84..95b9aca61f 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -96,8 +96,17 @@ void hreg_compute_hflags(CPUPPCState *env)
> hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
> hreg_compute_mem_idx(env);
> env->hflags = env->msr & hflags_mask;
> - /* Merge with hflags coming from other registers */
> - env->hflags |= env->hflags_nmsr;
> +
> + if (env->flags & POWERPC_FLAG_HID0_LE) {
> + /*
> + * Note that MSR_LE is not set in env->msr_mask for this cpu,
> + * and so will never be set in msr or hflags at this point.
> + */
> + uint32_t le = extract32(env->spr[SPR_HID0], 3, 1);
> + env->hflags |= le << MSR_LE;
> + /* Retain for backward compatibility with migration. */
> + env->hflags_nmsr = le << MSR_LE;
> + }
> }
>
> void cpu_interrupt_exittb(CPUState *cs)
> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
> index 5d6e0de396..63e3147eb4 100644
> --- a/target/ppc/misc_helper.c
> +++ b/target/ppc/misc_helper.c
> @@ -194,16 +194,14 @@ void helper_store_hid0_601(CPUPPCState *env, target_ulong val)
> target_ulong hid0;
>
> hid0 = env->spr[SPR_HID0];
> + env->spr[SPR_HID0] = (uint32_t)val;
> +
> if ((val ^ hid0) & 0x00000008) {
> /* Change current endianness */
> - env->hflags &= ~(1 << MSR_LE);
> - env->hflags_nmsr &= ~(1 << MSR_LE);
> - env->hflags_nmsr |= (1 << MSR_LE) & (((val >> 3) & 1) << MSR_LE);
> - env->hflags |= env->hflags_nmsr;
> + hreg_compute_hflags(env);
> qemu_log("%s: set endianness to %c => " TARGET_FMT_lx "\n", __func__,
> val & 0x8 ? 'l' : 'b', env->hflags);
> }
> - env->spr[SPR_HID0] = (uint32_t)val;
> }
>
> void helper_store_403_pbr(CPUPPCState *env, uint32_t num, target_ulong value)
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index c03a7c4f52..049d76cfd1 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -5441,7 +5441,7 @@ POWERPC_FAMILY(601)(ObjectClass *oc, void *data)
> pcc->excp_model = POWERPC_EXCP_601;
> pcc->bus_model = PPC_FLAGS_INPUT_6xx;
> pcc->bfd_mach = bfd_mach_ppc_601;
> - pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK;
> + pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK | POWERPC_FLAG_HID0_LE;
> }
>
> #define POWERPC_MSRR_601v (0x0000000000001040ULL)
> @@ -5485,7 +5485,7 @@ POWERPC_FAMILY(601v)(ObjectClass *oc, void *data)
> #endif
> pcc->bus_model = PPC_FLAGS_INPUT_6xx;
> pcc->bfd_mach = bfd_mach_ppc_601;
> - pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK;
> + pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK | POWERPC_FLAG_HID0_LE;
> }
>
> static void init_proc_602(CPUPPCState *env)
>
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v4 02/17] target/ppc: Move 601 hflags adjustment to hreg_compute_hflags
2021-03-15 18:46 ` [PATCH v4 02/17] target/ppc: Move 601 hflags adjustment to hreg_compute_hflags Richard Henderson
2021-03-16 8:12 ` Cédric Le Goater
@ 2021-03-22 3:35 ` David Gibson
1 sibling, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 3:35 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4670 bytes --]
On Mon, Mar 15, 2021 at 12:46:00PM -0600, Richard Henderson wrote:
> Keep all hflags computation in one place, as this will be
> especially important later.
>
> Introduce a new POWERPC_FLAG_HID0_LE bit to indicate when
> LE should be taken from HID0. This appears to be set if
> and only if POWERPC_FLAG_RTC_CLK is set, but we're not
> short of bits and having both names will avoid confusion.
>
> Note that this was the only user of hflags_nmsr, so we can
> perform a straight assignment rather than mask and set.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Applied to ppc-for-6.0, thanks.
> ---
> target/ppc/cpu.h | 2 ++
> target/ppc/helper_regs.c | 13 +++++++++++--
> target/ppc/misc_helper.c | 8 +++-----
> target/ppc/translate_init.c.inc | 4 ++--
> 4 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index e73416da68..061d2eed1b 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -581,6 +581,8 @@ enum {
> POWERPC_FLAG_TM = 0x00100000,
> /* Has SCV (ISA 3.00) */
> POWERPC_FLAG_SCV = 0x00200000,
> + /* Has HID0 for LE bit (601) */
> + POWERPC_FLAG_HID0_LE = 0x00400000,
> };
>
> /*****************************************************************************/
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index 5e18232b84..95b9aca61f 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -96,8 +96,17 @@ void hreg_compute_hflags(CPUPPCState *env)
> hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
> hreg_compute_mem_idx(env);
> env->hflags = env->msr & hflags_mask;
> - /* Merge with hflags coming from other registers */
> - env->hflags |= env->hflags_nmsr;
> +
> + if (env->flags & POWERPC_FLAG_HID0_LE) {
> + /*
> + * Note that MSR_LE is not set in env->msr_mask for this cpu,
> + * and so will never be set in msr or hflags at this point.
> + */
> + uint32_t le = extract32(env->spr[SPR_HID0], 3, 1);
> + env->hflags |= le << MSR_LE;
> + /* Retain for backward compatibility with migration. */
> + env->hflags_nmsr = le << MSR_LE;
> + }
> }
>
> void cpu_interrupt_exittb(CPUState *cs)
> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
> index 5d6e0de396..63e3147eb4 100644
> --- a/target/ppc/misc_helper.c
> +++ b/target/ppc/misc_helper.c
> @@ -194,16 +194,14 @@ void helper_store_hid0_601(CPUPPCState *env, target_ulong val)
> target_ulong hid0;
>
> hid0 = env->spr[SPR_HID0];
> + env->spr[SPR_HID0] = (uint32_t)val;
> +
> if ((val ^ hid0) & 0x00000008) {
> /* Change current endianness */
> - env->hflags &= ~(1 << MSR_LE);
> - env->hflags_nmsr &= ~(1 << MSR_LE);
> - env->hflags_nmsr |= (1 << MSR_LE) & (((val >> 3) & 1) << MSR_LE);
> - env->hflags |= env->hflags_nmsr;
> + hreg_compute_hflags(env);
> qemu_log("%s: set endianness to %c => " TARGET_FMT_lx "\n", __func__,
> val & 0x8 ? 'l' : 'b', env->hflags);
> }
> - env->spr[SPR_HID0] = (uint32_t)val;
> }
>
> void helper_store_403_pbr(CPUPPCState *env, uint32_t num, target_ulong value)
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index c03a7c4f52..049d76cfd1 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -5441,7 +5441,7 @@ POWERPC_FAMILY(601)(ObjectClass *oc, void *data)
> pcc->excp_model = POWERPC_EXCP_601;
> pcc->bus_model = PPC_FLAGS_INPUT_6xx;
> pcc->bfd_mach = bfd_mach_ppc_601;
> - pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK;
> + pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK | POWERPC_FLAG_HID0_LE;
> }
>
> #define POWERPC_MSRR_601v (0x0000000000001040ULL)
> @@ -5485,7 +5485,7 @@ POWERPC_FAMILY(601v)(ObjectClass *oc, void *data)
> #endif
> pcc->bus_model = PPC_FLAGS_INPUT_6xx;
> pcc->bfd_mach = bfd_mach_ppc_601;
> - pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK;
> + pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK | POWERPC_FLAG_HID0_LE;
> }
>
> static void init_proc_602(CPUPPCState *env)
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 03/17] target/ppc: Properly sync cpu state with new msr in cpu_load_old
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
2021-03-15 18:45 ` [PATCH v4 01/17] target/ppc: Move helper_regs.h functions out-of-line Richard Henderson
2021-03-15 18:46 ` [PATCH v4 02/17] target/ppc: Move 601 hflags adjustment to hreg_compute_hflags Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-16 8:15 ` Cédric Le Goater
2021-03-22 3:38 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 04/17] target/ppc: Do not call hreg_compute_mem_idx after ppc_store_msr Richard Henderson
` (14 subsequent siblings)
17 siblings, 2 replies; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
Match cpu_post_load in using ppc_store_msr to set all of
the cpu state implied by the value of msr. Do not restore
hflags or hflags_nmsr, as we recompute them in ppc_store_msr.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/machine.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 283db1d28a..87d7bffb86 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -21,6 +21,7 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
int32_t slb_nr;
#endif
target_ulong xer;
+ target_ulong msr;
for (i = 0; i < 32; i++) {
qemu_get_betls(f, &env->gpr[i]);
@@ -111,11 +112,19 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
qemu_get_betls(f, &env->ivpr_mask);
qemu_get_betls(f, &env->hreset_vector);
qemu_get_betls(f, &env->nip);
- qemu_get_betls(f, &env->hflags);
- qemu_get_betls(f, &env->hflags_nmsr);
+ qemu_get_sbetl(f); /* Discard unused hflags */
+ qemu_get_sbetl(f); /* Discard unused hflags_nmsr */
qemu_get_sbe32(f); /* Discard unused mmu_idx */
qemu_get_sbe32(f); /* Discard unused power_mode */
+ /*
+ * Invalidate all supported msr bits except MSR_TGPR/MSR_HVB
+ * before restoring. Note that this recomputes hflags and mem_idx.
+ */
+ msr = env->msr;
+ env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB);
+ ppc_store_msr(env, msr);
+
/* Recompute mmu indices */
hreg_compute_mem_idx(env);
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 03/17] target/ppc: Properly sync cpu state with new msr in cpu_load_old
2021-03-15 18:46 ` [PATCH v4 03/17] target/ppc: Properly sync cpu state with new msr in cpu_load_old Richard Henderson
@ 2021-03-16 8:15 ` Cédric Le Goater
2021-03-22 16:53 ` Richard Henderson
2021-03-22 3:38 ` David Gibson
1 sibling, 1 reply; 51+ messages in thread
From: Cédric Le Goater @ 2021-03-16 8:15 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc, david
On 3/15/21 7:46 PM, Richard Henderson wrote:
> Match cpu_post_load in using ppc_store_msr to set all of
> the cpu state implied by the value of msr. Do not restore
> hflags or hflags_nmsr, as we recompute them in ppc_store_msr.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Could we add a common routine used by cpu_post_load() and cpu_load_old() ?
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> target/ppc/machine.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 283db1d28a..87d7bffb86 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -21,6 +21,7 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> int32_t slb_nr;
> #endif
> target_ulong xer;
> + target_ulong msr;
>
> for (i = 0; i < 32; i++) {
> qemu_get_betls(f, &env->gpr[i]);
> @@ -111,11 +112,19 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> qemu_get_betls(f, &env->ivpr_mask);
> qemu_get_betls(f, &env->hreset_vector);
> qemu_get_betls(f, &env->nip);
> - qemu_get_betls(f, &env->hflags);
> - qemu_get_betls(f, &env->hflags_nmsr);
> + qemu_get_sbetl(f); /* Discard unused hflags */
> + qemu_get_sbetl(f); /* Discard unused hflags_nmsr */
> qemu_get_sbe32(f); /* Discard unused mmu_idx */
> qemu_get_sbe32(f); /* Discard unused power_mode */
>
> + /*
> + * Invalidate all supported msr bits except MSR_TGPR/MSR_HVB
> + * before restoring. Note that this recomputes hflags and mem_idx.
> + */
> + msr = env->msr;
> + env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB);
> + ppc_store_msr(env, msr);
> +
> /* Recompute mmu indices */
> hreg_compute_mem_idx(env);
>
>
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v4 03/17] target/ppc: Properly sync cpu state with new msr in cpu_load_old
2021-03-16 8:15 ` Cédric Le Goater
@ 2021-03-22 16:53 ` Richard Henderson
2021-03-22 23:19 ` David Gibson
0 siblings, 1 reply; 51+ messages in thread
From: Richard Henderson @ 2021-03-22 16:53 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel; +Cc: qemu-ppc, david
On 3/16/21 2:15 AM, Cédric Le Goater wrote:
> On 3/15/21 7:46 PM, Richard Henderson wrote:
>> Match cpu_post_load in using ppc_store_msr to set all of
>> the cpu state implied by the value of msr. Do not restore
>> hflags or hflags_nmsr, as we recompute them in ppc_store_msr.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
> Could we add a common routine used by cpu_post_load() and cpu_load_old() ?
Will do. David, would you like to unqueue this one, or shall I send another
patch on top?
r~
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v4 03/17] target/ppc: Properly sync cpu state with new msr in cpu_load_old
2021-03-22 16:53 ` Richard Henderson
@ 2021-03-22 23:19 ` David Gibson
0 siblings, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 23:19 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, Cédric Le Goater, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 919 bytes --]
On Mon, Mar 22, 2021 at 10:53:01AM -0600, Richard Henderson wrote:
> On 3/16/21 2:15 AM, Cédric Le Goater wrote:
> > On 3/15/21 7:46 PM, Richard Henderson wrote:
> > > Match cpu_post_load in using ppc_store_msr to set all of
> > > the cpu state implied by the value of msr. Do not restore
> > > hflags or hflags_nmsr, as we recompute them in ppc_store_msr.
> > >
> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >
> > Could we add a common routine used by cpu_post_load() and cpu_load_old() ?
>
> Will do. David, would you like to unqueue this one, or shall I send another
> patch on top?
Pulling that one out causes conflicts with later patches, so another
one on top, please.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v4 03/17] target/ppc: Properly sync cpu state with new msr in cpu_load_old
2021-03-15 18:46 ` [PATCH v4 03/17] target/ppc: Properly sync cpu state with new msr in cpu_load_old Richard Henderson
2021-03-16 8:15 ` Cédric Le Goater
@ 2021-03-22 3:38 ` David Gibson
1 sibling, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 3:38 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]
On Mon, Mar 15, 2021 at 12:46:01PM -0600, Richard Henderson wrote:
> Match cpu_post_load in using ppc_store_msr to set all of
> the cpu state implied by the value of msr. Do not restore
> hflags or hflags_nmsr, as we recompute them in ppc_store_msr.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Applied to ppc-for-6.0, thanks.
> ---
> target/ppc/machine.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 283db1d28a..87d7bffb86 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -21,6 +21,7 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> int32_t slb_nr;
> #endif
> target_ulong xer;
> + target_ulong msr;
>
> for (i = 0; i < 32; i++) {
> qemu_get_betls(f, &env->gpr[i]);
> @@ -111,11 +112,19 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> qemu_get_betls(f, &env->ivpr_mask);
> qemu_get_betls(f, &env->hreset_vector);
> qemu_get_betls(f, &env->nip);
> - qemu_get_betls(f, &env->hflags);
> - qemu_get_betls(f, &env->hflags_nmsr);
> + qemu_get_sbetl(f); /* Discard unused hflags */
> + qemu_get_sbetl(f); /* Discard unused hflags_nmsr */
> qemu_get_sbe32(f); /* Discard unused mmu_idx */
> qemu_get_sbe32(f); /* Discard unused power_mode */
>
> + /*
> + * Invalidate all supported msr bits except MSR_TGPR/MSR_HVB
> + * before restoring. Note that this recomputes hflags and mem_idx.
> + */
> + msr = env->msr;
> + env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB);
> + ppc_store_msr(env, msr);
> +
> /* Recompute mmu indices */
> hreg_compute_mem_idx(env);
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 04/17] target/ppc: Do not call hreg_compute_mem_idx after ppc_store_msr
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (2 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 03/17] target/ppc: Properly sync cpu state with new msr in cpu_load_old Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-16 8:16 ` Cédric Le Goater
2021-03-22 3:39 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 05/17] target/ppc: Retain hflags_nmsr only for migration Richard Henderson
` (13 subsequent siblings)
17 siblings, 2 replies; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
In ppc_store_msr we call hreg_compute_hflags, which itself
calls hreg_compute_mem_idx. Rely on ppc_store_msr to update
everything required by the msr update.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/machine.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 87d7bffb86..f6eeda9642 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -125,9 +125,6 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB);
ppc_store_msr(env, msr);
- /* Recompute mmu indices */
- hreg_compute_mem_idx(env);
-
return 0;
}
@@ -418,14 +415,12 @@ static int cpu_post_load(void *opaque, int version_id)
/*
* Invalidate all supported msr bits except MSR_TGPR/MSR_HVB
- * before restoring
+ * before restoring. Note that this recomputes hflags and mem_idx.
*/
msr = env->msr;
env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB);
ppc_store_msr(env, msr);
- hreg_compute_mem_idx(env);
-
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 04/17] target/ppc: Do not call hreg_compute_mem_idx after ppc_store_msr
2021-03-15 18:46 ` [PATCH v4 04/17] target/ppc: Do not call hreg_compute_mem_idx after ppc_store_msr Richard Henderson
@ 2021-03-16 8:16 ` Cédric Le Goater
2021-03-22 3:39 ` David Gibson
1 sibling, 0 replies; 51+ messages in thread
From: Cédric Le Goater @ 2021-03-16 8:16 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc, david
On 3/15/21 7:46 PM, Richard Henderson wrote:
> In ppc_store_msr we call hreg_compute_hflags, which itself
> calls hreg_compute_mem_idx. Rely on ppc_store_msr to update
> everything required by the msr update.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> target/ppc/machine.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 87d7bffb86..f6eeda9642 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -125,9 +125,6 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB);
> ppc_store_msr(env, msr);
>
> - /* Recompute mmu indices */
> - hreg_compute_mem_idx(env);
> -
> return 0;
> }
>
> @@ -418,14 +415,12 @@ static int cpu_post_load(void *opaque, int version_id)
>
> /*
> * Invalidate all supported msr bits except MSR_TGPR/MSR_HVB
> - * before restoring
> + * before restoring. Note that this recomputes hflags and mem_idx.
> */
> msr = env->msr;
> env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB);
> ppc_store_msr(env, msr);
>
> - hreg_compute_mem_idx(env);
> -
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v4 04/17] target/ppc: Do not call hreg_compute_mem_idx after ppc_store_msr
2021-03-15 18:46 ` [PATCH v4 04/17] target/ppc: Do not call hreg_compute_mem_idx after ppc_store_msr Richard Henderson
2021-03-16 8:16 ` Cédric Le Goater
@ 2021-03-22 3:39 ` David Gibson
1 sibling, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 3:39 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1573 bytes --]
On Mon, Mar 15, 2021 at 12:46:02PM -0600, Richard Henderson wrote:
> In ppc_store_msr we call hreg_compute_hflags, which itself
> calls hreg_compute_mem_idx. Rely on ppc_store_msr to update
> everything required by the msr update.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Applied to ppc-for-6.0.
> ---
> target/ppc/machine.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 87d7bffb86..f6eeda9642 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -125,9 +125,6 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
> env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB);
> ppc_store_msr(env, msr);
>
> - /* Recompute mmu indices */
> - hreg_compute_mem_idx(env);
> -
> return 0;
> }
>
> @@ -418,14 +415,12 @@ static int cpu_post_load(void *opaque, int version_id)
>
> /*
> * Invalidate all supported msr bits except MSR_TGPR/MSR_HVB
> - * before restoring
> + * before restoring. Note that this recomputes hflags and mem_idx.
> */
> msr = env->msr;
> env->msr ^= env->msr_mask & ~((1ULL << MSR_TGPR) | MSR_HVB);
> ppc_store_msr(env, msr);
>
> - hreg_compute_mem_idx(env);
> -
> return 0;
> }
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 05/17] target/ppc: Retain hflags_nmsr only for migration
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (3 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 04/17] target/ppc: Do not call hreg_compute_mem_idx after ppc_store_msr Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-16 8:16 ` Cédric Le Goater
2021-03-22 3:41 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 06/17] target/ppc: Fix comment for MSR_FE{0,1} Richard Henderson
` (12 subsequent siblings)
17 siblings, 2 replies; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
We have eliminated all normal uses of hflags_nmsr. We need
not even compute it except when we want to migrate. Rename
the field to emphasize this.
Remove the fixme comment for migrating access_type. This value
is only ever used with the current executing instruction, and
is never live when the cpu is halted for migration.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/cpu.h | 4 ++--
target/ppc/helper_regs.c | 2 --
target/ppc/machine.c | 9 ++++++---
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 061d2eed1b..79c4033a42 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1105,8 +1105,8 @@ struct CPUPPCState {
#endif
/* These resources are used only in QEMU core */
- target_ulong hflags; /* hflags is MSR & HFLAGS_MASK */
- target_ulong hflags_nmsr; /* specific hflags, not coming from MSR */
+ target_ulong hflags;
+ target_ulong hflags_compat_nmsr; /* for migration compatibility */
int immu_idx; /* precomputed MMU index to speed up insn accesses */
int dmmu_idx; /* precomputed MMU index to speed up data accesses */
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 95b9aca61f..a87e354ca2 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -104,8 +104,6 @@ void hreg_compute_hflags(CPUPPCState *env)
*/
uint32_t le = extract32(env->spr[SPR_HID0], 3, 1);
env->hflags |= le << MSR_LE;
- /* Retain for backward compatibility with migration. */
- env->hflags_nmsr = le << MSR_LE;
}
}
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index f6eeda9642..1f7a353c78 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -310,6 +310,10 @@ static int cpu_pre_save(void *opaque)
}
}
+ /* Retain migration compatibility for pre 6.0 for 601 machines. */
+ env->hflags_compat_nmsr = (env->flags & POWERPC_FLAG_HID0_LE
+ ? env->hflags & MSR_LE : 0);
+
return 0;
}
@@ -829,9 +833,8 @@ const VMStateDescription vmstate_ppc_cpu = {
/* Supervisor mode architected state */
VMSTATE_UINTTL(env.msr, PowerPCCPU),
- /* Internal state */
- VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
- /* FIXME: access_type? */
+ /* Backward compatible internal state */
+ VMSTATE_UINTTL(env.hflags_compat_nmsr, PowerPCCPU),
/* Sanity checking */
VMSTATE_UINTTL_TEST(mig_msr_mask, PowerPCCPU, cpu_pre_2_8_migration),
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 05/17] target/ppc: Retain hflags_nmsr only for migration
2021-03-15 18:46 ` [PATCH v4 05/17] target/ppc: Retain hflags_nmsr only for migration Richard Henderson
@ 2021-03-16 8:16 ` Cédric Le Goater
2021-03-22 3:41 ` David Gibson
1 sibling, 0 replies; 51+ messages in thread
From: Cédric Le Goater @ 2021-03-16 8:16 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc, david
On 3/15/21 7:46 PM, Richard Henderson wrote:
> We have eliminated all normal uses of hflags_nmsr. We need
> not even compute it except when we want to migrate. Rename
> the field to emphasize this.
>
> Remove the fixme comment for migrating access_type. This value
> is only ever used with the current executing instruction, and
> is never live when the cpu is halted for migration.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> target/ppc/cpu.h | 4 ++--
> target/ppc/helper_regs.c | 2 --
> target/ppc/machine.c | 9 ++++++---
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 061d2eed1b..79c4033a42 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1105,8 +1105,8 @@ struct CPUPPCState {
> #endif
>
> /* These resources are used only in QEMU core */
> - target_ulong hflags; /* hflags is MSR & HFLAGS_MASK */
> - target_ulong hflags_nmsr; /* specific hflags, not coming from MSR */
> + target_ulong hflags;
> + target_ulong hflags_compat_nmsr; /* for migration compatibility */
> int immu_idx; /* precomputed MMU index to speed up insn accesses */
> int dmmu_idx; /* precomputed MMU index to speed up data accesses */
>
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index 95b9aca61f..a87e354ca2 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -104,8 +104,6 @@ void hreg_compute_hflags(CPUPPCState *env)
> */
> uint32_t le = extract32(env->spr[SPR_HID0], 3, 1);
> env->hflags |= le << MSR_LE;
> - /* Retain for backward compatibility with migration. */
> - env->hflags_nmsr = le << MSR_LE;
> }
> }
>
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index f6eeda9642..1f7a353c78 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -310,6 +310,10 @@ static int cpu_pre_save(void *opaque)
> }
> }
>
> + /* Retain migration compatibility for pre 6.0 for 601 machines. */
> + env->hflags_compat_nmsr = (env->flags & POWERPC_FLAG_HID0_LE
> + ? env->hflags & MSR_LE : 0);
> +
> return 0;
> }
>
> @@ -829,9 +833,8 @@ const VMStateDescription vmstate_ppc_cpu = {
> /* Supervisor mode architected state */
> VMSTATE_UINTTL(env.msr, PowerPCCPU),
>
> - /* Internal state */
> - VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
> - /* FIXME: access_type? */
> + /* Backward compatible internal state */
> + VMSTATE_UINTTL(env.hflags_compat_nmsr, PowerPCCPU),
>
> /* Sanity checking */
> VMSTATE_UINTTL_TEST(mig_msr_mask, PowerPCCPU, cpu_pre_2_8_migration),
>
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v4 05/17] target/ppc: Retain hflags_nmsr only for migration
2021-03-15 18:46 ` [PATCH v4 05/17] target/ppc: Retain hflags_nmsr only for migration Richard Henderson
2021-03-16 8:16 ` Cédric Le Goater
@ 2021-03-22 3:41 ` David Gibson
1 sibling, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 3:41 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3105 bytes --]
On Mon, Mar 15, 2021 at 12:46:03PM -0600, Richard Henderson wrote:
> We have eliminated all normal uses of hflags_nmsr. We need
> not even compute it except when we want to migrate. Rename
> the field to emphasize this.
>
> Remove the fixme comment for migrating access_type. This value
> is only ever used with the current executing instruction, and
> is never live when the cpu is halted for migration.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Applied to ppc-for-6.0, thanks.
> ---
> target/ppc/cpu.h | 4 ++--
> target/ppc/helper_regs.c | 2 --
> target/ppc/machine.c | 9 ++++++---
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 061d2eed1b..79c4033a42 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1105,8 +1105,8 @@ struct CPUPPCState {
> #endif
>
> /* These resources are used only in QEMU core */
> - target_ulong hflags; /* hflags is MSR & HFLAGS_MASK */
> - target_ulong hflags_nmsr; /* specific hflags, not coming from MSR */
> + target_ulong hflags;
> + target_ulong hflags_compat_nmsr; /* for migration compatibility */
> int immu_idx; /* precomputed MMU index to speed up insn accesses */
> int dmmu_idx; /* precomputed MMU index to speed up data accesses */
>
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index 95b9aca61f..a87e354ca2 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -104,8 +104,6 @@ void hreg_compute_hflags(CPUPPCState *env)
> */
> uint32_t le = extract32(env->spr[SPR_HID0], 3, 1);
> env->hflags |= le << MSR_LE;
> - /* Retain for backward compatibility with migration. */
> - env->hflags_nmsr = le << MSR_LE;
> }
> }
>
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index f6eeda9642..1f7a353c78 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -310,6 +310,10 @@ static int cpu_pre_save(void *opaque)
> }
> }
>
> + /* Retain migration compatibility for pre 6.0 for 601 machines. */
> + env->hflags_compat_nmsr = (env->flags & POWERPC_FLAG_HID0_LE
> + ? env->hflags & MSR_LE : 0);
> +
> return 0;
> }
>
> @@ -829,9 +833,8 @@ const VMStateDescription vmstate_ppc_cpu = {
> /* Supervisor mode architected state */
> VMSTATE_UINTTL(env.msr, PowerPCCPU),
>
> - /* Internal state */
> - VMSTATE_UINTTL(env.hflags_nmsr, PowerPCCPU),
> - /* FIXME: access_type? */
> + /* Backward compatible internal state */
> + VMSTATE_UINTTL(env.hflags_compat_nmsr, PowerPCCPU),
>
> /* Sanity checking */
> VMSTATE_UINTTL_TEST(mig_msr_mask, PowerPCCPU, cpu_pre_2_8_migration),
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 06/17] target/ppc: Fix comment for MSR_FE{0,1}
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (4 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 05/17] target/ppc: Retain hflags_nmsr only for migration Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-16 8:16 ` Cédric Le Goater
2021-03-22 3:42 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 07/17] target/ppc: Disconnect hflags from MSR Richard Henderson
` (11 subsequent siblings)
17 siblings, 2 replies; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
As per hreg_compute_hflags:
We 'forget' FE0 & FE1: we'll never generate imprecise exceptions
remove the hflags marker from the respective comments.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/cpu.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 79c4033a42..fd13489dce 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -322,13 +322,13 @@ typedef struct ppc_v3_pate_t {
#define MSR_PR 14 /* Problem state hflags */
#define MSR_FP 13 /* Floating point available hflags */
#define MSR_ME 12 /* Machine check interrupt enable */
-#define MSR_FE0 11 /* Floating point exception mode 0 hflags */
+#define MSR_FE0 11 /* Floating point exception mode 0 */
#define MSR_SE 10 /* Single-step trace enable x hflags */
#define MSR_DWE 10 /* Debug wait enable on 405 x */
#define MSR_UBLE 10 /* User BTB lock enable on e500 x */
#define MSR_BE 9 /* Branch trace enable x hflags */
#define MSR_DE 9 /* Debug interrupts enable on embedded PowerPC x */
-#define MSR_FE1 8 /* Floating point exception mode 1 hflags */
+#define MSR_FE1 8 /* Floating point exception mode 1 */
#define MSR_AL 7 /* AL bit on POWER */
#define MSR_EP 6 /* Exception prefix on 601 */
#define MSR_IR 5 /* Instruction relocate */
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 06/17] target/ppc: Fix comment for MSR_FE{0,1}
2021-03-15 18:46 ` [PATCH v4 06/17] target/ppc: Fix comment for MSR_FE{0,1} Richard Henderson
@ 2021-03-16 8:16 ` Cédric Le Goater
2021-03-22 3:42 ` David Gibson
1 sibling, 0 replies; 51+ messages in thread
From: Cédric Le Goater @ 2021-03-16 8:16 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc, david
On 3/15/21 7:46 PM, Richard Henderson wrote:
> As per hreg_compute_hflags:
>
> We 'forget' FE0 & FE1: we'll never generate imprecise exceptions
>
> remove the hflags marker from the respective comments.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> target/ppc/cpu.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 79c4033a42..fd13489dce 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -322,13 +322,13 @@ typedef struct ppc_v3_pate_t {
> #define MSR_PR 14 /* Problem state hflags */
> #define MSR_FP 13 /* Floating point available hflags */
> #define MSR_ME 12 /* Machine check interrupt enable */
> -#define MSR_FE0 11 /* Floating point exception mode 0 hflags */
> +#define MSR_FE0 11 /* Floating point exception mode 0 */
> #define MSR_SE 10 /* Single-step trace enable x hflags */
> #define MSR_DWE 10 /* Debug wait enable on 405 x */
> #define MSR_UBLE 10 /* User BTB lock enable on e500 x */
> #define MSR_BE 9 /* Branch trace enable x hflags */
> #define MSR_DE 9 /* Debug interrupts enable on embedded PowerPC x */
> -#define MSR_FE1 8 /* Floating point exception mode 1 hflags */
> +#define MSR_FE1 8 /* Floating point exception mode 1 */
> #define MSR_AL 7 /* AL bit on POWER */
> #define MSR_EP 6 /* Exception prefix on 601 */
> #define MSR_IR 5 /* Instruction relocate */
>
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v4 06/17] target/ppc: Fix comment for MSR_FE{0,1}
2021-03-15 18:46 ` [PATCH v4 06/17] target/ppc: Fix comment for MSR_FE{0,1} Richard Henderson
2021-03-16 8:16 ` Cédric Le Goater
@ 2021-03-22 3:42 ` David Gibson
1 sibling, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 3:42 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2091 bytes --]
On Mon, Mar 15, 2021 at 12:46:04PM -0600, Richard Henderson wrote:
> As per hreg_compute_hflags:
>
> We 'forget' FE0 & FE1: we'll never generate imprecise exceptions
>
> remove the hflags marker from the respective comments.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Applied to ppc-for-6.0, thanks.
> ---
> target/ppc/cpu.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 79c4033a42..fd13489dce 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -322,13 +322,13 @@ typedef struct ppc_v3_pate_t {
> #define MSR_PR 14 /* Problem state hflags */
> #define MSR_FP 13 /* Floating point available hflags */
> #define MSR_ME 12 /* Machine check interrupt enable */
> -#define MSR_FE0 11 /* Floating point exception mode 0 hflags */
> +#define MSR_FE0 11 /* Floating point exception mode 0 */
> #define MSR_SE 10 /* Single-step trace enable x hflags */
> #define MSR_DWE 10 /* Debug wait enable on 405 x */
> #define MSR_UBLE 10 /* User BTB lock enable on e500 x */
> #define MSR_BE 9 /* Branch trace enable x hflags */
> #define MSR_DE 9 /* Debug interrupts enable on embedded PowerPC x */
> -#define MSR_FE1 8 /* Floating point exception mode 1 hflags */
> +#define MSR_FE1 8 /* Floating point exception mode 1 */
> #define MSR_AL 7 /* AL bit on POWER */
> #define MSR_EP 6 /* Exception prefix on 601 */
> #define MSR_IR 5 /* Instruction relocate */
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 07/17] target/ppc: Disconnect hflags from MSR
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (5 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 06/17] target/ppc: Fix comment for MSR_FE{0,1} Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-22 3:52 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 08/17] target/ppc: Reduce env->hflags to uint32_t Richard Henderson
` (10 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: Ivan Warren, qemu-ppc, david
Copying flags directly from msr has drawbacks: (1) msr bits
mean different things per cpu, (2) msr has 64 bits on 64 cpus
while tb->flags has only 32 bits.
Create a enum to define these bits. Document the origin of each bit.
This fixes the truncation of env->hflags to tb->flags, because we no
longer have hflags bits set above bit 31.
Most of the code in ppc_tr_init_disas_context is moved over to
hreg_compute_hflags. Some of it is simple extractions from msr,
some requires examining other cpu flags. Anything that is moved
becomes a simple extract from hflags in ppc_tr_init_disas_context.
Several existing bugs are left in ppc_tr_init_disas_context, where
additional changes are required -- to be addressed in future patches.
Remove a broken #if 0 block.
Reported-by: Ivan Warren <ivan@vmfacility.fr>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/cpu.h | 24 ++++++++++++++++++
target/ppc/helper_regs.c | 55 ++++++++++++++++++++++++++++++++--------
target/ppc/translate.c | 55 ++++++++++++----------------------------
3 files changed, 84 insertions(+), 50 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index fd13489dce..39f35b570c 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -585,6 +585,30 @@ enum {
POWERPC_FLAG_HID0_LE = 0x00400000,
};
+/*
+ * Bits for env->hflags.
+ *
+ * Most of these bits overlap with corresponding bits in MSR,
+ * but some come from other sources. Be cautious when modifying.
+ */
+enum {
+ HFLAGS_LE = 0, /* MSR_LE -- comes from elsewhere on 601 */
+ HFLAGS_HV = 1, /* computed from MSR_HV and other state */
+ HFLAGS_64 = 2, /* computed from MSR_CE and MSR_SF */
+ HFLAGS_PR = 3, /* MSR_PR */
+ HFLAGS_DR = 4, /* MSR_DR */
+ HFLAGS_IR = 5, /* MSR_IR */
+ HFLAGS_SPE = 6, /* from MSR_SPE if cpu has SPE; avoid overlap w/ MSR_VR */
+ HFLAGS_VSX = 7, /* from MSR_VSX if cpu has VSX; avoid overlap w/ MSR_AP */
+ HFLAGS_TM = 8, /* computed from MSR_TM */
+ HFLAGS_BE = 9, /* MSR_BE -- from elsewhere on embedded ppc */
+ HFLAGS_SE = 10, /* MSR_SE -- from elsewhere on embedded ppc */
+ HFLAGS_FP = 13, /* MSR_FP */
+ HFLAGS_SA = 22, /* MSR_SA */
+ HFLAGS_AP = 23, /* MSR_AP */
+ HFLAGS_VR = 25, /* MSR_VR if cpu has VRE */
+};
+
/*****************************************************************************/
/* Floating point status and control register */
#define FPSCR_DRN2 34 /* Decimal Floating-Point rounding control */
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index a87e354ca2..0a746bffd7 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -18,6 +18,7 @@
*/
#include "qemu/osdep.h"
+#include "cpu.h"
#include "qemu/main-loop.h"
#include "exec/exec-all.h"
#include "sysemu/kvm.h"
@@ -87,24 +88,56 @@ void hreg_compute_mem_idx(CPUPPCState *env)
void hreg_compute_hflags(CPUPPCState *env)
{
- target_ulong hflags_mask;
+ target_ulong msr = env->msr;
+ uint32_t ppc_flags = env->flags;
+ uint32_t hflags = 0;
+ uint32_t msr_mask;
- /* We 'forget' FE0 & FE1: we'll never generate imprecise exceptions */
- hflags_mask = (1 << MSR_VR) | (1 << MSR_AP) | (1 << MSR_SA) |
- (1 << MSR_PR) | (1 << MSR_FP) | (1 << MSR_SE) | (1 << MSR_BE) |
- (1 << MSR_LE) | (1 << MSR_VSX) | (1 << MSR_IR) | (1 << MSR_DR);
- hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
- hreg_compute_mem_idx(env);
- env->hflags = env->msr & hflags_mask;
+ /* Some bits come straight across from MSR. */
+ msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |
+ (1 << MSR_DR) | (1 << MSR_IR) |
+ (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));
- if (env->flags & POWERPC_FLAG_HID0_LE) {
+ if (ppc_flags & POWERPC_FLAG_HID0_LE) {
/*
* Note that MSR_LE is not set in env->msr_mask for this cpu,
- * and so will never be set in msr or hflags at this point.
+ * and so will never be set in msr.
*/
uint32_t le = extract32(env->spr[SPR_HID0], 3, 1);
- env->hflags |= le << MSR_LE;
+ hflags |= le << MSR_LE;
}
+
+ if (ppc_flags & POWERPC_FLAG_BE) {
+ msr_mask |= 1 << MSR_BE;
+ }
+ if (ppc_flags & POWERPC_FLAG_SE) {
+ msr_mask |= 1 << MSR_SE;
+ }
+
+ if (msr_is_64bit(env, msr)) {
+ hflags |= 1 << HFLAGS_64;
+ }
+ if ((ppc_flags & POWERPC_FLAG_SPE) && (msr & (1 << MSR_SPE))) {
+ hflags |= 1 << HFLAGS_SPE;
+ }
+ if (ppc_flags & POWERPC_FLAG_VRE) {
+ msr_mask |= 1 << MSR_VR;
+ }
+ if ((ppc_flags & POWERPC_FLAG_VSX) && (msr & (1 << MSR_VSX))) {
+ hflags |= 1 << HFLAGS_VSX;
+ }
+ if ((ppc_flags & POWERPC_FLAG_TM) && (msr & (1ull << MSR_TM))) {
+ hflags |= 1 << HFLAGS_TM;
+ }
+
+#ifndef CONFIG_USER_ONLY
+ if (!env->has_hv_mode || (msr & (1ull << MSR_HV))) {
+ hflags |= 1 << HFLAGS_HV;
+ }
+#endif
+
+ env->hflags = hflags | (msr & msr_mask);
+ hreg_compute_mem_idx(env);
}
void cpu_interrupt_exittb(CPUState *cs)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 0984ce637b..a9325a12e5 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7879,67 +7879,48 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
{
DisasContext *ctx = container_of(dcbase, DisasContext, base);
CPUPPCState *env = cs->env_ptr;
+ uint32_t hflags = ctx->base.tb->flags;
int bound;
ctx->exception = POWERPC_EXCP_NONE;
ctx->spr_cb = env->spr_cb;
- ctx->pr = msr_pr;
+ ctx->pr = (hflags >> HFLAGS_PR) & 1;
ctx->mem_idx = env->dmmu_idx;
- ctx->dr = msr_dr;
-#if !defined(CONFIG_USER_ONLY)
- ctx->hv = msr_hv || !env->has_hv_mode;
-#endif
+ ctx->dr = (hflags >> HFLAGS_DR) & 1;
+ ctx->hv = (hflags >> HFLAGS_HV) & 1;
ctx->insns_flags = env->insns_flags;
ctx->insns_flags2 = env->insns_flags2;
ctx->access_type = -1;
ctx->need_access_type = !mmu_is_64bit(env->mmu_model);
- ctx->le_mode = !!(env->hflags & (1 << MSR_LE));
+ ctx->le_mode = (hflags >> HFLAGS_LE) & 1;
ctx->default_tcg_memop_mask = ctx->le_mode ? MO_LE : MO_BE;
ctx->flags = env->flags;
#if defined(TARGET_PPC64)
- ctx->sf_mode = msr_is_64bit(env, env->msr);
+ ctx->sf_mode = (hflags >> HFLAGS_64) & 1;
ctx->has_cfar = !!(env->flags & POWERPC_FLAG_CFAR);
#endif
ctx->lazy_tlb_flush = env->mmu_model == POWERPC_MMU_32B
|| env->mmu_model == POWERPC_MMU_601
|| env->mmu_model & POWERPC_MMU_64;
- ctx->fpu_enabled = !!msr_fp;
- if ((env->flags & POWERPC_FLAG_SPE) && msr_spe) {
- ctx->spe_enabled = !!msr_spe;
- } else {
- ctx->spe_enabled = false;
- }
- if ((env->flags & POWERPC_FLAG_VRE) && msr_vr) {
- ctx->altivec_enabled = !!msr_vr;
- } else {
- ctx->altivec_enabled = false;
- }
- if ((env->flags & POWERPC_FLAG_VSX) && msr_vsx) {
- ctx->vsx_enabled = !!msr_vsx;
- } else {
- ctx->vsx_enabled = false;
- }
+ ctx->fpu_enabled = (hflags >> HFLAGS_FP) & 1;
+ ctx->spe_enabled = (hflags >> HFLAGS_SPE) & 1;
+ ctx->altivec_enabled = (hflags >> HFLAGS_VR) & 1;
+ ctx->vsx_enabled = (hflags >> HFLAGS_VSX) & 1;
if ((env->flags & POWERPC_FLAG_SCV)
&& (env->spr[SPR_FSCR] & (1ull << FSCR_SCV))) {
ctx->scv_enabled = true;
} else {
ctx->scv_enabled = false;
}
-#if defined(TARGET_PPC64)
- if ((env->flags & POWERPC_FLAG_TM) && msr_tm) {
- ctx->tm_enabled = !!msr_tm;
- } else {
- ctx->tm_enabled = false;
- }
-#endif
+ ctx->tm_enabled = (hflags >> HFLAGS_TM) & 1;
ctx->gtse = !!(env->spr[SPR_LPCR] & LPCR_GTSE);
- if ((env->flags & POWERPC_FLAG_SE) && msr_se) {
- ctx->singlestep_enabled = CPU_SINGLE_STEP;
- } else {
- ctx->singlestep_enabled = 0;
+
+ ctx->singlestep_enabled = 0;
+ if ((hflags >> HFLAGS_SE) & 1) {
+ ctx->singlestep_enabled |= CPU_SINGLE_STEP;
}
- if ((env->flags & POWERPC_FLAG_BE) && msr_be) {
+ if ((hflags >> HFLAGS_BE) & 1) {
ctx->singlestep_enabled |= CPU_BRANCH_STEP;
}
if ((env->flags & POWERPC_FLAG_DE) && msr_de) {
@@ -7956,10 +7937,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
if (unlikely(ctx->base.singlestep_enabled)) {
ctx->singlestep_enabled |= GDBSTUB_SINGLE_STEP;
}
-#if defined(DO_SINGLE_STEP) && 0
- /* Single step trace mode */
- msr_se = 1;
-#endif
bound = -(ctx->base.pc_first | TARGET_PAGE_MASK) / 4;
ctx->base.max_insns = MIN(ctx->base.max_insns, bound);
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 07/17] target/ppc: Disconnect hflags from MSR
2021-03-15 18:46 ` [PATCH v4 07/17] target/ppc: Disconnect hflags from MSR Richard Henderson
@ 2021-03-22 3:52 ` David Gibson
2021-03-22 16:55 ` Richard Henderson
0 siblings, 1 reply; 51+ messages in thread
From: David Gibson @ 2021-03-22 3:52 UTC (permalink / raw)
To: Richard Henderson; +Cc: Ivan Warren, qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 10205 bytes --]
On Mon, Mar 15, 2021 at 12:46:05PM -0600, Richard Henderson wrote:
> Copying flags directly from msr has drawbacks: (1) msr bits
> mean different things per cpu, (2) msr has 64 bits on 64 cpus
> while tb->flags has only 32 bits.
>
> Create a enum to define these bits. Document the origin of each bit.
> This fixes the truncation of env->hflags to tb->flags, because we no
> longer have hflags bits set above bit 31.
>
> Most of the code in ppc_tr_init_disas_context is moved over to
> hreg_compute_hflags. Some of it is simple extractions from msr,
> some requires examining other cpu flags. Anything that is moved
> becomes a simple extract from hflags in ppc_tr_init_disas_context.
>
> Several existing bugs are left in ppc_tr_init_disas_context, where
> additional changes are required -- to be addressed in future patches.
>
> Remove a broken #if 0 block.
>
> Reported-by: Ivan Warren <ivan@vmfacility.fr>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/ppc/cpu.h | 24 ++++++++++++++++++
> target/ppc/helper_regs.c | 55 ++++++++++++++++++++++++++++++++--------
> target/ppc/translate.c | 55 ++++++++++++----------------------------
> 3 files changed, 84 insertions(+), 50 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index fd13489dce..39f35b570c 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -585,6 +585,30 @@ enum {
> POWERPC_FLAG_HID0_LE = 0x00400000,
> };
>
> +/*
> + * Bits for env->hflags.
> + *
> + * Most of these bits overlap with corresponding bits in MSR,
> + * but some come from other sources. Be cautious when modifying.
Yeah.. I'm not sure "be cautious" is enough of a warning. The exact
value of some but not all of these flags must equal that for the
corresponding MSR bits, which is terrifyingly subtle.
> + */
> +enum {
> + HFLAGS_LE = 0, /* MSR_LE -- comes from elsewhere on 601 */
> + HFLAGS_HV = 1, /* computed from MSR_HV and other state */
> + HFLAGS_64 = 2, /* computed from MSR_CE and MSR_SF */
> + HFLAGS_PR = 3, /* MSR_PR */
> + HFLAGS_DR = 4, /* MSR_DR */
> + HFLAGS_IR = 5, /* MSR_IR */
> + HFLAGS_SPE = 6, /* from MSR_SPE if cpu has SPE; avoid overlap w/ MSR_VR */
> + HFLAGS_VSX = 7, /* from MSR_VSX if cpu has VSX; avoid overlap w/ MSR_AP */
> + HFLAGS_TM = 8, /* computed from MSR_TM */
> + HFLAGS_BE = 9, /* MSR_BE -- from elsewhere on embedded ppc */
> + HFLAGS_SE = 10, /* MSR_SE -- from elsewhere on embedded ppc */
> + HFLAGS_FP = 13, /* MSR_FP */
> + HFLAGS_SA = 22, /* MSR_SA */
> + HFLAGS_AP = 23, /* MSR_AP */
> + HFLAGS_VR = 25, /* MSR_VR if cpu has VRE */
> +};
> +
> /*****************************************************************************/
> /* Floating point status and control register */
> #define FPSCR_DRN2 34 /* Decimal Floating-Point rounding control */
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index a87e354ca2..0a746bffd7 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -18,6 +18,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "cpu.h"
> #include "qemu/main-loop.h"
> #include "exec/exec-all.h"
> #include "sysemu/kvm.h"
> @@ -87,24 +88,56 @@ void hreg_compute_mem_idx(CPUPPCState *env)
>
> void hreg_compute_hflags(CPUPPCState *env)
> {
> - target_ulong hflags_mask;
> + target_ulong msr = env->msr;
> + uint32_t ppc_flags = env->flags;
> + uint32_t hflags = 0;
> + uint32_t msr_mask;
>
> - /* We 'forget' FE0 & FE1: we'll never generate imprecise exceptions */
> - hflags_mask = (1 << MSR_VR) | (1 << MSR_AP) | (1 << MSR_SA) |
> - (1 << MSR_PR) | (1 << MSR_FP) | (1 << MSR_SE) | (1 << MSR_BE) |
> - (1 << MSR_LE) | (1 << MSR_VSX) | (1 << MSR_IR) | (1 << MSR_DR);
> - hflags_mask |= (1ULL << MSR_CM) | (1ULL << MSR_SF) | MSR_HVB;
> - hreg_compute_mem_idx(env);
> - env->hflags = env->msr & hflags_mask;
> + /* Some bits come straight across from MSR. */
> + msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |
> + (1 << MSR_DR) | (1 << MSR_IR) |
> + (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));
>
> - if (env->flags & POWERPC_FLAG_HID0_LE) {
> + if (ppc_flags & POWERPC_FLAG_HID0_LE) {
> /*
> * Note that MSR_LE is not set in env->msr_mask for this cpu,
> - * and so will never be set in msr or hflags at this point.
> + * and so will never be set in msr.
> */
> uint32_t le = extract32(env->spr[SPR_HID0], 3, 1);
> - env->hflags |= le << MSR_LE;
> + hflags |= le << MSR_LE;
> }
> +
> + if (ppc_flags & POWERPC_FLAG_BE) {
> + msr_mask |= 1 << MSR_BE;
> + }
> + if (ppc_flags & POWERPC_FLAG_SE) {
> + msr_mask |= 1 << MSR_SE;
> + }
> +
> + if (msr_is_64bit(env, msr)) {
> + hflags |= 1 << HFLAGS_64;
> + }
> + if ((ppc_flags & POWERPC_FLAG_SPE) && (msr & (1 << MSR_SPE))) {
> + hflags |= 1 << HFLAGS_SPE;
> + }
> + if (ppc_flags & POWERPC_FLAG_VRE) {
> + msr_mask |= 1 << MSR_VR;
> + }
> + if ((ppc_flags & POWERPC_FLAG_VSX) && (msr & (1 << MSR_VSX))) {
> + hflags |= 1 << HFLAGS_VSX;
> + }
> + if ((ppc_flags & POWERPC_FLAG_TM) && (msr & (1ull << MSR_TM))) {
> + hflags |= 1 << HFLAGS_TM;
> + }
> +
> +#ifndef CONFIG_USER_ONLY
> + if (!env->has_hv_mode || (msr & (1ull << MSR_HV))) {
> + hflags |= 1 << HFLAGS_HV;
> + }
> +#endif
> +
> + env->hflags = hflags | (msr & msr_mask);
I feel like it would be better to explicitly translate *all* the bits,
so that the hflags bits numbers really are disentangled from the MSR
bit numbers. Or if we really must do this, then have some build time
asserts to ensure that (MSR_LE == HFLAGS_LE) and so forth.
> + hreg_compute_mem_idx(env);
> }
>
> void cpu_interrupt_exittb(CPUState *cs)
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 0984ce637b..a9325a12e5 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7879,67 +7879,48 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> {
> DisasContext *ctx = container_of(dcbase, DisasContext, base);
> CPUPPCState *env = cs->env_ptr;
> + uint32_t hflags = ctx->base.tb->flags;
> int bound;
>
> ctx->exception = POWERPC_EXCP_NONE;
> ctx->spr_cb = env->spr_cb;
> - ctx->pr = msr_pr;
> + ctx->pr = (hflags >> HFLAGS_PR) & 1;
> ctx->mem_idx = env->dmmu_idx;
> - ctx->dr = msr_dr;
> -#if !defined(CONFIG_USER_ONLY)
> - ctx->hv = msr_hv || !env->has_hv_mode;
> -#endif
> + ctx->dr = (hflags >> HFLAGS_DR) & 1;
> + ctx->hv = (hflags >> HFLAGS_HV) & 1;
> ctx->insns_flags = env->insns_flags;
> ctx->insns_flags2 = env->insns_flags2;
> ctx->access_type = -1;
> ctx->need_access_type = !mmu_is_64bit(env->mmu_model);
> - ctx->le_mode = !!(env->hflags & (1 << MSR_LE));
> + ctx->le_mode = (hflags >> HFLAGS_LE) & 1;
> ctx->default_tcg_memop_mask = ctx->le_mode ? MO_LE : MO_BE;
> ctx->flags = env->flags;
> #if defined(TARGET_PPC64)
> - ctx->sf_mode = msr_is_64bit(env, env->msr);
> + ctx->sf_mode = (hflags >> HFLAGS_64) & 1;
> ctx->has_cfar = !!(env->flags & POWERPC_FLAG_CFAR);
> #endif
> ctx->lazy_tlb_flush = env->mmu_model == POWERPC_MMU_32B
> || env->mmu_model == POWERPC_MMU_601
> || env->mmu_model & POWERPC_MMU_64;
>
> - ctx->fpu_enabled = !!msr_fp;
> - if ((env->flags & POWERPC_FLAG_SPE) && msr_spe) {
> - ctx->spe_enabled = !!msr_spe;
> - } else {
> - ctx->spe_enabled = false;
> - }
> - if ((env->flags & POWERPC_FLAG_VRE) && msr_vr) {
> - ctx->altivec_enabled = !!msr_vr;
> - } else {
> - ctx->altivec_enabled = false;
> - }
> - if ((env->flags & POWERPC_FLAG_VSX) && msr_vsx) {
> - ctx->vsx_enabled = !!msr_vsx;
> - } else {
> - ctx->vsx_enabled = false;
> - }
> + ctx->fpu_enabled = (hflags >> HFLAGS_FP) & 1;
> + ctx->spe_enabled = (hflags >> HFLAGS_SPE) & 1;
> + ctx->altivec_enabled = (hflags >> HFLAGS_VR) & 1;
> + ctx->vsx_enabled = (hflags >> HFLAGS_VSX) & 1;
> if ((env->flags & POWERPC_FLAG_SCV)
> && (env->spr[SPR_FSCR] & (1ull << FSCR_SCV))) {
> ctx->scv_enabled = true;
> } else {
> ctx->scv_enabled = false;
> }
> -#if defined(TARGET_PPC64)
> - if ((env->flags & POWERPC_FLAG_TM) && msr_tm) {
> - ctx->tm_enabled = !!msr_tm;
> - } else {
> - ctx->tm_enabled = false;
> - }
> -#endif
> + ctx->tm_enabled = (hflags >> HFLAGS_TM) & 1;
> ctx->gtse = !!(env->spr[SPR_LPCR] & LPCR_GTSE);
> - if ((env->flags & POWERPC_FLAG_SE) && msr_se) {
> - ctx->singlestep_enabled = CPU_SINGLE_STEP;
> - } else {
> - ctx->singlestep_enabled = 0;
> +
> + ctx->singlestep_enabled = 0;
> + if ((hflags >> HFLAGS_SE) & 1) {
> + ctx->singlestep_enabled |= CPU_SINGLE_STEP;
> }
> - if ((env->flags & POWERPC_FLAG_BE) && msr_be) {
> + if ((hflags >> HFLAGS_BE) & 1) {
> ctx->singlestep_enabled |= CPU_BRANCH_STEP;
> }
> if ((env->flags & POWERPC_FLAG_DE) && msr_de) {
> @@ -7956,10 +7937,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> if (unlikely(ctx->base.singlestep_enabled)) {
> ctx->singlestep_enabled |= GDBSTUB_SINGLE_STEP;
> }
> -#if defined(DO_SINGLE_STEP) && 0
> - /* Single step trace mode */
> - msr_se = 1;
> -#endif
>
> bound = -(ctx->base.pc_first | TARGET_PAGE_MASK) / 4;
> ctx->base.max_insns = MIN(ctx->base.max_insns, bound);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v4 07/17] target/ppc: Disconnect hflags from MSR
2021-03-22 3:52 ` David Gibson
@ 2021-03-22 16:55 ` Richard Henderson
2021-03-22 23:54 ` David Gibson
0 siblings, 1 reply; 51+ messages in thread
From: Richard Henderson @ 2021-03-22 16:55 UTC (permalink / raw)
To: David Gibson; +Cc: Ivan Warren, qemu-ppc, qemu-devel
On 3/21/21 9:52 PM, David Gibson wrote:
>> +/*
>> + * Bits for env->hflags.
>> + *
>> + * Most of these bits overlap with corresponding bits in MSR,
>> + * but some come from other sources. Be cautious when modifying.
>
> Yeah.. I'm not sure "be cautious" is enough of a warning. The exact
> value of some but not all of these flags must equal that for the
> corresponding MSR bits, which is terrifyingly subtle.
Fair. How about, for the comment here, "This is validated in hreg_compute_hflags."
>> + /* Some bits come straight across from MSR. */
>> + msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |
>> + (1 << MSR_DR) | (1 << MSR_IR) |
>> + (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));
Here, and in every other spot within this function where we manipulate msr_mask,
QEMU_BUILD_BUG_ON(MSR_LE != HFLAGS_LE);
etc.
r~
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v4 07/17] target/ppc: Disconnect hflags from MSR
2021-03-22 16:55 ` Richard Henderson
@ 2021-03-22 23:54 ` David Gibson
2021-03-23 17:04 ` Richard Henderson
0 siblings, 1 reply; 51+ messages in thread
From: David Gibson @ 2021-03-22 23:54 UTC (permalink / raw)
To: Richard Henderson; +Cc: Ivan Warren, qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1213 bytes --]
On Mon, Mar 22, 2021 at 10:55:46AM -0600, Richard Henderson wrote:
> On 3/21/21 9:52 PM, David Gibson wrote:
> > > +/*
> > > + * Bits for env->hflags.
> > > + *
> > > + * Most of these bits overlap with corresponding bits in MSR,
> > > + * but some come from other sources. Be cautious when modifying.
> >
> > Yeah.. I'm not sure "be cautious" is enough of a warning. The exact
> > value of some but not all of these flags must equal that for the
> > corresponding MSR bits, which is terrifyingly subtle.
>
> Fair. How about, for the comment here, "This is validated in hreg_compute_hflags."
>
> > > + /* Some bits come straight across from MSR. */
> > > + msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |
> > > + (1 << MSR_DR) | (1 << MSR_IR) |
> > > + (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));
>
> Here, and in every other spot within this function where we manipulate msr_mask,
>
> QEMU_BUILD_BUG_ON(MSR_LE != HFLAGS_LE);
Seems reasonable.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v4 07/17] target/ppc: Disconnect hflags from MSR
2021-03-22 23:54 ` David Gibson
@ 2021-03-23 17:04 ` Richard Henderson
2021-03-24 1:42 ` David Gibson
0 siblings, 1 reply; 51+ messages in thread
From: Richard Henderson @ 2021-03-23 17:04 UTC (permalink / raw)
To: David Gibson; +Cc: Ivan Warren, qemu-ppc, qemu-devel
On 3/22/21 5:54 PM, David Gibson wrote:
> On Mon, Mar 22, 2021 at 10:55:46AM -0600, Richard Henderson wrote:
>> On 3/21/21 9:52 PM, David Gibson wrote:
>>>> +/*
>>>> + * Bits for env->hflags.
>>>> + *
>>>> + * Most of these bits overlap with corresponding bits in MSR,
>>>> + * but some come from other sources. Be cautious when modifying.
>>>
>>> Yeah.. I'm not sure "be cautious" is enough of a warning. The exact
>>> value of some but not all of these flags must equal that for the
>>> corresponding MSR bits, which is terrifyingly subtle.
>>
>> Fair. How about, for the comment here, "This is validated in hreg_compute_hflags."
>>
>>>> + /* Some bits come straight across from MSR. */
>>>> + msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |
>>>> + (1 << MSR_DR) | (1 << MSR_IR) |
>>>> + (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));
>>
>> Here, and in every other spot within this function where we manipulate msr_mask,
>>
>> QEMU_BUILD_BUG_ON(MSR_LE != HFLAGS_LE);
>
> Seems reasonable.
>
Hah. Your paranoia was well-founded. Typo in HFLAGS_PR. :-)
r~
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH v4 07/17] target/ppc: Disconnect hflags from MSR
2021-03-23 17:04 ` Richard Henderson
@ 2021-03-24 1:42 ` David Gibson
0 siblings, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-24 1:42 UTC (permalink / raw)
To: Richard Henderson; +Cc: Ivan Warren, qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1502 bytes --]
On Tue, Mar 23, 2021 at 11:04:03AM -0600, Richard Henderson wrote:
> On 3/22/21 5:54 PM, David Gibson wrote:
> > On Mon, Mar 22, 2021 at 10:55:46AM -0600, Richard Henderson wrote:
> > > On 3/21/21 9:52 PM, David Gibson wrote:
> > > > > +/*
> > > > > + * Bits for env->hflags.
> > > > > + *
> > > > > + * Most of these bits overlap with corresponding bits in MSR,
> > > > > + * but some come from other sources. Be cautious when modifying.
> > > >
> > > > Yeah.. I'm not sure "be cautious" is enough of a warning. The exact
> > > > value of some but not all of these flags must equal that for the
> > > > corresponding MSR bits, which is terrifyingly subtle.
> > >
> > > Fair. How about, for the comment here, "This is validated in hreg_compute_hflags."
> > >
> > > > > + /* Some bits come straight across from MSR. */
> > > > > + msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |
> > > > > + (1 << MSR_DR) | (1 << MSR_IR) |
> > > > > + (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));
> > >
> > > Here, and in every other spot within this function where we manipulate msr_mask,
> > >
> > > QEMU_BUILD_BUG_ON(MSR_LE != HFLAGS_LE);
> >
> > Seems reasonable.
>
> Hah. Your paranoia was well-founded. Typo in HFLAGS_PR. :-)
Heh :).
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 08/17] target/ppc: Reduce env->hflags to uint32_t
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (6 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 07/17] target/ppc: Disconnect hflags from MSR Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-16 8:17 ` Cédric Le Goater
2021-03-22 3:53 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 09/17] target/ppc: Put dbcr0 single-step bits into hflags Richard Henderson
` (9 subsequent siblings)
17 siblings, 2 replies; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
It will be stored in tb->flags, which is also uint32_t,
so let's use the correct size.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/cpu.h | 4 ++--
target/ppc/misc_helper.c | 2 +-
target/ppc/translate.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 39f35b570c..2abaf56869 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1128,8 +1128,8 @@ struct CPUPPCState {
bool resume_as_sreset;
#endif
- /* These resources are used only in QEMU core */
- target_ulong hflags;
+ /* These resources are used only in TCG */
+ uint32_t hflags;
target_ulong hflags_compat_nmsr; /* for migration compatibility */
int immu_idx; /* precomputed MMU index to speed up insn accesses */
int dmmu_idx; /* precomputed MMU index to speed up data accesses */
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index 63e3147eb4..b04b4d7c6e 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -199,7 +199,7 @@ void helper_store_hid0_601(CPUPPCState *env, target_ulong val)
if ((val ^ hid0) & 0x00000008) {
/* Change current endianness */
hreg_compute_hflags(env);
- qemu_log("%s: set endianness to %c => " TARGET_FMT_lx "\n", __func__,
+ qemu_log("%s: set endianness to %c => %08x\n", __func__,
val & 0x8 ? 'l' : 'b', env->hflags);
}
}
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index a9325a12e5..a85b890bb0 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7657,7 +7657,7 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
env->nip, env->lr, env->ctr, cpu_read_xer(env),
cs->cpu_index);
qemu_fprintf(f, "MSR " TARGET_FMT_lx " HID0 " TARGET_FMT_lx " HF "
- TARGET_FMT_lx " iidx %d didx %d\n",
+ "%08x iidx %d didx %d\n",
env->msr, env->spr[SPR_HID0],
env->hflags, env->immu_idx, env->dmmu_idx);
#if !defined(NO_TIMER_DUMP)
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 08/17] target/ppc: Reduce env->hflags to uint32_t
2021-03-15 18:46 ` [PATCH v4 08/17] target/ppc: Reduce env->hflags to uint32_t Richard Henderson
@ 2021-03-16 8:17 ` Cédric Le Goater
2021-03-22 3:53 ` David Gibson
1 sibling, 0 replies; 51+ messages in thread
From: Cédric Le Goater @ 2021-03-16 8:17 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: qemu-ppc, david
On 3/15/21 7:46 PM, Richard Henderson wrote:
> It will be stored in tb->flags, which is also uint32_t,
> so let's use the correct size.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> target/ppc/cpu.h | 4 ++--
> target/ppc/misc_helper.c | 2 +-
> target/ppc/translate.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 39f35b570c..2abaf56869 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1128,8 +1128,8 @@ struct CPUPPCState {
> bool resume_as_sreset;
> #endif
>
> - /* These resources are used only in QEMU core */
> - target_ulong hflags;
> + /* These resources are used only in TCG */
> + uint32_t hflags;
> target_ulong hflags_compat_nmsr; /* for migration compatibility */
> int immu_idx; /* precomputed MMU index to speed up insn accesses */
> int dmmu_idx; /* precomputed MMU index to speed up data accesses */
> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
> index 63e3147eb4..b04b4d7c6e 100644
> --- a/target/ppc/misc_helper.c
> +++ b/target/ppc/misc_helper.c
> @@ -199,7 +199,7 @@ void helper_store_hid0_601(CPUPPCState *env, target_ulong val)
> if ((val ^ hid0) & 0x00000008) {
> /* Change current endianness */
> hreg_compute_hflags(env);
> - qemu_log("%s: set endianness to %c => " TARGET_FMT_lx "\n", __func__,
> + qemu_log("%s: set endianness to %c => %08x\n", __func__,
> val & 0x8 ? 'l' : 'b', env->hflags);
> }
> }
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index a9325a12e5..a85b890bb0 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7657,7 +7657,7 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> env->nip, env->lr, env->ctr, cpu_read_xer(env),
> cs->cpu_index);
> qemu_fprintf(f, "MSR " TARGET_FMT_lx " HID0 " TARGET_FMT_lx " HF "
> - TARGET_FMT_lx " iidx %d didx %d\n",
> + "%08x iidx %d didx %d\n",
> env->msr, env->spr[SPR_HID0],
> env->hflags, env->immu_idx, env->dmmu_idx);
> #if !defined(NO_TIMER_DUMP)
>
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v4 08/17] target/ppc: Reduce env->hflags to uint32_t
2021-03-15 18:46 ` [PATCH v4 08/17] target/ppc: Reduce env->hflags to uint32_t Richard Henderson
2021-03-16 8:17 ` Cédric Le Goater
@ 2021-03-22 3:53 ` David Gibson
1 sibling, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 3:53 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2589 bytes --]
On Mon, Mar 15, 2021 at 12:46:06PM -0600, Richard Henderson wrote:
> It will be stored in tb->flags, which is also uint32_t,
> so let's use the correct size.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> target/ppc/cpu.h | 4 ++--
> target/ppc/misc_helper.c | 2 +-
> target/ppc/translate.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 39f35b570c..2abaf56869 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1128,8 +1128,8 @@ struct CPUPPCState {
> bool resume_as_sreset;
> #endif
>
> - /* These resources are used only in QEMU core */
> - target_ulong hflags;
> + /* These resources are used only in TCG */
> + uint32_t hflags;
> target_ulong hflags_compat_nmsr; /* for migration compatibility */
> int immu_idx; /* precomputed MMU index to speed up insn accesses */
> int dmmu_idx; /* precomputed MMU index to speed up data accesses */
> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
> index 63e3147eb4..b04b4d7c6e 100644
> --- a/target/ppc/misc_helper.c
> +++ b/target/ppc/misc_helper.c
> @@ -199,7 +199,7 @@ void helper_store_hid0_601(CPUPPCState *env, target_ulong val)
> if ((val ^ hid0) & 0x00000008) {
> /* Change current endianness */
> hreg_compute_hflags(env);
> - qemu_log("%s: set endianness to %c => " TARGET_FMT_lx "\n", __func__,
> + qemu_log("%s: set endianness to %c => %08x\n", __func__,
> val & 0x8 ? 'l' : 'b', env->hflags);
> }
> }
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index a9325a12e5..a85b890bb0 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7657,7 +7657,7 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> env->nip, env->lr, env->ctr, cpu_read_xer(env),
> cs->cpu_index);
> qemu_fprintf(f, "MSR " TARGET_FMT_lx " HID0 " TARGET_FMT_lx " HF "
> - TARGET_FMT_lx " iidx %d didx %d\n",
> + "%08x iidx %d didx %d\n",
> env->msr, env->spr[SPR_HID0],
> env->hflags, env->immu_idx, env->dmmu_idx);
> #if !defined(NO_TIMER_DUMP)
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 09/17] target/ppc: Put dbcr0 single-step bits into hflags
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (7 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 08/17] target/ppc: Reduce env->hflags to uint32_t Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-22 3:55 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 10/17] target/ppc: Create helper_scv Richard Henderson
` (8 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
Because these bits were not in hflags, the code generated
for single-stepping on BookE was essentially random.
Recompute hflags when storing to dbcr0.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/helper_regs.c | 20 +++++++++++++++-----
target/ppc/misc_helper.c | 3 +++
target/ppc/translate.c | 11 -----------
3 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 0a746bffd7..c735540333 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -107,11 +107,21 @@ void hreg_compute_hflags(CPUPPCState *env)
hflags |= le << MSR_LE;
}
- if (ppc_flags & POWERPC_FLAG_BE) {
- msr_mask |= 1 << MSR_BE;
- }
- if (ppc_flags & POWERPC_FLAG_SE) {
- msr_mask |= 1 << MSR_SE;
+ if (ppc_flags & POWERPC_FLAG_DE) {
+ target_ulong dbcr0 = env->spr[SPR_BOOKE_DBCR0];
+ if (dbcr0 & DBCR0_ICMP) {
+ hflags |= 1 << HFLAGS_SE;
+ }
+ if (dbcr0 & DBCR0_BRT) {
+ hflags |= 1 << HFLAGS_BE;
+ }
+ } else {
+ if (ppc_flags & POWERPC_FLAG_BE) {
+ msr_mask |= 1 << MSR_BE;
+ }
+ if (ppc_flags & POWERPC_FLAG_SE) {
+ msr_mask |= 1 << MSR_SE;
+ }
}
if (msr_is_64bit(env, msr)) {
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index b04b4d7c6e..a5ee1fd63c 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -215,6 +215,9 @@ void helper_store_403_pbr(CPUPPCState *env, uint32_t num, target_ulong value)
void helper_store_40x_dbcr0(CPUPPCState *env, target_ulong val)
{
+ /* Bits 26 & 27 affect single-stepping */
+ hreg_compute_hflags(env);
+ /* Bits 28 & 29 affect reset or shutdown. */
store_40x_dbcr0(env, val);
}
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index a85b890bb0..7912495f28 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7923,17 +7923,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
if ((hflags >> HFLAGS_BE) & 1) {
ctx->singlestep_enabled |= CPU_BRANCH_STEP;
}
- if ((env->flags & POWERPC_FLAG_DE) && msr_de) {
- ctx->singlestep_enabled = 0;
- target_ulong dbcr0 = env->spr[SPR_BOOKE_DBCR0];
- if (dbcr0 & DBCR0_ICMP) {
- ctx->singlestep_enabled |= CPU_SINGLE_STEP;
- }
- if (dbcr0 & DBCR0_BRT) {
- ctx->singlestep_enabled |= CPU_BRANCH_STEP;
- }
-
- }
if (unlikely(ctx->base.singlestep_enabled)) {
ctx->singlestep_enabled |= GDBSTUB_SINGLE_STEP;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 09/17] target/ppc: Put dbcr0 single-step bits into hflags
2021-03-15 18:46 ` [PATCH v4 09/17] target/ppc: Put dbcr0 single-step bits into hflags Richard Henderson
@ 2021-03-22 3:55 ` David Gibson
0 siblings, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 3:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3224 bytes --]
On Mon, Mar 15, 2021 at 12:46:07PM -0600, Richard Henderson wrote:
> Because these bits were not in hflags, the code generated
> for single-stepping on BookE was essentially random.
> Recompute hflags when storing to dbcr0.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> target/ppc/helper_regs.c | 20 +++++++++++++++-----
> target/ppc/misc_helper.c | 3 +++
> target/ppc/translate.c | 11 -----------
> 3 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index 0a746bffd7..c735540333 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -107,11 +107,21 @@ void hreg_compute_hflags(CPUPPCState *env)
> hflags |= le << MSR_LE;
> }
>
> - if (ppc_flags & POWERPC_FLAG_BE) {
> - msr_mask |= 1 << MSR_BE;
> - }
> - if (ppc_flags & POWERPC_FLAG_SE) {
> - msr_mask |= 1 << MSR_SE;
> + if (ppc_flags & POWERPC_FLAG_DE) {
> + target_ulong dbcr0 = env->spr[SPR_BOOKE_DBCR0];
> + if (dbcr0 & DBCR0_ICMP) {
> + hflags |= 1 << HFLAGS_SE;
> + }
> + if (dbcr0 & DBCR0_BRT) {
> + hflags |= 1 << HFLAGS_BE;
> + }
> + } else {
> + if (ppc_flags & POWERPC_FLAG_BE) {
> + msr_mask |= 1 << MSR_BE;
> + }
> + if (ppc_flags & POWERPC_FLAG_SE) {
> + msr_mask |= 1 << MSR_SE;
> + }
> }
>
> if (msr_is_64bit(env, msr)) {
> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
> index b04b4d7c6e..a5ee1fd63c 100644
> --- a/target/ppc/misc_helper.c
> +++ b/target/ppc/misc_helper.c
> @@ -215,6 +215,9 @@ void helper_store_403_pbr(CPUPPCState *env, uint32_t num, target_ulong value)
>
> void helper_store_40x_dbcr0(CPUPPCState *env, target_ulong val)
> {
> + /* Bits 26 & 27 affect single-stepping */
> + hreg_compute_hflags(env);
> + /* Bits 28 & 29 affect reset or shutdown. */
> store_40x_dbcr0(env, val);
> }
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index a85b890bb0..7912495f28 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7923,17 +7923,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> if ((hflags >> HFLAGS_BE) & 1) {
> ctx->singlestep_enabled |= CPU_BRANCH_STEP;
> }
> - if ((env->flags & POWERPC_FLAG_DE) && msr_de) {
> - ctx->singlestep_enabled = 0;
> - target_ulong dbcr0 = env->spr[SPR_BOOKE_DBCR0];
> - if (dbcr0 & DBCR0_ICMP) {
> - ctx->singlestep_enabled |= CPU_SINGLE_STEP;
> - }
> - if (dbcr0 & DBCR0_BRT) {
> - ctx->singlestep_enabled |= CPU_BRANCH_STEP;
> - }
> -
> - }
> if (unlikely(ctx->base.singlestep_enabled)) {
> ctx->singlestep_enabled |= GDBSTUB_SINGLE_STEP;
> }
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 10/17] target/ppc: Create helper_scv
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (8 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 09/17] target/ppc: Put dbcr0 single-step bits into hflags Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-22 4:00 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 11/17] target/ppc: Put LPCR[GTSE] in hflags Richard Henderson
` (7 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
Perform the test against FSCR_SCV at runtime, in the helper.
This means we can remove the incorrect set against SCV in
ppc_tr_init_disas_context and do not need to add an HFLAGS bit.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/helper.h | 1 +
target/ppc/excp_helper.c | 9 +++++++++
target/ppc/translate.c | 20 +++++++-------------
3 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 6a4dccf70c..513066d54d 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -13,6 +13,7 @@ DEF_HELPER_1(rfci, void, env)
DEF_HELPER_1(rfdi, void, env)
DEF_HELPER_1(rfmci, void, env)
#if defined(TARGET_PPC64)
+DEF_HELPER_2(scv, noreturn, env, i32)
DEF_HELPER_2(pminsn, void, env, i32)
DEF_HELPER_1(rfid, void, env)
DEF_HELPER_1(rfscv, void, env)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 85de7e6c90..5c95e0c103 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1130,6 +1130,15 @@ void helper_store_msr(CPUPPCState *env, target_ulong val)
}
#if defined(TARGET_PPC64)
+void helper_scv(CPUPPCState *env, uint32_t lev)
+{
+ if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) {
+ raise_exception_err(env, POWERPC_EXCP_SYSCALL_VECTORED, lev);
+ } else {
+ raise_exception_err(env, POWERPC_EXCP_FU, FSCR_IC_SCV);
+ }
+}
+
void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t insn)
{
CPUState *cs;
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 7912495f28..d48c554290 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -173,7 +173,6 @@ struct DisasContext {
bool vsx_enabled;
bool spe_enabled;
bool tm_enabled;
- bool scv_enabled;
bool gtse;
ppc_spr_t *spr_cb; /* Needed to check rights for mfspr/mtspr */
int singlestep_enabled;
@@ -4081,15 +4080,16 @@ static void gen_sc(DisasContext *ctx)
#if !defined(CONFIG_USER_ONLY)
static void gen_scv(DisasContext *ctx)
{
- uint32_t lev;
+ uint32_t lev = (ctx->opcode >> 5) & 0x7F;
- if (unlikely(!ctx->scv_enabled)) {
- gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_SCV);
- return;
+ /* Set the PC back to the faulting instruction. */
+ if (ctx->exception == POWERPC_EXCP_NONE) {
+ gen_update_nip(ctx, ctx->base.pc_next - 4);
}
+ gen_helper_scv(cpu_env, tcg_constant_i32(lev));
- lev = (ctx->opcode >> 5) & 0x7F;
- gen_exception_err(ctx, POWERPC_SYSCALL_VECTORED, lev);
+ /* This need not be exact, just not POWERPC_EXCP_NONE */
+ ctx->exception = POWERPC_SYSCALL_VECTORED;
}
#endif
#endif
@@ -7907,12 +7907,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
ctx->spe_enabled = (hflags >> HFLAGS_SPE) & 1;
ctx->altivec_enabled = (hflags >> HFLAGS_VR) & 1;
ctx->vsx_enabled = (hflags >> HFLAGS_VSX) & 1;
- if ((env->flags & POWERPC_FLAG_SCV)
- && (env->spr[SPR_FSCR] & (1ull << FSCR_SCV))) {
- ctx->scv_enabled = true;
- } else {
- ctx->scv_enabled = false;
- }
ctx->tm_enabled = (hflags >> HFLAGS_TM) & 1;
ctx->gtse = !!(env->spr[SPR_LPCR] & LPCR_GTSE);
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 10/17] target/ppc: Create helper_scv
2021-03-15 18:46 ` [PATCH v4 10/17] target/ppc: Create helper_scv Richard Henderson
@ 2021-03-22 4:00 ` David Gibson
2021-03-22 17:05 ` Richard Henderson
0 siblings, 1 reply; 51+ messages in thread
From: David Gibson @ 2021-03-22 4:00 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4003 bytes --]
On Mon, Mar 15, 2021 at 12:46:08PM -0600, Richard Henderson wrote:
> Perform the test against FSCR_SCV at runtime, in the helper.
>
> This means we can remove the incorrect set against SCV in
> ppc_tr_init_disas_context and do not need to add an HFLAGS bit.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/ppc/helper.h | 1 +
> target/ppc/excp_helper.c | 9 +++++++++
> target/ppc/translate.c | 20 +++++++-------------
> 3 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index 6a4dccf70c..513066d54d 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -13,6 +13,7 @@ DEF_HELPER_1(rfci, void, env)
> DEF_HELPER_1(rfdi, void, env)
> DEF_HELPER_1(rfmci, void, env)
> #if defined(TARGET_PPC64)
> +DEF_HELPER_2(scv, noreturn, env, i32)
> DEF_HELPER_2(pminsn, void, env, i32)
> DEF_HELPER_1(rfid, void, env)
> DEF_HELPER_1(rfscv, void, env)
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 85de7e6c90..5c95e0c103 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1130,6 +1130,15 @@ void helper_store_msr(CPUPPCState *env, target_ulong val)
> }
>
> #if defined(TARGET_PPC64)
> +void helper_scv(CPUPPCState *env, uint32_t lev)
> +{
> + if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) {
> + raise_exception_err(env, POWERPC_EXCP_SYSCALL_VECTORED, lev);
> + } else {
> + raise_exception_err(env, POWERPC_EXCP_FU, FSCR_IC_SCV);
> + }
> +}
> +
> void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t insn)
> {
> CPUState *cs;
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 7912495f28..d48c554290 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -173,7 +173,6 @@ struct DisasContext {
> bool vsx_enabled;
> bool spe_enabled;
> bool tm_enabled;
> - bool scv_enabled;
> bool gtse;
> ppc_spr_t *spr_cb; /* Needed to check rights for mfspr/mtspr */
> int singlestep_enabled;
> @@ -4081,15 +4080,16 @@ static void gen_sc(DisasContext *ctx)
> #if !defined(CONFIG_USER_ONLY)
> static void gen_scv(DisasContext *ctx)
> {
> - uint32_t lev;
> + uint32_t lev = (ctx->opcode >> 5) & 0x7F;
>
> - if (unlikely(!ctx->scv_enabled)) {
> - gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_SCV);
> - return;
> + /* Set the PC back to the faulting instruction. */
> + if (ctx->exception == POWERPC_EXCP_NONE) {
> + gen_update_nip(ctx, ctx->base.pc_next - 4);
> }
I don't quite understand this. Don't we need the NIP to be on the scv
instruction itself for the case where we get a facility unavailable
exception, but on the next instruction if we actually take the system
call? This appears to be unconditional.
> + gen_helper_scv(cpu_env, tcg_constant_i32(lev));
>
> - lev = (ctx->opcode >> 5) & 0x7F;
> - gen_exception_err(ctx, POWERPC_SYSCALL_VECTORED, lev);
> + /* This need not be exact, just not POWERPC_EXCP_NONE */
> + ctx->exception = POWERPC_SYSCALL_VECTORED;
> }
> #endif
> #endif
> @@ -7907,12 +7907,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> ctx->spe_enabled = (hflags >> HFLAGS_SPE) & 1;
> ctx->altivec_enabled = (hflags >> HFLAGS_VR) & 1;
> ctx->vsx_enabled = (hflags >> HFLAGS_VSX) & 1;
> - if ((env->flags & POWERPC_FLAG_SCV)
> - && (env->spr[SPR_FSCR] & (1ull << FSCR_SCV))) {
> - ctx->scv_enabled = true;
> - } else {
> - ctx->scv_enabled = false;
> - }
> ctx->tm_enabled = (hflags >> HFLAGS_TM) & 1;
> ctx->gtse = !!(env->spr[SPR_LPCR] & LPCR_GTSE);
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v4 10/17] target/ppc: Create helper_scv
2021-03-22 4:00 ` David Gibson
@ 2021-03-22 17:05 ` Richard Henderson
2021-03-22 23:55 ` David Gibson
0 siblings, 1 reply; 51+ messages in thread
From: Richard Henderson @ 2021-03-22 17:05 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel
On 3/21/21 10:00 PM, David Gibson wrote:
> On Mon, Mar 15, 2021 at 12:46:08PM -0600, Richard Henderson wrote:
>> Perform the test against FSCR_SCV at runtime, in the helper.
>>
>> This means we can remove the incorrect set against SCV in
>> ppc_tr_init_disas_context and do not need to add an HFLAGS bit.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> target/ppc/helper.h | 1 +
>> target/ppc/excp_helper.c | 9 +++++++++
>> target/ppc/translate.c | 20 +++++++-------------
>> 3 files changed, 17 insertions(+), 13 deletions(-)
>>
>> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
>> index 6a4dccf70c..513066d54d 100644
>> --- a/target/ppc/helper.h
>> +++ b/target/ppc/helper.h
>> @@ -13,6 +13,7 @@ DEF_HELPER_1(rfci, void, env)
>> DEF_HELPER_1(rfdi, void, env)
>> DEF_HELPER_1(rfmci, void, env)
>> #if defined(TARGET_PPC64)
>> +DEF_HELPER_2(scv, noreturn, env, i32)
>> DEF_HELPER_2(pminsn, void, env, i32)
>> DEF_HELPER_1(rfid, void, env)
>> DEF_HELPER_1(rfscv, void, env)
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 85de7e6c90..5c95e0c103 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -1130,6 +1130,15 @@ void helper_store_msr(CPUPPCState *env, target_ulong val)
>> }
>>
>> #if defined(TARGET_PPC64)
>> +void helper_scv(CPUPPCState *env, uint32_t lev)
>> +{
>> + if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) {
>> + raise_exception_err(env, POWERPC_EXCP_SYSCALL_VECTORED, lev);
>> + } else {
>> + raise_exception_err(env, POWERPC_EXCP_FU, FSCR_IC_SCV);
>> + }
>> +}
>> +
>> void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t insn)
>> {
>> CPUState *cs;
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index 7912495f28..d48c554290 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -173,7 +173,6 @@ struct DisasContext {
>> bool vsx_enabled;
>> bool spe_enabled;
>> bool tm_enabled;
>> - bool scv_enabled;
>> bool gtse;
>> ppc_spr_t *spr_cb; /* Needed to check rights for mfspr/mtspr */
>> int singlestep_enabled;
>> @@ -4081,15 +4080,16 @@ static void gen_sc(DisasContext *ctx)
>> #if !defined(CONFIG_USER_ONLY)
>> static void gen_scv(DisasContext *ctx)
>> {
>> - uint32_t lev;
>> + uint32_t lev = (ctx->opcode >> 5) & 0x7F;
>>
>> - if (unlikely(!ctx->scv_enabled)) {
>> - gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_SCV);
>> - return;
>> + /* Set the PC back to the faulting instruction. */
>> + if (ctx->exception == POWERPC_EXCP_NONE) {
>> + gen_update_nip(ctx, ctx->base.pc_next - 4);
>> }
>
> I don't quite understand this. Don't we need the NIP to be on the scv
> instruction itself for the case where we get a facility unavailable
> exception, but on the next instruction if we actually take the system
> call? This appears to be unconditional.
>
>> + gen_helper_scv(cpu_env, tcg_constant_i32(lev));
>>
>> - lev = (ctx->opcode >> 5) & 0x7F;
>> - gen_exception_err(ctx, POWERPC_SYSCALL_VECTORED, lev);
Hmm. In the old code, both paths use gen_exception_err, without otherwise
manipulating NIP. That suggests to me that both exceptions receive the same
value in NIP.
Is there an adjustment to NIP when delivering the SCV exception? Yep:
case POWERPC_EXCP_SYSCALL_VECTORED:
lev = env->error_code;
dump_syscall_vectored(env);
env->nip += 4;
new_msr |= env->msr & ((target_ulong)1 << MSR_EE);
new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
break;
r~
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v4 10/17] target/ppc: Create helper_scv
2021-03-22 17:05 ` Richard Henderson
@ 2021-03-22 23:55 ` David Gibson
0 siblings, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 23:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4330 bytes --]
On Mon, Mar 22, 2021 at 11:05:00AM -0600, Richard Henderson wrote:
> On 3/21/21 10:00 PM, David Gibson wrote:
> > On Mon, Mar 15, 2021 at 12:46:08PM -0600, Richard Henderson wrote:
> > > Perform the test against FSCR_SCV at runtime, in the helper.
> > >
> > > This means we can remove the incorrect set against SCV in
> > > ppc_tr_init_disas_context and do not need to add an HFLAGS bit.
> > >
> > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > > ---
> > > target/ppc/helper.h | 1 +
> > > target/ppc/excp_helper.c | 9 +++++++++
> > > target/ppc/translate.c | 20 +++++++-------------
> > > 3 files changed, 17 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> > > index 6a4dccf70c..513066d54d 100644
> > > --- a/target/ppc/helper.h
> > > +++ b/target/ppc/helper.h
> > > @@ -13,6 +13,7 @@ DEF_HELPER_1(rfci, void, env)
> > > DEF_HELPER_1(rfdi, void, env)
> > > DEF_HELPER_1(rfmci, void, env)
> > > #if defined(TARGET_PPC64)
> > > +DEF_HELPER_2(scv, noreturn, env, i32)
> > > DEF_HELPER_2(pminsn, void, env, i32)
> > > DEF_HELPER_1(rfid, void, env)
> > > DEF_HELPER_1(rfscv, void, env)
> > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > > index 85de7e6c90..5c95e0c103 100644
> > > --- a/target/ppc/excp_helper.c
> > > +++ b/target/ppc/excp_helper.c
> > > @@ -1130,6 +1130,15 @@ void helper_store_msr(CPUPPCState *env, target_ulong val)
> > > }
> > > #if defined(TARGET_PPC64)
> > > +void helper_scv(CPUPPCState *env, uint32_t lev)
> > > +{
> > > + if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) {
> > > + raise_exception_err(env, POWERPC_EXCP_SYSCALL_VECTORED, lev);
> > > + } else {
> > > + raise_exception_err(env, POWERPC_EXCP_FU, FSCR_IC_SCV);
> > > + }
> > > +}
> > > +
> > > void helper_pminsn(CPUPPCState *env, powerpc_pm_insn_t insn)
> > > {
> > > CPUState *cs;
> > > diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> > > index 7912495f28..d48c554290 100644
> > > --- a/target/ppc/translate.c
> > > +++ b/target/ppc/translate.c
> > > @@ -173,7 +173,6 @@ struct DisasContext {
> > > bool vsx_enabled;
> > > bool spe_enabled;
> > > bool tm_enabled;
> > > - bool scv_enabled;
> > > bool gtse;
> > > ppc_spr_t *spr_cb; /* Needed to check rights for mfspr/mtspr */
> > > int singlestep_enabled;
> > > @@ -4081,15 +4080,16 @@ static void gen_sc(DisasContext *ctx)
> > > #if !defined(CONFIG_USER_ONLY)
> > > static void gen_scv(DisasContext *ctx)
> > > {
> > > - uint32_t lev;
> > > + uint32_t lev = (ctx->opcode >> 5) & 0x7F;
> > > - if (unlikely(!ctx->scv_enabled)) {
> > > - gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_SCV);
> > > - return;
> > > + /* Set the PC back to the faulting instruction. */
> > > + if (ctx->exception == POWERPC_EXCP_NONE) {
> > > + gen_update_nip(ctx, ctx->base.pc_next - 4);
> > > }
> >
> > I don't quite understand this. Don't we need the NIP to be on the scv
> > instruction itself for the case where we get a facility unavailable
> > exception, but on the next instruction if we actually take the system
> > call? This appears to be unconditional.
> >
> > > + gen_helper_scv(cpu_env, tcg_constant_i32(lev));
> > > - lev = (ctx->opcode >> 5) & 0x7F;
> > > - gen_exception_err(ctx, POWERPC_SYSCALL_VECTORED, lev);
>
> Hmm. In the old code, both paths use gen_exception_err, without otherwise
> manipulating NIP. That suggests to me that both exceptions receive the same
> value in NIP.
>
> Is there an adjustment to NIP when delivering the SCV exception? Yep:
Ok. Just shows my ignorance of the exception handling code.
>
> case POWERPC_EXCP_SYSCALL_VECTORED:
> lev = env->error_code;
> dump_syscall_vectored(env);
> env->nip += 4;
> new_msr |= env->msr & ((target_ulong)1 << MSR_EE);
> new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
> break;
>
>
> r~
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 11/17] target/ppc: Put LPCR[GTSE] in hflags
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (9 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 10/17] target/ppc: Create helper_scv Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-22 4:18 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 12/17] target/ppc: Remove MSR_SA and MSR_AP from hflags Richard Henderson
` (6 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
Because this bit was not in hflags, the privilege check
for tlb instructions was essentially random.
Recompute hflags when storing to LPCR.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/cpu.h | 1 +
target/ppc/helper_regs.c | 3 +++
target/ppc/mmu-hash64.c | 3 +++
target/ppc/translate.c | 2 +-
4 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 2abaf56869..07a4331eec 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -603,6 +603,7 @@ enum {
HFLAGS_TM = 8, /* computed from MSR_TM */
HFLAGS_BE = 9, /* MSR_BE -- from elsewhere on embedded ppc */
HFLAGS_SE = 10, /* MSR_SE -- from elsewhere on embedded ppc */
+ HFLAGS_GTSE = 11, /* computed from SPR_LPCR[GTSE] */
HFLAGS_FP = 13, /* MSR_FP */
HFLAGS_SA = 22, /* MSR_SA */
HFLAGS_AP = 23, /* MSR_AP */
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index c735540333..8479789e24 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -139,6 +139,9 @@ void hreg_compute_hflags(CPUPPCState *env)
if ((ppc_flags & POWERPC_FLAG_TM) && (msr & (1ull << MSR_TM))) {
hflags |= 1 << HFLAGS_TM;
}
+ if (env->spr[SPR_LPCR] & LPCR_GTSE) {
+ hflags |= 1 << HFLAGS_GTSE;
+ }
#ifndef CONFIG_USER_ONLY
if (!env->has_hv_mode || (msr & (1ull << MSR_HV))) {
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 0fabc10302..d517a99832 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -30,6 +30,7 @@
#include "exec/log.h"
#include "hw/hw.h"
#include "mmu-book3s-v3.h"
+#include "helper_regs.h"
/* #define DEBUG_SLB */
@@ -1125,6 +1126,8 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
CPUPPCState *env = &cpu->env;
env->spr[SPR_LPCR] = val & pcc->lpcr_mask;
+ /* The gtse bit affects hflags */
+ hreg_compute_hflags(env);
}
void helper_store_lpcr(CPUPPCState *env, target_ulong val)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index d48c554290..5e629291d3 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7908,7 +7908,7 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
ctx->altivec_enabled = (hflags >> HFLAGS_VR) & 1;
ctx->vsx_enabled = (hflags >> HFLAGS_VSX) & 1;
ctx->tm_enabled = (hflags >> HFLAGS_TM) & 1;
- ctx->gtse = !!(env->spr[SPR_LPCR] & LPCR_GTSE);
+ ctx->gtse = (hflags >> HFLAGS_GTSE) & 1;
ctx->singlestep_enabled = 0;
if ((hflags >> HFLAGS_SE) & 1) {
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 11/17] target/ppc: Put LPCR[GTSE] in hflags
2021-03-15 18:46 ` [PATCH v4 11/17] target/ppc: Put LPCR[GTSE] in hflags Richard Henderson
@ 2021-03-22 4:18 ` David Gibson
0 siblings, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 4:18 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3337 bytes --]
On Mon, Mar 15, 2021 at 12:46:09PM -0600, Richard Henderson wrote:
> Because this bit was not in hflags, the privilege check
> for tlb instructions was essentially random.
> Recompute hflags when storing to LPCR.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Ouch. Unlike the others which come from ancient strata of qemu code,
this one is pretty recent, and demonstrates that I don't really
understand how hflags and TCG code generation work. Anyway,
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> target/ppc/cpu.h | 1 +
> target/ppc/helper_regs.c | 3 +++
> target/ppc/mmu-hash64.c | 3 +++
> target/ppc/translate.c | 2 +-
> 4 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 2abaf56869..07a4331eec 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -603,6 +603,7 @@ enum {
> HFLAGS_TM = 8, /* computed from MSR_TM */
> HFLAGS_BE = 9, /* MSR_BE -- from elsewhere on embedded ppc */
> HFLAGS_SE = 10, /* MSR_SE -- from elsewhere on embedded ppc */
> + HFLAGS_GTSE = 11, /* computed from SPR_LPCR[GTSE] */
> HFLAGS_FP = 13, /* MSR_FP */
> HFLAGS_SA = 22, /* MSR_SA */
> HFLAGS_AP = 23, /* MSR_AP */
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index c735540333..8479789e24 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -139,6 +139,9 @@ void hreg_compute_hflags(CPUPPCState *env)
> if ((ppc_flags & POWERPC_FLAG_TM) && (msr & (1ull << MSR_TM))) {
> hflags |= 1 << HFLAGS_TM;
> }
> + if (env->spr[SPR_LPCR] & LPCR_GTSE) {
> + hflags |= 1 << HFLAGS_GTSE;
> + }
>
> #ifndef CONFIG_USER_ONLY
> if (!env->has_hv_mode || (msr & (1ull << MSR_HV))) {
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index 0fabc10302..d517a99832 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -30,6 +30,7 @@
> #include "exec/log.h"
> #include "hw/hw.h"
> #include "mmu-book3s-v3.h"
> +#include "helper_regs.h"
>
> /* #define DEBUG_SLB */
>
> @@ -1125,6 +1126,8 @@ void ppc_store_lpcr(PowerPCCPU *cpu, target_ulong val)
> CPUPPCState *env = &cpu->env;
>
> env->spr[SPR_LPCR] = val & pcc->lpcr_mask;
> + /* The gtse bit affects hflags */
> + hreg_compute_hflags(env);
> }
>
> void helper_store_lpcr(CPUPPCState *env, target_ulong val)
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index d48c554290..5e629291d3 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7908,7 +7908,7 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> ctx->altivec_enabled = (hflags >> HFLAGS_VR) & 1;
> ctx->vsx_enabled = (hflags >> HFLAGS_VSX) & 1;
> ctx->tm_enabled = (hflags >> HFLAGS_TM) & 1;
> - ctx->gtse = !!(env->spr[SPR_LPCR] & LPCR_GTSE);
> + ctx->gtse = (hflags >> HFLAGS_GTSE) & 1;
>
> ctx->singlestep_enabled = 0;
> if ((hflags >> HFLAGS_SE) & 1) {
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 12/17] target/ppc: Remove MSR_SA and MSR_AP from hflags
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (10 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 11/17] target/ppc: Put LPCR[GTSE] in hflags Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-22 4:20 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 13/17] target/ppc: Remove env->immu_idx and env->dmmu_idx Richard Henderson
` (5 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
Nothing within the translator -- or anywhere else for that
matter -- checks MSR_SA or MSR_AP on the 602. This may be
a mistake. However, for the moment, we need not record these
bits in hflags.
This allows us to simplify HFLAGS_VSX computation by moving
it to overlap with MSR_VSX.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/cpu.h | 4 +---
target/ppc/helper_regs.c | 7 +++----
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 07a4331eec..23ff16c154 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -599,14 +599,12 @@ enum {
HFLAGS_DR = 4, /* MSR_DR */
HFLAGS_IR = 5, /* MSR_IR */
HFLAGS_SPE = 6, /* from MSR_SPE if cpu has SPE; avoid overlap w/ MSR_VR */
- HFLAGS_VSX = 7, /* from MSR_VSX if cpu has VSX; avoid overlap w/ MSR_AP */
HFLAGS_TM = 8, /* computed from MSR_TM */
HFLAGS_BE = 9, /* MSR_BE -- from elsewhere on embedded ppc */
HFLAGS_SE = 10, /* MSR_SE -- from elsewhere on embedded ppc */
HFLAGS_GTSE = 11, /* computed from SPR_LPCR[GTSE] */
HFLAGS_FP = 13, /* MSR_FP */
- HFLAGS_SA = 22, /* MSR_SA */
- HFLAGS_AP = 23, /* MSR_AP */
+ HFLAGS_VSX = 23, /* MSR_VSX if cpu has VSX */
HFLAGS_VR = 25, /* MSR_VR if cpu has VRE */
};
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 8479789e24..d62921c322 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -95,8 +95,7 @@ void hreg_compute_hflags(CPUPPCState *env)
/* Some bits come straight across from MSR. */
msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |
- (1 << MSR_DR) | (1 << MSR_IR) |
- (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));
+ (1 << MSR_DR) | (1 << MSR_IR) | (1 << MSR_FP));
if (ppc_flags & POWERPC_FLAG_HID0_LE) {
/*
@@ -133,8 +132,8 @@ void hreg_compute_hflags(CPUPPCState *env)
if (ppc_flags & POWERPC_FLAG_VRE) {
msr_mask |= 1 << MSR_VR;
}
- if ((ppc_flags & POWERPC_FLAG_VSX) && (msr & (1 << MSR_VSX))) {
- hflags |= 1 << HFLAGS_VSX;
+ if (ppc_flags & POWERPC_FLAG_VSX) {
+ msr_mask |= 1 << MSR_VSX;
}
if ((ppc_flags & POWERPC_FLAG_TM) && (msr & (1ull << MSR_TM))) {
hflags |= 1 << HFLAGS_TM;
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 12/17] target/ppc: Remove MSR_SA and MSR_AP from hflags
2021-03-15 18:46 ` [PATCH v4 12/17] target/ppc: Remove MSR_SA and MSR_AP from hflags Richard Henderson
@ 2021-03-22 4:20 ` David Gibson
0 siblings, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 4:20 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2951 bytes --]
On Mon, Mar 15, 2021 at 12:46:10PM -0600, Richard Henderson wrote:
> Nothing within the translator -- or anywhere else for that
> matter -- checks MSR_SA or MSR_AP on the 602. This may be
> a mistake. However, for the moment, we need not record these
> bits in hflags.
>
> This allows us to simplify HFLAGS_VSX computation by moving
> it to overlap with MSR_VSX.
This leans into the requirement that certain hflags and msr bits line
up, which makes me nervous. Apart from that
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/ppc/cpu.h | 4 +---
> target/ppc/helper_regs.c | 7 +++----
> 2 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 07a4331eec..23ff16c154 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -599,14 +599,12 @@ enum {
> HFLAGS_DR = 4, /* MSR_DR */
> HFLAGS_IR = 5, /* MSR_IR */
> HFLAGS_SPE = 6, /* from MSR_SPE if cpu has SPE; avoid overlap w/ MSR_VR */
> - HFLAGS_VSX = 7, /* from MSR_VSX if cpu has VSX; avoid overlap w/ MSR_AP */
> HFLAGS_TM = 8, /* computed from MSR_TM */
> HFLAGS_BE = 9, /* MSR_BE -- from elsewhere on embedded ppc */
> HFLAGS_SE = 10, /* MSR_SE -- from elsewhere on embedded ppc */
> HFLAGS_GTSE = 11, /* computed from SPR_LPCR[GTSE] */
> HFLAGS_FP = 13, /* MSR_FP */
> - HFLAGS_SA = 22, /* MSR_SA */
> - HFLAGS_AP = 23, /* MSR_AP */
> + HFLAGS_VSX = 23, /* MSR_VSX if cpu has VSX */
> HFLAGS_VR = 25, /* MSR_VR if cpu has VRE */
> };
>
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index 8479789e24..d62921c322 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -95,8 +95,7 @@ void hreg_compute_hflags(CPUPPCState *env)
>
> /* Some bits come straight across from MSR. */
> msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |
> - (1 << MSR_DR) | (1 << MSR_IR) |
> - (1 << MSR_FP) | (1 << MSR_SA) | (1 << MSR_AP));
> + (1 << MSR_DR) | (1 << MSR_IR) | (1 << MSR_FP));
>
> if (ppc_flags & POWERPC_FLAG_HID0_LE) {
> /*
> @@ -133,8 +132,8 @@ void hreg_compute_hflags(CPUPPCState *env)
> if (ppc_flags & POWERPC_FLAG_VRE) {
> msr_mask |= 1 << MSR_VR;
> }
> - if ((ppc_flags & POWERPC_FLAG_VSX) && (msr & (1 << MSR_VSX))) {
> - hflags |= 1 << HFLAGS_VSX;
> + if (ppc_flags & POWERPC_FLAG_VSX) {
> + msr_mask |= 1 << MSR_VSX;
> }
> if ((ppc_flags & POWERPC_FLAG_TM) && (msr & (1ull << MSR_TM))) {
> hflags |= 1 << HFLAGS_TM;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 13/17] target/ppc: Remove env->immu_idx and env->dmmu_idx
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (11 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 12/17] target/ppc: Remove MSR_SA and MSR_AP from hflags Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-22 4:26 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 14/17] hw/ppc/pnv_core: Update hflags after setting msr Richard Henderson
` (4 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
We weren't recording MSR_GS in hflags, which means that BookE
memory accesses were essentially random vs Guest State.
Instead of adding this bit directly, record the completed mmu
indexes instead. This makes it obvious that we are recording
exactly the information that we need.
This also means that we can stop directly recording MSR_IR.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/cpu.h | 12 ++++--
target/ppc/helper_regs.h | 1 -
target/ppc/helper_regs.c | 88 ++++++++++++++++++++--------------------
target/ppc/mem_helper.c | 2 +-
target/ppc/translate.c | 6 +--
5 files changed, 55 insertions(+), 54 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 23ff16c154..2f8d7fa13c 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -597,7 +597,6 @@ enum {
HFLAGS_64 = 2, /* computed from MSR_CE and MSR_SF */
HFLAGS_PR = 3, /* MSR_PR */
HFLAGS_DR = 4, /* MSR_DR */
- HFLAGS_IR = 5, /* MSR_IR */
HFLAGS_SPE = 6, /* from MSR_SPE if cpu has SPE; avoid overlap w/ MSR_VR */
HFLAGS_TM = 8, /* computed from MSR_TM */
HFLAGS_BE = 9, /* MSR_BE -- from elsewhere on embedded ppc */
@@ -606,6 +605,9 @@ enum {
HFLAGS_FP = 13, /* MSR_FP */
HFLAGS_VSX = 23, /* MSR_VSX if cpu has VSX */
HFLAGS_VR = 25, /* MSR_VR if cpu has VRE */
+
+ HFLAGS_IMMU_IDX = 26, /* 26..28 -- the composite immu_idx */
+ HFLAGS_DMMU_IDX = 29, /* 29..31 -- the composite dmmu_idx */
};
/*****************************************************************************/
@@ -1130,8 +1132,6 @@ struct CPUPPCState {
/* These resources are used only in TCG */
uint32_t hflags;
target_ulong hflags_compat_nmsr; /* for migration compatibility */
- int immu_idx; /* precomputed MMU index to speed up insn accesses */
- int dmmu_idx; /* precomputed MMU index to speed up data accesses */
/* Power management */
int (*check_pow)(CPUPPCState *env);
@@ -1367,7 +1367,11 @@ int ppc_dcr_write(ppc_dcr_t *dcr_env, int dcrn, uint32_t val);
#define MMU_USER_IDX 0
static inline int cpu_mmu_index(CPUPPCState *env, bool ifetch)
{
- return ifetch ? env->immu_idx : env->dmmu_idx;
+#ifdef CONFIG_USER_ONLY
+ return MMU_USER_IDX;
+#else
+ return (env->hflags >> (ifetch ? HFLAGS_IMMU_IDX : HFLAGS_DMMU_IDX)) & 7;
+#endif
}
/* Compatibility modes */
diff --git a/target/ppc/helper_regs.h b/target/ppc/helper_regs.h
index 4148a442b3..42f26870b9 100644
--- a/target/ppc/helper_regs.h
+++ b/target/ppc/helper_regs.h
@@ -21,7 +21,6 @@
#define HELPER_REGS_H
void hreg_swap_gpr_tgpr(CPUPPCState *env);
-void hreg_compute_mem_idx(CPUPPCState *env);
void hreg_compute_hflags(CPUPPCState *env);
void cpu_interrupt_exittb(CPUState *cs);
int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv);
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index d62921c322..b28037ca24 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -43,49 +43,6 @@ void hreg_swap_gpr_tgpr(CPUPPCState *env)
env->tgpr[3] = tmp;
}
-void hreg_compute_mem_idx(CPUPPCState *env)
-{
- /*
- * This is our encoding for server processors. The architecture
- * specifies that there is no such thing as userspace with
- * translation off, however it appears that MacOS does it and some
- * 32-bit CPUs support it. Weird...
- *
- * 0 = Guest User space virtual mode
- * 1 = Guest Kernel space virtual mode
- * 2 = Guest User space real mode
- * 3 = Guest Kernel space real mode
- * 4 = HV User space virtual mode
- * 5 = HV Kernel space virtual mode
- * 6 = HV User space real mode
- * 7 = HV Kernel space real mode
- *
- * For BookE, we need 8 MMU modes as follow:
- *
- * 0 = AS 0 HV User space
- * 1 = AS 0 HV Kernel space
- * 2 = AS 1 HV User space
- * 3 = AS 1 HV Kernel space
- * 4 = AS 0 Guest User space
- * 5 = AS 0 Guest Kernel space
- * 6 = AS 1 Guest User space
- * 7 = AS 1 Guest Kernel space
- */
- if (env->mmu_model & POWERPC_MMU_BOOKE) {
- env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
- env->immu_idx += msr_is ? 2 : 0;
- env->dmmu_idx += msr_ds ? 2 : 0;
- env->immu_idx += msr_gs ? 4 : 0;
- env->dmmu_idx += msr_gs ? 4 : 0;
- } else {
- env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
- env->immu_idx += msr_ir ? 0 : 2;
- env->dmmu_idx += msr_dr ? 0 : 2;
- env->immu_idx += msr_hv ? 4 : 0;
- env->dmmu_idx += msr_hv ? 4 : 0;
- }
-}
-
void hreg_compute_hflags(CPUPPCState *env)
{
target_ulong msr = env->msr;
@@ -95,7 +52,7 @@ void hreg_compute_hflags(CPUPPCState *env)
/* Some bits come straight across from MSR. */
msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |
- (1 << MSR_DR) | (1 << MSR_IR) | (1 << MSR_FP));
+ (1 << MSR_DR) | (1 << MSR_FP));
if (ppc_flags & POWERPC_FLAG_HID0_LE) {
/*
@@ -146,10 +103,51 @@ void hreg_compute_hflags(CPUPPCState *env)
if (!env->has_hv_mode || (msr & (1ull << MSR_HV))) {
hflags |= 1 << HFLAGS_HV;
}
+
+ /*
+ * This is our encoding for server processors. The architecture
+ * specifies that there is no such thing as userspace with
+ * translation off, however it appears that MacOS does it and some
+ * 32-bit CPUs support it. Weird...
+ *
+ * 0 = Guest User space virtual mode
+ * 1 = Guest Kernel space virtual mode
+ * 2 = Guest User space real mode
+ * 3 = Guest Kernel space real mode
+ * 4 = HV User space virtual mode
+ * 5 = HV Kernel space virtual mode
+ * 6 = HV User space real mode
+ * 7 = HV Kernel space real mode
+ *
+ * For BookE, we need 8 MMU modes as follow:
+ *
+ * 0 = AS 0 HV User space
+ * 1 = AS 0 HV Kernel space
+ * 2 = AS 1 HV User space
+ * 3 = AS 1 HV Kernel space
+ * 4 = AS 0 Guest User space
+ * 5 = AS 0 Guest Kernel space
+ * 6 = AS 1 Guest User space
+ * 7 = AS 1 Guest Kernel space
+ */
+ unsigned immu_idx, dmmu_idx;
+ dmmu_idx = msr & (1 << MSR_PR) ? 0 : 1;
+ if (env->mmu_model & POWERPC_MMU_BOOKE) {
+ dmmu_idx |= msr & (1 << MSR_GS) ? 4 : 0;
+ immu_idx = dmmu_idx;
+ immu_idx |= msr & (1 << MSR_IS) ? 2 : 0;
+ dmmu_idx |= msr & (1 << MSR_DS) ? 2 : 0;
+ } else {
+ dmmu_idx |= msr & (1ull << MSR_HV) ? 4 : 0;
+ immu_idx = dmmu_idx;
+ immu_idx |= msr & (1 << MSR_IR) ? 0 : 2;
+ dmmu_idx |= msr & (1 << MSR_DR) ? 0 : 2;
+ }
+ hflags |= immu_idx << HFLAGS_IMMU_IDX;
+ hflags |= dmmu_idx << HFLAGS_DMMU_IDX;
#endif
env->hflags = hflags | (msr & msr_mask);
- hreg_compute_mem_idx(env);
}
void cpu_interrupt_exittb(CPUState *cs)
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index f4f7e730de..444b2a30ef 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -278,7 +278,7 @@ static void dcbz_common(CPUPPCState *env, target_ulong addr,
target_ulong mask, dcbz_size = env->dcache_line_size;
uint32_t i;
void *haddr;
- int mmu_idx = epid ? PPC_TLB_EPID_STORE : env->dmmu_idx;
+ int mmu_idx = epid ? PPC_TLB_EPID_STORE : cpu_mmu_index(env, false);
#if defined(TARGET_PPC64)
/* Check for dcbz vs dcbzl on 970 */
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 5e629291d3..a53463b9b8 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7658,8 +7658,8 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
cs->cpu_index);
qemu_fprintf(f, "MSR " TARGET_FMT_lx " HID0 " TARGET_FMT_lx " HF "
"%08x iidx %d didx %d\n",
- env->msr, env->spr[SPR_HID0],
- env->hflags, env->immu_idx, env->dmmu_idx);
+ env->msr, env->spr[SPR_HID0], env->hflags,
+ cpu_mmu_index(env, true), cpu_mmu_index(env, false));
#if !defined(NO_TIMER_DUMP)
qemu_fprintf(f, "TB %08" PRIu32 " %08" PRIu64
#if !defined(CONFIG_USER_ONLY)
@@ -7885,7 +7885,7 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
ctx->exception = POWERPC_EXCP_NONE;
ctx->spr_cb = env->spr_cb;
ctx->pr = (hflags >> HFLAGS_PR) & 1;
- ctx->mem_idx = env->dmmu_idx;
+ ctx->mem_idx = (hflags >> HFLAGS_DMMU_IDX) & 7;
ctx->dr = (hflags >> HFLAGS_DR) & 1;
ctx->hv = (hflags >> HFLAGS_HV) & 1;
ctx->insns_flags = env->insns_flags;
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 13/17] target/ppc: Remove env->immu_idx and env->dmmu_idx
2021-03-15 18:46 ` [PATCH v4 13/17] target/ppc: Remove env->immu_idx and env->dmmu_idx Richard Henderson
@ 2021-03-22 4:26 ` David Gibson
2021-03-22 17:27 ` Richard Henderson
0 siblings, 1 reply; 51+ messages in thread
From: David Gibson @ 2021-03-22 4:26 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 9669 bytes --]
On Mon, Mar 15, 2021 at 12:46:11PM -0600, Richard Henderson wrote:
> We weren't recording MSR_GS in hflags, which means that BookE
> memory accesses were essentially random vs Guest State.
>
> Instead of adding this bit directly, record the completed mmu
> indexes instead. This makes it obvious that we are recording
> exactly the information that we need.
>
> This also means that we can stop directly recording MSR_IR.
What still uses MSR_DR, that you can't also drop it?
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> target/ppc/cpu.h | 12 ++++--
> target/ppc/helper_regs.h | 1 -
> target/ppc/helper_regs.c | 88 ++++++++++++++++++++--------------------
> target/ppc/mem_helper.c | 2 +-
> target/ppc/translate.c | 6 +--
> 5 files changed, 55 insertions(+), 54 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 23ff16c154..2f8d7fa13c 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -597,7 +597,6 @@ enum {
> HFLAGS_64 = 2, /* computed from MSR_CE and MSR_SF */
> HFLAGS_PR = 3, /* MSR_PR */
> HFLAGS_DR = 4, /* MSR_DR */
> - HFLAGS_IR = 5, /* MSR_IR */
> HFLAGS_SPE = 6, /* from MSR_SPE if cpu has SPE; avoid overlap w/ MSR_VR */
> HFLAGS_TM = 8, /* computed from MSR_TM */
> HFLAGS_BE = 9, /* MSR_BE -- from elsewhere on embedded ppc */
> @@ -606,6 +605,9 @@ enum {
> HFLAGS_FP = 13, /* MSR_FP */
> HFLAGS_VSX = 23, /* MSR_VSX if cpu has VSX */
> HFLAGS_VR = 25, /* MSR_VR if cpu has VRE */
> +
> + HFLAGS_IMMU_IDX = 26, /* 26..28 -- the composite immu_idx */
> + HFLAGS_DMMU_IDX = 29, /* 29..31 -- the composite dmmu_idx */
> };
>
> /*****************************************************************************/
> @@ -1130,8 +1132,6 @@ struct CPUPPCState {
> /* These resources are used only in TCG */
> uint32_t hflags;
> target_ulong hflags_compat_nmsr; /* for migration compatibility */
> - int immu_idx; /* precomputed MMU index to speed up insn accesses */
> - int dmmu_idx; /* precomputed MMU index to speed up data accesses */
>
> /* Power management */
> int (*check_pow)(CPUPPCState *env);
> @@ -1367,7 +1367,11 @@ int ppc_dcr_write(ppc_dcr_t *dcr_env, int dcrn, uint32_t val);
> #define MMU_USER_IDX 0
> static inline int cpu_mmu_index(CPUPPCState *env, bool ifetch)
> {
> - return ifetch ? env->immu_idx : env->dmmu_idx;
> +#ifdef CONFIG_USER_ONLY
> + return MMU_USER_IDX;
> +#else
> + return (env->hflags >> (ifetch ? HFLAGS_IMMU_IDX : HFLAGS_DMMU_IDX)) & 7;
> +#endif
> }
>
> /* Compatibility modes */
> diff --git a/target/ppc/helper_regs.h b/target/ppc/helper_regs.h
> index 4148a442b3..42f26870b9 100644
> --- a/target/ppc/helper_regs.h
> +++ b/target/ppc/helper_regs.h
> @@ -21,7 +21,6 @@
> #define HELPER_REGS_H
>
> void hreg_swap_gpr_tgpr(CPUPPCState *env);
> -void hreg_compute_mem_idx(CPUPPCState *env);
> void hreg_compute_hflags(CPUPPCState *env);
> void cpu_interrupt_exittb(CPUState *cs);
> int hreg_store_msr(CPUPPCState *env, target_ulong value, int alter_hv);
> diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
> index d62921c322..b28037ca24 100644
> --- a/target/ppc/helper_regs.c
> +++ b/target/ppc/helper_regs.c
> @@ -43,49 +43,6 @@ void hreg_swap_gpr_tgpr(CPUPPCState *env)
> env->tgpr[3] = tmp;
> }
>
> -void hreg_compute_mem_idx(CPUPPCState *env)
> -{
> - /*
> - * This is our encoding for server processors. The architecture
> - * specifies that there is no such thing as userspace with
> - * translation off, however it appears that MacOS does it and some
> - * 32-bit CPUs support it. Weird...
> - *
> - * 0 = Guest User space virtual mode
> - * 1 = Guest Kernel space virtual mode
> - * 2 = Guest User space real mode
> - * 3 = Guest Kernel space real mode
> - * 4 = HV User space virtual mode
> - * 5 = HV Kernel space virtual mode
> - * 6 = HV User space real mode
> - * 7 = HV Kernel space real mode
> - *
> - * For BookE, we need 8 MMU modes as follow:
> - *
> - * 0 = AS 0 HV User space
> - * 1 = AS 0 HV Kernel space
> - * 2 = AS 1 HV User space
> - * 3 = AS 1 HV Kernel space
> - * 4 = AS 0 Guest User space
> - * 5 = AS 0 Guest Kernel space
> - * 6 = AS 1 Guest User space
> - * 7 = AS 1 Guest Kernel space
> - */
> - if (env->mmu_model & POWERPC_MMU_BOOKE) {
> - env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
> - env->immu_idx += msr_is ? 2 : 0;
> - env->dmmu_idx += msr_ds ? 2 : 0;
> - env->immu_idx += msr_gs ? 4 : 0;
> - env->dmmu_idx += msr_gs ? 4 : 0;
> - } else {
> - env->immu_idx = env->dmmu_idx = msr_pr ? 0 : 1;
> - env->immu_idx += msr_ir ? 0 : 2;
> - env->dmmu_idx += msr_dr ? 0 : 2;
> - env->immu_idx += msr_hv ? 4 : 0;
> - env->dmmu_idx += msr_hv ? 4 : 0;
> - }
> -}
> -
> void hreg_compute_hflags(CPUPPCState *env)
> {
> target_ulong msr = env->msr;
> @@ -95,7 +52,7 @@ void hreg_compute_hflags(CPUPPCState *env)
>
> /* Some bits come straight across from MSR. */
> msr_mask = ((1 << MSR_LE) | (1 << MSR_PR) |
> - (1 << MSR_DR) | (1 << MSR_IR) | (1 << MSR_FP));
> + (1 << MSR_DR) | (1 << MSR_FP));
>
> if (ppc_flags & POWERPC_FLAG_HID0_LE) {
> /*
> @@ -146,10 +103,51 @@ void hreg_compute_hflags(CPUPPCState *env)
> if (!env->has_hv_mode || (msr & (1ull << MSR_HV))) {
> hflags |= 1 << HFLAGS_HV;
> }
> +
> + /*
> + * This is our encoding for server processors. The architecture
> + * specifies that there is no such thing as userspace with
> + * translation off, however it appears that MacOS does it and some
> + * 32-bit CPUs support it. Weird...
> + *
> + * 0 = Guest User space virtual mode
> + * 1 = Guest Kernel space virtual mode
> + * 2 = Guest User space real mode
> + * 3 = Guest Kernel space real mode
> + * 4 = HV User space virtual mode
> + * 5 = HV Kernel space virtual mode
> + * 6 = HV User space real mode
> + * 7 = HV Kernel space real mode
> + *
> + * For BookE, we need 8 MMU modes as follow:
> + *
> + * 0 = AS 0 HV User space
> + * 1 = AS 0 HV Kernel space
> + * 2 = AS 1 HV User space
> + * 3 = AS 1 HV Kernel space
> + * 4 = AS 0 Guest User space
> + * 5 = AS 0 Guest Kernel space
> + * 6 = AS 1 Guest User space
> + * 7 = AS 1 Guest Kernel space
> + */
> + unsigned immu_idx, dmmu_idx;
> + dmmu_idx = msr & (1 << MSR_PR) ? 0 : 1;
> + if (env->mmu_model & POWERPC_MMU_BOOKE) {
> + dmmu_idx |= msr & (1 << MSR_GS) ? 4 : 0;
> + immu_idx = dmmu_idx;
> + immu_idx |= msr & (1 << MSR_IS) ? 2 : 0;
> + dmmu_idx |= msr & (1 << MSR_DS) ? 2 : 0;
> + } else {
> + dmmu_idx |= msr & (1ull << MSR_HV) ? 4 : 0;
> + immu_idx = dmmu_idx;
> + immu_idx |= msr & (1 << MSR_IR) ? 0 : 2;
> + dmmu_idx |= msr & (1 << MSR_DR) ? 0 : 2;
> + }
> + hflags |= immu_idx << HFLAGS_IMMU_IDX;
> + hflags |= dmmu_idx << HFLAGS_DMMU_IDX;
> #endif
>
> env->hflags = hflags | (msr & msr_mask);
> - hreg_compute_mem_idx(env);
> }
>
> void cpu_interrupt_exittb(CPUState *cs)
> diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
> index f4f7e730de..444b2a30ef 100644
> --- a/target/ppc/mem_helper.c
> +++ b/target/ppc/mem_helper.c
> @@ -278,7 +278,7 @@ static void dcbz_common(CPUPPCState *env, target_ulong addr,
> target_ulong mask, dcbz_size = env->dcache_line_size;
> uint32_t i;
> void *haddr;
> - int mmu_idx = epid ? PPC_TLB_EPID_STORE : env->dmmu_idx;
> + int mmu_idx = epid ? PPC_TLB_EPID_STORE : cpu_mmu_index(env, false);
>
> #if defined(TARGET_PPC64)
> /* Check for dcbz vs dcbzl on 970 */
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 5e629291d3..a53463b9b8 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7658,8 +7658,8 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> cs->cpu_index);
> qemu_fprintf(f, "MSR " TARGET_FMT_lx " HID0 " TARGET_FMT_lx " HF "
> "%08x iidx %d didx %d\n",
> - env->msr, env->spr[SPR_HID0],
> - env->hflags, env->immu_idx, env->dmmu_idx);
> + env->msr, env->spr[SPR_HID0], env->hflags,
> + cpu_mmu_index(env, true), cpu_mmu_index(env, false));
> #if !defined(NO_TIMER_DUMP)
> qemu_fprintf(f, "TB %08" PRIu32 " %08" PRIu64
> #if !defined(CONFIG_USER_ONLY)
> @@ -7885,7 +7885,7 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> ctx->exception = POWERPC_EXCP_NONE;
> ctx->spr_cb = env->spr_cb;
> ctx->pr = (hflags >> HFLAGS_PR) & 1;
> - ctx->mem_idx = env->dmmu_idx;
> + ctx->mem_idx = (hflags >> HFLAGS_DMMU_IDX) & 7;
> ctx->dr = (hflags >> HFLAGS_DR) & 1;
> ctx->hv = (hflags >> HFLAGS_HV) & 1;
> ctx->insns_flags = env->insns_flags;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v4 13/17] target/ppc: Remove env->immu_idx and env->dmmu_idx
2021-03-22 4:26 ` David Gibson
@ 2021-03-22 17:27 ` Richard Henderson
2021-03-23 0:01 ` David Gibson
0 siblings, 1 reply; 51+ messages in thread
From: Richard Henderson @ 2021-03-22 17:27 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel
On 3/21/21 10:26 PM, David Gibson wrote:
> On Mon, Mar 15, 2021 at 12:46:11PM -0600, Richard Henderson wrote:
>> We weren't recording MSR_GS in hflags, which means that BookE
>> memory accesses were essentially random vs Guest State.
>>
>> Instead of adding this bit directly, record the completed mmu
>> indexes instead. This makes it obvious that we are recording
>> exactly the information that we need.
>>
>> This also means that we can stop directly recording MSR_IR.
>
> What still uses MSR_DR, that you can't also drop it?
#define CHK_HVRM \
do { \
if (unlikely(ctx->pr || !ctx->hv || ctx->dr)) { \
I have this notion that this (and CHK_HV and CHK_SV) could be a test against
mmu_idx instead, but was reluctant to make that change.
r~
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH v4 13/17] target/ppc: Remove env->immu_idx and env->dmmu_idx
2021-03-22 17:27 ` Richard Henderson
@ 2021-03-23 0:01 ` David Gibson
0 siblings, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-23 0:01 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]
On Mon, Mar 22, 2021 at 11:27:49AM -0600, Richard Henderson wrote:
> On 3/21/21 10:26 PM, David Gibson wrote:
> > On Mon, Mar 15, 2021 at 12:46:11PM -0600, Richard Henderson wrote:
> > > We weren't recording MSR_GS in hflags, which means that BookE
> > > memory accesses were essentially random vs Guest State.
> > >
> > > Instead of adding this bit directly, record the completed mmu
> > > indexes instead. This makes it obvious that we are recording
> > > exactly the information that we need.
> > >
> > > This also means that we can stop directly recording MSR_IR.
> >
> > What still uses MSR_DR, that you can't also drop it?
>
> #define CHK_HVRM \
> do { \
> if (unlikely(ctx->pr || !ctx->hv || ctx->dr)) { \
>
> I have this notion that this (and CHK_HV and CHK_SV) could be a test against
> mmu_idx instead, but was reluctant to make that change.
Yeah, that's checking for hypervisor real mode (hence "HVRM") for
ldcix and friends, so it should be equivalent to (mmu_idx != 7).
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 14/17] hw/ppc/pnv_core: Update hflags after setting msr
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (12 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 13/17] target/ppc: Remove env->immu_idx and env->dmmu_idx Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-22 4:27 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 15/17] hw/ppc/spapr_rtas: " Richard Henderson
` (3 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/ppc/pnv_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index bd2bf2e044..8c2a15a0fb 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -29,6 +29,7 @@
#include "hw/ppc/pnv_xscom.h"
#include "hw/ppc/xics.h"
#include "hw/qdev-properties.h"
+#include "helper_regs.h"
static const char *pnv_core_cpu_typename(PnvCore *pc)
{
@@ -55,8 +56,8 @@ static void pnv_core_cpu_reset(PnvCore *pc, PowerPCCPU *cpu)
env->gpr[3] = PNV_FDT_ADDR;
env->nip = 0x10;
env->msr |= MSR_HVB; /* Hypervisor mode */
-
env->spr[SPR_HRMOR] = pc->hrmor;
+ hreg_compute_hflags(env);
pcc->intc_reset(pc->chip, cpu);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 14/17] hw/ppc/pnv_core: Update hflags after setting msr
2021-03-15 18:46 ` [PATCH v4 14/17] hw/ppc/pnv_core: Update hflags after setting msr Richard Henderson
@ 2021-03-22 4:27 ` David Gibson
0 siblings, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 4:27 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]
On Mon, Mar 15, 2021 at 12:46:12PM -0600, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Applied to ppc-for-6.0, thanks.
> ---
> hw/ppc/pnv_core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index bd2bf2e044..8c2a15a0fb 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -29,6 +29,7 @@
> #include "hw/ppc/pnv_xscom.h"
> #include "hw/ppc/xics.h"
> #include "hw/qdev-properties.h"
> +#include "helper_regs.h"
>
> static const char *pnv_core_cpu_typename(PnvCore *pc)
> {
> @@ -55,8 +56,8 @@ static void pnv_core_cpu_reset(PnvCore *pc, PowerPCCPU *cpu)
> env->gpr[3] = PNV_FDT_ADDR;
> env->nip = 0x10;
> env->msr |= MSR_HVB; /* Hypervisor mode */
> -
> env->spr[SPR_HRMOR] = pc->hrmor;
> + hreg_compute_hflags(env);
>
> pcc->intc_reset(pc->chip, cpu);
> }
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 15/17] hw/ppc/spapr_rtas: Update hflags after setting msr
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (13 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 14/17] hw/ppc/pnv_core: Update hflags after setting msr Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-22 4:27 ` David Gibson
2021-03-15 18:46 ` [PATCH v4 16/17] linux-user/ppc: Fix msr updates for signal handling Richard Henderson
` (2 subsequent siblings)
17 siblings, 1 reply; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
hw/ppc/spapr_rtas.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 8a79f9c628..6ec3e71757 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -51,6 +51,7 @@
#include "target/ppc/mmu-hash64.h"
#include "target/ppc/mmu-book3s-v3.h"
#include "migration/blocker.h"
+#include "helper_regs.h"
static void rtas_display_character(PowerPCCPU *cpu, SpaprMachineState *spapr,
uint32_t token, uint32_t nargs,
@@ -163,6 +164,7 @@ static void rtas_start_cpu(PowerPCCPU *callcpu, SpaprMachineState *spapr,
cpu_synchronize_state(CPU(newcpu));
env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
+ hreg_compute_hflags(env);
/* Enable Power-saving mode Exit Cause exceptions for the new CPU */
lpcr = env->spr[SPR_LPCR];
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 15/17] hw/ppc/spapr_rtas: Update hflags after setting msr
2021-03-15 18:46 ` [PATCH v4 15/17] hw/ppc/spapr_rtas: " Richard Henderson
@ 2021-03-22 4:27 ` David Gibson
0 siblings, 0 replies; 51+ messages in thread
From: David Gibson @ 2021-03-22 4:27 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]
On Mon, Mar 15, 2021 at 12:46:13PM -0600, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Applied to ppc-for-6.0, thanks.
> ---
> hw/ppc/spapr_rtas.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 8a79f9c628..6ec3e71757 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -51,6 +51,7 @@
> #include "target/ppc/mmu-hash64.h"
> #include "target/ppc/mmu-book3s-v3.h"
> #include "migration/blocker.h"
> +#include "helper_regs.h"
>
> static void rtas_display_character(PowerPCCPU *cpu, SpaprMachineState *spapr,
> uint32_t token, uint32_t nargs,
> @@ -163,6 +164,7 @@ static void rtas_start_cpu(PowerPCCPU *callcpu, SpaprMachineState *spapr,
> cpu_synchronize_state(CPU(newcpu));
>
> env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
> + hreg_compute_hflags(env);
>
> /* Enable Power-saving mode Exit Cause exceptions for the new CPU */
> lpcr = env->spr[SPR_LPCR];
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH v4 16/17] linux-user/ppc: Fix msr updates for signal handling
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (14 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 15/17] hw/ppc/spapr_rtas: " Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-15 18:46 ` [PATCH v4 17/17] target/ppc: Validate hflags with CONFIG_DEBUG_TCG Richard Henderson
2021-03-16 8:11 ` [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Cédric Le Goater
17 siblings, 0 replies; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
In save_user_regs, there are two bugs where we OR in a bit number
instead of the bit, clobbering the low bits of MSR. However:
The MSR_VR and MSR_SPE bits control the availability of the insns.
If the bits were not already set in MSR, then any attempt to access
those registers would result in SIGILL.
For linux-user, we always initialize MSR to the capabilities
of the cpu. We *could* add checks vs MSR where we currently
check insn_flags and insn_flags2, but we know they match.
Also, there's a stray cut-and-paste comment in restore.
Then, do not force little-endian binaries into big-endian mode.
Finally, use ppc_store_msr for the update to affect hflags.
Which is the reason none of these bugs were previously noticed.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/ppc/cpu_loop.c | 5 +++--
linux-user/ppc/signal.c | 23 +++++++++++------------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/linux-user/ppc/cpu_loop.c b/linux-user/ppc/cpu_loop.c
index df71e15a25..4a0f6c8dc2 100644
--- a/linux-user/ppc/cpu_loop.c
+++ b/linux-user/ppc/cpu_loop.c
@@ -492,11 +492,12 @@ void target_cpu_copy_regs(CPUArchState *env, struct target_pt_regs *regs)
#if defined(TARGET_PPC64)
int flag = (env->insns_flags2 & PPC2_BOOKE206) ? MSR_CM : MSR_SF;
#if defined(TARGET_ABI32)
- env->msr &= ~((target_ulong)1 << flag);
+ ppc_store_msr(env, env->msr & ~((target_ulong)1 << flag));
#else
- env->msr |= (target_ulong)1 << flag;
+ ppc_store_msr(env, env->msr | (target_ulong)1 << flag);
#endif
#endif
+
env->nip = regs->nip;
for(i = 0; i < 32; i++) {
env->gpr[i] = regs->gpr[i];
diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index b78613f7c8..bad38f8ed9 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -261,9 +261,6 @@ static void save_user_regs(CPUPPCState *env, struct target_mcontext *frame)
__put_user(avr->u64[PPC_VEC_HI], &vreg->u64[0]);
__put_user(avr->u64[PPC_VEC_LO], &vreg->u64[1]);
}
- /* Set MSR_VR in the saved MSR value to indicate that
- frame->mc_vregs contains valid data. */
- msr |= MSR_VR;
#if defined(TARGET_PPC64)
vrsave = (uint32_t *)&frame->mc_vregs.altivec[33];
/* 64-bit needs to put a pointer to the vectors in the frame */
@@ -300,9 +297,6 @@ static void save_user_regs(CPUPPCState *env, struct target_mcontext *frame)
for (i = 0; i < ARRAY_SIZE(env->gprh); i++) {
__put_user(env->gprh[i], &frame->mc_vregs.spe[i]);
}
- /* Set MSR_SPE in the saved MSR value to indicate that
- frame->mc_vregs contains valid data. */
- msr |= MSR_SPE;
__put_user(env->spe_fscr, &frame->mc_vregs.spe[32]);
}
#endif
@@ -354,8 +348,10 @@ static void restore_user_regs(CPUPPCState *env,
__get_user(msr, &frame->mc_gregs[TARGET_PT_MSR]);
/* If doing signal return, restore the previous little-endian mode. */
- if (sig)
- env->msr = (env->msr & ~(1ull << MSR_LE)) | (msr & (1ull << MSR_LE));
+ if (sig) {
+ ppc_store_msr(env, ((env->msr & ~(1ull << MSR_LE)) |
+ (msr & (1ull << MSR_LE))));
+ }
/* Restore Altivec registers if necessary. */
if (env->insns_flags & PPC_ALTIVEC) {
@@ -376,8 +372,6 @@ static void restore_user_regs(CPUPPCState *env,
__get_user(avr->u64[PPC_VEC_HI], &vreg->u64[0]);
__get_user(avr->u64[PPC_VEC_LO], &vreg->u64[1]);
}
- /* Set MSR_VEC in the saved MSR value to indicate that
- frame->mc_vregs contains valid data. */
#if defined(TARGET_PPC64)
vrsave = (uint32_t *)&v_regs[33];
#else
@@ -468,7 +462,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
env->nip = (target_ulong) ka->_sa_handler;
/* Signal handlers are entered in big-endian mode. */
- env->msr &= ~(1ull << MSR_LE);
+ ppc_store_msr(env, env->msr & ~(1ull << MSR_LE));
unlock_user_struct(frame, frame_addr, 1);
return;
@@ -563,8 +557,13 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
env->nip = (target_ulong) ka->_sa_handler;
#endif
+#ifdef TARGET_WORDS_BIGENDIAN
/* Signal handlers are entered in big-endian mode. */
- env->msr &= ~(1ull << MSR_LE);
+ ppc_store_msr(env, env->msr & ~(1ull << MSR_LE));
+#else
+ /* Signal handlers are entered in little-endian mode. */
+ ppc_store_msr(env, env->msr | (1ull << MSR_LE));
+#endif
unlock_user_struct(rt_sf, rt_sf_addr, 1);
return;
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH v4 17/17] target/ppc: Validate hflags with CONFIG_DEBUG_TCG
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (15 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 16/17] linux-user/ppc: Fix msr updates for signal handling Richard Henderson
@ 2021-03-15 18:46 ` Richard Henderson
2021-03-16 8:11 ` [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Cédric Le Goater
17 siblings, 0 replies; 51+ messages in thread
From: Richard Henderson @ 2021-03-15 18:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, david
Verify that hflags was updated correctly whenever we change
cpu state that is used by hflags.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/ppc/cpu.h | 5 +++++
target/ppc/helper_regs.c | 29 +++++++++++++++++++++++++++--
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 2f8d7fa13c..7ee5c9a66e 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2424,6 +2424,10 @@ void cpu_write_xer(CPUPPCState *env, target_ulong xer);
*/
#define is_book3s_arch2x(ctx) (!!((ctx)->insns_flags & PPC_SEGMENT_64B))
+#ifdef CONFIG_DEBUG_TCG
+void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc,
+ target_ulong *cs_base, uint32_t *flags);
+#else
static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc,
target_ulong *cs_base, uint32_t *flags)
{
@@ -2431,6 +2435,7 @@ static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc,
*cs_base = 0;
*flags = env->hflags;
}
+#endif
void QEMU_NORETURN raise_exception(CPUPPCState *env, uint32_t exception);
void QEMU_NORETURN raise_exception_ra(CPUPPCState *env, uint32_t exception,
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index b28037ca24..9df1098fec 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -43,7 +43,7 @@ void hreg_swap_gpr_tgpr(CPUPPCState *env)
env->tgpr[3] = tmp;
}
-void hreg_compute_hflags(CPUPPCState *env)
+static uint32_t hreg_compute_hflags_value(CPUPPCState *env)
{
target_ulong msr = env->msr;
uint32_t ppc_flags = env->flags;
@@ -147,9 +147,34 @@ void hreg_compute_hflags(CPUPPCState *env)
hflags |= dmmu_idx << HFLAGS_DMMU_IDX;
#endif
- env->hflags = hflags | (msr & msr_mask);
+ return hflags | (msr & msr_mask);
}
+void hreg_compute_hflags(CPUPPCState *env)
+{
+ env->hflags = hreg_compute_hflags_value(env);
+}
+
+#ifdef CONFIG_DEBUG_TCG
+void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc,
+ target_ulong *cs_base, uint32_t *flags)
+{
+ uint32_t hflags_current = env->hflags;
+ uint32_t hflags_rebuilt;
+
+ *pc = env->nip;
+ *cs_base = 0;
+ *flags = hflags_current;
+
+ hflags_rebuilt = hreg_compute_hflags_value(env);
+ if (unlikely(hflags_current != hflags_rebuilt)) {
+ cpu_abort(env_cpu(env),
+ "TCG hflags mismatch (current:0x%08x rebuilt:0x%08x)\n",
+ hflags_current, hflags_rebuilt);
+ }
+}
+#endif
+
void cpu_interrupt_exittb(CPUState *cs)
{
if (!kvm_enabled()) {
--
2.25.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags
2021-03-15 18:45 [PATCH v4 00/17] target/ppc: Fix truncation of env->hflags Richard Henderson
` (16 preceding siblings ...)
2021-03-15 18:46 ` [PATCH v4 17/17] target/ppc: Validate hflags with CONFIG_DEBUG_TCG Richard Henderson
@ 2021-03-16 8:11 ` Cédric Le Goater
17 siblings, 0 replies; 51+ messages in thread
From: Cédric Le Goater @ 2021-03-16 8:11 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: Mark Cave-Ayland, qemu-ppc, Greg Kurz, david
Hello,
I gave this series a try on some PPC machines : mac99, g3beige,
sam460ex, pseries, powernv, with linux, macos, darwin, aix and
didn't see any regression. Migration seems to work for pseries.
C.
On 3/15/21 7:45 PM, Richard Henderson wrote:
> Changes for v4:
> * Use hregs_recompute_hflags for hw/ppc/ reset.
> -- Incorporate Cedric's feedback.
>
> Changes for v3:
> * Fixes for linux-user, signal handling and startup.
> -- Oops, the directory in which I did testing for v2
> had a reduced set of targets.
>
> Changes for v2:
> * Do not put tcg internal state into migration, except to
> retain backward compatibility.
> * Do not touch anything in env in ppc_tr_init_disas_context.
> * Do make sure that hflags contains everything that it should.
> * Do verify that hflags is properly updated.
>
>
> r~
>
>
> Richard Henderson (17):
> target/ppc: Move helper_regs.h functions out-of-line
> target/ppc: Move 601 hflags adjustment to hreg_compute_hflags
> target/ppc: Properly sync cpu state with new msr in cpu_load_old
> target/ppc: Do not call hreg_compute_mem_idx after ppc_store_msr
> target/ppc: Retain hflags_nmsr only for migration
> target/ppc: Fix comment for MSR_FE{0,1}
> target/ppc: Disconnect hflags from MSR
> target/ppc: Reduce env->hflags to uint32_t
> target/ppc: Put dbcr0 single-step bits into hflags
> target/ppc: Create helper_scv
> target/ppc: Put LPCR[GTSE] in hflags
> target/ppc: Remove MSR_SA and MSR_AP from hflags
> target/ppc: Remove env->immu_idx and env->dmmu_idx
> hw/ppc/pnv_core: Update hflags after setting msr
> hw/ppc/spapr_rtas: Update hflags after setting msr
> linux-user/ppc: Fix msr updates for signal handling
> target/ppc: Validate hflags with CONFIG_DEBUG_TCG
>
> target/ppc/cpu.h | 50 +++++-
> target/ppc/helper.h | 1 +
> target/ppc/helper_regs.h | 183 +--------------------
> hw/ppc/pnv_core.c | 3 +-
> hw/ppc/spapr_rtas.c | 2 +
> linux-user/ppc/cpu_loop.c | 5 +-
> linux-user/ppc/signal.c | 23 ++-
> target/ppc/excp_helper.c | 9 ++
> target/ppc/helper_regs.c | 272 ++++++++++++++++++++++++++++++++
> target/ppc/int_helper.c | 1 +
> target/ppc/machine.c | 27 ++--
> target/ppc/mem_helper.c | 2 +-
> target/ppc/misc_helper.c | 13 +-
> target/ppc/mmu-hash64.c | 3 +
> target/ppc/translate.c | 98 ++++--------
> target/ppc/translate_init.c.inc | 4 +-
> target/ppc/meson.build | 1 +
> 17 files changed, 410 insertions(+), 287 deletions(-)
> create mode 100644 target/ppc/helper_regs.c
>
^ permalink raw reply [flat|nested] 51+ messages in thread