Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: init_excp_7x0: fix hreset entry point.

[Fabien Chouteau <chouteau@adacore.com>]

On 03/27/2013 03:04 PM, Alexander Graf wrote:
> On 27.03.2013, at 15:00, Alexander Graf wrote:
>> On 27.03.2013, at 14:54, Alexander Graf wrote:
>>> On 27.03.2013, at 14:50, Fabien Chouteau wrote:
>>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>>> index 781170f..a5bae1e 100644
>>>> --- a/target-ppc/translate_init.c
>>>> +++ b/target-ppc/translate_init.c
>>>> @@ -2885,7 +2885,7 @@ static void init_excp_7x0 (CPUPPCState *env)
>>>>   env->excp_vectors[POWERPC_EXCP_THERM]    = 0x00001700;
>>>>   env->hreset_excp_prefix = 0x00000000UL;
>>>>   /* Hardware reset vector */
>>>> -    env->hreset_vector = 0xFFFFFFFCUL;
>>>> +    env->hreset_vector = 0xFFF00100UL;
>>>
>>> As you properly explained above, the reset vector is 0x100 according to the spec. However, hreset_excp_prefix is 0x0. How do we end up getting to 0xfff00100 here?
>>
>> According to 7xx_um.pdf (740 / 750 User manual), the IP bit in MSR (bit 25 in ppc notion) controls whether excp_prefix is 0xfff00000 or 0x00000000. The spec also says:
>>
>> When either HRESET is negated or SRESET transitions to asserted, the processor attempts to fetch code from the system reset exception vector. The vector is located at offset 0x00100 from the exception prefix (all zeros or ones, depending on the setting of the exception prefix bit in the machine state register (MSR[IP]). The MSR[IP] bit is set for HRESET.
>>
>> So on reset, MSR[IP] = 1. That means that hreset_excp_prefix is also wrong here.
>>
>> Please add the respective logic that sets hreset_excp_prefix according to MSR[IP] on 740 / 750, otherwise whatever you're trying to execute will break as soon as it gets its first real exception :).
> 

It's actually already implemented (helper_regs.h:96). The question is:
what is the value of MSR[IP] at reset?

Also, we might want to call hreg_store_msr() in ppc_cpu_reset() instead
of just setting the value env->msr, this way we don't need
hreset_excp_prefix as the MSR[IP] will be used to set the value of
env->excp_prefix. Something like:

+++ b/target-ppc/translate_init.c
@@ -8182,19 +8182,23 @@ static void ppc_cpu_reset(CPUState *s)
     msr |= (target_ulong)1 << MSR_VR; /* Allow altivec usage */
     msr |= (target_ulong)1 << MSR_SPE; /* Allow SPE usage */
     msr |= (target_ulong)1 << MSR_PR;
-#else
-    env->excp_prefix = env->hreset_excp_prefix;
-    env->nip = env->hreset_vector | env->excp_prefix;
-    if (env->mmu_model != POWERPC_MMU_REAL) {
-        ppc_tlb_invalidate_all(env);
-    }
 #endif
-    env->msr = msr & env->msr_mask;
+
 #if defined(TARGET_PPC64)
     if (env->mmu_model & POWERPC_MMU_64) {
         env->msr |= (1ULL << MSR_SF);
     }
 #endif
+
+    hreg_store_msr(env, msr, 1);
+
+#if !defined(CONFIG_USER_ONLY)
+    env->nip = env->hreset_vector | env->excp_prefix;
+    if (env->mmu_model != POWERPC_MMU_REAL) {
+        ppc_tlb_invalidate_all(env);
+    }
+#endif
+
     hreg_compute_hflags(env);
     env->reserve_addr = (target_ulong)-1ULL;
     /* Be sure no exception or interrupt is pending */




> While at it, on exception delivery ILE, ME and IP do not get modified according to the spec. Please verify that we don't accidentally set them to 0 when we deliver an interrupt. 

They seems to be preserved.

> Except for machine check interrupts, where MSR.ME = 0.

This is done, excp_helper.c:148.

>
> Also, MSR.LE becomes the previous value of MSR.ILE. Not that we'd implement LE mode properly ;).
>

This is also already done, excp_helper.c:615.


Thanks,

-- 
Fabien Chouteau