Re: [Qemu-ppc] [Qemu-devel] [PATCH v4] spapr: make sure RMA is in first mode of first memory node

[Thomas Huth]

On Fri,  8 Nov 2013 14:27:40 +1100
Alexey Kardashevskiy <address@hidden> wrote:

> The SPAPR specification says that the RMA starts at the LPAR's logical
> address 0 and is the first logical memory block reported in
> the LPAR’s device tree.
> 
> So SLOF only maps the first block and that block needs to span
> the full RMA.
> 
> This makes sure that the RMA area is where SLOF expects it.
> 
> Cc: Thomas Huth <address@hidden>
> Cc: Benjamin Herrenschmidt <address@hidden>
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> Changes:
> v4:
> * fixed a bug with preallocated RMA (thanks to Thomas Huth)
> 
> v3:
> * removed unnecessary RMA fixup from spapr_populate_memory()
> 
> v2:
> * changed as recommended by Alex Graf
> ---
>  hw/ppc/spapr.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7e53a5f..036246c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -532,9 +532,6 @@ static int spapr_populate_memory(sPAPREnvironment *spapr, 
> void *fdt)
> 
>      /* memory node(s) */
>      node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> -    if (spapr->rma_size > node0_size) {
> -        spapr->rma_size = node0_size;
> -    }
> 
>      /* RMA */
>      mem_reg_property[0] = 0;
> @@ -1113,6 +1110,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>      MemoryRegion *sysmem = get_system_memory();
>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>      hwaddr rma_alloc_size;
> +    hwaddr node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
>      uint32_t initrd_base = 0;
>      long kernel_size = 0, initrd_size = 0;
>      long load_limit, rtas_limit, fw_size;
> @@ -1134,10 +1132,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>          exit(1);
>      }
> 
> -    if (rma_alloc_size && (rma_alloc_size < ram_size)) {
> +    if (rma_alloc_size && (rma_alloc_size < node0_size)) {
>          spapr->rma_size = rma_alloc_size;
>      } else {
> -        spapr->rma_size = ram_size;
> +        spapr->rma_size = node0_size;
> 
>          /* With KVM, we don't actually know whether KVM supports an
>           * unbounded RMA (PR KVM) or is limited by the hash table size
> @@ -1154,6 +1152,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>          }
>      }
> 
> +    if (spapr->rma_size > node0_size) {
> +        fprintf(stderr, "Error: Numa node 0 has to span the RMA 
> (%#08"HWADDR_PRIx")\n",
> +                spapr->rma_size);
> +        exit(1);
> +    }
> +
>      /* We place the device tree and RTAS just below either the top of the 
> RMA,
>       * or just below 2GB, whichever is lowere, so that it can be
>       * processed with 32-bit real mode code if necessary */

Looks fine to me now!

Reviewed-by: Thomas Huth <address@hidden>