[Qemu-devel] [PATCH qom-cpu-next] e500: Replace open-coded loop with qemu_get_cpu()

2013-02-15 Thread Andreas Färber 
Since we still need env for ppc-specific fields, obtain it via the new
env_ptr fields to avoid cpu name conflicts between CPUState and
PowerPCCPU for now.

This fixes a potential issue with env being NULL at the end of the loop
but cpu still being a valid pointer corresponding to a previous env.

Signed-off-by: Andreas Färber afaer...@suse.de
---
 Alex, this is a rebased and slimmed-down version of yesterday's paste.
 Can you ack please? Thanks, /-F

 hw/ppc/e500.c |   11 +++
 1 Datei geändert, 3 Zeilen hinzugefügt(+), 8 Zeilen entfernt(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index b7474c0..451682c 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -240,20 +240,15 @@ static int ppce500_load_device_tree(CPUPPCState *env,
 /* We need to generate the cpu nodes in reverse order, so Linux can pick
the first node as boot node and be happy */
 for (i = smp_cpus - 1; i = 0; i--) {
-CPUState *cpu = NULL;
+CPUState *cpu;
 char cpu_name[128];
 uint64_t cpu_release_addr = MPC8544_SPIN_BASE + (i * 0x20);
 
-for (env = first_cpu; env != NULL; env = env-next_cpu) {
-cpu = ENV_GET_CPU(env);
-if (cpu-cpu_index == i) {
-break;
-}
-}
-
+cpu = qemu_get_cpu(i);
 if (cpu == NULL) {
 continue;
 }
+env = cpu-env_ptr;
 
 snprintf(cpu_name, sizeof(cpu_name), /cpus/PowerPC,8544@%x,
  cpu-cpu_index);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH qom-cpu-next] e500: Replace open-coded loop with qemu_get_cpu()

2013-02-15 Thread Alexander Graf 

On 15.02.2013, at 15:25, Andreas Färber wrote:

 Since we still need env for ppc-specific fields, obtain it via the new
 env_ptr fields to avoid cpu name conflicts between CPUState and
 PowerPCCPU for now.
 
 This fixes a potential issue with env being NULL at the end of the loop
 but cpu still being a valid pointer corresponding to a previous env.
 
 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
 Alex, this is a rebased and slimmed-down version of yesterday's paste.
 Can you ack please? Thanks, /-F

In your QOM conversion you also converted other places like this. Have you 
verified that these don't suffer from the same problem?


Alex

 
 hw/ppc/e500.c |   11 +++
 1 Datei geändert, 3 Zeilen hinzugefügt(+), 8 Zeilen entfernt(-)
 
 diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
 index b7474c0..451682c 100644
 --- a/hw/ppc/e500.c
 +++ b/hw/ppc/e500.c
 @@ -240,20 +240,15 @@ static int ppce500_load_device_tree(CPUPPCState *env,
 /* We need to generate the cpu nodes in reverse order, so Linux can pick
the first node as boot node and be happy */
 for (i = smp_cpus - 1; i = 0; i--) {
 -CPUState *cpu = NULL;
 +CPUState *cpu;
 char cpu_name[128];
 uint64_t cpu_release_addr = MPC8544_SPIN_BASE + (i * 0x20);
 
 -for (env = first_cpu; env != NULL; env = env-next_cpu) {
 -cpu = ENV_GET_CPU(env);
 -if (cpu-cpu_index == i) {
 -break;
 -}
 -}
 -
 +cpu = qemu_get_cpu(i);
 if (cpu == NULL) {
 continue;
 }
 +env = cpu-env_ptr;
 
 snprintf(cpu_name, sizeof(cpu_name), /cpus/PowerPC,8544@%x,
  cpu-cpu_index);
 -- 
 1.7.10.4

Re: [Qemu-devel] [PATCH qom-cpu-next] e500: Replace open-coded loop with qemu_get_cpu()

2013-02-15 Thread Andreas Färber 
Am 15.02.2013 15:29, schrieb Alexander Graf:
 
 On 15.02.2013, at 15:25, Andreas Färber wrote:
 
 Since we still need env for ppc-specific fields, obtain it via the new
 env_ptr fields to avoid cpu name conflicts between CPUState and
 PowerPCCPU for now.

 This fixes a potential issue with env being NULL at the end of the loop
 but cpu still being a valid pointer corresponding to a previous env.

 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
 Alex, this is a rebased and slimmed-down version of yesterday's paste.
 Can you ack please? Thanks, /-F
 
 In your QOM conversion you also converted other places like this. Have you 
 verified that these don't suffer from the same problem?

Not yet. Not all callers are using this pattern though, so solutions
would differ and be per-case patches.

At some point I obviously intend to change first_cpu and -next_cpu to
CPUState and have been wondering whether we can use QTAILQ macros or
anything else more standardized, with better iteration support.

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 qom-cpu-next] e500: Replace open-coded loop with qemu_get_cpu()

2013-02-15 Thread Alexander Graf 

On 15.02.2013, at 15:35, Andreas Färber wrote:

 Am 15.02.2013 15:29, schrieb Alexander Graf:
 
 On 15.02.2013, at 15:25, Andreas Färber wrote:
 
 Since we still need env for ppc-specific fields, obtain it via the new
 env_ptr fields to avoid cpu name conflicts between CPUState and
 PowerPCCPU for now.
 
 This fixes a potential issue with env being NULL at the end of the loop
 but cpu still being a valid pointer corresponding to a previous env.
 
 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
 Alex, this is a rebased and slimmed-down version of yesterday's paste.
 Can you ack please? Thanks, /-F
 
 In your QOM conversion you also converted other places like this. Have you 
 verified that these don't suffer from the same problem?
 
 Not yet. Not all callers are using this pattern though, so solutions
 would differ and be per-case patches.

This specific patch is:

Acked-by: Alexander Graf ag...@suse.de

However, please make sure to fix the other places too :)


Alex

 
 At some point I obviously intend to change first_cpu and -next_cpu to
 CPUState and have been wondering whether we can use QTAILQ macros or
 anything else more standardized, with better iteration support.
 
 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 qom-cpu-next] e500: Replace open-coded loop with qemu_get_cpu()

2013-02-15 Thread Andreas Färber 
Am 15.02.2013 17:45, schrieb Alexander Graf:
 
 On 15.02.2013, at 15:35, Andreas Färber wrote:
 
 Am 15.02.2013 15:29, schrieb Alexander Graf:

 On 15.02.2013, at 15:25, Andreas Färber wrote:

 Since we still need env for ppc-specific fields, obtain it via the new
 env_ptr fields to avoid cpu name conflicts between CPUState and
 PowerPCCPU for now.

 This fixes a potential issue with env being NULL at the end of the loop
 but cpu still being a valid pointer corresponding to a previous env.

 Signed-off-by: Andreas Färber afaer...@suse.de
 ---
 Alex, this is a rebased and slimmed-down version of yesterday's paste.
 Can you ack please? Thanks, /-F

 In your QOM conversion you also converted other places like this. Have you 
 verified that these don't suffer from the same problem?

 Not yet. Not all callers are using this pattern though, so solutions
 would differ and be per-case patches.
 
 This specific patch is:
 
 Acked-by: Alexander Graf ag...@suse.de

Thanks, applied to qom-cpu-next queue:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next

 However, please make sure to fix the other places too :)

In whole there's two e500 patches fixing a potential bug, plus
non-functional cleanups for sPAPR, monitor and QMP code.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg