| Message ID | 1430971024-27899-3-git-send-email-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
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 <david@gibson.dropbear.id.au> > --- > 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. > 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; >
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 <david@gibson.dropbear.id.au> > >--- > > 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; > > > >
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 <david@gibson.dropbear.id.au> > >--- > > 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. Actually.. it's worse than that. There's both a global and this local which was masking it. Luckily the local had the same value, but it's still nasty. Respin will clean this up better.
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; 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;
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 <david@gibson.dropbear.id.au> --- hw/ppc/spapr.c | 3 +-- hw/ppc/spapr_hcall.c | 3 ++- include/hw/ppc/spapr.h | 1 - 3 files changed, 3 insertions(+), 4 deletions(-)