[Qemu-devel] [PATCH 00/12] ppc: preparing pnv landing (round 2)

[Qemu-devel] [PATCH 00/12] ppc: preparing pnv landing (round 2)

[Cédric Le Goater]

Hello,

Here is a new set of fixes extracted from Ben's PowerNV tree :

     https://github.com/ozbenh/qemu/commits/powernv

It was quickly tested with a pseries guest using KVM and TCG.

Thanks,

C.

Benjamin Herrenschmidt (11):
  ppc: Remove MMU_MODEn_SUFFIX definitions
  ppc: Use split I/D mmu modes to avoid flushes on interrupts
  ppc: Do some batching of TCG tlb flushes
  ppc: Add a bunch of hypervisor SPRs to Book3s
  ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV
  ppc: Fix rfi/rfid/hrfi/... emulation
  ppc: Better figure out if processor has HV mode
  ppc: tlbie, tlbia and tlbisync are HV only
  ppc: Change 'invalid' bit mask of tlbiel and tlbie
  ppc: Get out of emulation on SMT "OR" ops
  ppc: Add PPC_64H instruction flag to POWER7 and POWER8

Michael Neuling (1):
  ppc: Fix sign extension issue in mtmsr(d) emulation

 hw/ppc/spapr_hcall.c        |  14 ++++-
 target-ppc/cpu.h            |  20 ++++--
 target-ppc/excp_helper.c    |  77 +++++++++++------------
 target-ppc/helper.h         |   1 +
 target-ppc/helper_regs.h    |  71 +++++++++++++++++++---
 target-ppc/machine.c        |   5 +-
 target-ppc/mmu-hash64.c     |  11 +---
 target-ppc/mmu_helper.c     |   9 ++-
 target-ppc/translate.c      |  94 +++++++++++++++++++++++------
 target-ppc/translate_init.c | 144 +++++++++++++++++++++++++++++++++++++++++---
 10 files changed, 348 insertions(+), 98 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH 01/12] ppc: Remove MMU_MODEn_SUFFIX definitions

[Cédric Le Goater]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

We don't use the resulting accessors and this gets in the way of
the split I/D TLB work.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 target-ppc/cpu.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 5282533b3858..4c0dc02e86d5 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1303,9 +1303,6 @@ int ppc_dcr_write (ppc_dcr_t *dcr_env, int dcrn, uint32_t val);
 #define cpu_list ppc_cpu_list
 
 /* MMU modes definitions */
