[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH] spapr: make sure RMA is in first mod
From: |
Thomas Huth |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH] spapr: make sure RMA is in first mode of first memory node |
Date: |
Mon, 4 Nov 2013 12:50:04 +0100 |
On Mon, 4 Nov 2013 12:28:12 +0100
Alexander Graf <address@hidden> wrote:
>
> On 04.11.2013, at 11:55, Benjamin Herrenschmidt <address@hidden> wrote:
>
> > On Mon, 2013-11-04 at 11:44 +0100, Alexander Graf wrote:
> >> On 01.11.2013, at 11:21, Alexey Kardashevskiy <address@hidden> wrote:
> >>
> >>> SLOF gets really confused if RTAS/device-tree and everything else
> >>> what SLOF can use is not in the very first block of the very first
> >>> memory node.
> >>>
> >>> This makes sure that the RMA area is where SLOF expects it to be.
> >>>
> >>> Cc: Benjamin Herrenschmidt <address@hidden>
> >>> Cc: Nikunj A Dadhania <address@hidden>
> >>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> >>> ---
> >>> hw/ppc/spapr.c | 8 +++++++-
> >>> 1 file changed, 7 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >>> index 09dc635..09a5d94 100644
> >>> --- a/hw/ppc/spapr.c
> >>> +++ b/hw/ppc/spapr.c
> >>> @@ -1113,7 +1113,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs
> >>> *args)
> >>> int i;
> >>> MemoryRegion *sysmem = get_system_memory();
> >>> MemoryRegion *ram = g_new(MemoryRegion, 1);
> >>> - hwaddr rma_alloc_size;
> >>> + hwaddr rma_alloc_size, node0_size;
> >>> uint32_t initrd_base = 0;
> >>> long kernel_size = 0, initrd_size = 0;
> >>> long load_limit, rtas_limit, fw_size;
> >>> @@ -1154,6 +1154,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs
> >>> *args)
> >>> spapr->rma_size = MIN(spapr->rma_size, 0x10000000);
> >>> }
> >>> }
> >>> + /*
> >>> + * SLOF gets confused if RMA resides not in the first block
> >>> + * of the first memory node so let's fix it.
> >>> + */
> >>> + node0_size = (nb_numa_nodes > 1) ? node_mem[0] : ram_size;
> >>> + spapr->rma_size = MIN(spapr->rma_size, node0_size);
> >> So if I create a NUMA node of 4MB that will be my RMA? That sounds pretty
> >> broken, especially on 970.
> >>
> >> Why does SLOF have any issues with NUMA memory nodes? It can just ignore
> >> them, no?
> >
> > Because the only way SLOF knows about the RMA is by using the first
> > "reg" entry of the first memory node and that's *all* SLOF knows about.
> >
> > If we start putting things like the DT, SLOF itself, etc... outside of
> > that region, it will crash.
Ok, the question is whether this is a bug in SLOF and should be fixed
there or whether the RMA should really be limited to the RAM of the
first node only.
Looking at the function spapr_populate_memory(), it seems there is
already similar code there, so I assume the RMA should really be
limited to that size:
/* 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;
}
Maybe this piece of code could just be done earlier instead, before
setting up the fdt_addr and rtas_addr variables, instead of adding the
similar piece of code of this patch?
> > So we "constrain" things to the rma that way.
> >
> > Creating 4M nodes makes no sense anyway
>
> So why don't we just use the "limit VRMA to 256MB" code always and error out
> of node0 is smaller? I don't think SLOF can run with less than 256MB anyway.
It's 128 MB nowadays ... there is even a define called MIN_RMA_SLOF for
this in the code already.
Thomas