Message ID | 1360947119-26115-1-git-send-email-afaerber@suse.de |
---|---|
State | New |
Headers | show |
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 <afaerber@suse.de> > --- > 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 >
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 <afaerber@suse.de> >> --- >> 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 >> >
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 <afaerber@suse.de> >>> --- >>> 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 <agraf@suse.de> 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
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 <afaerber@suse.de> >>>> --- >>>> 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 <agraf@suse.de> Thanks, applied to qom-cpu-next: https://github.com/afaerber/qemu-cpu/commits/qom-cpu-next /-F
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); } }
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 <afaerber@suse.de> --- hw/ppce500_spin.c | 15 ++++----------- 1 Datei geändert, 4 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-)