Re: [PATCH REPOST v3 65/80] ppc/{ppc440_bamboo, sam460ex}: use memdev for RAM

[David Gibson <david@gibson.dropbear.id.au>]

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

On Thu, Jan 23, 2020 at 12:38:30PM +0100, Igor Mammedov wrote:
> memory_region_allocate_system_memory() API is going away, so
> replace it with memdev allocated MemoryRegion. The later is
> initialized by generic code, so board only needs to opt in
> to memdev scheme by providing
>   MachineClass::default_ram_id
> and using MachineState::ram instead of manually initializing
> RAM memory region.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> v3:
>   * drop ram_size from comment above ppc4xx_sdram_banks
>      (BALATON Zoltan <balaton@eik.bme.hu>)
>   * move memory alias initialization into the same loop
>     where RAM is split on banks.
>      (BALATON Zoltan <balaton@eik.bme.hu>)
> v3.1
>   * rebase on top of previous patch due to changed context
> 
> CC: balaton@eik.bme.hu
> CC: david@gibson.dropbear.id.au
> CC: qemu-ppc@nongnu.org
> ---
>  include/hw/ppc/ppc4xx.h |  2 +-
>  hw/ppc/ppc440_bamboo.c  |  3 ++-
>  hw/ppc/ppc4xx_devs.c    | 25 +++++++++----------------
>  hw/ppc/sam460ex.c       |  3 ++-
>  4 files changed, 14 insertions(+), 19 deletions(-)
> 
> diff --git a/include/hw/ppc/ppc4xx.h b/include/hw/ppc/ppc4xx.h
> index b8c8f32..cc19c8d 100644
> --- a/include/hw/ppc/ppc4xx.h
> +++ b/include/hw/ppc/ppc4xx.h
> @@ -42,7 +42,7 @@ enum {
>  qemu_irq *ppcuic_init (CPUPPCState *env, qemu_irq *irqs,
>                         uint32_t dcr_base, int has_ssr, int has_vr);
>  
> -void ppc4xx_sdram_banks(ram_addr_t ram_size, int nr_banks,
> +void ppc4xx_sdram_banks(MemoryRegion *ram, int nr_banks,
>                          MemoryRegion ram_memories[],
>                          hwaddr ram_bases[], hwaddr ram_sizes[],
>                          const ram_addr_t sdram_bank_sizes[]);
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index 29a9642..1d4a11d 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -202,7 +202,7 @@ static void bamboo_init(MachineState *machine)
>      /* SDRAM controller */
>      memset(ram_bases, 0, sizeof(ram_bases));
>      memset(ram_sizes, 0, sizeof(ram_sizes));
> -    ppc4xx_sdram_banks(machine->ram_size, PPC440EP_SDRAM_NR_BANKS, ram_memories,
> +    ppc4xx_sdram_banks(machine->ram, PPC440EP_SDRAM_NR_BANKS, ram_memories,
>                         ram_bases, ram_sizes, ppc440ep_sdram_bank_sizes);
>      /* XXX 440EP's ECC interrupts are on UIC1, but we've only created UIC0. */
>      ppc4xx_sdram_init(env, pic[14], PPC440EP_SDRAM_NR_BANKS, ram_memories,
> @@ -289,6 +289,7 @@ static void bamboo_machine_init(MachineClass *mc)
>      mc->desc = "bamboo";
>      mc->init = bamboo_init;
>      mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("440epb");
> +    mc->default_ram_id = "ppc4xx.sdram";
>  }
>  
>  DEFINE_MACHINE("bamboo", bamboo_machine_init)
> diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
> index d89008a..0e2f81c 100644
> --- a/hw/ppc/ppc4xx_devs.c
> +++ b/hw/ppc/ppc4xx_devs.c
> @@ -666,7 +666,7 @@ void ppc4xx_sdram_init (CPUPPCState *env, qemu_irq irq, int nbanks,
>          sdram_map_bcr(sdram);
>  }
>  
> -/* Fill in consecutive SDRAM banks with 'ram_size' bytes of memory.
> +/* Split RAM between SDRAM banks.
>   *
>   * sdram_bank_sizes[] must be in descending order, that is sizes[i] > sizes[i+1]
>   * and must be 0-terminated.
> @@ -674,16 +674,14 @@ void ppc4xx_sdram_init (CPUPPCState *env, qemu_irq irq, int nbanks,
>   * The 4xx SDRAM controller supports a small number of banks, and each bank
>   * must be one of a small set of sizes. The number of banks and the supported
>   * sizes varies by SoC. */
> -void ppc4xx_sdram_banks(ram_addr_t ram_size, int nr_banks,
> +void ppc4xx_sdram_banks(MemoryRegion *ram, int nr_banks,
>                          MemoryRegion ram_memories[],
>                          hwaddr ram_bases[], hwaddr ram_sizes[],
>                          const ram_addr_t sdram_bank_sizes[])
>  {
> -    MemoryRegion *ram = g_malloc0(sizeof(*ram));
> -    ram_addr_t size_left = ram_size;
> +    ram_addr_t size_left = memory_region_size(ram);
>      ram_addr_t base = 0;
>      ram_addr_t bank_size;
> -    int last_bank = 0;
>      int i;
>      int j;
>  
> @@ -691,11 +689,15 @@ void ppc4xx_sdram_banks(ram_addr_t ram_size, int nr_banks,
>          for (j = 0; sdram_bank_sizes[j] != 0; j++) {
>              bank_size = sdram_bank_sizes[j];
>              if (bank_size <= size_left) {
> +                char name[32];
> +
>                  ram_bases[i] = base;
>                  ram_sizes[i] = bank_size;
>                  base += bank_size;
>                  size_left -= bank_size;
> -                last_bank = i;
> +                snprintf(name, sizeof(name), "ppc4xx.sdram%d", i);
> +                memory_region_init_alias(&ram_memories[i], NULL, name, ram,
> +                                         ram_bases[i], ram_sizes[i]);
>                  break;
>              }
>          }
> @@ -706,7 +708,7 @@ void ppc4xx_sdram_banks(ram_addr_t ram_size, int nr_banks,
>      }
>  
>      if (size_left) {
> -        ram_addr_t used_size = ram_size - size_left;
> +        ram_addr_t used_size = memory_region_size(ram) - size_left;
>          GString *s = g_string_new(NULL);
>  
>          for (i = 0; sdram_bank_sizes[i]; i++) {
> @@ -722,15 +724,6 @@ void ppc4xx_sdram_banks(ram_addr_t ram_size, int nr_banks,
>          g_string_free(s, true);
>          exit(EXIT_FAILURE);
>      }
> -
> -    memory_region_allocate_system_memory(ram, NULL, "ppc4xx.sdram", ram_size);
> -
> -    for (i = 0; i <= last_bank; i++) {
> -        char name[32];
> -        snprintf(name, sizeof(name), "ppc4xx.sdram%d", i);
> -        memory_region_init_alias(&ram_memories[i], NULL, name, ram,
> -                                 ram_bases[i], ram_sizes[i]);
> -    }
>  }
>  
>  /*****************************************************************************/
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 949acba..b48aea5 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -324,7 +324,7 @@ static void sam460ex_init(MachineState *machine)
>      /* SDRAM controller */
>      /* put all RAM on first bank because board has one slot
>       * and firmware only checks that */
> -    ppc4xx_sdram_banks(machine->ram_size, 1, ram_memories, ram_bases, ram_sizes,
> +    ppc4xx_sdram_banks(machine->ram, 1, ram_memories, ram_bases, ram_sizes,
>                         ppc460ex_sdram_bank_sizes);
>  
>      /* FIXME: does 460EX have ECC interrupts? */
> @@ -483,6 +483,7 @@ static void sam460ex_machine_init(MachineClass *mc)
>      mc->init = sam460ex_init;
>      mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("460exb");
>      mc->default_ram_size = 512 * MiB;
> +    mc->default_ram_id = "ppc4xx.sdram";
>  }
>  
>  DEFINE_MACHINE("sam460ex", sam460ex_machine_init)

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

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