On Tue, May 26, 2015 at 10:00:31AM +0200, Thomas Huth wrote:
> On Tue, 26 May 2015 12:22:57 +1000
> David Gibson <da...@gibson.dropbear.id.au> wrote:
>
> > The ram_limit field was imported from sPAPREnvironment where it predates
> > the machine's ram size being available generically from machine->ram_size.
> >
> > Worse, the existing code was inconsistent about where it got the ram size
> > from. Sometimes it used spapr->ram_limit, sometimes the global 'ram_size'
> > and sometimes a local 'ram_size' masking the global.
> >
> > This cleans up the code to consistently use machine->ram_size, eliminating
> > spapr->ram_limit in the process.
> >
> > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> > ---
> > hw/ppc/spapr.c | 22 ++++++++++++----------
> > hw/ppc/spapr_hcall.c | 3 ++-
> > include/hw/ppc/spapr.h | 1 -
> > 3 files changed, 14 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 0016f25..31b29d6 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> [...]
> > @@ -649,6 +652,7 @@ static void spapr_populate_memory_node(void *fdt, int
> > nodeid, hwaddr start,
> >
> > static int spapr_populate_memory(sPAPRMachineState *spapr, void *fdt)
> > {
> > + MachineState *machine = &spapr->parent_obj;
>
> Here you use &spapr->parent_obj ... below you use MACHINE(spapr) ...
> looks somewhat inconsequent ==> maybe also use MACHINE(spapr) here?
Ah, good catch. Clearly not quite back into QOM headspace.
> ...
> > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> > index 1a20884..652ddf6 100644
> > --- a/hw/ppc/spapr_hcall.c
> > +++ b/hw/ppc/spapr_hcall.c
> > @@ -87,6 +87,7 @@ static inline bool valid_pte_index(CPUPPCState *env,
> > target_ulong pte_index)
> > static target_ulong h_enter(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> > target_ulong opcode, target_ulong *args)
> > {
> > + MachineState *machine = MACHINE(spapr);
> > CPUPPCState *env = &cpu->env;
> > target_ulong flags = args[0];
> > target_ulong pte_index = args[1];
>
> Apart from the above nit, patch looks fine to me, so:
>
> Reviewed-by: Thomas Huth <th...@redhat.com>
>
> Another question out of curiosity: Do you know if the global "ram_size"
> variable is scheduled to be removed soon in the future?
I don't know, I'm afraid.
--
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
pgp7WbNroD0wl.pgp
Description: PGP signature
