diff mbox

[v2] ppc: Fix support for odd MSR combinations

Message ID 1468035691.20552.29.camel@kernel.crashing.org (mailing list archive)
State New, archived
Headers show

Commit Message

Benjamin Herrenschmidt July 9, 2016, 3:41 a.m. UTC
MacOS uses an architecturally illegal MSR combination that
seems nonetheless supported by 32-bit processors, which is
to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.

This adds support for it. To work properly we need to also
properly include support for PR=1,{I,D}R=0 to the MMU index
used by the qemu TLB.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

v2. Use the correct flags

 target-ppc/helper_regs.h | 46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

Comments

Benjamin Herrenschmidt July 9, 2016, 3:42 a.m. UTC | #1
On Sat, 2016-07-09 at 13:41 +1000, Benjamin Herrenschmidt wrote:
> MacOS uses an architecturally illegal MSR combination that
> seems nonetheless supported by 32-bit processors, which is
> to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.
> 
> This adds support for it. To work properly we need to also
> properly include support for PR=1,{I,D}R=0 to the MMU index
> used by the qemu TLB.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Note Mark that with this patch on top of the dbdma series (which I did
manage to find in your repository :-), my OSX 9.2.2 installer now boots
without any hangs.

Cheers,
Ben.
Mark Cave-Ayland July 9, 2016, 9:56 a.m. UTC | #2
On 09/07/16 04:42, Benjamin Herrenschmidt wrote:

> On Sat, 2016-07-09 at 13:41 +1000, Benjamin Herrenschmidt wrote:
>> MacOS uses an architecturally illegal MSR combination that
>> seems nonetheless supported by 32-bit processors, which is
>> to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.
>>
>> This adds support for it. To work properly we need to also
>> properly include support for PR=1,{I,D}R=0 to the MMU index
>> used by the qemu TLB.
>>
>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Note Mark that with this patch on top of the dbdma series (which I did
> manage to find in your repository :-), my OSX 9.2.2 installer now boots
> without any hangs.

Wow, just wow. This fixes the hard lockup after disk initialization I
was seeing with OS 9.2.2, and I've just done a complete install and boot
cycle of both OS 9.2.1 and OS 9.2.2 with no issues.

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.
David Gibson July 11, 2016, 1:55 a.m. UTC | #3
On Sat, Jul 09, 2016 at 01:41:31PM +1000, Benjamin Herrenschmidt wrote:
> MacOS uses an architecturally illegal MSR combination that
> seems nonetheless supported by 32-bit processors, which is
> to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.
> 
> This adds support for it. To work properly we need to also
> properly include support for PR=1,{I,D}R=0 to the MMU index
> used by the qemu TLB.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Applied to ppc-for-2.7, thanks.


> ---
> 
> v2. Use the correct flags
> 
>  target-ppc/helper_regs.h | 46 ++++++++++++++++++++++------------------------
>  1 file changed, 22 insertions(+), 24 deletions(-)
> 
> diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
> index 8fdfa5c..466ad67 100644
> --- a/target-ppc/helper_regs.h
> +++ b/target-ppc/helper_regs.h
> @@ -41,17 +41,19 @@ static inline void hreg_swap_gpr_tgpr(CPUPPCState *env)
>  
>  static inline void hreg_compute_mem_idx(CPUPPCState *env)
>  {
> -    /* This is our encoding for server processors
> +    /* 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 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
> +     *   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:
>       *
> @@ -71,20 +73,11 @@ static inline void hreg_compute_mem_idx(CPUPPCState *env)
>          env->immu_idx += msr_gs ? 4 : 0;
>          env->dmmu_idx += msr_gs ? 4 : 0;
>      } else {
> -        /* 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;
> -        }
> +        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;
>      }
>  }
>  
> @@ -136,8 +129,13 @@ static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
>          /* 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 */
> -    if ((value >> MSR_PR) & 1) {
> +    /* If PR=1 then EE, IR and DR must be 1
> +     *
> +     * Note: We only enforce this on 64-bit processors. It appears that
> +     * 32-bit implementations supports PR=1 and EE/DR/IR=0 and MacOS
> +     * exploits it.
> +     */
> +    if ((env->insns_flags & PPC_64B) && ((value >> MSR_PR) & 1)) {
>          value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
>      }
>  #endif
> 
>
Mark Cave-Ayland July 11, 2016, 6:30 p.m. UTC | #4
On 11/07/16 02:55, David Gibson wrote:

> On Sat, Jul 09, 2016 at 01:41:31PM +1000, Benjamin Herrenschmidt wrote:
>> MacOS uses an architecturally illegal MSR combination that
>> seems nonetheless supported by 32-bit processors, which is
>> to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.
>>
>> This adds support for it. To work properly we need to also
>> properly include support for PR=1,{I,D}R=0 to the MMU index
>> used by the qemu TLB.
>>
>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Applied to ppc-for-2.7, thanks.

Hi David,

I can't see this in the ppc-for-2.7 branch on github - does it need a push?


ATB,

Mark.
David Gibson July 12, 2016, 12:57 a.m. UTC | #5
On Mon, Jul 11, 2016 at 07:30:08PM +0100, Mark Cave-Ayland wrote:
1;4402;0c> On 11/07/16 02:55, David Gibson wrote:
> 
> > On Sat, Jul 09, 2016 at 01:41:31PM +1000, Benjamin Herrenschmidt wrote:
> >> MacOS uses an architecturally illegal MSR combination that
> >> seems nonetheless supported by 32-bit processors, which is
> >> to have MSR[PR]=1 and one or more of MSR[DR/IR/EE]=0.
> >>
> >> This adds support for it. To work properly we need to also
> >> properly include support for PR=1,{I,D}R=0 to the MMU index
> >> used by the qemu TLB.
> >>
> >> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > 
> > Applied to ppc-for-2.7, thanks.
> 
> Hi David,
> 
> I can't see this in the ppc-for-2.7 branch on github - does it need
> a push?

Yes it did.  Done now.
diff mbox

Patch

diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
index 8fdfa5c..466ad67 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -41,17 +41,19 @@  static inline void hreg_swap_gpr_tgpr(CPUPPCState *env)
 
 static inline void hreg_compute_mem_idx(CPUPPCState *env)
 {
-    /* This is our encoding for server processors
+    /* 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 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
+     *   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:
      *
@@ -71,20 +73,11 @@  static inline void hreg_compute_mem_idx(CPUPPCState *env)
         env->immu_idx += msr_gs ? 4 : 0;
         env->dmmu_idx += msr_gs ? 4 : 0;
     } else {
-        /* 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;
-        }
+        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;
     }
 }
 
@@ -136,8 +129,13 @@  static inline int hreg_store_msr(CPUPPCState *env, target_ulong value,
         /* 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 */
-    if ((value >> MSR_PR) & 1) {
+    /* If PR=1 then EE, IR and DR must be 1
+     *
+     * Note: We only enforce this on 64-bit processors. It appears that
+     * 32-bit implementations supports PR=1 and EE/DR/IR=0 and MacOS
+     * exploits it.
+     */
+    if ((env->insns_flags & PPC_64B) && ((value >> MSR_PR) & 1)) {
         value |= (1 << MSR_EE) | (1 << MSR_DR) | (1 << MSR_IR);
     }
 #endif