On Mon, 4 Nov 2013 12:28:12 +0100 Alexander Graf <ag...@suse.de> wrote:
> > On 04.11.2013, at 11:55, Benjamin Herrenschmidt <b...@kernel.crashing.org> > wrote: > > > On Mon, 2013-11-04 at 11:44 +0100, Alexander Graf wrote: > >> On 01.11.2013, at 11:21, Alexey Kardashevskiy <a...@ozlabs.ru> 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 <b...@kernel.crashing.org> > >>> Cc: Nikunj A Dadhania <nik...@linux.vnet.ibm.com> > >>> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru> > >>> --- > >>> 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