-#define MMU_MODE0_SUFFIX _user
-#define MMU_MODE1_SUFFIX _kernel
-#define MMU_MODE2_SUFFIX _hypv
 #define MMU_USER_IDX 0
 static inline int cpu_mmu_index (CPUPPCState *env, bool ifetch)
 {
-- 
2.1.4

[Qemu-devel] [PATCH 02/12] ppc: Use split I/D mmu modes to avoid flushes on interrupts

[Cédric Le Goater]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

We rework the way the MMU indices are calculated, providing separate
indices for I and D side based on MSR:IR and MSR:DR respectively,
and thus no longer need to flush the TLB on context changes. This also
adds correct support for HV as a separate address space.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 target-ppc/cpu.h         | 11 +++++++---
 target-ppc/excp_helper.c | 11 ----------
 target-ppc/helper_regs.h | 54 +++++++++++++++++++++++++++++++++++++++++-------
 target-ppc/machine.c     |  5 ++++-
 target-ppc/translate.c   |  7 ++++---
 5 files changed, 63 insertions(+), 25 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 4c0dc02e86d5..9588b30ee855 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -470,6 +470,8 @@ struct ppc_slb_t {
 #define MSR_EP   6  /* Exception prefix on 601                               */
 #define MSR_IR   5  /* Instruction relocate                                  */
 #define MSR_DR   4  /* Data relocate                                         */
+#define MSR_IS   5  /* Instruction address space (BookE)                     */
+#define MSR_DS   4  /* Data address space (BookE)                            */
 #define MSR_PE   3  /* Protection enable on 403                              */
 #define MSR_PX   2  /* Protection exclusive on 403                  x        */
 #define MSR_PMM  2  /* Performance monitor mark on POWER            x        */
@@ -521,6 +523,8 @@ struct ppc_slb_t {
 #define msr_ep   ((env->msr >> MSR_EP)   & 1)
 #define msr_ir   ((env->msr >> MSR_IR)   & 1)
 #define msr_dr   ((env->msr >> MSR_DR)   & 1)
+#define msr_is   ((env->msr >> MSR_IS)   & 1)
+#define msr_ds   ((env->msr >> MSR_DS)   & 1)
 #define msr_pe   ((env->msr >> MSR_PE)   & 1)
 #define msr_px   ((env->msr >> MSR_PX)   & 1)
 #define msr_pmm  ((env->msr >> MSR_PMM)  & 1)
@@ -1000,7 +1004,7 @@ struct ppc_segment_page_sizes {
 
 /*****************************************************************************/
 /* The whole PowerPC CPU context */
-#define NB_MMU_MODES 3
+#define NB_MMU_MODES    8
 
 #define PPC_CPU_OPCODES_LEN          0x40
 #define PPC_CPU_INDIRECT_OPCODES_LEN 0x20
@@ -1164,7 +1168,8 @@ struct CPUPPCState {
     /* Those resources are used only in QEMU core */
     target_ulong hflags;      /* hflags is a MSR & HFLAGS_MASK         */
     target_ulong hflags_nmsr; /* specific hflags, not coming from MSR */
-    int mmu_idx;         /* precomputed MMU index to speed up mem accesses */
+    int immu_idx;         /* precomputed MMU index to speed up insn access */
+    int dmmu_idx;         /* precomputed MMU index to speed up data accesses */
 
     /* Power management */
     int (*check_pow)(CPUPPCState *env);
@@ -1306,7 +1311,7 @@ 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 env->mmu_idx;
+    return ifetch ? env->immu_idx : env->dmmu_idx;
 }
 
 #include "exec/cpu-all.h"
diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index ca4ffe8ad660..cf882ebdad4c 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -645,9 +645,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
 
     if (env->spr[SPR_LPCR] & LPCR_AIL) {
         new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
-    } else if (msr & ((1 << MSR_IR) | (1 << MSR_DR))) {
-        /* If we disactivated any translation, flush TLBs */
-        tlb_flush(cs, 1);
     }
 
 #ifdef TARGET_PPC64
@@ -720,14 +717,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
     /* Reset exception state */
     cs->exception_index = POWERPC_EXCP_NONE;
     env->error_code = 0;
-
-    if ((env->mmu_model == POWERPC_MMU_BOOKE) ||
-        (env->mmu_model == POWERPC_MMU_BOOKE206)) {
-        /* XXX: The BookE changes address space when switching modes,
-                we should probably implement that as different MMU indexes,
-                but for the moment we do it the slow way and flush all.  */
-        tlb_flush(cs, 1);
-    }
 }
 
 void ppc_cpu_do_interrupt(CPUState *cs)
diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
index 271fddf17f0a..f7edd5bc5945 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -41,11 +41,50 @@ static inline void hreg_swap_gpr_tgpr(CPUPPCState *env)
 
 static inline void hreg_compute_mem_idx(CPUPPCState *env)
 {
-    /* Precompute MMU index */
-    if (msr_pr == 0 && msr_hv != 0) {
-        env->mmu_idx = 2;
+    /* This is our encoding for server processors
+     *
+     *   0 = Guest User space virtual mode
+     *   1 = Guest Kernel space virtual mode
+     *   2 = Guest Kernel space real mode
+     *   3 = HV User space virtual mode
+     *   4 = HV Kernel space virtual mode
+     *   5 = HV Kernel space real mode
+     *
+     * The combination PR=1 IR&DR=0 is invalid, we will treat
+     * it as IR=DR=1
+     *
+     * 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->mmu_idx = 1 - msr_pr;
+        /* First calucalte a base value independent of HV */
+        if (msr_pr != 0) {
+            /* User space, ignore IR and DR */
+            env->immu_idx = env->dmmu_idx = 0;
+        } else {
+            /* Kernel, setup a base I/D value */
+            env->immu_idx = msr_ir ? 1 : 2;
+            env->dmmu_idx = msr_dr ? 1 : 2;
+        }
+        /* Then offset it for HV */
+        if (msr_hv) {
+            env->immu_idx += 3;
+            env->dmmu_idx += 3;
+        }
     }
 }
 
@@ -82,9 +121,10 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
     }
     if (((value >> MSR_IR) & 1) != msr_ir ||
         ((value >> MSR_DR) & 1) != msr_dr) {
-        /* Flush all tlb when changing translation mode */
-        tlb_flush(cs, 1);
-        excp = POWERPC_EXCP_NONE;
+        cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
+    }
+    if ((env->mmu_model & POWERPC_MMU_BOOKE) &&
+        ((value >> MSR_GS) & 1) != msr_gs) {
         cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
     }
     if (unlikely((env->flags & POWERPC_FLAG_TGPR) &&
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index 46684fb93372..cca2a57e54af 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -92,9 +92,12 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
     qemu_get_betls(f, &env->nip);
     qemu_get_betls(f, &env->hflags);
     qemu_get_betls(f, &env->hflags_nmsr);
-    qemu_get_sbe32s(f, &env->mmu_idx);
+    qemu_get_sbe32(f); /* Discard unused mmu_idx */
     qemu_get_sbe32(f); /* Discard unused power_mode */
 
+    /* Recompute mmu indices */
+    hreg_compute_mem_idx(env);
+
     return 0;
 }
 
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index b3860ecdea9c..1119a301154c 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -11247,8 +11247,9 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf,
                 env->nip, env->lr, env->ctr, cpu_read_xer(env),
                 cs->cpu_index);
     cpu_fprintf(f, "MSR " TARGET_FMT_lx " HID0 " TARGET_FMT_lx "  HF "
-                TARGET_FMT_lx " idx %d\n", env->msr, env->spr[SPR_HID0],
-                env->hflags, env->mmu_idx);
+                TARGET_FMT_lx " iidx %d didx %d\n",
+                env->msr, env->spr[SPR_HID0],
+                env->hflags, env->immu_idx, env->dmmu_idx);
 #if !defined(NO_TIMER_DUMP)
     cpu_fprintf(f, "TB %08" PRIu32 " %08" PRIu64
 #if !defined(CONFIG_USER_ONLY)
@@ -11455,7 +11456,7 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
     ctx.spr_cb = env->spr_cb;
     ctx.pr = msr_pr;
     ctx.hv = !msr_pr && msr_hv;
-    ctx.mem_idx = env->mmu_idx;
+    ctx.mem_idx = env->dmmu_idx;
     ctx.insns_flags = env->insns_flags;
     ctx.insns_flags2 = env->insns_flags2;
     ctx.access_type = -1;
-- 
2.1.4

[Qemu-devel] [PATCH 03/12] ppc: Do some batching of TCG tlb flushes

[Cédric Le Goater]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

On ppc64 especially, we flush the tlb on any slbie or tlbie instruction.

However, those instructions often come in bursts of 3 or more (context
switch will favor a series of slbie's for example to an slbia if the
SLB has less than a certain number of entries in it, and tlbie's can
happen in a series, with PAPR, H_BULK_REMOVE can remove up to 4 entries
at a time.

Doing a tlb_flush() each time is a waste of time. We end up doing a memset
of the whole TLB, reloading it for the next instruction, memset'ing again,
etc...

Those instructions don't have to take effect immediately. For slbie, they
can wait for the next context synchronizing event. For tlbie, the next
tlbsync.

This implements batching by keeping a flag that indicates that we have a
TLB in need of flushing. We check it on interrupts, rfi's, isync's and
tlbsync and flush the TLB if needed.

This reduces the number of tlb_flush() on a boot to a ubuntu installer
first dialog screen from roughly 360K down to 36K.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[clg: added a 'CPUPPCState *' variable in h_remove() and
      h_bulk_remove() ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/spapr_hcall.c     | 14 +++++++++++---
 target-ppc/cpu.h         |  2 ++
 target-ppc/excp_helper.c |  9 +++++++++
 target-ppc/helper.h      |  1 +
 target-ppc/helper_regs.h | 13 +++++++++++++
 target-ppc/mmu-hash64.c  | 11 +++--------
 target-ppc/mmu_helper.c  |  9 ++++++++-
 target-ppc/translate.c   | 39 ++++++++++++++++++++++++++++++++++++---
 8 files changed, 83 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 8f40602a5efb..2713087c1e5d 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -183,6 +183,7 @@ static RemoveResult remove_hpte(PowerPCCPU *cpu, target_ulong ptex,
 static target_ulong h_remove(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                              target_ulong opcode, target_ulong *args)
 {
+    CPUPPCState *env = &cpu->env;
     target_ulong flags = args[0];
     target_ulong pte_index = args[1];
     target_ulong avpn = args[2];
@@ -193,6 +194,7 @@ static target_ulong h_remove(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 
     switch (ret) {
     case REMOVE_SUCCESS:
+        check_tlb_flush(env);
         return H_SUCCESS;
 
     case REMOVE_NOT_FOUND:
@@ -229,7 +231,9 @@ static target_ulong h_remove(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 static target_ulong h_bulk_remove(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                                   target_ulong opcode, target_ulong *args)
 {
+    CPUPPCState *env = &cpu->env;
     int i;
+    target_ulong rc = H_SUCCESS;
 
     for (i = 0; i < H_BULK_REMOVE_MAX_BATCH; i++) {
         target_ulong *tsh = &args[i*2];
@@ -262,14 +266,18 @@ static target_ulong h_bulk_remove(PowerPCCPU *cpu, sPAPRMachineState *spapr,
             break;
 
         case REMOVE_PARM:
-            return H_PARAMETER;
+            rc = H_PARAMETER;
+            goto exit;
 
         case REMOVE_HW:
-            return H_HARDWARE;
+            rc = H_HARDWARE;
+            goto exit;
         }
     }
+ exit:
+    check_tlb_flush(env);
 
-    return H_SUCCESS;
+    return rc;
 }
 
 static target_ulong h_protect(PowerPCCPU *cpu, sPAPRMachineState *spapr,
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 9588b30ee855..2a96efcbf813 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1069,6 +1069,8 @@ struct CPUPPCState {
     /* PowerPC 64 SLB area */
     ppc_slb_t slb[MAX_SLB_ENTRIES];
     int32_t slb_nr;
+    /* tcg TLB needs flush (deferred slb inval instruction typically) */
+    uint32_t tlb_need_flush;
 #endif
     /* segment registers */
     hwaddr htab_base;
diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index cf882ebdad4c..85f38640bdf4 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -717,6 +717,11 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
     /* Reset exception state */
     cs->exception_index = POWERPC_EXCP_NONE;
     env->error_code = 0;
+
+    /* Any interrupt is context synchronizing, check if TCG TLB
+     * needs a delayed flush on ppc64
+     */
+    check_tlb_flush(env);
 }
 
 void ppc_cpu_do_interrupt(CPUState *cs)
@@ -738,6 +743,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
                   __func__, env, env->pending_interrupts,
                   cs->interrupt_request, (int)msr_me, (int)msr_ee);
 #endif
+
     /* External reset */
     if (env->pending_interrupts & (1 << PPC_INTERRUPT_RESET)) {
         env->pending_interrupts &= ~(1 << PPC_INTERRUPT_RESET);
@@ -942,6 +948,9 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr,
      * as rfi is always the last insn of a TB
      */
     cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
+
+    /* Context synchronizing: check if TCG TLB needs flush */
+    check_tlb_flush(env);
 }
 
 void helper_rfi(CPUPPCState *env)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index e5a8f7b9b539..0526322f4d27 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -16,6 +16,7 @@ DEF_HELPER_1(rfmci, void, env)
 DEF_HELPER_1(rfid, void, env)
 DEF_HELPER_1(hrfid, void, env)
 #endif
+DEF_HELPER_1(check_tlb_flush, void, env)
 #endif
 
 DEF_HELPER_3(lmw, void, env, tl, i32)
diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
index f7edd5bc5945..57da931e3c4d 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -151,4 +151,17 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
     return excp;
 }
 
+#if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64)
+static inline void check_tlb_flush(CPUPPCState *env)
+{
+    CPUState *cs = CPU(ppc_env_get_cpu(env));
+    if (env->tlb_need_flush) {
+        env->tlb_need_flush = 0;
+        tlb_flush(cs, 1);
+    }
+}
+#else
+static inline void check_tlb_flush(CPUPPCState *env) { }
+#endif
+
 #endif /* !defined(__HELPER_REGS_H__) */
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 72c4ab5d751c..44fc1bfc288c 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -98,10 +98,8 @@ void dump_slb(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
 
 void helper_slbia(CPUPPCState *env)
 {
-    PowerPCCPU *cpu = ppc_env_get_cpu(env);
-    int n, do_invalidate;
+    int n;
 
-    do_invalidate = 0;
     /* XXX: Warning: slbia never invalidates the first segment */
     for (n = 1; n < env->slb_nr; n++) {
         ppc_slb_t *slb = &env->slb[n];
@@ -112,12 +110,9 @@ void helper_slbia(CPUPPCState *env)
              *      and we still don't have a tlb_flush_mask(env, n, mask)
              *      in QEMU, we just invalidate all TLBs
              */
-            do_invalidate = 1;
+            env->tlb_need_flush = true;
         }
     }
-    if (do_invalidate) {
-        tlb_flush(CPU(cpu), 1);
-    }
 }
 
 void helper_slbie(CPUPPCState *env, target_ulong addr)
@@ -137,7 +132,7 @@ void helper_slbie(CPUPPCState *env, target_ulong addr)
          *      and we still don't have a tlb_flush_mask(env, n, mask)
          *      in QEMU, we just invalidate all TLBs
          */
-        tlb_flush(CPU(cpu), 1);
+        env->tlb_need_flush = true;
     }
 }
 
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index ff217941b5a7..930e9d31cfde 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -26,6 +26,7 @@
 #include "mmu-hash32.h"
 #include "exec/cpu_ldst.h"
 #include "exec/log.h"
+#include "helper_regs.h"
 
 //#define DEBUG_MMU
 //#define DEBUG_BATS
@@ -1923,6 +1924,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
     case POWERPC_MMU_2_06a:
     case POWERPC_MMU_2_07:
     case POWERPC_MMU_2_07a:
+        env->tlb_need_flush = 0;
 #endif /* defined(TARGET_PPC64) */
         tlb_flush(CPU(cpu), 1);
         break;
@@ -1985,7 +1987,7 @@ void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr)
          *      and we still don't have a tlb_flush_mask(env, n, mask) in QEMU,
          *      we just invalidate all TLBs
          */
-        tlb_flush(CPU(cpu), 1);
+        env->tlb_need_flush = 1;
         break;
 #endif /* defined(TARGET_PPC64) */
     default:
@@ -2874,6 +2876,11 @@ void helper_booke206_tlbflush(CPUPPCState *env, target_ulong type)
 }
 
 
+void helper_check_tlb_flush(CPUPPCState *env)
+{
+    check_tlb_flush(env);
+}
+
 /*****************************************************************************/
 
 /* try to fill the TLB and return an exception if error. If retaddr is
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 1119a301154c..62fabe952c35 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3312,9 +3312,32 @@ static void gen_eieio(DisasContext *ctx)
 {
 }
 
+#if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64)
+static inline void gen_check_tlb_flush(DisasContext *ctx)
+{
+    TCGv_i32 t = tcg_temp_new_i32();
+    TCGLabel *l = gen_new_label();
+
+    tcg_gen_ld_i32(t, cpu_env, offsetof(CPUPPCState, tlb_need_flush));
+    tcg_gen_brcondi_i32(TCG_COND_EQ, t, 0, l);
+    gen_helper_check_tlb_flush(cpu_env);
+    gen_set_label(l);
+    tcg_temp_free_i32(t);
+}
+#else
+static inline void gen_check_tlb_flush(DisasContext *ctx) { }
+#endif
+
 /* isync */
 static void gen_isync(DisasContext *ctx)
 {
+    /*
+     * We need to check for a pending TLB flush. This can only happen in
+     * kernel mode however so check MSR_PR
+     */
+    if (!ctx->pr) {
+        gen_check_tlb_flush(ctx);
+    }
     gen_stop_exception(ctx);
 }
 
@@ -3471,6 +3494,15 @@ STCX(stqcx_, 16);
 /* sync */
 static void gen_sync(DisasContext *ctx)
 {
+    uint32_t l = (ctx->opcode >> 21) & 3;
+
+    /*
+     * For l == 2, it's a ptesync, We need to check for a pending TLB flush.
+     * This can only happen in kernel mode however so check MSR_PR as well.
+     */
+    if (l == 2 && !ctx->pr) {
+        gen_check_tlb_flush(ctx);
+    }
 }
 
 /* wait */
@@ -4878,10 +4910,11 @@ static void gen_tlbsync(DisasContext *ctx)
         gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
         return;
     }
-    /* This has no effect: it should ensure that all previous
-     * tlbie have completed
+    /* tlbsync is a nop for server, ptesync handles delayed tlb flush,
+     * embedded however needs to deal with tlbsync. We don't try to be
+     * fancy and swallow the overhead of checking for both.
      */
-    gen_stop_exception(ctx);
+    gen_check_tlb_flush(ctx);
 #endif
 }
 
-- 
2.1.4

[Qemu-devel] [PATCH 04/12] ppc: Add a bunch of hypervisor SPRs to Book3s

[Cédric Le Goater]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

We don't give them a KVM reg number yet as no current KVM version
supports HV mode.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[clg: SPRs AMOR,DAWR,DARWX were already included in commit f401dd32cb8e9]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target-ppc/translate_init.c | 119 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 116 insertions(+), 3 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index f51572552bc2..10a92fdbbdd7 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7788,15 +7788,116 @@ static void gen_spr_power5p_lpar(CPUPPCState *env)
     spr_register_kvm(env, SPR_LPCR, "LPCR",
                      SPR_NOACCESS, SPR_NOACCESS,
                      &spr_read_generic, &spr_write_generic,
-                     KVM_REG_PPC_LPCR, 0x00000000);
+                     KVM_REG_PPC_LPCR, LPCR_LPES0 | LPCR_LPES1);
 }
 
