From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::143; helo=mail-it1-x143.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@ml01.01.org Received: from mail-it1-x143.google.com (mail-it1-x143.google.com [IPv6:2607:f8b0:4864:20::143]) (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 7043B211A6D56 for ; Mon, 28 Jan 2019 02:28:05 -0800 (PST) Received: by mail-it1-x143.google.com with SMTP id b5so18592182iti.2 for ; Mon, 28 Jan 2019 02:28:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=MbaPcaHEquOyjMiqwi1My5wxHZVTWgfo+l61B0CK1hQ=; b=f4O8y6OLZtgHbn/U8ERnhc3RunziaVdkUN6jbWipy8ApaIQdMD+fHvIvLBYv1HnzGF UZJvizaKPIm+4v/Q784E7u/wyI8+hKWZ4RkSbcv/f+u8q8YMpCqjyt5nqKu8kOK0G19d UKCKT8aefKn8uatq9Ur7jLMkX/N8mT4kSJwCI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=MbaPcaHEquOyjMiqwi1My5wxHZVTWgfo+l61B0CK1hQ=; b=M+6SBltpeCOhVpAxNGBZwCeP/WwrSo5cEiTUyUH90lrVM6INUuPwUIP0wuCVVmBv0b 70CUHvItT7pTnvTg2wgNwjp5eHKxTvtod1XSdMIfP0XmzjC+K0WPOkDkmIAgc8qx57JF kv9oousSBeW+547Wk6OJe+hcHpdSnUUc75FQse/TYR2trcujaXTqo41g6sxVNstYG6dX HyTbSc/vJWvJJz+HpY71hzjJm4qjOJnUk8e3+Hm3lknd7iY9o7In7EjizbnqedRYOy9e lYpiN0ns1OZBh+8AEiMqtrlCfAnFTzkh2AaNwmqWgaoPnnj/7gh5yzBp9drfNSErA2bA WPLw== X-Gm-Message-State: AJcUukfGevYwcrNJSD7QwEGhEqI+9iK2ABb+xDQE442ah6FO378t+gNn 2WoDUjsWwO8/eCfyJOqJ3SEyMyTmcVadZqgFB3MNHg== X-Google-Smtp-Source: ALg8bN4ZI5Wc4Z7ZCnoU8re5RuvNXOtOPki64lHv+XdRpLSf4+5iovzFSL2PPY0XGQQ1eG0zdbSMN+5Lpq99YV7dO7s= X-Received: by 2002:a05:660c:4b:: with SMTP id p11mr10186273itk.71.1548671284517; Mon, 28 Jan 2019 02:28:04 -0800 (PST) MIME-Version: 1.0 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> <20190128102311.ailneth53n63tsbh@blommer> In-Reply-To: <20190128102311.ailneth53n63tsbh@blommer> From: Ard Biesheuvel Date: Mon, 28 Jan 2019 11:27:53 +0100 Message-ID: To: Mark Rutland Cc: Laszlo Ersek , Marc Zyngier , "edk2-devel@lists.01.org" , Christoffer Dall , Tanxiaojun 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: Mon, 28 Jan 2019 10:28:05 -0000 X-List-Received-Date: Mon, 28 Jan 2019 10:28:05 -0000 X-List-Received-Date: Mon, 28 Jan 2019 10:28:05 -0000 X-List-Received-Date: Mon, 28 Jan 2019 10:28:05 -0000 X-List-Received-Date: Mon, 28 Jan 2019 10:28:05 -0000 X-List-Received-Date: Mon, 28 Jan 2019 10:28:05 -0000 Content-Type: text/plain; charset="UTF-8" On Mon, 28 Jan 2019 at 11:23, Mark Rutland wrote: > > On Wed, Jan 23, 2019 at 03:02:14PM +0100, 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 > > Assuming that hardware is working correctly, this change shouldn't be > necessary. > > KVM sets HCR_EL2.FB, so all TLBI ops will behave as their *IS variant. > Likewise it sets HCR_EL2.BSU, so barriers apply to the inner shareable domain too. > > On bare-metal, NSH should be sufficient. > Ah wonderful, thanks for clarifying.