Re: [Qemu-ppc] [Qemu-devel] [PATCH 0/3] ppc: complete the new HV mode

[Cédric Le Goater]

Hello Mark,

On 06/03/2016 03:52 PM, Mark Cave-Ayland wrote:
> On 03/06/16 13:11, Cédric Le Goater wrote:
> 
>> This is follow up to complete the serie "ppc: preparing pnv landing
>> (round 2)" plus a little fix on instruction privileges.
>>
>> Tested on a POWER8 pserie guest and on mac99.
>>
>> Benjamin Herrenschmidt (2):
>>   ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV
>>   ppc: Better figure out if processor has HV mode
>>
>> Cédric Le Goater (1):
>>   ppc: fix hrfid, tlbia and slbia privilege
>>
>>  target-ppc/cpu.h            |  4 ++++
>>  target-ppc/excp_helper.c    |  8 ++++++--
>>  target-ppc/helper_regs.h    |  4 ++--
>>  target-ppc/translate.c      | 10 ++++++----
>>  target-ppc/translate_init.c | 19 +++++++++++++++----
>>  5 files changed, 33 insertions(+), 12 deletions(-)
> 
> Hi Cédric,
> 
> I can confirm that this patchset fixes starting up OpenBIOS for both
> g3beige/mac99 in my tests here. With the escc fix also applied, the only
> outstanding issue is the removal of the tlb_flush() statements which
> causes Darwin, MacOS X and HelenOS 0.60 to panic on boot
> 
> My only request is if it would be possible to move patch 2 "ppc: Better
> figure out if processor has HV mode" to the front of this patchset which
> will make the remaining patches bisectable for the Mac machines. With that:
> 
> Tested-by: Mark Cave-Ayland <address@hidden>
> 
> Does anyone know if Ben has any ideas as to why the MMU tlb_flush
> changes patch is causing such problems?


Here is a fix I think. Could you give it a try ? 

Cheers,

C. 

From: Cédric Le Goater <address@hidden>
Subject: [PATCH] ppc: fix tlb flushes for 32bit environment
Date: Sun, 05 Jun 2016 18:46:19 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

commit cd0c6f473532 ('ppc: Do some batching of TCG tlb flushes')
introduced an optimisation to flush TLBs only when a context
synchronizing event is reached (interrupt, rfi). This was done for
ppc64 but 32bit was forgotten on the way.

Tested on mac99 and g3beige with

    qemu-system-ppc -cdrom darwinppc-602.cdr -boot d

Signed-off-by: Cédric Le Goater <address@hidden>
---

 I think the hunk in powerpc_excp() is needed if we don't generate a
 context synchronizing event. what is best to do ?

 target-ppc/cpu.h         |    2 +-
 target-ppc/excp_helper.c |   10 ++++++++++
 target-ppc/helper_regs.h |    9 ++++++++-
 target-ppc/translate.c   |    2 +-
 4 files changed, 20 insertions(+), 3 deletions(-)

Index: qemu-dgibson-for-2.7.git/target-ppc/translate.c
===================================================================
--- qemu-dgibson-for-2.7.git.orig/target-ppc/translate.c
+++ qemu-dgibson-for-2.7.git/target-ppc/translate.c
@@ -3290,7 +3290,7 @@ static void gen_eieio(DisasContext *ctx)
 {
 }
 
-#if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64)
+#if !defined(CONFIG_USER_ONLY)
 static inline void gen_check_tlb_flush(DisasContext *ctx)
 {
     TCGv_i32 t = tcg_temp_new_i32();
Index: qemu-dgibson-for-2.7.git/target-ppc/cpu.h
===================================================================
--- qemu-dgibson-for-2.7.git.orig/target-ppc/cpu.h
+++ qemu-dgibson-for-2.7.git/target-ppc/cpu.h
@@ -958,9 +958,9 @@ struct CPUPPCState {
     /* PowerPC 64 SLB area */
     ppc_slb_t slb[MAX_SLB_ENTRIES];
     int32_t slb_nr;
+#endif
     /* tcg TLB needs flush (deferred slb inval instruction typically) */
     uint32_t tlb_need_flush;
-#endif
     /* segment registers */
     hwaddr htab_base;
     /* mask used to normalize hash value to PTEG index */
Index: qemu-dgibson-for-2.7.git/target-ppc/helper_regs.h
===================================================================
--- qemu-dgibson-for-2.7.git.orig/target-ppc/helper_regs.h
+++ qemu-dgibson-for-2.7.git/target-ppc/helper_regs.h
@@ -121,6 +121,13 @@ static inline int hreg_store_msr(CPUPPCS
     }
     if (((value >> MSR_IR) & 1) != msr_ir ||
         ((value >> MSR_DR) & 1) != msr_dr) {
+        /* A change of the instruction relocation bit in the MSR can
+         * cause an implicit branch in the address space. This
+         * requires a tlb flush.
+         */
+        if (env->mmu_model & POWERPC_MMU_32B) {
+            env->tlb_need_flush = 1;
+        }
         cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
     }
     if ((env->mmu_model & POWERPC_MMU_BOOKE) &&
@@ -151,7 +158,7 @@ static inline int hreg_store_msr(CPUPPCS
     return excp;
 }
 
-#if !defined(CONFIG_USER_ONLY) && defined(TARGET_PPC64)
+#if !defined(CONFIG_USER_ONLY)
 static inline void check_tlb_flush(CPUPPCState *env)
 {
     CPUState *cs = CPU(ppc_env_get_cpu(env));
Index: qemu-dgibson-for-2.7.git/target-ppc/excp_helper.c
===================================================================
--- qemu-dgibson-for-2.7.git.orig/target-ppc/excp_helper.c
+++ qemu-dgibson-for-2.7.git/target-ppc/excp_helper.c
@@ -709,6 +709,16 @@ static inline void powerpc_excp(PowerPCC
         }
     }
 #endif
+    if (((new_msr >> MSR_IR) & 1) != msr_ir ||
+        ((new_msr >> MSR_DR) & 1) != msr_dr) {
+        /* A change of the instruction relocation bit in the MSR can
+         * cause an implicit branch in the address space. This
+         * requires a tlb flush.
+         */
+        if (env->mmu_model & POWERPC_MMU_32B) {
+            env->tlb_need_flush = 1;
+        }
+    }
     /* We don't use hreg_store_msr here as already have treated
      * any special case that could occur. Just store MSR and update hflags
      *