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

[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 <address@hidden>
>>>> ---
>>>> 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 <address@hidden>

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