+#if !defined(CONFIG_USER_ONLY)
+static void spr_write_hmer(DisasContext *ctx, int sprn, int gprn)
+{
+    TCGv hmer = tcg_temp_new();
+
+    gen_load_spr(hmer, sprn);
+    tcg_gen_and_tl(hmer, cpu_gpr[gprn], hmer);
+    gen_store_spr(sprn, hmer);
+    spr_store_dump_spr(sprn);
+    tcg_temp_free(hmer);
+}
+#endif
+
 static void gen_spr_book3s_ids(CPUPPCState *env)
 {
+    /* FIXME: Will need to deal with thread vs core only SPRs */
+
     /* Processor identification */
-    spr_register(env, SPR_PIR, "PIR",
+    spr_register_hv(env, SPR_PIR, "PIR",
                  SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_pir,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, NULL,
+                 0x00000000);
+    spr_register_hv(env, SPR_HID0, "HID0",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_TSCR, "TSCR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HMER, "HMER",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_hmer,
+                 0x00000000);
+    spr_register_hv(env, SPR_HMEER, "HMEER",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_TFMR, "TFMR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_LPIDR, "LPIDR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HFSCR, "HFSCR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_MMCRC, "MMCRC",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_MMCRH, "MMCRH",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HSPRG0, "HSPRG0",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HSPRG1, "HSPRG1",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HSRR0, "HSRR0",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HSRR1, "HSRR1",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HDAR, "HDAR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HDSISR, "HDSISR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_RMOR, "RMOR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
+                 0x00000000);
+    spr_register_hv(env, SPR_HRMOR, "HRMOR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
                  0x00000000);
 }
 
@@ -8040,6 +8141,17 @@ static void gen_spr_power8_book4(CPUPPCState *env)
 #endif
 }
 
