From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755559AbbINQNo (ORCPT ); Mon, 14 Sep 2015 12:13:44 -0400 Received: from mail-by2on0130.outbound.protection.outlook.com ([207.46.100.130]:30153 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753511AbbINQNm (ORCPT ); Mon, 14 Sep 2015 12:13:42 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=scottwood@freescale.com; Message-ID: <1442247213.2909.71.camel@freescale.com> Subject: Re: [PATCH v3] powerpc32: memset: only use dcbz once cache is enabled From: Scott Wood To: Christophe LEROY CC: Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , , , Date: Mon, 14 Sep 2015 11:13:33 -0500 In-Reply-To: <55F6EB5F.1070203@c-s.fr> References: <20150914062159.353701A2413@localhost.localdomain> <1442244054.2909.66.camel@freescale.com> <55F6EB5F.1070203@c-s.fr> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.16.0-fta1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Originating-IP: [2601:448:8100:f9f:12bf:48ff:fe84:c9a0] X-ClientProxiedBy: CY1PR0101CA0011.prod.exchangelabs.com (25.162.170.21) To BY1PR03MB1482.namprd03.prod.outlook.com (25.162.210.140) X-Microsoft-Exchange-Diagnostics: 1;BY1PR03MB1482;2:NzmiCufIyjcgs3nBmiw2d639wIHqnMBl15GcJTOTl9NENXXnzVEZji6MY6CmDo25dmJlBO+DlXce08wAYYj3L/e5yi0Laq55vs57NUy4LBBxFedN89/P6GqAS1GjMw0ATx5jJBm0lkG0bmtCdUceTevpAfAQHV0LNitIFcSaBoQ=;3:VWzCzdkpmkuCVcxs0dfCZOKa7shcgMPPwnZ88tfS7Qbz4BaR0j0lP8BtbrC9nucAoTJ9I9DPzGBRa4whjaRmo0QYFdYov4JJ3qeL/skUtBxtdz5SAvrgencotIHiBBeNUfmBqABCRE5JIlv8IMNqtA==;25:zHMYk2kBGBLqymtKveBBb7uYQ53yy1IN8dNmREzNCSieZWM0tDG44i2kqQEzd24QpMij7/MTrT4Ywj4aK4V/TClpdAWEU0A47m3TTBtyy+2mRx11Qm1U//dbjVUY53IUsc6IWX4GmFx9fwvbWHLJgIdI0x6XsZrPuYwzbFx9ynlXjXlQmyn22hC69yQqxLy99v8b7jgF/iGXx7ei97Xy/8We6+WLzK9sAVQPrgGrcp6Agp9v6wh+qGucdCXliv+VZ9rt2DfDN6eHLmfJy1shgA== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY1PR03MB1482; X-Microsoft-Exchange-Diagnostics: 1;BY1PR03MB1482;20:6VQuziKjMUkNlL47+L7xt7yQKvYvKuf1lbRyR0EKymCLahPpl4um0M2W/ZTZQLkxfzyKqlNWMVB1DDliX/0ms1njvaCSh7DDP34XejJ5Ca+86aqIb3WsIjjq1m5A7cruSRIi/hsN7SQDPvnIQVunCTFW2lEiikOQJKHQ7/pHDo18D2aWcvSzr/RcX718EAZ1+mgifKi2m05+Rdop8XdbbSG7axfAKCp+ZZ1X0sDfXIClRVhc+BKJpfRcb6oX99o0EK0S374JfOsW1IOzU8xrmrin/jY0N5Qd/Au1r4OmO7bq8jdDaC9q14Gmj8hRZiHUhB/rrcbSKMF0WUBX5hewKukEAQ/gnd2p1UZ0UmKKbferTkEDftPbk4QgUQF3J6Gs2VskDjQ7pD4TUvUTC59B7Q7t8UXzqyZlPRXpZN+P9J6sqfHEnYOrE58sS+a9IcWG47r3CmhlPc0D8FFhe1tYaqhSMBtxhBc+Z4D88/2FdlK8/5zZm6SAqSTYi+qBpHUs;4:lw25CtIBlcm9QIlddWbyFE7/Yx5S0OFXIY7AfHtg2JobDTFni2+Df3B78iCKq2/4PbHbuMg7aBoU6TbxDf9Yeg1a1/GF049pAr4a7QS0UL0okG4aESvMCKn2DXmg6QOuAT1Xjiu65p4Skpc/73pE52+CImVcmx/XvBx0c7yTcl6v5avO6pgL+BLXwwQw6BneI0Z7/oSiHKuD9vVhRsHXRGsD5K6J+zJTTJutQ91FKeCmW2vQAyap46NViH/a+D2iPkVpBUGPHoipzKcKkasxZSdkzYBZG3/cPQyVj5pCsXx81jrYlHD93PreRuwd40BZ X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(5005006)(8121501046)(3002001);SRVR:BY1PR03MB1482;BCL:0;PCL:0;RULEID:;SRVR:BY1PR03MB1482; X-Forefront-PRVS: 0699FCD394 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(6009001)(377424004)(199003)(24454002)(189002)(479174004)(101416001)(50226001)(105586002)(50986999)(42186005)(5001960100002)(106356001)(189998001)(33646002)(62966003)(110136002)(23676002)(19580405001)(5001860100001)(5820100001)(92566002)(86362001)(77156002)(2950100001)(122386002)(40100003)(81156007)(4001540100001)(97736004)(77096005)(5001830100001)(87976001)(76176999)(50466002)(68736005)(103116003)(5007970100001)(46102003)(64706001)(47776003)(36756003)(5004730100002)(99106002)(3826002)(5001840100002);DIR:OUT;SFP:1102;SCL:1;SRVR:BY1PR03MB1482;H:[IPv6:2601:448:8100:f9f:12bf:48ff:fe84:c9a0];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCWTFQUjAzTUIxNDgyOzIzOnEzc1M3UktzNnptdllMYi9USHNmTVMvMk43?= =?utf-8?B?eWFoR0RyazM2REl0bEZmYmN3UldtNWwwVFl1SkFiOWF6SXl3cjlRZW5jM1Zs?= =?utf-8?B?Ny96TU9YMXZNZ2p2UUMrMW9QeGgzdTBJNHduSWhuNUU0YnBxKzlQV1Nwdjln?= =?utf-8?B?VHoxMlJSS2JqZ0xwcDJxa09ERGRIUUM5T0FPUVpId2d2cWpYREVNbUNkMDAy?= =?utf-8?B?WnZDQnZpODhFZU0wd2hqNXBSY05ERTRSZGdmUzhLd01yMDlqZVppTFZlZzdz?= =?utf-8?B?OSs0b2d6ajVZeW1tKy9BWWZwSnBYUC9pOHpnZ1ZpWVpHY3VFeVJCNkdoUy9t?= =?utf-8?B?Z3dqS2tnZDFoYnc2UWtscW5ZcjhwT1k2ckM2dGFvV3hzTnNseVQzdUttSmhD?= =?utf-8?B?MXFWWHd1WXdsWUtSbjV0NkJvVE83SlI0bnZPV01RUnAxYncvM2ZUb2hTY2o5?= =?utf-8?B?R3g3WkFBQ3YxYVM1eVlVbjFBM1NBcjl6U1FMNVB1K0JnN2J6eTV0djFEK215?= =?utf-8?B?eGtKaWhyZ1U4U2pTWXR4YTBWSmd3ZERsRkVUT0dzRzJBVlVOeTlOaWpuV3Zt?= =?utf-8?B?bkFxVHRoRlR6QWN3N0NOQ1paMFJxUGF5Vnd0THdDRTNiU1FzTTNBQnA5ellC?= =?utf-8?B?ZW1GZVBWZnd6QU1md3NHMXhoenZ0QVdneHhDRXM2cVcwNUFHT0RoMllsMkZ1?= =?utf-8?B?dXFYMmlBUk1zY0FTQTkxT2VWOVBKcS9rTzhUNFUrZldta1N2OGxuUitkSjQ5?= =?utf-8?B?WmlMMXQ0Uk0zSUNhOG0xSTNLYjRkQS9CdEtDaEpWbG0xRlI3T3FOM1BUUEJH?= =?utf-8?B?eUdFdXZRMzZiOGoveUF2M2twcmJ2dDE3ZnlucmJmNmsxM0JsRnRvOElMMEpo?= =?utf-8?B?a0NIKzdQZkMvd3g4OVA0aEdkUmxGelZXOW50TEdmSUdkQ0FVa0JOZkFvYzVB?= =?utf-8?B?R0tWTUVpaHlKZHBCT2w3ZzZkNUJvUEg1Mmg5M1lUWjZUVlpDdXkwV2dIZzFT?= =?utf-8?B?bFpIQmRtYkV1dUJTTEZNYVRPZjJXbFhvK21IanNSVCsyTW1BUGZuMkpNdHJF?= =?utf-8?B?TllhY1JYbWgvL0gyeDN0c2hsTkFzdS9IUG0wOFRYWTArMXFjb1ZrUW1KaTQv?= =?utf-8?B?RVpzZVE1ZzI4Skd3akFiVkVqQjhiaU8rbnJ4ZWZaY1BLWjlPU2hLSS9YbW5K?= =?utf-8?B?QzBmUC9DeDF4VWsybWhTNDV0alVoN1N4YndBSFppKzVldjBhVUtvejFQL2VP?= =?utf-8?B?U0xqVmI5bVRTWm16NGRZS1cyRVZiUWliN0VNNDdSU3BGL3NDYmNUK2Q0M3Bv?= =?utf-8?B?YVkrdU5pM0VJcHZ5WWVxVzRSUHhmb3NFcHVNcDVReWxsT0xlQ1JoNmFCVVpo?= =?utf-8?B?N1JCR3c0NzdkS1A3Uzl2SzZmYlhHWUpBbGVqcys3SEdjbTE4NllVUVo3K0t4?= =?utf-8?B?UDVQVHdJT3oraHNmRXgxemtEY3paUTNIbE5BR21RRUd0b3llRzFJSkpFeCtB?= =?utf-8?B?K2R2UjRHdm1NTmp0Nis5SDIwa2ZyY2ljSmVYcXNNeU1zYjM2TG5YTFZsUkZj?= =?utf-8?B?SG1pNkpHQkk4anJRRmg2ZjNIWmFPODZIUHBwa2pSTitSZUlUclV2V1VXdWRo?= =?utf-8?B?dGwrSEVNZVMwb283T1hnZ0ZqRnBBNzdjb2JEZndINVN1SHUzWUMwd3pGOTFS?= =?utf-8?Q?wO4Hvq/Asr9dAcKbsA=3D?= X-Microsoft-Exchange-Diagnostics: 1;BY1PR03MB1482;5:2WdB8El7uiDwGN6yIiuIwGMeFQoB51zRwurLUnvXkId7R/hGT9phSRn7kwnY7kYv8FHvyVDaXdA0w3Nq/N5/FR4OiPsh30Bz8JNL9Vimhkm2AQkV3S/xrIC30AeFWu8OJyiOar0ErCK2+wDAUWFe7A==;24:Rh5TJjrG4Utlbf9U/kTGWuPtZNvDj137PZIHeUBXS1Pv3Yqatbgo5yeUy1vWNdk5QsVFTpwUi8FeA7UxWacAqnANcgqcKeFcSY0UZrujXSQ=;20:9rhMyw29CZizAhVK620YS6d9JLPgURcMgKplQFRfDfyYQNneZvZrhlPuofVFXg68lr7nhXcrSpNh/z+7BJsfSA== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: freescale.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Sep 2015 16:13:40.3298 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR03MB1482 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2015-09-14 at 17:44 +0200, Christophe LEROY wrote: > Le 14/09/2015 17:20, Scott Wood a écrit : > > On Mon, 2015-09-14 at 08:21 +0200, Christophe Leroy wrote: > > > memset() uses instruction dcbz to speed up clearing by not wasting time > > > loading cache line with data that will be overwritten. > > > Some platform like mpc52xx do no have cache active at startup and > > > can therefore not use memset(). Allthough no part of the code > > > explicitly uses memset(), GCC may makes calls to it. > > > > > > This patch modifies memset() such that at startup, memset() > > > unconditionally jumps to simple_memset() which doesn't use > > > the dcbz instruction. > > > > > > Once the initial MMU is set up, in machine_init() we patch memset() > > > by replacing this inconditional jump by a NOP > > > > > > Signed-off-by: Christophe Leroy > > > --- > > > This patch goes on to of [v3] powerpc32: memcpy: only use dcbz once > > > cache > > > is enabled > > > > > > Changes in v2: > > > was part of [v2] powerpc32: memcpy/memset: only use dcbz once cache > > > is > > > enabled > > > changes in v3: > > > Not using anymore feature-fixups > > > Handling of memcpy() and memset() split in two patches > > > > > > arch/powerpc/kernel/setup_32.c | 1 + > > > arch/powerpc/lib/copy_32.S | 15 +++++++++++++++ > > > 2 files changed, 16 insertions(+) > > > > > > diff --git a/arch/powerpc/kernel/setup_32.c > > > b/arch/powerpc/kernel/setup_32.c > > > index 362495f..345ec3a 100644 > > > --- a/arch/powerpc/kernel/setup_32.c > > > +++ b/arch/powerpc/kernel/setup_32.c > > > @@ -124,6 +124,7 @@ notrace void __init machine_init(u64 dt_ptr) > > > udbg_early_init(); > > > > > > patch_instruction((unsigned int *)&memcpy, PPC_INST_NOP); > > > + patch_instruction((unsigned int *)&memset, PPC_INST_NOP); > > > > > > /* Do some early initialization based on the flat device tree */ > > > early_init_devtree(__va(dt_ptr)); > > > diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S > > > index da5847d..68a59d4 100644 > > > --- a/arch/powerpc/lib/copy_32.S > > > +++ b/arch/powerpc/lib/copy_32.S > > > @@ -73,8 +73,13 @@ CACHELINE_MASK = (L1_CACHE_BYTES-1) > > > * Use dcbz on the complete cache lines in the destination > > > * to set them to zero. This requires that the destination > > > * area is cacheable. -- paulus > > > + * > > > + * During early init, cache might not be active yet, so dcbz cannot be > > > used. > > > + * We therefore jump to simple_memset which doesn't use dcbz. This > > > jump is > > > + * replaced by a nop once cache is active. This is done in > > > machine_init() > > > */ > > > _GLOBAL(memset) > > > + b simple_memset > > > rlwimi r4,r4,8,16,23 > > > rlwimi r4,r4,16,0,15 > > > > > > @@ -122,6 +127,16 @@ _GLOBAL(memset) > > > bdnz 8b > > > blr > > > > > > +/* Simple version of memset used during early boot until cache is > > > enabled > > > */ > > > +simple_memset: > > > + cmplwi cr0,r5,0 > > > + addi r6,r3,-1 > > > + beqlr > > > + mtctr r5 > > > +1: stbu r4,1(r6) > > > + bdnz 1b > > > + blr > > Instead couldn't you use the generic memset at label 2: and patch the "bne > > 2f"? > > Yes I could but it means adding a global symbol there at the "bne 2f". I > thought it was what Michael didn't like in my v1 of memcpy(). > What name could I give to that symbol ? Something like > memcpy_nocache_patch: ? > What would be the best ? Having b 2f, and replacing it with bne 2f ? Or > have b 2f just above and replace it by nop once cache is up ? I'm not sure why a global symbol for an instruction to be patched would be a big deal (similar to kvm_emul.S)... I thought Michael just wanted existing infrastructure to be used if it's practical to do so. -Scott