Re: [Qemu-devel] [PATCH 2.0] PPC: E500: Set PIR default reset value rather than SPR value
Am 04.04.2014 um 02:26 schrieb Alexey Kardashevskiy a...@ozlabs.ru: On 04/04/2014 05:58 AM, Alexander Graf wrote: On 03.04.2014, at 20:55, Peter Maydell peter.mayd...@linaro.org wrote: On 3 April 2014 19:48, Alexander Graf ag...@suse.de 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 fred.kon...@greensocs.com Signed-off-by: Alexander Graf ag...@suse.de --- 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
Re: [Qemu-devel] [PATCH 2.0] PPC: E500: Set PIR default reset value rather than SPR value
Hi Alex, Seems to works for me ;). On 03/04/2014 20:48, Alexander Graf 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 fred.kon...@greensocs.com Signed-off-by: Alexander Graf ag...@suse.de Tested-by: KONRAD Frederic fred.kon...@greensocs.com Thanks, Fred --- 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; env-mpic_iack = MPC8544_CCSRBAR_BASE + MPC8544_MPIC_REGS_OFFSET + 0xa0;
[Qemu-devel] [PATCH 2.0] PPC: E500: Set PIR default reset value rather than SPR value
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 fred.kon...@greensocs.com Signed-off-by: Alexander Graf ag...@suse.de --- 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; env-mpic_iack = MPC8544_CCSRBAR_BASE + MPC8544_MPIC_REGS_OFFSET + 0xa0; -- 1.8.1.4
Re: [Qemu-devel] [PATCH 2.0] PPC: E500: Set PIR default reset value rather than SPR value
Am 03.04.2014 20:48, schrieb Alexander Graf: 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 fred.kon...@greensocs.com Suggested-by: Andreas Färber afaer...@suse.de Signed-off-by: Alexander Graf ag...@suse.de --- 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; env-mpic_iack = MPC8544_CCSRBAR_BASE + MPC8544_MPIC_REGS_OFFSET + 0xa0; .default_value then. ;) Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 2.0] PPC: E500: Set PIR default reset value rather than SPR value
On 3 April 2014 19:48, Alexander Graf ag...@suse.de 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 fred.kon...@greensocs.com Signed-off-by: Alexander Graf ag...@suse.de --- 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... thanks -- PMM
Re: [Qemu-devel] [PATCH 2.0] PPC: E500: Set PIR default reset value rather than SPR value
On 03.04.2014, at 20:55, Peter Maydell peter.mayd...@linaro.org wrote: On 3 April 2014 19:48, Alexander Graf ag...@suse.de 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 fred.kon...@greensocs.com Signed-off-by: Alexander Graf ag...@suse.de --- 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 :) Alex
Re: [Qemu-devel] [PATCH 2.0] PPC: E500: Set PIR default reset value rather than SPR value
On 04/04/2014 05:58 AM, Alexander Graf wrote: On 03.04.2014, at 20:55, Peter Maydell peter.mayd...@linaro.org wrote: On 3 April 2014 19:48, Alexander Graf ag...@suse.de 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 fred.kon...@greensocs.com Signed-off-by: Alexander Graf ag...@suse.de --- 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? -- Alexey