+static void gen_spr_power8_rpr(CPUPPCState *env)
+{
+#if !defined(CONFIG_USER_ONLY)
+    spr_register_hv(env, SPR_RPR, "RPR",
+                    SPR_NOACCESS, SPR_NOACCESS,
+                    SPR_NOACCESS, SPR_NOACCESS,
+                    &spr_read_generic, &spr_write_generic,
+                    0x00000103070F1F3F);
+#endif
+}
+
 static void init_proc_book3s_64(CPUPPCState *env, int version)
 {
     gen_spr_ne_601(env);
@@ -8094,6 +8206,7 @@ static void init_proc_book3s_64(CPUPPCState *env, int version)
         gen_spr_vtb(env);
         gen_spr_power8_ic(env);
         gen_spr_power8_book4(env);
+        gen_spr_power8_rpr(env);
     }
     if (version < BOOK3S_CPU_POWER8) {
         gen_spr_book3s_dbg(env);
-- 
2.1.4

[Qemu-devel] [PATCH 05/12] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV

[Cédric Le Goater]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

This helper is only used by the various instructions that can alter
MSR and not interrupts. Add a comment to that effect to the interrupt
code as well in case somebody wants to change this

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/excp_helper.c | 8 ++++++--
 target-ppc/helper_regs.h | 4 ++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index 85f38640bdf4..b48a34bfab8b 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -708,8 +708,12 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
         }
     }
 #endif
-    /* XXX: we don't use hreg_store_msr here as already have treated
-     *      any special case that could occur. Just store MSR and update hflags
+    /* We don't use hreg_store_msr here as already have treated
+     * any special case that could occur. Just store MSR and update hflags
+     *
+     * Note: We *MUST* not use hreg_store_msr() as-is anyway because it
+     * will prevent setting of the HV bit which some exceptions might need
+     * to do.
      */
     env->msr = new_msr & env->msr_mask;
     hreg_compute_hflags(env);
diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
index 57da931e3c4d..12af61cbf19b 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -114,8 +114,8 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
     excp = 0;
     value &= env->msr_mask;
 #if !defined(CONFIG_USER_ONLY)
-    if (!alter_hv) {
-        /* mtmsr cannot alter the hypervisor state */
+    /* Neither mtmsr nor guest state can alter HV */
+    if (!alter_hv || !(env->msr & MSR_HVB)) {
         value &= ~MSR_HVB;
         value |= env->msr & MSR_HVB;
     }
-- 
2.1.4

[Qemu-devel] [PATCH 06/12] ppc: Fix rfi/rfid/hrfi/... emulation

[Cédric Le Goater]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

This reworks emulation of the various "rfi" variants. I removed
some masking bits that I couldn't make sense of, the only bit that
I am aware we should mask here is POW, the CPU's MSR mask should
take care of the rest.

This also fixes some problems when running 32-bit userspace under
a 64-bit kernel.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/excp_helper.c | 51 +++++++++++++++++++-----------------------------
 target-ppc/translate.c   |  7 +++++++
 2 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index b48a34bfab8b..e3560aa0ea05 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -922,25 +922,20 @@ void helper_store_msr(CPUPPCState *env, target_ulong val)
     }
 }
 
-static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr,
-                          target_ulong msrm, int keep_msrh)
+static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
 {
     CPUState *cs = CPU(ppc_env_get_cpu(env));
 
+    /* MSR:POW cannot be set by any form of rfi */
+    msr &= ~(1ULL << MSR_POW);
+
 #if defined(TARGET_PPC64)
-    if (msr_is_64bit(env, msr)) {
-        nip = (uint64_t)nip;
-        msr &= (uint64_t)msrm;
-    } else {
+    /* Switching to 32-bit ? Crop the nip */
+    if (!msr_is_64bit(env, msr)) {
         nip = (uint32_t)nip;
-        msr = (uint32_t)(msr & msrm);
-        if (keep_msrh) {
-            msr |= env->msr & ~((uint64_t)0xFFFFFFFF);
-        }
     }
 #else
     nip = (uint32_t)nip;
-    msr &= (uint32_t)msrm;
 #endif
     /* XXX: beware: this is false if VLE is supported */
     env->nip = nip & ~((target_ulong)0x00000003);
@@ -959,26 +954,24 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr,
 
 void helper_rfi(CPUPPCState *env)
 {
-    if (env->excp_model == POWERPC_EXCP_BOOKE) {
-        do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1],
-               ~((target_ulong)0), 0);
-    } else {
-        do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1],
-               ~((target_ulong)0x783F0000), 1);
-    }
+    do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xfffffffful);
 }
 
+#define MSR_BOOK3S_MASK
 #if defined(TARGET_PPC64)
 void helper_rfid(CPUPPCState *env)
 {
-    do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1],
-           ~((target_ulong)0x783F0000), 0);
+    /* The architeture defines a number of rules for which bits
+     * can change but in practice, we handle this in hreg_store_msr()
+     * which will be called by do_rfi(), so there is no need to filter
+     * here
+     */
+    do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1]);
 }
 
 void helper_hrfid(CPUPPCState *env)
 {
-    do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1],
-           ~((target_ulong)0x783F0000), 0);
+    do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
 }
 #endif
 
@@ -986,28 +979,24 @@ void helper_hrfid(CPUPPCState *env)
 /* Embedded PowerPC specific helpers */
 void helper_40x_rfci(CPUPPCState *env)
 {
-    do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3],
-           ~((target_ulong)0xFFFF0000), 0);
+    do_rfi(env, env->spr[SPR_40x_SRR2], env->spr[SPR_40x_SRR3]);
 }
 
 void helper_rfci(CPUPPCState *env)
 {
-    do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1],
-           ~((target_ulong)0), 0);
+    do_rfi(env, env->spr[SPR_BOOKE_CSRR0], env->spr[SPR_BOOKE_CSRR1]);
 }
 
 void helper_rfdi(CPUPPCState *env)
 {
     /* FIXME: choose CSRR1 or DSRR1 based on cpu type */
-    do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1],
-           ~((target_ulong)0), 0);
+    do_rfi(env, env->spr[SPR_BOOKE_DSRR0], env->spr[SPR_BOOKE_DSRR1]);
 }
 
 void helper_rfmci(CPUPPCState *env)
 {
     /* FIXME: choose CSRR1 or MCSRR1 based on cpu type */
-    do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1],
-           ~((target_ulong)0), 0);
+    do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1]);
 }
 #endif
 
