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

2014-04-04 Thread Alexander Graf


 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

2014-04-04 Thread Frederic Konrad

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

2014-04-03 Thread 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
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

2014-04-03 Thread Andreas Färber
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

2014-04-03 Thread Peter Maydell
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

2014-04-03 Thread Alexander Graf

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

2014-04-03 Thread Alexey Kardashevskiy
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