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

2013-02-15 Thread Andreas Färber 
Potentially env could be NULL whereas cpu would still be valid and
correspond to a previous env.

Wrapping this in qemu_get_cpu(), env is no longer needed, so simplify
code that existed before 55e5c2850293547203874098f7cec148ffd12dfa.

Signed-off-by: Andreas Färber 
---
 hw/ppce500_spin.c |   15 ---
 1 Datei geändert, 4 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-)

diff --git a/hw/ppce500_spin.c b/hw/ppce500_spin.c
index 7e90fb9..5bdce52 100644
--- a/hw/ppce500_spin.c
+++ b/hw/ppce500_spin.c
@@ -123,18 +123,11 @@ static void spin_write(void *opaque, hwaddr addr, 
uint64_t value,
 {
 SpinState *s = opaque;
 int env_idx = addr / sizeof(SpinInfo);
-CPUPPCState *env;
-CPUState *cpu = NULL;
+CPUState *cpu;
 SpinInfo *curspin = &s->spin[env_idx];
 uint8_t *curspin_p = (uint8_t*)curspin;
 
-for (env = first_cpu; env != NULL; env = env->next_cpu) {
-cpu = CPU(ppc_env_get_cpu(env));
-if (cpu->cpu_index == env_idx) {
-break;
-}
-}
-
+cpu = qemu_get_cpu(env_idx);
 if (cpu == NULL) {
 /* Unknown CPU */
 return;
@@ -161,11 +154,11 @@ static void spin_write(void *opaque, hwaddr addr, 
uint64_t value,
 if (!(ldq_p(&curspin->addr) & 1)) {
 /* run CPU */
 SpinKick kick = {
-.cpu = ppc_env_get_cpu(env),
+.cpu = POWERPC_CPU(cpu),
 .spin = curspin,
 };
 
-run_on_cpu(CPU(kick.cpu), spin_kick, &kick);
+run_on_cpu(cpu, spin_kick, &kick);
 }
 }
 
-- 
1.7.10.4

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

2013-02-15 Thread Alexander Graf 

On 15.02.2013, at 17:51, Andreas Färber wrote:

> Potentially env could be NULL whereas cpu would still be valid and
> correspond to a previous env.
> 
> Wrapping this in qemu_get_cpu(), env is no longer needed, so simplify
> code that existed before 55e5c2850293547203874098f7cec148ffd12dfa.
> 
> Signed-off-by: Andreas Färber 
> ---
> hw/ppce500_spin.c |   15 ---
> 1 Datei geändert, 4 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-)
> 
> diff --git a/hw/ppce500_spin.c b/hw/ppce500_spin.c
> index 7e90fb9..5bdce52 100644
> --- a/hw/ppce500_spin.c
> +++ b/hw/ppce500_spin.c
> @@ -123,18 +123,11 @@ static void spin_write(void *opaque, hwaddr addr, 
> uint64_t value,
> {
> SpinState *s = opaque;
> int env_idx = addr / sizeof(SpinInfo);
> -CPUPPCState *env;
> -CPUState *cpu = NULL;
> +CPUState *cpu;
> SpinInfo *curspin = &s->spin[env_idx];
> uint8_t *curspin_p = (uint8_t*)curspin;
> 
> -for (env = first_cpu; env != NULL; env = env->next_cpu) {
> -cpu = CPU(ppc_env_get_cpu(env));
> -if (cpu->cpu_index == env_idx) {
> -break;
> -}
> -}
> -
> +cpu = qemu_get_cpu(env_idx);
> if (cpu == NULL) {
> /* Unknown CPU */
> return;
> @@ -161,11 +154,11 @@ static void spin_write(void *opaque, hwaddr addr, 
> uint64_t value,
> if (!(ldq_p(&curspin->addr) & 1)) {
> /* run CPU */
> SpinKick kick = {
> -.cpu = ppc_env_get_cpu(env),
> +.cpu = POWERPC_CPU(cpu),

Why not just cpu?

Alex

> .spin = curspin,
> };
> 
> -run_on_cpu(CPU(kick.cpu), spin_kick, &kick);
> +run_on_cpu(cpu, spin_kick, &kick);
> }
> }
> 
> -- 
> 1.7.10.4
>

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

2013-02-15 Thread Andreas Färber 
Am 15.02.2013 17:54, schrieb Alexander Graf:
> 
> On 15.02.2013, at 17:51, Andreas Färber wrote:
> 
>> Potentially env could be NULL whereas cpu would still be valid and
>> correspond to a previous env.
>>
>> Wrapping this in qemu_get_cpu(), env is no longer needed, so simplify
>> code that existed before 55e5c2850293547203874098f7cec148ffd12dfa.
>>
>> Signed-off-by: Andreas Färber 
>> ---
>> hw/ppce500_spin.c |   15 ---
>> 1 Datei geändert, 4 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-)
>>
>> diff --git a/hw/ppce500_spin.c b/hw/ppce500_spin.c
>> index 7e90fb9..5bdce52 100644
>> --- a/hw/ppce500_spin.c
>> +++ b/hw/ppce500_spin.c
>> @@ -123,18 +123,11 @@ static void spin_write(void *opaque, hwaddr addr, 
>> uint64_t value,
>> {
>> SpinState *s = opaque;
>> int env_idx = addr / sizeof(SpinInfo);
>> -CPUPPCState *env;
>> -CPUState *cpu = NULL;
>> +CPUState *cpu;
>> SpinInfo *curspin = &s->spin[env_idx];
>> uint8_t *curspin_p = (uint8_t*)curspin;
>>
>> -for (env = first_cpu; env != NULL; env = env->next_cpu) {
>> -cpu = CPU(ppc_env_get_cpu(env));
>> -if (cpu->cpu_index == env_idx) {
>> -break;
>> -}
>> -}
>> -
>> +cpu = qemu_get_cpu(env_idx);
>> if (cpu == NULL) {
>> /* Unknown CPU */
>> return;
>> @@ -161,11 +154,11 @@ static void spin_write(void *opaque, hwaddr addr, 
>> uint64_t value,
>> if (!(ldq_p(&curspin->addr) & 1)) {
>> /* run CPU */
>> SpinKick kick = {
>> -.cpu = ppc_env_get_cpu(env),
>> +.cpu = POWERPC_CPU(cpu),
> 
> Why not just cpu?

PowerPCCPU vs. CPUState type.
Having the specific type in ppc code allows direct access to ->env.

If you see a performance issue, we could also use (PowerPCCPU *)cpu.

Andreas

>> .spin = curspin,
>> };
>>
>> -run_on_cpu(CPU(kick.cpu), spin_kick, &kick);
>> +run_on_cpu(cpu, spin_kick, &kick);
>> }
>> }
>>
>> -- 
>> 1.7.10.4
>>
> 


-- 
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] ppce500_spin: Replace open-coded CPU loop with qemu_get_cpu()

2013-02-15 Thread Alexander Graf 

On 15.02.2013, at 17:58, Andreas Färber wrote:

> Am 15.02.2013 17:54, schrieb Alexander Graf:
>> 
>> On 15.02.2013, at 17:51, Andreas Färber wrote:
>> 
>>> Potentially env could be NULL whereas cpu would still be valid and
>>> correspond to a previous env.
>>> 
>>> Wrapping this in qemu_get_cpu(), env is no longer needed, so simplify
>>> code that existed before 55e5c2850293547203874098f7cec148ffd12dfa.
>>> 
>>> Signed-off-by: Andreas Färber 
>>> ---
>>> hw/ppce500_spin.c |   15 ---
>>> 1 Datei geändert, 4 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-)
>>> 
>>> diff --git a/hw/ppce500_spin.c b/hw/ppce500_spin.c
>>> index 7e90fb9..5bdce52 100644
>>> --- a/hw/ppce500_spin.c
>>> +++ b/hw/ppce500_spin.c
>>> @@ -123,18 +123,11 @@ static void spin_write(void *opaque, hwaddr addr, 
>>> uint64_t value,
>>> {
>>>SpinState *s = opaque;
>>>int env_idx = addr / sizeof(SpinInfo);
>>> -CPUPPCState *env;
>>> -CPUState *cpu = NULL;
>>> +CPUState *cpu;
>>>SpinInfo *curspin = &s->spin[env_idx];
>>>uint8_t *curspin_p = (uint8_t*)curspin;
>>> 
>>> -for (env = first_cpu; env != NULL; env = env->next_cpu) {
>>> -cpu = CPU(ppc_env_get_cpu(env));
>>> -if (cpu->cpu_index == env_idx) {
>>> -break;
>>> -}
>>> -}
>>> -
>>> +cpu = qemu_get_cpu(env_idx);
>>>if (cpu == NULL) {
>>>/* Unknown CPU */
>>>return;
>>> @@ -161,11 +154,11 @@ static void spin_write(void *opaque, hwaddr addr, 
>>> uint64_t value,
>>>if (!(ldq_p(&curspin->addr) & 1)) {
>>>/* run CPU */
>>>SpinKick kick = {
>>> -.cpu = ppc_env_get_cpu(env),
>>> +.cpu = POWERPC_CPU(cpu),
>> 
>> Why not just cpu?
> 
> PowerPCCPU vs. CPUState type.
> Having the specific type in ppc code allows direct access to ->env.
> 
> If you see a performance issue, we could also use (PowerPCCPU *)cpu.

There are no performance issues in the spin code :). By definition. This thing 
could be written in bash and still be fast enough.

I was mostly wondering whether ppc_env_get_cpu(env) returns a PowerPCCPU *. But 
apparently it does. Ok then :)


Acked-by: Alexander Graf 


Alex

> 
> Andreas
> 
>>>.spin = curspin,
>>>};
>>> 
>>> -run_on_cpu(CPU(kick.cpu), spin_kick, &kick);
>>> +run_on_cpu(cpu, spin_kick, &kick);
>>>}
>>> }
>>> 
>>> -- 
>>> 1.7.10.4
>>> 
>> 
> 
> 
> -- 
> 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] ppce500_spin: Replace open-coded CPU loop with qemu_get_cpu()

2013-02-15 Thread Andreas Färber 
Am 15.02.2013 18:04, schrieb Alexander Graf:
> 
> On 15.02.2013, at 17:58, Andreas Färber wrote:
> 
>> Am 15.02.2013 17:54, schrieb Alexander Graf:
>>>
>>> On 15.02.2013, at 17:51, Andreas Färber wrote:
>>>
 Potentially env could be NULL whereas cpu would still be valid and
 correspond to a previous env.

 Wrapping this in qemu_get_cpu(), env is no longer needed, so simplify
 code that existed before 55e5c2850293547203874098f7cec148ffd12dfa.

 Signed-off-by: Andreas Färber 
 ---
 hw/ppce500_spin.c |   15 ---
 1 Datei geändert, 4 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-)

 diff --git a/hw/ppce500_spin.c b/hw/ppce500_spin.c
 index 7e90fb9..5bdce52 100644
 --- a/hw/ppce500_spin.c
 +++ b/hw/ppce500_spin.c
 @@ -123,18 +123,11 @@ static void spin_write(void *opaque, hwaddr addr, 
 uint64_t value,
 {
SpinState *s = opaque;
int env_idx = addr / sizeof(SpinInfo);
 -CPUPPCState *env;
 -CPUState *cpu = NULL;
 +CPUState *cpu;
SpinInfo *curspin = &s->spin[env_idx];
uint8_t *curspin_p = (uint8_t*)curspin;

 -for (env = first_cpu; env != NULL; env = env->next_cpu) {
 -cpu = CPU(ppc_env_get_cpu(env));
 -if (cpu->cpu_index == env_idx) {
 -break;
 -}
 -}
 -
 +cpu = qemu_get_cpu(env_idx);
if (cpu == NULL) {
/* Unknown CPU */
return;
 @@ -161,11 +154,11 @@ static void spin_write(void *opaque, hwaddr addr, 
 uint64_t value,
if (!(ldq_p(&curspin->addr) & 1)) {
/* run CPU */
SpinKick kick = {
 -.cpu = ppc_env_get_cpu(env),
 +.cpu = POWERPC_CPU(cpu),
>>>
>>> Why not just cpu?
>>
>> PowerPCCPU vs. CPUState type.
>> Having the specific type in ppc code allows direct access to ->env.
>>
>> If you see a performance issue, we could also use (PowerPCCPU *)cpu.
> 
> There are no performance issues in the spin code :). By definition. This 
> thing could be written in bash and still be fast enough.
> 
> I was mostly wondering whether ppc_env_get_cpu(env) returns a PowerPCCPU *. 
> But apparently it does. Ok then :)
> 
> 
> Acked-by: Alexander Graf 

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

/-F

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