@@ -1045,7 +1034,7 @@ void helper_td(CPUPPCState *env, target_ulong arg1, target_ulong arg2,
 
 void helper_rfsvc(CPUPPCState *env)
 {
-    do_rfi(env, env->lr, env->ctr, 0x0000FFFF, 0);
+    do_rfi(env, env->lr, env->ctr & 0x0000FFFF);
 }
 
 /* Embedded.Processor Control */
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 62fabe952c35..7a672cba796d 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -4086,6 +4086,13 @@ static void gen_rfi(DisasContext *ctx)
 #if defined(CONFIG_USER_ONLY)
     gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
 #else
+    /* This instruction doesn't exist anymore on 64-bit server
+     * processors compliant with arch 2.x
+     */
+    if (ctx->insns_flags & PPC_SEGMENT_64B) {
+        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
+        return;
+    }
     /* Restore CPU state */
     if (unlikely(ctx->pr)) {
         gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
-- 
2.1.4

[Qemu-devel] [PATCH 07/12] ppc: Better figure out if processor has HV mode

[Cédric Le Goater]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

We use an env. flag which is set to the initial value of MSR_HVB in
the msr_mask. We also adjust the POWER8 mask to set SHV.

Also use this to adjust ctx.hv so that it is *set* when the processor
doesn't have an HV mode (970 with Apple mode for example), thus enabling
hypervisor instructions/SPRs.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/cpu.h            |  4 ++++
 target-ppc/translate.c      |  4 +++-
 target-ppc/translate_init.c | 21 ++++++++++++++++-----
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 2a96efcbf813..02f2e72e6d14 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1161,6 +1161,10 @@ struct CPUPPCState {
     hwaddr mpic_iack;
     /* true when the external proxy facility mode is enabled */
     bool mpic_proxy;
+    /* set when the processor has an HV mode, thus HV priv
+     * instructions and SPRs are diallowed if MSR:HV is 0
+     */
+    bool has_hv_mode;
 #endif
 
     /* Those resources are used only during code translation */
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 7a672cba796d..6f55bcd34a74 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -11495,8 +11495,10 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
     ctx.exception = POWERPC_EXCP_NONE;
     ctx.spr_cb = env->spr_cb;
     ctx.pr = msr_pr;
-    ctx.hv = !msr_pr && msr_hv;
     ctx.mem_idx = env->dmmu_idx;
+#if !defined(CONFIG_USER_ONLY)
+    ctx.hv = msr_hv || !env->has_hv_mode;
+#endif
     ctx.insns_flags = env->insns_flags;
     ctx.insns_flags2 = env->insns_flags2;
     ctx.access_type = -1;
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 10a92fdbbdd7..df656e6021b4 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8579,7 +8579,8 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
                         PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
                         PPC2_TM;
     pcc->msr_mask = (1ull << MSR_SF) |
-                    (1ull << MSR_TM) |
+                    (1ull << MSR_SHV) |
+		    (1ull << MSR_TM) |
                     (1ull << MSR_VR) |
                     (1ull << MSR_VSX) |
                     (1ull << MSR_EE) |
@@ -9975,10 +9976,7 @@ static void ppc_cpu_reset(CPUState *s)
     pcc->parent_reset(s);
 
     msr = (target_ulong)0;
-    if (0) {
-        /* XXX: find a suitable condition to enable the hypervisor mode */
-        msr |= (target_ulong)MSR_HVB;
-    }
+    msr |= (target_ulong)MSR_HVB;
     msr |= (target_ulong)0 << MSR_AP; /* TO BE CHECKED */
     msr |= (target_ulong)0 << MSR_SA; /* TO BE CHECKED */
     msr |= (target_ulong)1 << MSR_EP;
@@ -10079,6 +10077,19 @@ static void ppc_cpu_initfn(Object *obj)
     env->bfd_mach = pcc->bfd_mach;
     env->check_pow = pcc->check_pow;
 
+    /* Mark HV mode as supported if the CPU has an MSR_HV bit
+     * in the msr_mask. The mask can later be cleared by PAPR
+     * mode but the hv mode support will remain, thus enforcing
+     * that we cannot use priv. instructions in guest in PAPR
+     * mode. For 970 we currently simply don't set HV in msr_mask
+     * thus simulating an "Apple mode" 970. If we ever want to
+     * support 970 HV mode, we'll have to add a processor attribute
+     * of some sort.
+     */
+#if !defined(CONFIG_USER_ONLY)
+    env->has_hv_mode = !!(env->msr_mask & MSR_HVB);
+#endif
+
 #if defined(TARGET_PPC64)
     if (pcc->sps) {
         env->sps = *pcc->sps;
-- 
2.1.4

[Qemu-devel] [PATCH 08/12] ppc: tlbie, tlbia and tlbisync are HV only

[Cédric Le Goater]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Not that anything remotely recent supports tlbia but ...

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 target-ppc/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 6f55bcd34a74..5518f01ec3cd 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -4864,7 +4864,7 @@ static void gen_tlbia(DisasContext *ctx)
 #if defined(CONFIG_USER_ONLY)
     gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
 #else
-    if (unlikely(ctx->pr)) {
+    if (unlikely(ctx->pr || !ctx->hv)) {
         gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
         return;
     }
@@ -4878,7 +4878,7 @@ static void gen_tlbiel(DisasContext *ctx)
 #if defined(CONFIG_USER_ONLY)
     gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
 #else
-    if (unlikely(ctx->pr)) {
+    if (unlikely(ctx->pr || !ctx->hv)) {
         gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
         return;
     }
@@ -4892,7 +4892,7 @@ static void gen_tlbie(DisasContext *ctx)
 #if defined(CONFIG_USER_ONLY)
     gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
 #else
-    if (unlikely(ctx->pr)) {
+    if (unlikely(ctx->pr || !ctx->hv)) {
         gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
         return;
     }
-- 
2.1.4

[Qemu-devel] [PATCH 09/12] ppc: Change 'invalid' bit mask of tlbiel and tlbie

[Cédric Le Goater]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Otherwise it will trip on the forms used in recent architecture.

Ideally, we should have different handlers for different architecture
levels but our current implementation of TLB flushing is dumb enough
that this will do for now.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/translate.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 5518f01ec3cd..4f18dc8bca2f 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -9980,8 +9980,10 @@ GEN_HANDLER2(slbmfee, "slbmfee", 0x1F, 0x13, 0x1C, 0x001F0001, PPC_SEGMENT_64B),
 GEN_HANDLER2(slbmfev, "slbmfev", 0x1F, 0x13, 0x1A, 0x001F0001, PPC_SEGMENT_64B),
 #endif
 GEN_HANDLER(tlbia, 0x1F, 0x12, 0x0B, 0x03FFFC01, PPC_MEM_TLBIA),
-GEN_HANDLER(tlbiel, 0x1F, 0x12, 0x08, 0x03FF0001, PPC_MEM_TLBIE),
-GEN_HANDLER(tlbie, 0x1F, 0x12, 0x09, 0x03FF0001, PPC_MEM_TLBIE),
+/* XXX Those instructions will need to be handled differently for
+ * different ISA versions */
+GEN_HANDLER(tlbiel, 0x1F, 0x12, 0x08, 0x001F0001, PPC_MEM_TLBIE),
+GEN_HANDLER(tlbie, 0x1F, 0x12, 0x09, 0x001F0001, PPC_MEM_TLBIE),
 GEN_HANDLER(tlbsync, 0x1F, 0x16, 0x11, 0x03FFF801, PPC_MEM_TLBSYNC),
 #if defined(TARGET_PPC64)
 GEN_HANDLER(slbia, 0x1F, 0x12, 0x0F, 0x03FFFC01, PPC_SLBI),
-- 
2.1.4

[Qemu-devel] [PATCH 10/12] ppc: Fix sign extension issue in mtmsr(d) emulation

[Cédric Le Goater]

From: Michael Neuling <mikey@neuling.org>

Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 4f18dc8bca2f..3fe08e0920d8 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -4415,7 +4415,7 @@ static void gen_mtmsrd(DisasContext *ctx)
         /* Special form that does not need any synchronisation */
         TCGv t0 = tcg_temp_new();
         tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)], (1 << MSR_RI) | (1 << MSR_EE));
-        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~((1 << MSR_RI) | (1 << MSR_EE)));
+        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
         tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
         tcg_temp_free(t0);
     } else {
@@ -4446,7 +4446,7 @@ static void gen_mtmsr(DisasContext *ctx)
         /* Special form that does not need any synchronisation */
         TCGv t0 = tcg_temp_new();
         tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)], (1 << MSR_RI) | (1 << MSR_EE));
-        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~((1 << MSR_RI) | (1 << MSR_EE)));
+        tcg_gen_andi_tl(cpu_msr, cpu_msr, ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
         tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
         tcg_temp_free(t0);
     } else {
-- 
2.1.4

[Qemu-devel] [PATCH 11/12] ppc: Get out of emulation on SMT "OR" ops

[Cédric Le Goater]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Otherwise tight loops at smt_low for example, which OPAL does,
eat so much CPU that we can't boot a kernel anymore. With that,
I can boot 8 CPUs just fine with powernv.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/translate.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 3fe08e0920d8..875862db33ee 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1398,6 +1398,19 @@ GEN_LOGICAL2(nand, tcg_gen_nand_tl, 0x0E, PPC_INTEGER);
 /* nor & nor. */
 GEN_LOGICAL2(nor, tcg_gen_nor_tl, 0x03, PPC_INTEGER);
 
+#if defined(TARGET_PPC64)
+static void gen_pause(DisasContext *ctx)
+{
+    TCGv_i32 t0 = tcg_const_i32(0);
+    tcg_gen_st_i32(t0, cpu_env,
+                   -offsetof(PowerPCCPU, env) + offsetof(CPUState, halted));
+    tcg_temp_free_i32(t0);
+
+    /* Stop translation, this gives other CPUs a chance to run */
+    gen_exception_err(ctx, EXCP_HLT, 1);
+}
+#endif /* defined(TARGET_PPC64) */
+
 /* or & or. */
 static void gen_or(DisasContext *ctx)
 {
@@ -1453,7 +1466,7 @@ static void gen_or(DisasContext *ctx)
             }
             break;
         case 7:
-            if (ctx->hv) {
+            if (ctx->hv && !ctx->pr) {
                 /* Set process priority to very high */
                 prio = 7;
             }
@@ -1470,6 +1483,10 @@ static void gen_or(DisasContext *ctx)
             tcg_gen_ori_tl(t0, t0, ((uint64_t)prio) << 50);
             gen_store_spr(SPR_PPR, t0);
             tcg_temp_free(t0);
+            /* Pause us out of TCG otherwise spin loops with smt_low
+             * eat too much CPU and the kernel hangs
+             */
+            gen_pause(ctx);
         }
 #endif
     }
