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

[Fabien Chouteau]

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