Patchwork Fix PReP NIP reset value

login
register
mail settings
Submitter Fabien Chouteau
Date April 30, 2013, 3:07 p.m.
Message ID <1367334424-10656-1-git-send-email-chouteau@adacore.com>
Download mbox | patch
Permalink /patch/240639/
State New
Headers show

Comments

Fabien Chouteau - April 30, 2013, 3:07 p.m.
The value was changed by the "PPC: fix hreset_vector..." patch.

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
---
 hw/ppc/prep.c |    3 +++
 1 file changed, 3 insertions(+)
Alexander Graf - April 30, 2013, 3:24 p.m.
On 04/30/2013 05:07 PM, Fabien Chouteau wrote:
> The value was changed by the "PPC: fix hreset_vector..." patch.
>
> Signed-off-by: Fabien Chouteau<chouteau@adacore.com>
> ---
>   hw/ppc/prep.c |    3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index cceab3e..2d0c4fe 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -427,6 +427,9 @@ static void ppc_prep_reset(void *opaque)
>       PowerPCCPU *cpu = opaque;
>
>       cpu_reset(CPU(cpu));
> +
> +    /* Reset address */
> +    cpu->env.nip = 0xfffffffc;

Why does PREP reset at this vector? Is it architected to that? Does 601 
reset to that offset?

As an interim fix, I think this patch is ok however.


Alex

>   }
>
>   /* PowerPC PREP hardware initialisation */
Fabien Chouteau - April 30, 2013, 4 p.m.
On 04/30/2013 05:24 PM, Alexander Graf wrote:
> On 04/30/2013 05:07 PM, Fabien Chouteau wrote:
>> The value was changed by the "PPC: fix hreset_vector..." patch.
>>
>> Signed-off-by: Fabien Chouteau<chouteau@adacore.com>
>> ---
>>   hw/ppc/prep.c |    3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
>> index cceab3e..2d0c4fe 100644
>> --- a/hw/ppc/prep.c
>> +++ b/hw/ppc/prep.c
>> @@ -427,6 +427,9 @@ static void ppc_prep_reset(void *opaque)
>>       PowerPCCPU *cpu = opaque;
>>
>>       cpu_reset(CPU(cpu));
>> +
>> +    /* Reset address */
>> +    cpu->env.nip = 0xfffffffc;
> 
> Why does PREP reset at this vector? Is it architected to that? Does 601 reset to that offset?
> 

I don't know why PReP reset here. As I said in the hreset_vector patch,
even if the core manual says that hreset is at 0xfff00100, the value is
in fact board specific. OpenHackWare expects 0xfffffffc as reset
address.
Alexander Graf - April 30, 2013, 4:06 p.m.
On 04/30/2013 06:00 PM, Fabien Chouteau wrote:
> On 04/30/2013 05:24 PM, Alexander Graf wrote:
>> On 04/30/2013 05:07 PM, Fabien Chouteau wrote:
>>> The value was changed by the "PPC: fix hreset_vector..." patch.
>>>
>>> Signed-off-by: Fabien Chouteau<chouteau@adacore.com>
>>> ---
>>>    hw/ppc/prep.c |    3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
>>> index cceab3e..2d0c4fe 100644
>>> --- a/hw/ppc/prep.c
>>> +++ b/hw/ppc/prep.c
>>> @@ -427,6 +427,9 @@ static void ppc_prep_reset(void *opaque)
>>>        PowerPCCPU *cpu = opaque;
>>>
>>>        cpu_reset(CPU(cpu));
>>> +
>>> +    /* Reset address */
>>> +    cpu->env.nip = 0xfffffffc;
>> Why does PREP reset at this vector? Is it architected to that? Does 601 reset to that offset?
>>
> I don't know why PReP reset here. As I said in the hreset_vector patch,
> even if the core manual says that hreset is at 0xfff00100, the value is
> in fact board specific. OpenHackWare expects 0xfffffffc as reset
> address.

Do you have 601e or 601 specs handy? Maybe they have a different reset 
vector.

Maybe OHW is also just wrong ;).


