From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR03-DB5-obe.outbound.protection.outlook.com (mail-db5eur03on062c.outbound.protection.outlook.com [IPv6:2a01:111:f400:fe0a::62c]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A37DF81E9C for ; Fri, 20 Jan 2017 03:15:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=3nAmDh4PxsqPJc695RVUdnqGjO/ivIdadu9ZIKJcdFI=; b=owcnnUq38RbpKO5NDgnchn/FSEKxoD5GDDJBuPbGzWdknJhEl6qJCyb/VLsUnpjVF8svJIQHbcpptHP9QMKwPzPZQXMuxJNIxHUQ9+sL3lPLnqXMmjYK5ytOpvkP5JAYlX3agWtizgXskQPpuogb8oNhuYmK9KAnEyYn6NwY2Ww= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Achin.Gupta@arm.com; Received: from e104320-lin (217.140.96.140) by VI1PR08MB1197.eurprd08.prod.outlook.com (10.166.45.150) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.860.13; Fri, 20 Jan 2017 11:14:58 +0000 Date: Fri, 20 Jan 2017 11:15:15 +0000 From: Achin Gupta To: Ard Biesheuvel CC: Leif Lindholm , "edk2-devel@lists.01.org" , , Supreeth Venkatesh Message-ID: <20170120111514.GF4132@e104320-lin> References: <1484771046-21296-1-git-send-email-achin.gupta@arm.com> <20170118220500.GW25883@bivouac.eciton.net> <20170119123135.GB24076@e102648.cambridge.arm.com> <20170119215731.GE4132@e104320-lin> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Originating-IP: [217.140.96.140] X-ClientProxiedBy: HE1PR0902CA0016.eurprd09.prod.outlook.com (10.171.90.26) To VI1PR08MB1197.eurprd08.prod.outlook.com (10.166.45.150) X-MS-Office365-Filtering-Correlation-Id: de48e42c-7cf4-40d2-4709-08d441259232 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:VI1PR08MB1197; X-Microsoft-Exchange-Diagnostics: 1; VI1PR08MB1197; 3:XyQllmAhQ/DPpbSJIoR5P7DiQXiE05QTaZZescwiqlzdJds2h6phFPwq0ec7HTxR+LnDsaWZvvekYzYwdNALfgiYIZ1G3jrguJ5wrFBdAa9tCFjfAC3iZnTI32vjDS7chIug7rDaEqRZHdbp5a8vlZI4KAvhBvING2/0NWVfn0WSDoexKOMk4RTo+EQT3ZfC/J3eoOZssDxS/Wzzsyw0s3mGB48CWQaCpCYbp3NpgP9QXjrgbAFeAa2UHac7cxMvqHcQYY79REZA/HhxNhIelg==; 25:nZHEvIBRSGQnyP7ttdnFCZHoV0DW/i3NWgGqeyeFu3i8qR7T54l9FPlTYP5laeg0bVKiR4NYNBaJrO0iu4idVJ0okHYVXRBMJyx6ITvLStx4wprfGg/Pp8jJqF7xxEBkjL9Y9CiFJwOXMIceCLaxBbMxbXWwBWlNmWW9H24dLKCRjtpSi2NhtdS935h4hXBr4RE8N0SBvr2PtddBnKEilR0Ikm0kzsZ2kRNC0kn2qanqw+SmysROw1G2DTqxsztfLTuLjFNtZ19A3UVmWN9MFokY+bQAB0l6w7YaUlegRV0xkbvPFjnVoj+fpCWkNFK9F4ejaFSYcwK/SDrmpWmRyAHIYyWtuwUgrSIILLsUS6inqbVVe8kQiT0kqTKYR4VGh1JyP/BJAn6dCGU26Yh+F1gm0GGiG/dNjh5XmHOc5rf/ULwc8FkW9WlhKvke2mgjs3lLK2GddagTI3DNMff6+w== X-LD-Processed: f34e5979-57d9-4aaa-ad4d-b122a662184d,ExtAddr X-Microsoft-Exchange-Diagnostics: 1; VI1PR08MB1197; 31:OE7gTVN4ZkDip3cuq+5Mun6zNvvQslfdhqF5L3N80/1r+u427Xs4qCERes4WLjNKJoFb7QT5QRYA7b2nZ4CABHL/rFjzbyYDjoSIiMUfjKpowc/1xtWlt7LFHVA7mASDZ+4Xu4s6oGgJmlVW+7izyxLDZiTZ4ZsWnwHXazfgho+cxXYI7fPab29GvvTir8HjTT+3zn1lmG4+FlalDnY6AykfV7oUDhlEJNcAS6teLXlfNlOBKg2jvAU3R466cd4P; 20:4Yzrm39PxhsvAm0HUvcrb+RzaTxgkqI+8tJ/57B5P1V9F6iPogVdjPfgRw2Uep32wd8biVnbkmWunAnehIOYG46THo3tJ/pV0waYxfT2MSLVfM/Qhcs5TSIW9hMzoLgdquhvdvpfS1rQR5AlyusqVY88MLyohYLpArVczgZ7db4= NoDisclaimer: True X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(180628864354917)(166708455590820); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(6040375)(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001)(6055026)(6041248)(20161123564025)(20161123562025)(20161123555025)(20161123558021)(20161123560025)(6072148); SRVR:VI1PR08MB1197; BCL:0; PCL:0; RULEID:; SRVR:VI1PR08MB1197; X-Microsoft-Exchange-Diagnostics: 1; VI1PR08MB1197; 4:WVIkCE6YeY5SB7Wg5pcX52yvAGg++HQx2U+HXD14MwMJWOl4kgGM74AcFk3+fXzCHxaTV93X5IwKpaPhoOSPqh7IJ9zNlmu/kFYqQewuMXAUjXDoO/mKBbnMrqlCSYhAcIGH7Gue3NyBigAmKm5iiFjGvX9PXpzp2vdLXmYSS2GT5m4ayKjTsg9KtRnIiZ77wjn2JyH4siq7sC+/7IdMYru2eKzxBtAFiQz6yI+vhIPFXQjM1aNEvEEAdF2aXkv0J0EExNnb3tJYk0wrehyx5duwV8f0691CGEriVeEE5whhqIR5Smmr1S2K6GoKtu9Au3bGmhGU+HeAyv7/Y2Wvvpn95p+2AQLCoJ1rGmZtYF6qiTEXKdrywH7t23UxCsmafCeFk/AHnAzJYulgz+GM+wBHRcr8TC/dvevEsAq1smaw5bYEQQtR7DepNOlNEMMK9LTOBHZgxHlPytE05XbpIXQrl51asFTPvEWZncf/NsabYKfNGDDXsLfwfg8465ROy+4jWOQJ/eNuYpLPk06tBKob5IZnURonwQO5PPT6Qin7xc2fclL3bQmUVtb0ytXaOPs0BJx5DtbEeYsfqHkZ5lFMikC+3za2b1d5Xx3J7i+rtKwIoljrgehzT5ALsA+LRW5yBxoDmBwvtxoGyrlObVHET5i8ry2WwHVbI/KoWyZ4QJMgBNCKUwvOZSPIoS4wbCuLI4RjSYjbPYNxhU3vPw== X-Forefront-PRVS: 01930B2BA8 X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(4630300001)(6009001)(7916002)(39410400002)(39850400002)(39450400003)(39840400002)(39860400002)(199003)(51914003)(189002)(24454002)(6496003)(81156014)(7736002)(101416001)(76176999)(54356999)(86362001)(50986999)(305945005)(105586002)(106356001)(4326007)(50466002)(53936002)(5660300001)(93886004)(33716001)(575784001)(110136003)(33656002)(42186005)(2950100002)(6916009)(46406003)(92566002)(97756001)(4001350100001)(47776003)(6666003)(66066001)(9686003)(3846002)(189998001)(2906002)(54906002)(83506001)(38730400001)(97736004)(55016002)(68736007)(8676002)(25786008)(81166006)(1076002)(6116002)(6306002)(229853002)(23726003)(18370500001)(107986001)(19627235001); DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR08MB1197; H:e104320-lin; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; Received-SPF: None (protection.outlook.com: arm.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; VI1PR08MB1197; 23:IrjBT0JCfzfv/0f6sH4TtGM6lJZZKdo6qoXIJuz/T?= =?us-ascii?Q?dj6cZXajeD/V6YsT6/eH115amoUwQSfQMw0SYVHM1Djg2Zb7fwDd9j/jkeZw?= =?us-ascii?Q?JsC2Mb92GWiXH1LyXT3W8H37EOnfSbJuxfftughnmQI+nac1YfAkxfPKtsf/?= =?us-ascii?Q?cqSr+qPX3hfjDNSTA2/SLQhoTiag8GvQwy/zAC75gfrPkVqdDEvggKEGFHPH?= =?us-ascii?Q?7mqaBgkt0Wz9l2JzYOHQeXipfMsAnGcMuV3VwcCFAUYXvWJ2KYovmDMRM3qf?= =?us-ascii?Q?jT5KHTXsy7RWpNk1le2/Itds7DFmiX+i5PUpjgPrGjAKZ9YKaI0YO6NE3Nwg?= =?us-ascii?Q?Mumi7by/SBY9x5tGuGmF/dhcVZFAYElhIdWafCkiWDdO6N/6nXSYxVq/RYjG?= =?us-ascii?Q?g2STYGh25L8e0laLftPixGSDKj2Of5QZwDOtRIFFImZUho4Pbxz4aijcxX4w?= =?us-ascii?Q?qcUIw6hcDNLf3EPuruETaMWKzHmk4u/xfY6fLeIJjja76ZRm1asYo74yNF/D?= =?us-ascii?Q?YSlpTs3QK8OG080Ay3uk6f6eeyuABVNOpnYbq2CGZC35PIF1qQ1dUzLkoJW/?= =?us-ascii?Q?Gqu3/0EhDUpnO1jGcALSjabteQ89YDeiTDUhjQfgtzbTXzlakqjNp1lVg3oB?= =?us-ascii?Q?uFAdWKWxh2eD0mucBp1ryGRrrpX8cFWMlireISOqupJNecSS4q8pX8s+2lFv?= =?us-ascii?Q?ThR8hUjzmxL6i0YLyuXZdyDWFp9A08k2kPsLJbY7YqHeCol5SEqMUKvPIsGF?= =?us-ascii?Q?S4LXU4yPs5lOYdu+t17bWJxVNQtCAe3PQyNODmLRjNwPPoVw04K5MI+QnaHp?= =?us-ascii?Q?tcC38+d+PgBQbuDVZvB+FeEvAN1hIRvdDeSaAjkDBGHUiJq/BmO/4KD2bXuo?= =?us-ascii?Q?2nc6pZX5N2ObgtL+mGl9xO5HGtgqbVDW1BiG+TmXyJ0WLZ1OqsnUcRduD3ym?= =?us-ascii?Q?/+xw0qPV/lzjgmDUmevpDkGtdY9pt6j0ORVWxDqS9/JW0wd3m0GGG1yB8R2G?= =?us-ascii?Q?GbhcdUOQG90sGbzOOFcThLu6JT3WR+pZOd603XL9ie+kc/FxjdgZfyu3NiW6?= =?us-ascii?Q?Ol2J/2d6n2XJTVilRJyRQpdDJdTgpxJGgP8Iy46DbqTtl5zT1TzRuzzoR5om?= =?us-ascii?Q?jcwUWsT5gwW+Go7gL+LVNRfGQnu5deqF9nXZhDrweuC0/F55oyh2dDRWMmdl?= =?us-ascii?Q?UUQeAcc3AyYiOt5VGyOP+q12JiwiqHWkvRGZZ3rrB7kcU9xGgoYtNPDKdcaV?= =?us-ascii?Q?AWAStGsYvaI+3C5COXakm2D82/N4CYOWCWEAvFUSNzwTcrl5lmeHtJiEpxLP?= =?us-ascii?Q?pB9HCfsIz1TMExA6/Cl2VpCIF3yrVRVXxAY5Cl5NWjUyahAPk6p//+LRMD1u?= =?us-ascii?Q?2LBFoCYW8c9bm9UybbJtckMvypwc3iqLSJh1wLCUN4MwKVLDCwFabjEvKGuQ?= =?us-ascii?Q?/FsUALinTLCaWtg9Fm24XMPJM//2HNHRpkNQYmP0XLaq1OM/N7kizg2rlCVW?= =?us-ascii?Q?mPw/CPBDDgtM+Zb9YkE1O7Ao43MqKn1dSA=3D?= X-Microsoft-Exchange-Diagnostics: 1; VI1PR08MB1197; 6:bEf33ODlsxVcbC1QHHTArFn5r/5x3Yf8QVueHFw0TbcuiFnQMLCbCfGdM1U6Mc1ym02Ponul+xIi+PFZCegcps429fFPwq7Z8cPEWNYWT2R/tIf1yrJXn+AsgVpGvBSlw9wtl6Frwa60/ira5/FJyKsCu8PtLhIScxwovMr6eGynDkUFSaW3gG7WtdzhO3s2KNnsBuv2vgsVkeyIsSgaCDXENLKkT+SP4gB4axZkvXwrCmGJc01s+kpppyWfGnwolNzsO642TWlYDyv42Q8E89uKuSCpq+fZaKexbmnvMwSagjH/htJSadhiMl94jKWPD0ZdG/MP6HHpvlXX1ygHPjZKMZOvxfOKrxbXGFvt0pXaDCRmJR9Kxv5FMz0QwdeO/Pqq4SG4yePyekKBn286faESWlfPHyQF5r7mMQ4SSHGnDyGOsUlOZx22LtpxulKw7/87XRf3tqJeExaMqv1IMA==; 5:KHtz3p9a1Nkni7PhN9DbxqyROFeT61D6bu/gU803rzPDd1w7IWeglAr3iYUeBMxzC27XWml+IHFiAtZ5WJDbIUhPleiCuxzg6r34VhcmGbCU3wu3M18DaqyWNtx7dVhdnBXDAmvnyoi9awBxQALB/Q==; 24:Q3wagRHfukImOSQj9LYdi/36SFH8E5mOCVBg+ONKtw1SnsIma3+A5ateA0+jJDPwUfP8Do2VeU8D86dMZV2JDOediX73r5hqZyu55lpDk6c= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1; VI1PR08MB1197; 7:wlEi5D8MaDn3Mldw6WLHMizg0/jW/ZHtaFQ6T5Uq9MaMx8SIiHEN9z0tadseLyphcAKKvEuNx87uDThPyODMqzN/pAtAghZ/g31tHi5QRivzDDos7xEDbnEwgu5ivAtES33jvFBSeHdCxfl5k46h6FusciqUeNvOjRdPQEC8/6TxwnLesjkQIO4BhuKmcsbvLireao2XXXo8ZU+QC7xC1ovd81Crt0Bxyotrevm+9NdCpWpHiCkf47xEFt35FvM+YtqriLFVuDlounSb7bjBgwhd12BeUrCeJn+ObnHJdUJvJgJnYuQCae9J2pivym41VnHVmvViS7aZsHKJBa92NUYZoQNBRMfFTRu8sIOGE19eP0RUwlyNrWnB//+2BoMpt4i+5xnii25tcWcG6a9aY0e3ExOYg1LVsOoRgtze6xTHV6uSZCBP3fTTVp7/r00+2w+hljHDLATSLTKAXyYkWA== X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Jan 2017 11:14:58.1941 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB1197 Subject: Re: [PATCH] ArmPlatformPkg/ArmVExpressPkg: Fix memory attributes for NOR Flash X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Jan 2017 11:15:04 -0000 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline Hi Ard, On Thu, Jan 19, 2017 at 10:01:07PM +0000, Ard Biesheuvel wrote: > On 19 January 2017 at 21:57, Achin Gupta wrote: > > Hi Ard, > > > > On Thu, Jan 19, 2017 at 06:16:00PM +0000, Ard Biesheuvel wrote: > >> On 19 January 2017 at 12:31, Achin Gupta wrote: > >> > Hi Leif/Ard, > >> > > >> > On Wed, Jan 18, 2017 at 10:05:00PM +0000, Leif Lindholm wrote: > >> >> Hi Achin, > >> >> > >> >> On Wed, Jan 18, 2017 at 08:24:06PM +0000, achin.gupta@arm.com wrote: > >> >> > From: Achin Gupta > >> >> > > >> >> > The NOR flash banks were being mapped in the translation tables with the same > >> >> > memory attributes as RAM in the system. These attributes mark the region as > >> >> > Normal Memory and could additionally be cacheable or non-cacheable. > >> >> > > >> >> > Either type of attributes are unsuitable for NOR Flash since write operations > >> >> > could be performed on it. Normal Memory does not guarantee ordering of > >> >> > transactions that Device memory does. So the commands sent to the Flash device > >> >> > may not arrive in the right order unless barriers are used. Commands might not > >> >> > get past the CPU caches in case the region has been mapped with cacheable > >> >> > attributes. > >> >> > > >> >> > This patch fixes the problem by mapping the NOR Flash memory region with Device > >> >> > memory attributes. > >> >> > >> >> To add some background to Ard's comment - this was not unintentionally > >> >> done: > >> >> https://github.com/tianocore/edk2/commit/19bb46c411279dcd30d540c56e5993c5f771c319 > >> > > >> > Thanks! I missed this commit. There is some background to the problem I am > >> > facing below. > >> > > >> >> > >> >> Was the reasoning behind this commit incorrect - do you have a > >> >> (pre-DXE?) use-case that creates a problem? > >> > > >> > AFAIU, The current memory attributes for NOR Flash work only for reads. They > >> > should additionally be RO to flag any unexpected writes. > >> > > >> > Mine is a DXE use case! In NorFlashDxe.c, commands are send to the Flash (erase > >> > block etc.). They might never reach the device if there is a write-back cache in > >> > between. So either device or Normal memory with non-cacheable/write-through > >> > attributes and barriers should work. > >> > > >> > If I turn on cache state modelling in the Base FVP, the code gets stuck in > >> > NorFlashReadStatusRegister() in the below loop in NorFlashEraseSingleBlock(): > >> > > >> > // Wait until the status register gives us the all clear > >> > do { > >> > StatusRegister = NorFlashReadStatusRegister (Instance, BlockAddress); > >> > } while ((StatusRegister & P30_SR_BIT_WRITE) != P30_SR_BIT_WRITE); > >> > > >> > I think the SEND_NOR_COMMANDs at the beginning of the function get stuck in the > >> > cache. Changing the flash memory attributes as per this patch solves the > >> > problem. > >> > > >> > The original patch from Ard mentions that the NOR Flash DXE driver should change > >> > the attributes of the region it wants to write to. Is this what is missing? > >> > > >> > >> NorFlashFvbInitialize() in > >> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c contains the > >> following calls: > >> > >> Status = gDS->AddMemorySpace ( > >> EfiGcdMemoryTypeMemoryMappedIo, > >> Instance->DeviceBaseAddress, RuntimeMmioRegionSize, > >> EFI_MEMORY_UC | EFI_MEMORY_RUNTIME > >> ); > >> ASSERT_EFI_ERROR (Status); > >> > >> Status = gDS->SetMemorySpaceAttributes ( > >> Instance->DeviceBaseAddress, RuntimeMmioRegionSize, > >> EFI_MEMORY_UC | EFI_MEMORY_RUNTIME); > >> ASSERT_EFI_ERROR (Status); > >> > >> which is supposed to set device attributes on the NOR region that is > >> actually exposed to the upper layers as read-write capable. > >> > >> Perhaps you can enable GCD debugging to trace these calls, to ensure > >> that the address you are stalled on is actually covered? > > > > Thanks for the pointer. Somehow NorFlashFvbInitialize() gets called and a valid > > firmware volume header is not found. This irrespective of the state of cache > > modelling. The function proceeds to install a header for which it first tries to > > erase the Flash blocks reserved for variable storage. > > > > I am not sure why a FV header is not found. However the flash erase in response > > happens with the pre-DXE memory attributes for the flash region. The GCD > > services to change the attributes are called later. So this seems like a logical > > error in the code. Does this make sense to you? Here is the trace for > > reference. The last print was added by me. > > > >>>>> > > add-symbol-file /home/achgup01/work/genfw/uefi/edk2/Build/ArmVExpress-FVP-AArch64/DEBUG_GCC49/AARCH64/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe/DEBUG/ArmVeNorFlashDxe.dll 0xEAEA0000 > > Loading driver at 0x000EAE90000 EntryPoint=0x000EAEA0044 ArmVeNorFlashDxe.efi > > NorFlashWriteBlocks: informational - Had to enable HSYS_FLASH flag. > > FvbRead(Parameters: Lba=0, Offset=0x0, *NumBytes=0x40, Buffer @ 0xF3FFF8A8) > > NorFlashFvbInitialize > > ValidateFvHeader: No Firmware Volume header present > > NorFlashFvbInitialize: The FVB Header is not valid. > > NorFlashFvbInitialize: Installing a correct one for this volume. > > FvbEraseBlocks() > > FvbEraseBlocks: Check if: ( StartingLba=0 + NumOfLba=3 - 1 ) > LastBlock=2. > > FvbEraseBlocks: Erasing Lba=0 @ 0x0FFC0000. > > NorFlashEraseSingleBlock: BlockAddress=0x0FFC0000 > >>>>> > > > > Any pointers on the absent FV header and how NorFlashFvbInitialize() gets called > > would be helpful. > > > > Right, there is a 'feature' in the code to reinit the FV if it is > corrupted, which is useful for emulators given that their NOR flash > images are usually not backed by anything. For QEMU, we've added an > FDF definition similar to what OVMF uses which is simply a binary dump > of a pristine FV header. > > So yes, this is a logic error in the code: the reinit should not occur > before the remapping of the region. OK! I moved the remapping code to the beginning of the function i.e. first remap the flash before poking it and this seems to have solved the problem. I will send a separate patch for that. Further issues can be discussed in that thread. cheers, Achin