@@ -1495,8 +1512,6 @@ static void gen_ori(DisasContext *ctx)
     target_ulong uimm = UIMM(ctx->opcode);
 
     if (rS(ctx->opcode) == rA(ctx->opcode) && uimm == 0) {
-        /* NOP */
-        /* XXX: should handle special NOPs for POWER series */
         return;
     }
     tcg_gen_ori_tl(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)], uimm);
-- 
2.1.4

[Qemu-devel] [PATCH 12/12] ppc: Add PPC_64H instruction flag to POWER7 and POWER8

[Cédric Le Goater]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

This will enable decoding of hrfid

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/translate_init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index df656e6021b4..ccb1b45c1c6e 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8488,7 +8488,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
                        PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
                        PPC_MEM_SYNC | PPC_MEM_EIEIO |
                        PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
-                       PPC_64B | PPC_ALTIVEC |
+                       PPC_64B | PPC_64H | PPC_ALTIVEC |
                        PPC_SEGMENT_64B | PPC_SLBI |
                        PPC_POPCNTB | PPC_POPCNTWD;
     pcc->insns_flags2 = PPC2_VSX | PPC2_DFP | PPC2_DBRX | PPC2_ISA205 |
@@ -8568,7 +8568,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
                        PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
                        PPC_MEM_SYNC | PPC_MEM_EIEIO |
                        PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
-                       PPC_64B | PPC_64BX | PPC_ALTIVEC |
+                       PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
                        PPC_SEGMENT_64B | PPC_SLBI |
                        PPC_POPCNTB | PPC_POPCNTWD;
     pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
-- 
2.1.4

Re: [Qemu-devel] [PATCH 00/12] ppc: preparing pnv landing (round 2)

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1840 bytes --]

On Tue, May 03, 2016 at 06:03:22PM +0200, Cédric Le Goater wrote:
> Hello,
> 
> Here is a new set of fixes extracted from Ben's PowerNV tree :
> 
>      https://github.com/ozbenh/qemu/commits/powernv
> 
> It was quickly tested with a pseries guest using KVM and TCG.

I'm on holiday - please send to agraf.

> 
> Thanks,
> 
> C.
> 
> Benjamin Herrenschmidt (11):
>   ppc: Remove MMU_MODEn_SUFFIX definitions
>   ppc: Use split I/D mmu modes to avoid flushes on interrupts
>   ppc: Do some batching of TCG tlb flushes
>   ppc: Add a bunch of hypervisor SPRs to Book3s
>   ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV
>   ppc: Fix rfi/rfid/hrfi/... emulation
>   ppc: Better figure out if processor has HV mode
>   ppc: tlbie, tlbia and tlbisync are HV only
>   ppc: Change 'invalid' bit mask of tlbiel and tlbie
>   ppc: Get out of emulation on SMT "OR" ops
>   ppc: Add PPC_64H instruction flag to POWER7 and POWER8
> 
> Michael Neuling (1):
>   ppc: Fix sign extension issue in mtmsr(d) emulation
> 
>  hw/ppc/spapr_hcall.c        |  14 ++++-
>  target-ppc/cpu.h            |  20 ++++--
>  target-ppc/excp_helper.c    |  77 +++++++++++------------
>  target-ppc/helper.h         |   1 +
>  target-ppc/helper_regs.h    |  71 +++++++++++++++++++---
>  target-ppc/machine.c        |   5 +-
>  target-ppc/mmu-hash64.c     |  11 +---
>  target-ppc/mmu_helper.c     |   9 ++-
>  target-ppc/translate.c      |  94 +++++++++++++++++++++++------
>  target-ppc/translate_init.c | 144 +++++++++++++++++++++++++++++++++++++++++---
>  10 files changed, 348 insertions(+), 98 deletions(-)
> 

-- 
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: 819 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH 07/12] ppc: Better figure out if processor has HV mode

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 4338 bytes --]

On Tue, May 03, 2016 at 06:03:29PM +0200, Cédric Le Goater wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> We use an env. flag which is set to the initial value of MSR_HVB in
> the msr_mask. We also adjust the POWER8 mask to set SHV.
> 
> Also use this to adjust ctx.hv so that it is *set* when the processor
> doesn't have an HV mode (970 with Apple mode for example), thus enabling
> hypervisor instructions/SPRs.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  target-ppc/cpu.h            |  4 ++++
>  target-ppc/translate.c      |  4 +++-
>  target-ppc/translate_init.c | 21 ++++++++++++++++-----
>  3 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 2a96efcbf813..02f2e72e6d14 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -1161,6 +1161,10 @@ struct CPUPPCState {
>      hwaddr mpic_iack;
>      /* true when the external proxy facility mode is enabled */
>      bool mpic_proxy;
> +    /* set when the processor has an HV mode, thus HV priv
> +     * instructions and SPRs are diallowed if MSR:HV is 0
> +     */
> +    bool has_hv_mode;
>  #endif
>  
>      /* Those resources are used only during code translation */
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 7a672cba796d..6f55bcd34a74 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -11495,8 +11495,10 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
>      ctx.exception = POWERPC_EXCP_NONE;
>      ctx.spr_cb = env->spr_cb;
>      ctx.pr = msr_pr;
> -    ctx.hv = !msr_pr && msr_hv;

The test for msr_pr has been removed in the new version.  Maybe that's
safe, but I think it needs some justification.

>      ctx.mem_idx = env->dmmu_idx;
> +#if !defined(CONFIG_USER_ONLY)
> +    ctx.hv = msr_hv || !env->has_hv_mode;
> +#endif
>      ctx.insns_flags = env->insns_flags;
>      ctx.insns_flags2 = env->insns_flags2;
>      ctx.access_type = -1;
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 10a92fdbbdd7..df656e6021b4 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -8579,7 +8579,8 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>                          PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>                          PPC2_TM;
>      pcc->msr_mask = (1ull << MSR_SF) |
> -                    (1ull << MSR_TM) |
> +                    (1ull << MSR_SHV) |
> +		    (1ull << MSR_TM) |
>                      (1ull << MSR_VR) |
>                      (1ull << MSR_VSX) |
>                      (1ull << MSR_EE) |
> @@ -9975,10 +9976,7 @@ static void ppc_cpu_reset(CPUState *s)
>      pcc->parent_reset(s);
>  
>      msr = (target_ulong)0;
> -    if (0) {
> -        /* XXX: find a suitable condition to enable the hypervisor mode */
> -        msr |= (target_ulong)MSR_HVB;
> -    }
> +    msr |= (target_ulong)MSR_HVB;
>      msr |= (target_ulong)0 << MSR_AP; /* TO BE CHECKED */
>      msr |= (target_ulong)0 << MSR_SA; /* TO BE CHECKED */
>      msr |= (target_ulong)1 << MSR_EP;
> @@ -10079,6 +10077,19 @@ static void ppc_cpu_initfn(Object *obj)
>      env->bfd_mach = pcc->bfd_mach;
>      env->check_pow = pcc->check_pow;
>  
> +    /* Mark HV mode as supported if the CPU has an MSR_HV bit
> +     * in the msr_mask. The mask can later be cleared by PAPR
> +     * mode but the hv mode support will remain, thus enforcing
> +     * that we cannot use priv. instructions in guest in PAPR
> +     * mode. For 970 we currently simply don't set HV in msr_mask
> +     * thus simulating an "Apple mode" 970. If we ever want to
> +     * support 970 HV mode, we'll have to add a processor attribute
> +     * of some sort.
> +     */
> +#if !defined(CONFIG_USER_ONLY)
> +    env->has_hv_mode = !!(env->msr_mask & MSR_HVB);
> +#endif
> +
>  #if defined(TARGET_PPC64)
>      if (pcc->sps) {
>          env->sps = *pcc->sps;

-- 
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: 819 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH 00/12] ppc: preparing pnv landing (round 2)

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1930 bytes --]

