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

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

[Fabien Chouteau]

According to the PowePC 750 user's manual, the vector offset for system
reset (both /HRESET and /SRESET) is 0x00100.

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
 target-ppc/translate_init.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

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;
 #endif
 }
 
@@ -2931,7 +2931,7 @@ static void init_excp_750cx (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;
 #endif
 }
 
@@ -2959,7 +2959,7 @@ static void init_excp_7x5 (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;
 #endif
 }
 
@@ -2985,7 +2985,7 @@ static void init_excp_7400 (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;
 #endif
 }
 
-- 
1.7.9.5

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

[Alexander Graf]

On 27.03.2013, at 14:50, Fabien Chouteau wrote:

> According to the PowePC 750 user's manual, the vector offset for system

PowerPC?

> reset (both /HRESET and /SRESET) is 0x00100.
> 
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
> target-ppc/translate_init.c |    8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
> 
> 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?


Alex

> #endif
> }
> 
> @@ -2931,7 +2931,7 @@ static void init_excp_750cx (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;
> #endif
> }
> 
> @@ -2959,7 +2959,7 @@ static void init_excp_7x5 (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;
> #endif
> }
> 
> @@ -2985,7 +2985,7 @@ static void init_excp_7400 (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;
> #endif
> }
> 
> -- 
> 1.7.9.5
> 

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

[Alexander Graf]

On 27.03.2013, at 14:54, Alexander Graf wrote:

> 
> On 27.03.2013, at 14:50, Fabien Chouteau wrote:
> 
>> According to the PowePC 750 user's manual, the vector offset for system
> 
> PowerPC?
> 
>> reset (both /HRESET and /SRESET) is 0x00100.
>> 
>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>> ---
>> target-ppc/translate_init.c |    8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> 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 :).


Alex

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

[Alexander Graf]

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:
>> 
>>> According to the PowePC 750 user's manual, the vector offset for system
>> 
>> PowerPC?
>> 
>>> reset (both /HRESET and /SRESET) is 0x00100.
>>> 
>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
>>> ---
>>> target-ppc/translate_init.c |    8 ++++----
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>> 
>>> 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 :).

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. Except for machine check interrupts, where MSR.ME = 0.

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


Alex

Re: [Qemu-devel] [Qemu-ppc] [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

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

[Fabien Chouteau]

On 03/27/2013 03:59 PM, Fabien Chouteau wrote:

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

Right now in QEMU, MSR[IP] is always set at reset (MSR_EP == MSR_IP):

translate_init.c:8174:
    msr |= (target_ulong)1 << MSR_EP; 


-- 
Fabien Chouteau

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

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

[Fabien Chouteau]

On 03/27/2013 04:10 PM, Alexander Graf wrote:
>> 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.

Is it true for all PPC, because MSR_EP/IP is set for all kind of CPU right now.

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

Alright, we can get rid of hreset_excp_prefix then.

Thanks,

-- 
Fabien Chouteau

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

[Alexander Graf]

On 27.03.2013, at 16:49, Fabien Chouteau wrote:

> On 03/27/2013 04:10 PM, Alexander Graf wrote:
>>> 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.
> 
> Is it true for all PPC, because MSR_EP/IP is set for all kind of CPU right now.
> 
>> 
>>> 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 :)
>> 
> 
> Alright, we can get rid of hreset_excp_prefix then.

I don't think we can get rid of the variable. G5s for example set the prefix through some HID register.


Alex

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

[Fabien Chouteau]

On 03/27/2013 05:07 PM, Alexander Graf wrote:
> 
> On 27.03.2013, at 16:49, Fabien Chouteau wrote:
>> Alright, we can get rid of hreset_excp_prefix then.
> 
> I don't think we can get rid of the variable. G5s for example set the prefix through some HID register.
> 

I can't find this in the code...

-- 
Fabien Chouteau

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

[Alexander Graf]

On 27.03.2013, at 17:17, Fabien Chouteau wrote:

> On 03/27/2013 05:07 PM, Alexander Graf wrote:
>> 
>> On 27.03.2013, at 16:49, Fabien Chouteau wrote:
>>> Alright, we can get rid of hreset_excp_prefix then.
>> 
>> I don't think we can get rid of the variable. G5s for example set the prefix through some HID register.
>> 
> 
> I can't find this in the code...

Ah, hreset_excp_prefix vs excp_prefix. My bad. I was thinking of excp_prefix.

The SPR I was thinking of was HIOR:

static void spr_write_hior (void *opaque, int sprn, int gprn)
{
    TCGv t0 = tcg_temp_new();
    tcg_gen_andi_tl(t0, cpu_gpr[gprn], 0x3FFFFF00000ULL);
    tcg_gen_st_tl(t0, cpu_env, offsetof(CPUPPCState, excp_prefix));
    tcg_temp_free(t0);
}


Alex

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

[Fabien Chouteau]

On 03/27/2013 05:22 PM, Alexander Graf wrote:
> 
> On 27.03.2013, at 17:17, Fabien Chouteau wrote:
> 
>> On 03/27/2013 05:07 PM, Alexander Graf wrote:
>>>
>>> On 27.03.2013, at 16:49, Fabien Chouteau wrote:
>>>> Alright, we can get rid of hreset_excp_prefix then.
>>>
>>> I don't think we can get rid of the variable. G5s for example set the prefix through some HID register.
>>>
>>
>> I can't find this in the code...
> 
> Ah, hreset_excp_prefix vs excp_prefix. My bad. I was thinking of excp_prefix.
> 

OK thanks, I'm working on a patch to remove hreset_excp_prefix.

-- 
Fabien Chouteau

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

[Fabien Chouteau]

On 03/27/2013 04:49 PM, Fabien Chouteau wrote:
> On 03/27/2013 04:10 PM, Alexander Graf wrote:
>>> 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.
> 
> Is it true for all PPC, because MSR_EP/IP is set for all kind of CPU right now.
> 

Well, when the msr_maks allows it...

-- 
Fabien Chouteau