From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c0c::244; helo=mail-wr0-x244.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x244.google.com (mail-wr0-x244.google.com [IPv6:2a00:1450:400c:c0c::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C41F222361E5D for ; Thu, 8 Feb 2018 09:39:56 -0800 (PST) Received: by mail-wr0-x244.google.com with SMTP id 111so2737329wrb.13 for ; Thu, 08 Feb 2018 09:45:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Sq/LZxNyncFXxn/JZSQJWlbiUQRfNRQ68/BS9Nir7ps=; b=L/SyIthpE+S2sbyFPrt/3ym/7ei3gcJu665FzICiUBaWXT69bG9dtDP8aDOutPXO3L RntvXZXgSpvNIKAa77Dw6oipkywAWWZawSw7pVKlngdukPsvpJ7cXQ/x1lOf6uFBBJtL hWpwKsWKoCx9863QXL8I4mV6Pcxav48YWBVXQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Sq/LZxNyncFXxn/JZSQJWlbiUQRfNRQ68/BS9Nir7ps=; b=DoQH2rd/acrsvSIgMhsqUbCNtNHvmZ20S0cR4xitho4sXSX6GBos4lDvX6Tl9fbVum QAoJCTlPl1HzygLAzNVtX0Tw8mCiKCc7/lPMn8/zS8dQzH122ZAP39E9R5LHeYLewtNT 0MEPdSUwxicCd4GiQOZ5FdSIR1hvlEGIA+Gf3oCprwrqFAJvTJiBi+U7DzSELK9tHj0y fx7XLHy5LDHyH0FqZdGYTPjWNVONE1S1BiLqYWUdRm4d7pWNU0NMK99bn/Nf/34IyH1h W7iuSAFqeP/pUWWZJTzUAQX+CXDpOu1QQpuX2DY5jOK3lGtqCI9igH9IxUgDGL/OJ0wi KuQQ== X-Gm-Message-State: APf1xPDlJkQHMlyrLVliFk43kewxOM3hZWHpQV4HLoKD9vS/Ciu49eAw 6PopcMeluruHlHwpzU9p5YpP8LR5qlE= X-Google-Smtp-Source: AH8x226VeXu8A14NQrPANqfIx3dwtoPox1omCT1s3IDApIMnQidCXHl1yDpFabzKpNfIvc1u7cGAFA== X-Received: by 10.223.209.6 with SMTP id a6mr1577694wri.169.1518111939931; Thu, 08 Feb 2018 09:45:39 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id 14sm559997wry.95.2018.02.08.09.45.38 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 08 Feb 2018 09:45:38 -0800 (PST) Date: Thu, 8 Feb 2018 17:45:36 +0000 From: Leif Lindholm To: Alexei Fedorov Cc: "edk2-devel@lists.01.org" , "ard.biesheuvel@linaro.org" , Evan Lloyd , Sami Mujawar , Girish Pathak , Mitch Ishihara , Matteo Carlini Message-ID: <20180208174536.fz3teftgsvk7m35h@bivouac.eciton.net> References: <20180206171225.10676-1-leif.lindholm@linaro.org> MIME-Version: 1.0 In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH edk2-platforms 1/3] Platform/ARM: drop unused EmbeddedPkg Pcds X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Feb 2018 17:39:57 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Feb 08, 2018 at 03:08:19PM +0000, Alexei Fedorov wrote: > This patch causes Juno and other ARM platforms to raise an assert during MMU initialisation in ArmConfigureMmu() Huh... > (edk2\ArmPkg\Library\ArmMmuLib\AArch64\ArmMmuLibCore.c): > > > ASSERT (TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK || > TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK); > > > see debug output: > > UEFI firmware (version built at 12:01:21 on Feb 8 2018) > add-symbol-file n:\edk2\Build\ArmJuno\DEBUG_GCC5\AARCH64\ArmPlatformPkg\PrePi\PeiUniCore\DEBUG\ArmPlatformPrePiUniCore.dll 0xE0000800 > ASSERT [ArmPlatformPrePiUniCore] n:\edk2\ArmPkg\Library\ArmMmuLib\AArch64\ArmMmuLibCore.c(744): TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK || TranslationTableAttribute == ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK > > because removal of > - gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE > from ArmVExpress.dsc.inc now makes ArmPlatformGetVirtualMemoryMap () function in > \edk2-platforms\Platform\ARM\JunoPkg\Library\ArmJunoLib\ArmJunoMem.c > > use default FALSE value defined in EmbeddedPkg.dec: > > > [PcdsFeatureFlag.common] > gEmbeddedTokenSpaceGuid.PcdCacheEnable|FALSE|BOOLEAN|0x00000042 > > > & set CacheAttributes to DDR_ATTRIBUTES_UNCACHED: > > if (FeaturePcdGet(PcdCacheEnable) == TRUE) { > CacheAttributes = DDR_ATTRIBUTES_CACHED; > } else { > CacheAttributes = DDR_ATTRIBUTES_UNCACHED; > } So. I spent a while scratching my head here over how i didn't spot this when I did my final test build before generating the patches. Turns out, somewhere amongst my rebasing, gEmbeddedTokenSpaceGuid.PcdCacheEnable managed to reintroduce itself into EmbeddedPkg.dec (I explicitly deleted it). Since I _knew_ I'd deleted it, and all the platforms still built, I didn't bother grepping for it through all of edk2-platforms. Ho hum. Well, the fix is trivial - just excise it from all platforms, and turn all conditional statements into the unconditional execution of the TRUE path. There is no code in public trees that actually does anything different initialisation-wise depending on the value of this Pcd. Scream if you disagree. > Thank you for your cooperation. Thanks for reporting! / Leif > Alexei. > > ________________________________ > From: edk2-devel on behalf of Leif Lindholm > Sent: 06 February 2018 17:12 > To: edk2-devel@lists.01.org > Cc: ard.biesheuvel@linaro.org > Subject: [edk2] [PATCH edk2-platforms 1/3] Platform/ARM: drop unused EmbeddedPkg Pcds > > A set of mostly Ebl-related Pcds are about to be deleted from > edk2. Delete references to them here to keep platforms building. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Leif Lindholm > --- > Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 18 ------------------ > 1 file changed, 18 deletions(-) > > diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc > index 717f7332c9..b026ce3a42 100644 > --- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc > +++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc > @@ -257,20 +257,6 @@ [PcdsFeatureFlag.common] > gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable|TRUE > gEfiMdePkgTokenSpaceGuid.PcdDriverDiagnostics2Disable|TRUE > > - # > - # Control what commands are supported from the UI > - # Turn these on and off to add features or save size > - # > - gEmbeddedTokenSpaceGuid.PcdEmbeddedMacBoot|TRUE > - gEmbeddedTokenSpaceGuid.PcdEmbeddedDirCmd|TRUE > - gEmbeddedTokenSpaceGuid.PcdEmbeddedHobCmd|TRUE > - gEmbeddedTokenSpaceGuid.PcdEmbeddedHwDebugCmd|TRUE > - gEmbeddedTokenSpaceGuid.PcdEmbeddedPciDebugCmd|TRUE > - gEmbeddedTokenSpaceGuid.PcdEmbeddedIoEnable|FALSE > - gEmbeddedTokenSpaceGuid.PcdEmbeddedScriptCmd|FALSE > - > - gEmbeddedTokenSpaceGuid.PcdCacheEnable|TRUE > - > # Use the Vector Table location in CpuDxe. We will not copy the Vector Table at PcdCpuVectorBaseAddress > gArmTokenSpaceGuid.PcdRelocateVectorTable|FALSE > > @@ -333,10 +319,6 @@ [PcdsFixedAtBuild.common] > > gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07 > > - gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|"" > - gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07 > - gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000 > - > # > # Optional feature to help prevent EFI memory map fragments > # Turned on and off via: PcdPrePiProduceMemoryTypeInformationHob > -- > 2.11.0 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.