From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1948058AbcHETEk (ORCPT ); Fri, 5 Aug 2016 15:04:40 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:12234 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1947926AbcHETEY (ORCPT ); Fri, 5 Aug 2016 15:04:24 -0400 X-IBM-Helo: d03dlp03.boulder.ibm.com X-IBM-MailFrom: pc@us.ibm.com Reply-To: pc@us.ibm.com Subject: Re: [PATCH 2/2] powerpc/pseries: Dynamically increase RMA size References: <20160805061041.GA31857@us.ibm.com> <20160805061426.GB31857@us.ibm.com> <20160805183055.GA842@us.ibm.com> To: Sukadev Bhattiprolu , Michael Ellerman Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org From: Paul Clarke Date: Fri, 5 Aug 2016 14:04:07 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2 MIME-Version: 1.0 In-Reply-To: <20160805183055.GA842@us.ibm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16080519-8235-0000-0000-000008EF314F X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00005552; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000178; SDB=6.00740485; UDB=6.00348282; IPR=6.00513067; BA=6.00004645; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00012181; XFM=3.00000011; UTC=2016-08-05 19:04:20 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16080519-8236-0000-0000-000033B9D473 Message-Id: X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-08-05_12:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1608050224 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Only nits from me...(see below) On 08/05/2016 01:30 PM, Sukadev Bhattiprolu wrote: > Here is an updated patch to fix the build when CONFIG_PPC_PSERIES=n. > --- > From d4f77a6ca7b6ea83f6588e7d541cc70bf001ae85 Mon Sep 17 00:00:00 2001 > From: root > Date: Thu, 4 Aug 2016 23:13:37 -0400 > Subject: [PATCH 2/2] powerpc/pseries: Dynamically grow RMA size > > When booting a very large system with a larg initrd we run out of space > for the flattened device tree (FDT). To fix this we must increase the > space allocated for the RMA region. > > The RMA size is hard-coded in the 'ibm_architecture_vec[]' and increasing > the size there will apply to all systems, small and large, so we want to > increase the RMA region only when necessary. > > When we run out of room for the FDT, set a new OF property, 'ibm,new-rma-size' > to the new RMA size (512MB) and issue a client-architecture-support (CAS) > call to the firmware. This will initiate a system reboot. Upon reboot we > notice the new property and update the RMA size accordingly. > > Fix suggested by Michael Ellerman. > > Signed-off-by: Sukadev Bhattiprolu > --- > > [v2]: - Add a comment in code regarding 'fixup_nr_cores_done' > - Fix build break when CONFIG_PPC_PSERIES=n > --- > arch/powerpc/kernel/prom_init.c | 96 ++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 95 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c > index f612a99..cbd5387 100644 > --- a/arch/powerpc/kernel/prom_init.c > +++ b/arch/powerpc/kernel/prom_init.c > @@ -679,6 +679,7 @@ unsigned char ibm_architecture_vec[] = { > W(0xffffffff), /* virt_base */ > W(0xffffffff), /* virt_size */ > W(0xffffffff), /* load_base */ > +#define IBM_ARCH_VEC_MIN_RMA_OFFSET 108 > W(256), /* 256MB min RMA */ > W(0xffffffff), /* full client load */ > 0, /* min RMA percentage of total RAM */ > @@ -867,6 +868,14 @@ static void fixup_nr_cores(void) > { > u32 cores; > unsigned char *ptcores; > + static bool fixup_nr_cores_done = false; > + > + /* > + * If this is a second CAS call in the same boot sequence, (see > + * increase_rma_size()), we don't need to do the fixup again. > + */ > + if (fixup_nr_cores_done) > + return; > > /* We need to tell the FW about the number of cores we support. > * > @@ -898,6 +907,41 @@ static void fixup_nr_cores(void) > ptcores[1] = (cores >> 16) & 0xff; > ptcores[2] = (cores >> 8) & 0xff; > ptcores[3] = cores & 0xff; > + fixup_nr_cores_done = true; > + } > +} > + > +static void __init fixup_rma_size(void) > +{ > + int rc; > + u64 size; > + unsigned char *min_rmap; > + phandle optnode; > + char str[64]; > + > + optnode = call_prom("finddevice", 1, 1, ADDR("/options")); > + if (!PHANDLE_VALID(optnode)) > + prom_panic("Cannot find /options"); > + > + /* > + * If a prior boot specified a new RMA size, use that size in > + * ibm_architecture_vec[]. See also increase_rma_size(). > + */ > + size = 0ULL; > + memset(str, 0, sizeof(str)); > + rc = prom_getprop(optnode, "ibm,new-rma-size", &str, sizeof(str)); > + if (rc <= 0) > + return; > + > + size = prom_strtoul(str, NULL); > + min_rmap = &ibm_architecture_vec[IBM_ARCH_VEC_MIN_RMA_OFFSET]; > + > + if (size) { > + prom_printf("Using RMA size %lu from ibm,new-rma-size.\n", size); > + min_rmap[0] = (size >> 24) & 0xff; > + min_rmap[1] = (size >> 16) & 0xff; > + min_rmap[2] = (size >> 8) & 0xff; > + min_rmap[3] = size & 0xff; > } > } > > @@ -911,6 +955,8 @@ static void __init prom_send_capabilities(void) > > fixup_nr_cores(); > > + fixup_rma_size(); > + > /* try calling the ibm,client-architecture-support method */ > prom_printf("Calling ibm,client-architecture-support..."); > if (call_prom_ret("call-method", 3, 2, &ret, > @@ -946,6 +992,52 @@ static void __init prom_send_capabilities(void) > } > #endif /* __BIG_ENDIAN__ */ > } > + > +static void __init increase_rma_size(void) > +{ > + int rc; > + u64 size; > + char str[64]; > + phandle optnode; > + > + optnode = call_prom("finddevice", 1, 1, ADDR("/options")); > + if (!PHANDLE_VALID(optnode)) > + prom_panic("Cannot find /options"); > + > + /* > + * If we already increased the RMA size, return. > + */ > + size = 0ULL; > + memset(str, 0, sizeof(str)); > + rc = prom_getprop(optnode, "ibm,new-rma-size", &str, sizeof(str)); > + > + size = prom_strtoul(str, NULL); > + if (size == 512ULL) { Is this preferred over strncmp? Using a string also helps with my suggestion below... > + prom_printf("RMA size already at %lu.\n", size); > + return; > + } > + /* > + * Otherwise, set the ibm,new-rma-size property and initiate a CAS > + * reboot so the RMA size can take effect. See also init_rma_size(). > + */ > + memset(str, 0, 4); > + memcpy(str, "512", 3); There's a "512" here and a few lines above. Would it be better to define the magic value once somewhere, then use that common name as needed? The string "ibm,new-rma-size" is used in a number of places, too. (I'm just saying that if it changes, you'd need to go back and find them all.) Also, instead of memset/memcpy, why not: memcpy(str, "512", 4); > + prom_printf("Setting ibm,new-rma-size property to %s\n", str); > + rc = prom_setprop(optnode, "/options", "ibm,new-rma-size", &str, > + strlen(str)+1); > + > + /* Force a reboot. Will work only if ibm,fw-override-cas==false */ > + prom_send_capabilities(); > + > + prom_printf("No CAS initiated reboot? Try setting ibm,fw-override-cas to 'false' in Open Firmware\n"); > +} > + > +#else > + > +static void __init increase_rma_size(void) > +{ > +} > + > #endif /* #if defined(CONFIG_PPC_PSERIES) || defined(CONFIG_PPC_POWERNV) */ > > /* > @@ -2027,9 +2119,11 @@ static void __init *make_room(unsigned long *mem_start, unsigned long *mem_end, > room = alloc_top - alloc_bottom; > if (room > DEVTREE_CHUNK_SIZE) > room = DEVTREE_CHUNK_SIZE; > - if (room < PAGE_SIZE) > + if (room < PAGE_SIZE) { > + increase_rma_size(); > prom_panic("No memory for flatten_device_tree " > "(no room)\n"); > + } > chunk = alloc_up(room, 0); > if (chunk == 0) > prom_panic("No memory for flatten_device_tree " > -- PC