On Tue, May 03, 2016 at 06:03:22PM +0200, Cédric Le Goater wrote:
> Hello,
> 
> Here is a new set of fixes extracted from Ben's PowerNV tree :
> 
>      https://github.com/ozbenh/qemu/commits/powernv
> 
> It was quickly tested with a pseries guest using KVM and TCG.

Ok, back now.

I've merged these into ppc-for-2.7 (after today's pull req), except
for 7/12 which I've replied to separately.

> 
> Thanks,
> 
> C.
> 
> Benjamin Herrenschmidt (11):
>   ppc: Remove MMU_MODEn_SUFFIX definitions
>   ppc: Use split I/D mmu modes to avoid flushes on interrupts
>   ppc: Do some batching of TCG tlb flushes
>   ppc: Add a bunch of hypervisor SPRs to Book3s
>   ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV
>   ppc: Fix rfi/rfid/hrfi/... emulation
>   ppc: Better figure out if processor has HV mode
>   ppc: tlbie, tlbia and tlbisync are HV only
>   ppc: Change 'invalid' bit mask of tlbiel and tlbie
>   ppc: Get out of emulation on SMT "OR" ops
>   ppc: Add PPC_64H instruction flag to POWER7 and POWER8
> 
> Michael Neuling (1):
>   ppc: Fix sign extension issue in mtmsr(d) emulation
> 
>  hw/ppc/spapr_hcall.c        |  14 ++++-
>  target-ppc/cpu.h            |  20 ++++--
>  target-ppc/excp_helper.c    |  77 +++++++++++------------
>  target-ppc/helper.h         |   1 +
>  target-ppc/helper_regs.h    |  71 +++++++++++++++++++---
>  target-ppc/machine.c        |   5 +-
>  target-ppc/mmu-hash64.c     |  11 +---
>  target-ppc/mmu_helper.c     |   9 ++-
>  target-ppc/translate.c      |  94 +++++++++++++++++++++++------
>  target-ppc/translate_init.c | 144 +++++++++++++++++++++++++++++++++++++++++---
>  10 files changed, 348 insertions(+), 98 deletions(-)
> 

-- 
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: 819 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH 07/12] ppc: Better figure out if processor has HV mode

[Thomas Huth]

[-- Attachment #1: Type: text/plain, Size: 1844 bytes --]

On 27.05.2016 05:38, David Gibson wrote:
> On Tue, May 03, 2016 at 06:03:29PM +0200, Cédric Le Goater wrote:
>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>
>> We use an env. flag which is set to the initial value of MSR_HVB in
>> the msr_mask. We also adjust the POWER8 mask to set SHV.
>>
>> Also use this to adjust ctx.hv so that it is *set* when the processor
>> doesn't have an HV mode (970 with Apple mode for example), thus enabling
>> hypervisor instructions/SPRs.
>>
>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>>  target-ppc/cpu.h            |  4 ++++
>>  target-ppc/translate.c      |  4 +++-
>>  target-ppc/translate_init.c | 21 ++++++++++++++++-----
>>  3 files changed, 23 insertions(+), 6 deletions(-)
...
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 10a92fdbbdd7..df656e6021b4 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -8579,7 +8579,8 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>>                          PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>>                          PPC2_TM;
>>      pcc->msr_mask = (1ull << MSR_SF) |
>> -                    (1ull << MSR_TM) |
>> +                    (1ull << MSR_SHV) |
>> +		    (1ull << MSR_TM) |
>>                      (1ull << MSR_VR) |
>>                      (1ull << MSR_VSX) |
>>                      (1ull << MSR_EE) |

This indentation looks somewhat suspicious ... and indeed, checkpatch
compains here:

ERROR: code indent should never use tabs
#153: FILE: target-ppc/translate_init.c:8583:
+^I^I    (1ull << MSR_TM) |$

total: 1 errors, 0 warnings, 60 lines checked

Please fix it to use spaces instead.

 Thomas




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH 07/12] ppc: Better figure out if processor has HV mode

[Cédric Le Goater]

On 05/27/2016 05:38 AM, David Gibson wrote:
> On Tue, May 03, 2016 at 06:03:29PM +0200, Cédric Le Goater wrote:
>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>
>> We use an env. flag which is set to the initial value of MSR_HVB in
>> the msr_mask. We also adjust the POWER8 mask to set SHV.
>>
>> Also use this to adjust ctx.hv so that it is *set* when the processor
>> doesn't have an HV mode (970 with Apple mode for example), thus enabling
>> hypervisor instructions/SPRs.
>>
>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>>  target-ppc/cpu.h            |  4 ++++
>>  target-ppc/translate.c      |  4 +++-
>>  target-ppc/translate_init.c | 21 ++++++++++++++++-----
>>  3 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index 2a96efcbf813..02f2e72e6d14 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -1161,6 +1161,10 @@ struct CPUPPCState {
>>      hwaddr mpic_iack;
>>      /* true when the external proxy facility mode is enabled */
>>      bool mpic_proxy;
>> +    /* set when the processor has an HV mode, thus HV priv
>> +     * instructions and SPRs are diallowed if MSR:HV is 0
>> +     */
>> +    bool has_hv_mode;
>>  #endif
>>  
>>      /* Those resources are used only during code translation */
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index 7a672cba796d..6f55bcd34a74 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -11495,8 +11495,10 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
>>      ctx.exception = POWERPC_EXCP_NONE;
>>      ctx.spr_cb = env->spr_cb;
>>      ctx.pr = msr_pr;
>> -    ctx.hv = !msr_pr && msr_hv;
> 
> The test for msr_pr has been removed in the new version.  Maybe that's
> safe, but I think it needs some justification.

I don't know if it is safe to run with (MSR_HV|MSR_PR) = 11
 
There is a note in the Power ISA book describing such a case but I am not 
sure qemu-ppc supports that. So I will keep the msr_pr check in the next 
version :

	ctx.hv = !msr_pr && (msr_hv || !env->has_hv_mode);
 

Thanks,

C.

>>      ctx.mem_idx = env->dmmu_idx;
>> +#if !defined(CONFIG_USER_ONLY)
>> +    ctx.hv = msr_hv || !env->has_hv_mode;
>> +#endif
>>      ctx.insns_flags = env->insns_flags;
>>      ctx.insns_flags2 = env->insns_flags2;
>>      ctx.access_type = -1;
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 10a92fdbbdd7..df656e6021b4 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -8579,7 +8579,8 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>>                          PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>>                          PPC2_TM;
>>      pcc->msr_mask = (1ull << MSR_SF) |
>> -                    (1ull << MSR_TM) |
>> +                    (1ull << MSR_SHV) |
>> +		    (1ull << MSR_TM) |
>>                      (1ull << MSR_VR) |
>>                      (1ull << MSR_VSX) |
>>                      (1ull << MSR_EE) |
>> @@ -9975,10 +9976,7 @@ static void ppc_cpu_reset(CPUState *s)
>>      pcc->parent_reset(s);
>>  
>>      msr = (target_ulong)0;
>> -    if (0) {
>> -        /* XXX: find a suitable condition to enable the hypervisor mode */
>> -        msr |= (target_ulong)MSR_HVB;
>> -    }
>> +    msr |= (target_ulong)MSR_HVB;
>>      msr |= (target_ulong)0 << MSR_AP; /* TO BE CHECKED */
>>      msr |= (target_ulong)0 << MSR_SA; /* TO BE CHECKED */
>>      msr |= (target_ulong)1 << MSR_EP;
>> @@ -10079,6 +10077,19 @@ static void ppc_cpu_initfn(Object *obj)
>>      env->bfd_mach = pcc->bfd_mach;
>>      env->check_pow = pcc->check_pow;
>>  
>> +    /* Mark HV mode as supported if the CPU has an MSR_HV bit
>> +     * in the msr_mask. The mask can later be cleared by PAPR
>> +     * mode but the hv mode support will remain, thus enforcing
>> +     * that we cannot use priv. instructions in guest in PAPR
>> +     * mode. For 970 we currently simply don't set HV in msr_mask
>> +     * thus simulating an "Apple mode" 970. If we ever want to
>> +     * support 970 HV mode, we'll have to add a processor attribute
>> +     * of some sort.
>> +     */
>> +#if !defined(CONFIG_USER_ONLY)
>> +    env->has_hv_mode = !!(env->msr_mask & MSR_HVB);
>> +#endif
>> +
>>  #if defined(TARGET_PPC64)
>>      if (pcc->sps) {
>>          env->sps = *pcc->sps;
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH 07/12] ppc: Better figure out if processor has HV mode

[Cédric Le Goater]

On 05/27/2016 06:41 AM, Thomas Huth wrote:
> On 27.05.2016 05:38, David Gibson wrote:
>> On Tue, May 03, 2016 at 06:03:29PM +0200, Cédric Le Goater wrote:
>>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>>
>>> We use an env. flag which is set to the initial value of MSR_HVB in
>>> the msr_mask. We also adjust the POWER8 mask to set SHV.
>>>
>>> Also use this to adjust ctx.hv so that it is *set* when the processor
>>> doesn't have an HV mode (970 with Apple mode for example), thus enabling
>>> hypervisor instructions/SPRs.
>>>
>>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>>> ---
>>>  target-ppc/cpu.h            |  4 ++++
>>>  target-ppc/translate.c      |  4 +++-
>>>  target-ppc/translate_init.c | 21 ++++++++++++++++-----
>>>  3 files changed, 23 insertions(+), 6 deletions(-)
> ...
>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>> index 10a92fdbbdd7..df656e6021b4 100644
>>> --- a/target-ppc/translate_init.c
>>> +++ b/target-ppc/translate_init.c
>>> @@ -8579,7 +8579,8 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>>>                          PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>>>                          PPC2_TM;
>>>      pcc->msr_mask = (1ull << MSR_SF) |
>>> -                    (1ull << MSR_TM) |
>>> +                    (1ull << MSR_SHV) |
>>> +		    (1ull << MSR_TM) |
>>>                      (1ull << MSR_VR) |
>>>                      (1ull << MSR_VSX) |
>>>                      (1ull << MSR_EE) |
> 
> This indentation looks somewhat suspicious ... and indeed, checkpatch
> compains here:
> 
> ERROR: code indent should never use tabs
> #153: FILE: target-ppc/translate_init.c:8583:
> +^I^I    (1ull << MSR_TM) |$
> 
> total: 1 errors, 0 warnings, 60 lines checked
> 
> Please fix it to use spaces instead.

Sure. I will. Thanks for checking. A couple of other patches for PowerNV 
are in the same state. 

Sometimes I wish I had a reasonable 'indent' command line. 

C. 

Re: [Qemu-devel] [Qemu-ppc] [PATCH 07/12] ppc: Better figure out if processor has HV mode

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2689 bytes --]

