Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 04/10] hw/ppc: Replace global smp variables with machine smp properties

From: Greg Kurz <groug@kaod.org>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Like Xu" <like.xu@linux.intel.com>,
	qemu-trivial@nongnu.org,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	qemu-devel@nongnu.org, "Hervé Poussineau" <hpoussin@reactos.org>,
	"Cédric Le Goater" <clg@kaod.org>,
	"Alistair Francis" <alistair23@gmail.com>,
	"Igor Mammedov" <imammedo@redhat.com>,
	qemu-ppc@nongnu.org
Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 04/10] hw/ppc: Replace global smp variables with machine smp properties
Date: Thu, 6 Jun 2019 10:20:17 +0200	[thread overview]
Message-ID: <20190606102017.264123c5@bahia.lab.toulouse-stg.fr.ibm.com> (raw)
In-Reply-To: <20190606030732.GL10319@umbus.fritz.box>

[-- Attachment #1: Type: text/plain, Size: 2533 bytes --]

On Thu, 6 Jun 2019 13:07:32 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Jun 05, 2019 at 11:54:56PM -0300, Eduardo Habkost wrote:
> > On Wed, Jun 05, 2019 at 11:52:41PM -0300, Eduardo Habkost wrote:  
> > > On Sun, May 19, 2019 at 04:54:22AM +0800, Like Xu wrote:  
> > > > The global smp variables in ppc are replaced with smp machine properties.
> > > > 
> > > > A local variable of the same name would be introduced in the declaration
> > > > phase if it's used widely in the context OR replace it on the spot if it's
> > > > only used once. No semantic changes.
> > > > 
> > > > Signed-off-by: Like Xu <like.xu@linux.intel.com>  
> > > 
> > > Any objections from the ppc maintainers to queueing this through
> > > the Machine Core tree?  
> > 
> > Oops, CCing the ppc maintainers.  
> 
> No objection here.
> 
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> 

Just one nit...

[...]

> > > > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> > > > index ee24212765..c9ffe9786c 100644
> > > > --- a/hw/ppc/spapr_rtas.c
> > > > +++ b/hw/ppc/spapr_rtas.c
> > > > @@ -231,6 +231,8 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
> > > >                                            target_ulong args,
> > > >                                            uint32_t nret, target_ulong rets)
> > > >  {
> > > > +    MachineState *ms = MACHINE(qdev_get_machine());

rtas_ibm_get_system_parameter() has a SpaprMachineState *spapr argument, no
need to rely on qdev_get_machine().

But this can be fixed in a followup patch I guess. Not worth holding the
patchset because of that.

> > > > +    unsigned int max_cpus = ms->smp.max_cpus;
> > > >      target_ulong parameter = rtas_ld(args, 0);
> > > >      target_ulong buffer = rtas_ld(args, 1);
> > > >      target_ulong length = rtas_ld(args, 2);
> > > > @@ -244,7 +246,7 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
> > > >                                            "MaxPlatProcs=%d",
> > > >                                            max_cpus,
> > > >                                            current_machine->ram_size / MiB,
> > > > -                                          smp_cpus,
> > > > +                                          ms->smp.cpus,
> > > >                                            max_cpus);
> > > >          ret = sysparm_st(buffer, length, param_val, strlen(param_val) + 1);
> > > >          g_free(param_val);  
> > >   
> >   
> 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]