Re: [Qemu-ppc] [PATCH 2/4] spapr: Remove obsolete ram_limit field from sPAPRMachineState

[David Gibson]

On Thu, May 07, 2015 at 02:12:58PM +1000, Alexey Kardashevskiy wrote:
> On 05/07/2015 01:57 PM, David Gibson wrote:
> >The ram_limit field was imported from sPAPREnvironment where it predates
> >the machine's ram size being available generically from machine->ram_size.
> >
> >Signed-off-by: David Gibson <address@hidden>
> >---
> >  hw/ppc/spapr.c         | 3 +--
> >  hw/ppc/spapr_hcall.c   | 3 ++-
> >  include/hw/ppc/spapr.h | 1 -
> >  3 files changed, 3 insertions(+), 4 deletions(-)
> >
> >diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >index 601c41b..1123358 100644
> >--- a/hw/ppc/spapr.c
> >+++ b/hw/ppc/spapr.c
> >@@ -1496,9 +1496,8 @@ static void ppc_spapr_init(MachineState *machine)
> >      }
> >
> >      /* allocate RAM */
> >-    sm->ram_limit = ram_size;
> 
> 
> 
> I had to look at the code to realize that ram_size is local and it can
> actually be removed.

Urgh.. it's actually worse than that.  There's a global that's
shadowed by the local.  ppc_spapr_init() uses the local,
spapr_populate_memory() uses the global.
> 
> 
> 
> >      memory_region_allocate_system_memory(ram, NULL, "ppc_spapr.ram",
> >-                                         sm->ram_limit);
> >+                                         machine->ram_size);
> >      memory_region_add_subregion(sysmem, 0, ram);
> >
> >      if (rma_alloc_size && rma) {
> >diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> >index bf4b05a..e52983a 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 *sm,
> >                              target_ulong opcode, target_ulong *args)
> >  {
> >+    MachineState *machine = MACHINE(sm);
> >      CPUPPCState *env = &cpu->env;
> >      target_ulong flags = args[0];
> >      target_ulong pte_index = args[1];
> >@@ -118,7 +119,7 @@ static target_ulong h_enter(PowerPCCPU *cpu, 
> >sPAPRMachineState *sm,
> >
> >      raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << page_shift) - 1);
> >
> >-    if (raddr < sm->ram_limit) {
> >+    if (raddr < machine->ram_size) {
> >          /* Regular RAM - should have WIMG=0010 */
> >          if ((ptel & HPTE64_R_WIMG) != HPTE64_R_M) {
> >              return H_PARAMETER;
> >diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> >index c3652aa..9e7cf0f 100644
> >--- a/include/hw/ppc/spapr.h
> >+++ b/include/hw/ppc/spapr.h
> >@@ -33,7 +33,6 @@ struct sPAPRMachineState {
> >      XICSState *icp;
> >      DeviceState *rtc;
> >
> >-    hwaddr ram_limit;
> >      void *htab;
> >      uint32_t htab_shift;
> >      hwaddr rma_size;
> >
> 
> 

-- 
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

pgpSrSGmRiyc2.pgp
Description: PGP signature