[4/8] ppc4xx: Use ram_addr_t in ppc4xx_sdram_adjust()

To avoid overflow if larger values are added later use ram_addr_t for
the sdram_bank_sizes parameter to match ram_size to which it is compared.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/ppc440_bamboo.c  | 2 +-
 hw/ppc/ppc4xx_devs.c    | 4 ++--
 hw/ppc/sam460ex.c       | 2 +-
 include/hw/ppc/ppc4xx.h | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

On Wed, Jan 02, 2019 at 03:06:38AM +0100, BALATON Zoltan wrote:
> To avoid overflow if larger values are added later use ram_addr_t for
> the sdram_bank_sizes parameter to match ram_size to which it is
> compared.

So, technically I think these should be 'hwaddr' (which represents a
guest physical address) rather tham ram_addr_t which
represents... something subtley different I've never properly
understood.

> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/ppc/ppc440_bamboo.c  | 2 +-
>  hw/ppc/ppc4xx_devs.c    | 4 ++--
>  hw/ppc/sam460ex.c       | 2 +-
>  include/hw/ppc/ppc4xx.h | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index b8aa55d526..8277c0f784 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -49,7 +49,7 @@
>  
>  #define PPC440EP_SDRAM_NR_BANKS 4
>  
> -static const unsigned int ppc440ep_sdram_bank_sizes[] = {
> +static const ram_addr_t ppc440ep_sdram_bank_sizes[] = {
>      256 * MiB, 128 * MiB, 64 * MiB, 32 * MiB, 16 * MiB, 8 * MiB, 0
>  };
>  
> diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
> index 9b6e4c60fa..9418478575 100644
> --- a/hw/ppc/ppc4xx_devs.c
> +++ b/hw/ppc/ppc4xx_devs.c
> @@ -679,12 +679,12 @@ ram_addr_t ppc4xx_sdram_adjust(ram_addr_t ram_size, int nr_banks,
>                                 MemoryRegion ram_memories[],
>                                 hwaddr ram_bases[],
>                                 hwaddr ram_sizes[],
> -                               const unsigned int sdram_bank_sizes[])
> +                               const ram_addr_t sdram_bank_sizes[])
>  {
>      MemoryRegion *ram = g_malloc0(sizeof(*ram));
>      ram_addr_t size_left = ram_size;
>      ram_addr_t base = 0;
> -    unsigned int bank_size;
> +    ram_addr_t bank_size;
>      int i;
>      int j;
>  
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 2bb91ed21b..7cbd2f54c6 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -77,7 +77,7 @@
>  #define SDRAM_NR_BANKS 4
>  
>  /* FIXME: See u-boot.git 8ac41e, also fix in ppc440_uc.c */
> -static const unsigned int ppc460ex_sdram_bank_sizes[] = {
> +static const ram_addr_t ppc460ex_sdram_bank_sizes[] = {
>      1 * GiB, 512 * MiB, 256 * MiB, 128 * MiB, 64 * MiB, 32 * MiB, 0
>  };
>  
> diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
> index 3a2a04c8ce..39a7ba1ce6 100644
> --- a/include/hw/ppc/ppc4xx.h
> +++ b/include/hw/ppc/ppc4xx.h
> @@ -43,7 +43,7 @@ ram_addr_t ppc4xx_sdram_adjust(ram_addr_t ram_size, int nr_banks,
>                                 MemoryRegion ram_memories[],
>                                 hwaddr ram_bases[],
>                                 hwaddr ram_sizes[],
> -                               const unsigned int sdram_bank_sizes[]);
> +                               const ram_addr_t sdram_bank_sizes[]);
>  
>  void ppc4xx_sdram_init (CPUPPCState *env, qemu_irq irq, int nbanks,
>                          MemoryRegion ram_memories[],

On Wed, 2 Jan 2019, David Gibson wrote:
> On Wed, Jan 02, 2019 at 03:06:38AM +0100, BALATON Zoltan wrote:
>> To avoid overflow if larger values are added later use ram_addr_t for
>> the sdram_bank_sizes parameter to match ram_size to which it is
>> compared.
>
> So, technically I think these should be 'hwaddr' (which represents a
> guest physical address) rather tham ram_addr_t which
> represents... something subtley different I've never properly
> understood.

I don't understand the difference either but ram_size in MachineState 
where this value comes from is ram_addr_t now so I've left is for now. If 
someone knows which type should this be can change it in another patch 
later.

Regards,
BALATON Zoltan

On Thu, Jan 03, 2019 at 03:03:20PM +0100, BALATON Zoltan wrote:
> On Wed, 2 Jan 2019, David Gibson wrote:
> > On Wed, Jan 02, 2019 at 03:06:38AM +0100, BALATON Zoltan wrote:
> > > To avoid overflow if larger values are added later use ram_addr_t for
> > > the sdram_bank_sizes parameter to match ram_size to which it is
> > > compared.
> > 
> > So, technically I think these should be 'hwaddr' (which represents a
> > guest physical address) rather tham ram_addr_t which
> > represents... something subtley different I've never properly
> > understood.
> 
> I don't understand the difference either but ram_size in MachineState where
> this value comes from is ram_addr_t now so I've left is for now. If someone
> knows which type should this be can change it in another patch
> later.

Ok, fair enough.

On Fri, 4 Jan 2019, David Gibson wrote:
> On Thu, Jan 03, 2019 at 03:03:20PM +0100, BALATON Zoltan wrote:
>> On Wed, 2 Jan 2019, David Gibson wrote:
>>> On Wed, Jan 02, 2019 at 03:06:38AM +0100, BALATON Zoltan wrote:
>>>> To avoid overflow if larger values are added later use ram_addr_t for
>>>> the sdram_bank_sizes parameter to match ram_size to which it is
>>>> compared.
>>>
>>> So, technically I think these should be 'hwaddr' (which represents a
>>> guest physical address) rather tham ram_addr_t which
>>> represents... something subtley different I've never properly
>>> understood.
>>
>> I don't understand the difference either but ram_size in MachineState where
>> this value comes from is ram_addr_t now so I've left is for now. If someone
>> knows which type should this be can change it in another patch
>> later.
>
> Ok, fair enough.

Then will you take v3 of this series or is there anything else that should 
be corrected?

Regards,
BALATON Zoltan