Alex
Fabien Chouteau - April 30, 2013, 4:23 p.m.
On 04/30/2013 06:06 PM, Alexander Graf wrote:
> On 04/30/2013 06:00 PM, Fabien Chouteau wrote:
>> On 04/30/2013 05:24 PM, Alexander Graf wrote:
>>> On 04/30/2013 05:07 PM, Fabien Chouteau wrote:
>>>> The value was changed by the "PPC: fix hreset_vector..." patch.
>>>>
>>>> Signed-off-by: Fabien Chouteau<chouteau@adacore.com>
>>>> ---
>>>>    hw/ppc/prep.c |    3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
>>>> index cceab3e..2d0c4fe 100644
>>>> --- a/hw/ppc/prep.c
>>>> +++ b/hw/ppc/prep.c
>>>> @@ -427,6 +427,9 @@ static void ppc_prep_reset(void *opaque)
>>>>        PowerPCCPU *cpu = opaque;
>>>>
>>>>        cpu_reset(CPU(cpu));
>>>> +
>>>> +    /* Reset address */
>>>> +    cpu->env.nip = 0xfffffffc;
>>> Why does PREP reset at this vector? Is it architected to that? Does 601 reset to that offset?
>>>
>> I don't know why PReP reset here. As I said in the hreset_vector patch,
>> even if the core manual says that hreset is at 0xfff00100, the value is
>> in fact board specific. OpenHackWare expects 0xfffffffc as reset
>> address.
> 
> Do you have 601e or 601 specs handy? Maybe they have a different reset vector.
> 

None of the specs I have talk about 0xfffffffc...

> Maybe OHW is also just wrong ;).
> 

Maybe, but I have no way to prove it.
Alexander Graf - April 30, 2013, 4:36 p.m.
On 04/30/2013 06:23 PM, Fabien Chouteau wrote:
> On 04/30/2013 06:06 PM, Alexander Graf wrote:
>> On 04/30/2013 06:00 PM, Fabien Chouteau wrote:
>>> On 04/30/2013 05:24 PM, Alexander Graf wrote:
>>>> On 04/30/2013 05:07 PM, Fabien Chouteau wrote:
>>>>> The value was changed by the "PPC: fix hreset_vector..." patch.
>>>>>
>>>>> Signed-off-by: Fabien Chouteau<chouteau@adacore.com>
>>>>> ---
>>>>>     hw/ppc/prep.c |    3 +++
>>>>>     1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
>>>>> index cceab3e..2d0c4fe 100644
>>>>> --- a/hw/ppc/prep.c
>>>>> +++ b/hw/ppc/prep.c
>>>>> @@ -427,6 +427,9 @@ static void ppc_prep_reset(void *opaque)
>>>>>         PowerPCCPU *cpu = opaque;
>>>>>
>>>>>         cpu_reset(CPU(cpu));
>>>>> +
>>>>> +    /* Reset address */
>>>>> +    cpu->env.nip = 0xfffffffc;
>>>> Why does PREP reset at this vector? Is it architected to that? Does 601 reset to that offset?
>>>>
>>> I don't know why PReP reset here. As I said in the hreset_vector patch,
>>> even if the core manual says that hreset is at 0xfff00100, the value is
>>> in fact board specific. OpenHackWare expects 0xfffffffc as reset
>>> address.
>> Do you have 601e or 601 specs handy? Maybe they have a different reset vector.
>>
> None of the specs I have talk about 0xfffffffc...
>
>> Maybe OHW is also just wrong ;).
>>
> Maybe, but I have no way to prove it.

Ok, let's leave it at that patch then ;). It'd be good to know if herve 
can boot his real images from 0x100. If so, OHW is likely at fault and 
we can revert the change once Andreas switched to OpenBIOS.


