* [Qemu-devel] [PATCH v2] spapr_cpu_core: cleaning up qdev_get_machine() calls
@ 2017-09-11 21:10 Greg Kurz
2017-09-12 3:27 ` David Gibson
0 siblings, 1 reply; 3+ messages in thread
From: Greg Kurz @ 2017-09-11 21:10 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-ppc, David Gibson, Daniel Henrique Barboza
This patch removes the qdev_get_machine() calls that are made
in spapr_cpu_core.c in situations where we can get an existing
pointer for the MachineState by either passing it as an argument
to the function or by using other already available pointers.
Credits to Daniel Henrique Barboza for the idea and the changelog
text.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
v2: - fixed typo in spapr_cpu_reset()
---
hw/ppc/spapr_cpu_core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index dc9df0d393d1..0f32532abe99 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -73,8 +73,8 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
static void spapr_cpu_reset(void *opaque)
{
- sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
PowerPCCPU *cpu = opaque;
+ sPAPRMachineState *spapr = SPAPR_MACHINE(cpu->vhyp);
CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
@@ -162,10 +162,10 @@ static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp)
g_free(sc->threads);
}
-static void spapr_cpu_core_realize_child(Object *child, Error **errp)
+static void spapr_cpu_core_realize_child(Object *child,
+ sPAPRMachineState *spapr, Error **errp)
{
Error *local_err = NULL;
- sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
CPUState *cs = CPU(child);
PowerPCCPU *cpu = POWERPC_CPU(cs);
Object *obj;
@@ -254,7 +254,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
for (j = 0; j < cc->nr_threads; j++) {
obj = sc->threads + j * size;
- spapr_cpu_core_realize_child(obj, &local_err);
+ spapr_cpu_core_realize_child(obj, spapr, &local_err);
if (local_err) {
goto err;
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v2] spapr_cpu_core: cleaning up qdev_get_machine() calls
2017-09-11 21:10 [Qemu-devel] [PATCH v2] spapr_cpu_core: cleaning up qdev_get_machine() calls Greg Kurz
@ 2017-09-12 3:27 ` David Gibson
2017-09-12 3:58 ` David Gibson
0 siblings, 1 reply; 3+ messages in thread
From: David Gibson @ 2017-09-12 3:27 UTC (permalink / raw)
To: Greg Kurz; +Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
[-- Attachment #1: Type: text/plain, Size: 2331 bytes --]
On Mon, Sep 11, 2017 at 11:10:57PM +0200, Greg Kurz wrote:
> This patch removes the qdev_get_machine() calls that are made
> in spapr_cpu_core.c in situations where we can get an existing
> pointer for the MachineState by either passing it as an argument
> to the function or by using other already available pointers.
>
> Credits to Daniel Henrique Barboza for the idea and the changelog
> text.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> v2: - fixed typo in spapr_cpu_reset()
Applied to ppc-for-2.11.
> ---
> hw/ppc/spapr_cpu_core.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index dc9df0d393d1..0f32532abe99 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -73,8 +73,8 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
>
> static void spapr_cpu_reset(void *opaque)
> {
> - sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> PowerPCCPU *cpu = opaque;
> + sPAPRMachineState *spapr = SPAPR_MACHINE(cpu->vhyp);
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
>
> @@ -162,10 +162,10 @@ static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp)
> g_free(sc->threads);
> }
>
> -static void spapr_cpu_core_realize_child(Object *child, Error **errp)
> +static void spapr_cpu_core_realize_child(Object *child,
> + sPAPRMachineState *spapr, Error **errp)
> {
> Error *local_err = NULL;
> - sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> CPUState *cs = CPU(child);
> PowerPCCPU *cpu = POWERPC_CPU(cs);
> Object *obj;
> @@ -254,7 +254,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
> for (j = 0; j < cc->nr_threads; j++) {
> obj = sc->threads + j * size;
>
> - spapr_cpu_core_realize_child(obj, &local_err);
> + spapr_cpu_core_realize_child(obj, spapr, &local_err);
> if (local_err) {
> goto err;
> }
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v2] spapr_cpu_core: cleaning up qdev_get_machine() calls
2017-09-12 3:27 ` David Gibson
@ 2017-09-12 3:58 ` David Gibson
0 siblings, 0 replies; 3+ messages in thread
From: David Gibson @ 2017-09-12 3:58 UTC (permalink / raw)
To: Greg Kurz; +Cc: qemu-devel, qemu-ppc, Daniel Henrique Barboza
[-- Attachment #1: Type: text/plain, Size: 2945 bytes --]
On Tue, Sep 12, 2017 at 01:27:53PM +1000, David Gibson wrote:
> On Mon, Sep 11, 2017 at 11:10:57PM +0200, Greg Kurz wrote:
> > This patch removes the qdev_get_machine() calls that are made
> > in spapr_cpu_core.c in situations where we can get an existing
> > pointer for the MachineState by either passing it as an argument
> > to the function or by using other already available pointers.
> >
> > Credits to Daniel Henrique Barboza for the idea and the changelog
> > text.
> >
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> > v2: - fixed typo in spapr_cpu_reset()
>
> Applied to ppc-for-2.11.
Wait... no. The second hunk is fine, but the first isn't.
>
> > ---
> > hw/ppc/spapr_cpu_core.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > index dc9df0d393d1..0f32532abe99 100644
> > --- a/hw/ppc/spapr_cpu_core.c
> > +++ b/hw/ppc/spapr_cpu_core.c
> > @@ -73,8 +73,8 @@ void spapr_cpu_parse_features(sPAPRMachineState *spapr)
> >
> > static void spapr_cpu_reset(void *opaque)
> > {
> > - sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> > PowerPCCPU *cpu = opaque;
> > + sPAPRMachineState *spapr = SPAPR_MACHINE(cpu->vhyp);
Although we prefer to avoid the global when we can, it's preferable to
breaking abstraction by maching the machine reach into the cpu
internals here.
Arguably, instead of having this special machine-registered cpu reset
function we should instead have a hook in vhyp which is called by the
cpu core's reset function, but that would be a different change.
> > CPUState *cs = CPU(cpu);
> > CPUPPCState *env = &cpu->env;
> >
> > @@ -162,10 +162,10 @@ static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp)
> > g_free(sc->threads);
> > }
> >
> > -static void spapr_cpu_core_realize_child(Object *child, Error **errp)
> > +static void spapr_cpu_core_realize_child(Object *child,
> > + sPAPRMachineState *spapr, Error **errp)
> > {
> > Error *local_err = NULL;
> > - sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> > CPUState *cs = CPU(child);
> > PowerPCCPU *cpu = POWERPC_CPU(cs);
> > Object *obj;
> > @@ -254,7 +254,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
> > for (j = 0; j < cc->nr_threads; j++) {
> > obj = sc->threads + j * size;
> >
> > - spapr_cpu_core_realize_child(obj, &local_err);
> > + spapr_cpu_core_realize_child(obj, spapr, &local_err);
> > if (local_err) {
> > goto err;
> > }
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-09-12 4:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-11 21:10 [Qemu-devel] [PATCH v2] spapr_cpu_core: cleaning up qdev_get_machine() calls Greg Kurz
2017-09-12 3:27 ` David Gibson
2017-09-12 3:58 ` David Gibson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).