From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@ml01.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B29A0211B696B for ; Wed, 23 Jan 2019 15:04:47 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 189E8C0C8CCA; Wed, 23 Jan 2019 23:04:47 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-117.rdu2.redhat.com [10.10.120.117]) by smtp.corp.redhat.com (Postfix) with ESMTP id 12CDDBA8D; Wed, 23 Jan 2019 23:04:44 +0000 (UTC) To: Ard Biesheuvel Cc: Leif Lindholm , "Cohen, Eugene" , "edk2-devel@lists.01.org" , Tanxiaojun , Marc Zyngier , Christoffer Dall References: <1449471969-16949-1-git-send-email-ard.biesheuvel@linaro.org> <2dd4294c-76f0-f433-cbd2-bf0b37114aee@redhat.com> <12fa0861-e25d-eba7-48ea-2bd7d47d58fb@redhat.com> From: Laszlo Ersek Message-ID: <6053bb93-0219-49fb-28cd-19f8c0bb9cc6@redhat.com> Date: Thu, 24 Jan 2019 00:04:43 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 23 Jan 2019 23:04:47 +0000 (UTC) Subject: Re: [PATCH] ArmPkg: update InvalidateInstructionCacheRange to flush only to PoU X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Jan 2019 23:04:47 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 01/23/19 15:02, Ard Biesheuvel wrote: > On Wed, 23 Jan 2019 at 10:55, Laszlo Ersek wrote: >> >> On 01/23/19 10:26, Ard Biesheuvel wrote: >>> On Wed, 23 Jan 2019 at 10:14, Laszlo Ersek wrote: >>>> On 01/22/19 16:37, Ard Biesheuvel wrote: >> >>>>> Is SetUefiImageMemoryAttributes() being >>>>> called to remap the memory R-X ? >>>> >>>> No, it is not; the grub binary in question doesn't have the required >>>> section alignment (... I hope at least that that's what your question >>>> refers to): >>>> >>>>> ProtectUefiImageCommon - 0x3E6C54C0 >>>>> - 0x000000013BEEF000 - 0x0000000000030600 >>>>> !!!!!!!! ProtectUefiImageCommon - Section Alignment(0x200) is >>>> incorrect !!!!!!!! >>>> >>> >>> This is puzzling, given that the exact same binary works on Mustang. >> >> And even on the original (unspecified) hardware, the same binary works >> frequently. My understanding is that there are five VMs executing reboot >> loops in parallel, on the same host, and 4 out of 5 may hit the issue in >> a reasonable time period (300 reboots or so). >> >>> So when loaded, GRUB should cover the following regions: >>> >>> 0x13beef0000 - 0x13bf000000 (0x11000) >>> 0x13bf000000 - 0x13bf01f600 (0x1f600) >>> >>> where neither covers a 2 MB block fully, which means that the TLB >>> entry that we are hitting is stale. >>> >>> Since ProtectUefiImageCommon() does not do anything in this case, the >>> stale translation must be the result of >>> PcdDxeNxMemoryProtectionPolicy, which either sets the wrong >>> permissions for EfiLoaderCode (relying on ProtectUefiImageCommon), or >>> we don't flush the TLBs correctly after updating the permissions when >>> converting the memory from EfiConventionalMemory to EfiLoaderCode >>> >>> Are you using the default value for PcdDxeNxMemoryProtectionPolicy? >> >> Yes, we have >> >> ArmVirtPkg/ArmVirt.dsc.inc: >> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1 >> >> from commit 1acd7c54a724 ("ArmVirtPkg AARCH64: enable NX memory >> protection for all platforms", 2017-03-01). >> >> The binary is from the RPM >> "edk2-aarch64-20180508gitee3198e672e2-5.el8+1789+f0947240.noarch", which >> is basically upstream ee3198e672e2 plus a small number of backports and >> downstream customizations. >> > > This might help: > > diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S > b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S > index b7173e00b039..4c0b4b4efbd5 100644 > --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S > +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S > @@ -138,7 +138,7 @@ ASM_FUNC(ArmUpdateTranslationTableEntry) > > ASM_FUNC(ArmInvalidateTlb) > EL1_OR_EL2_OR_EL3(x0) > -1: tlbi vmalle1 > +1: tlbi vmalle1is > b 4f > 2: tlbi alle2 > b 4f > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S > b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S > index 90192df24f55..d54b1c19accf 100644 > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibReplaceEntry.S > @@ -34,7 +34,7 @@ > > // flush the TLBs > .if \el == 1 > - tlbi vmalle1 > + tlbi vmalle1is > .else > tlbi alle\el > .endif > "Invalidate all EL1&0 regime stage 1 TLB entries for the current VMID >>>on all PEs in the same Inner Shareable domain<<<". I'll soon try to build a new pkg with this applied, for testing by the reporter. Many thanks! Laszlo