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

[Alexander Graf]

On 27.03.2013, at 15:59, Fabien Chouteau wrote:

> 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?

For 740 / 750, it's 1. All other bits are 0.

> 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:

Sounds good :)


Alex

> 
> +++ 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