Alex
Hervé Poussineau - April 30, 2013, 7:55 p.m.
Alexander Graf a écrit :
> On 04/30/2013 06:23 PM, Fabien Chouteau wrote:
>> On 04/30/2013 06:06 PM, Alexander Graf wrote:
>>> On 04/30/2013 06:00 PM, Fabien Chouteau wrote:
>>>> On 04/30/2013 05:24 PM, Alexander Graf wrote:
>>>>> On 04/30/2013 05:07 PM, Fabien Chouteau wrote:
>>>>>> The value was changed by the "PPC: fix hreset_vector..." patch.
>>>>>>
>>>>>> Signed-off-by: Fabien Chouteau<chouteau@adacore.com>
>>>>>> ---
>>>>>>     hw/ppc/prep.c |    3 +++
>>>>>>     1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
>>>>>> index cceab3e..2d0c4fe 100644
>>>>>> --- a/hw/ppc/prep.c
>>>>>> +++ b/hw/ppc/prep.c
>>>>>> @@ -427,6 +427,9 @@ static void ppc_prep_reset(void *opaque)
>>>>>>         PowerPCCPU *cpu = opaque;
>>>>>>
>>>>>>         cpu_reset(CPU(cpu));
>>>>>> +
>>>>>> +    /* Reset address */
>>>>>> +    cpu->env.nip = 0xfffffffc;
>>>>> Why does PREP reset at this vector? Is it architected to that? Does 
>>>>> 601 reset to that offset?
>>>>>
>>>> I don't know why PReP reset here. As I said in the hreset_vector patch,
>>>> even if the core manual says that hreset is at 0xfff00100, the value is
>>>> in fact board specific. OpenHackWare expects 0xfffffffc as reset
>>>> address.
>>> Do you have 601e or 601 specs handy? Maybe they have a different 
>>> reset vector.
>>>
>> None of the specs I have talk about 0xfffffffc...
>>
>>> Maybe OHW is also just wrong ;).
>>>
>> Maybe, but I have no way to prove it.
> 
> Ok, let's leave it at that patch then ;). It'd be good to know if herve 
> can boot his real images from 0x100. If so, OHW is likely at fault and 
> we can revert the change once Andreas switched to OpenBIOS.

My real image only booted with a 601 and 604 which had a reset vector of 
0x00000100. With the committed change, 602 and 603 also work.
So I think that the change is correct, and that OpenHackWare incorrectly 
assumes a reset vector of 0xfffffffc...

Hervé
Andreas Färber - May 1, 2013, 11:16 a.m.
Am 30.04.2013 17:07, schrieb Fabien Chouteau:
> The value was changed by the "PPC: fix hreset_vector..." patch.
> 
> Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
> ---
>  hw/ppc/prep.c |    3 +++
>  1 file changed, 3 insertions(+)

Thanks, applied to prep-up (with enhanced commit message):
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/prep-up

Andreas

> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index cceab3e..2d0c4fe 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -427,6 +427,9 @@ static void ppc_prep_reset(void *opaque)
>      PowerPCCPU *cpu = opaque;
>  
>      cpu_reset(CPU(cpu));
> +
> +    /* Reset address */
> +    cpu->env.nip = 0xfffffffc;
>  }
>  
>  /* PowerPC PREP hardware initialisation */
>
Fabien Chouteau - May 2, 2013, 8:23 a.m.
On 04/30/2013 09:55 PM, Hervé Poussineau wrote:
> Alexander Graf a écrit :
>> On 04/30/2013 06:23 PM, Fabien Chouteau wrote:
>>> On 04/30/2013 06:06 PM, Alexander Graf wrote:
>>>> On 04/30/2013 06:00 PM, Fabien Chouteau wrote:
>>>>> On 04/30/2013 05:24 PM, Alexander Graf wrote:
>>>> Maybe OHW is also just wrong ;).
>>>>
>>> Maybe, but I have no way to prove it.
>>
>> Ok, let's leave it at that patch then ;). It'd be good to know if herve can boot his real images from 0x100. If so, OHW is likely at fault and we can revert the change once Andreas switched to OpenBIOS.
> 
> My real image only booted with a 601 and 604 which had a reset vector of 0x00000100. With the committed change, 602 and 603 also work.
> So I think that the change is correct, and that OpenHackWare incorrectly assumes a reset vector of 0xfffffffc...
> 

Thanks, so Alexander is right we should revert this patch when we
switch to OpenBios.

Patch

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index cceab3e..2d0c4fe 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -427,6 +427,9 @@  static void ppc_prep_reset(void *opaque)
     PowerPCCPU *cpu = opaque;
 
     cpu_reset(CPU(cpu));
+
+    /* Reset address */
+    cpu->env.nip = 0xfffffffc;
 }
 
 /* PowerPC PREP hardware initialisation */