On Fri, May 27, 2016 at 09:56:32AM +0200, Cédric Le Goater wrote:
> On 05/27/2016 05:38 AM, David Gibson wrote:
> > On Tue, May 03, 2016 at 06:03:29PM +0200, Cédric Le Goater wrote:
> >> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >>
> >> We use an env. flag which is set to the initial value of MSR_HVB in
> >> the msr_mask. We also adjust the POWER8 mask to set SHV.
> >>
> >> Also use this to adjust ctx.hv so that it is *set* when the processor
> >> doesn't have an HV mode (970 with Apple mode for example), thus enabling
> >> hypervisor instructions/SPRs.
> >>
> >> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> >> ---
> >>  target-ppc/cpu.h            |  4 ++++
> >>  target-ppc/translate.c      |  4 +++-
> >>  target-ppc/translate_init.c | 21 ++++++++++++++++-----
> >>  3 files changed, 23 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> >> index 2a96efcbf813..02f2e72e6d14 100644
> >> --- a/target-ppc/cpu.h
> >> +++ b/target-ppc/cpu.h
> >> @@ -1161,6 +1161,10 @@ struct CPUPPCState {
> >>      hwaddr mpic_iack;
> >>      /* true when the external proxy facility mode is enabled */
> >>      bool mpic_proxy;
> >> +    /* set when the processor has an HV mode, thus HV priv
> >> +     * instructions and SPRs are diallowed if MSR:HV is 0
> >> +     */
> >> +    bool has_hv_mode;
> >>  #endif
> >>  
> >>      /* Those resources are used only during code translation */
> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> >> index 7a672cba796d..6f55bcd34a74 100644
> >> --- a/target-ppc/translate.c
> >> +++ b/target-ppc/translate.c
> >> @@ -11495,8 +11495,10 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
> >>      ctx.exception = POWERPC_EXCP_NONE;
> >>      ctx.spr_cb = env->spr_cb;
> >>      ctx.pr = msr_pr;
> >> -    ctx.hv = !msr_pr && msr_hv;
> > 
> > The test for msr_pr has been removed in the new version.  Maybe that's
> > safe, but I think it needs some justification.
> 
> I don't know if it is safe to run with (MSR_HV|MSR_PR) = 11

Um.. I believe host userland runs routinely in that state.

> There is a note in the Power ISA book describing such a case but I am not 
> sure qemu-ppc supports that. So I will keep the msr_pr check in the next 
> version :
> 
> 	ctx.hv = !msr_pr && (msr_hv || !env->has_hv_mode);

Ok.

-- 
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: 819 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCH 07/12] ppc: Better figure out if processor has HV mode

[Cédric Le Goater]

On 05/28/2016 11:52 AM, David Gibson wrote:
> On Fri, May 27, 2016 at 09:56:32AM +0200, Cédric Le Goater wrote:
>> On 05/27/2016 05:38 AM, David Gibson wrote:
>>> On Tue, May 03, 2016 at 06:03:29PM +0200, Cédric Le Goater wrote:
>>>> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>>>
>>>> We use an env. flag which is set to the initial value of MSR_HVB in
>>>> the msr_mask. We also adjust the POWER8 mask to set SHV.
>>>>
>>>> Also use this to adjust ctx.hv so that it is *set* when the processor
>>>> doesn't have an HV mode (970 with Apple mode for example), thus enabling
>>>> hypervisor instructions/SPRs.
>>>>
>>>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>>>> ---
>>>>  target-ppc/cpu.h            |  4 ++++
>>>>  target-ppc/translate.c      |  4 +++-
>>>>  target-ppc/translate_init.c | 21 ++++++++++++++++-----
>>>>  3 files changed, 23 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>>> index 2a96efcbf813..02f2e72e6d14 100644
>>>> --- a/target-ppc/cpu.h
>>>> +++ b/target-ppc/cpu.h
>>>> @@ -1161,6 +1161,10 @@ struct CPUPPCState {
>>>>      hwaddr mpic_iack;
>>>>      /* true when the external proxy facility mode is enabled */
>>>>      bool mpic_proxy;
>>>> +    /* set when the processor has an HV mode, thus HV priv
>>>> +     * instructions and SPRs are diallowed if MSR:HV is 0
>>>> +     */
>>>> +    bool has_hv_mode;
>>>>  #endif
>>>>  
>>>>      /* Those resources are used only during code translation */
>>>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>>>> index 7a672cba796d..6f55bcd34a74 100644
>>>> --- a/target-ppc/translate.c
>>>> +++ b/target-ppc/translate.c
>>>> @@ -11495,8 +11495,10 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
>>>>      ctx.exception = POWERPC_EXCP_NONE;
>>>>      ctx.spr_cb = env->spr_cb;
>>>>      ctx.pr = msr_pr;
>>>> -    ctx.hv = !msr_pr && msr_hv;
>>>
>>> The test for msr_pr has been removed in the new version.  Maybe that's
>>> safe, but I think it needs some justification.
>>
>> I don't know if it is safe to run with (MSR_HV|MSR_PR) = 11
> 
> Um.. I believe host userland runs routinely in that state.

oui oui oui ... of course. hmm, I need to dig more the consequences of 
the *HV* patches. 

Thanks,

C. 

>> There is a note in the Power ISA book describing such a case but I am not 
>> sure qemu-ppc supports that. So I will keep the msr_pr check in the next 
>> version :
>>
>> 	ctx.hv = !msr_pr && (msr_hv || !env->has_hv_mode);
> 
> Ok.
>