Re: [Qemu-devel] [PATCH 2.0] PPC: E500: Set PIR default reset value rather than SPR value

[Alexander Graf]

> Am 04.04.2014 um 02:26 schrieb Alexey Kardashevskiy <address@hidden>:
> 
>> On 04/04/2014 05:58 AM, Alexander Graf wrote:
>> 
>>> On 03.04.2014, at 20:55, Peter Maydell <address@hidden> wrote:
>>> 
>>>> On 3 April 2014 19:48, Alexander Graf <address@hidden> wrote:
>>>> We now reset SPRs to their reset values on CPU reset. So if we want
>>>> to have an SPR persistently changed, we need to change its default
>>>> reset value rather than the value itself manually.
>>>> 
>>>> Do this for SPR_BOOKE_PIR, fixing e500v2 SMP boot.
>>>> 
>>>> Reported-by: Frederic Konrad <address@hidden>
>>>> Signed-off-by: Alexander Graf <address@hidden>
>>>> ---
>>>> hw/ppc/e500.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> 
>>>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>>>> index d7ba25f..f984b3e 100644
>>>> --- a/hw/ppc/e500.c
>>>> +++ b/hw/ppc/e500.c
>>>> @@ -649,7 +649,7 @@ void ppce500_init(QEMUMachineInitArgs *args, 
>>>> PPCE500Params *params)
>>>>        input = (qemu_irq *)env->irq_inputs;
>>>>        irqs[i][OPENPIC_OUTPUT_INT] = input[PPCE500_INPUT_INT];
>>>>        irqs[i][OPENPIC_OUTPUT_CINT] = input[PPCE500_INPUT_CINT];
>>>> -        env->spr[SPR_BOOKE_PIR] = cs->cpu_index = i;
>>>> +        env->spr_cb[SPR_BOOKE_PIR].default_value = cs->cpu_index = i;
>>> 
>>> Not an issue introduced with this patch, but should we really
>>> be missing with CPUState::cpu_index in a board file?
>>> (Nowhere else does, it's otherwise simply set to an
>>> appropriately incremented index by cpu_exec_init().)
>>> I rather suspect that cpu_index will already be set
>>> to the value we set it, in any case...
>> 
>> Most likely, but I'd rather not touch that logic for 2.0 :)
> 
> git grep "cpu_index\s\+=[^=]":
> 
> cpus.c:1404:        cpu_index = 0;
> exec.c:482:    cpu_index = 0;
> exec.c:486:    cpu->cpu_index = cpu_index;
> hmp.c:742:    cpu_index = qdict_get_int(qdict, "index");
> hw/ppc/e500.c:652:        env->spr[SPR_BOOKE_PIR] = cs->cpu_index = i;
> monitor.c:2225:    int cpu_index = qdict_get_int(qdict, "cpu_index");
> scripts/qmp/qmp-shell:183:        self.__cpu_index = 0
> scripts/qmp/qmp-shell:216:    def __cmd_passthrough(self, cmdline,
> cpu_index = 0):
> scripts/qmp/qmp-shell:229:                self.__cpu_index = idx
> 
> 
> Assigning cpu_index in e500.c looks to me as a strange idea. And it is
> hidden so nicely with two "=" in a row :-/
> 
> Out of curiosity - in the real e500 hardware, PIR does not get reset on CPU
> reset?

On real hw PIR is a hardwired read-only register :)

Alex


> 
> 
> -- 
> Alexey