From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::244; helo=mail-it0-x244.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x244.google.com (mail-it0-x244.google.com [IPv6:2607:f8b0:4001:c0b::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 111E12035A7AF for ; Thu, 16 Nov 2017 08:02:06 -0800 (PST) Received: by mail-it0-x244.google.com with SMTP id 72so557265itl.5 for ; Thu, 16 Nov 2017 08:06:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=EGpuh2tmu+U5JD23tEzhzHbMkPo+cBPZL5t5bJDjfvA=; b=jlw7x+6fWj9lETLOH3xxAcfcbieAGnu7Ww6He5hjxmA9Jk36Jw27wGhtVe1b3Z+0OD sr9zIRJjVuOMt6+eIrGECIE50qEYVM8WlOfbdPMI+PobIRSIpaZ4zbCykiObqUTy0UQE 9G4dBFIvMrkxi9Ilz1syTyqIYnzwBfe0iL7tA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=EGpuh2tmu+U5JD23tEzhzHbMkPo+cBPZL5t5bJDjfvA=; b=sKiJ6CRPoSlz46kNEOJBRKf+ixnJ6hLR31Mx0gXjXqMB9Emm//jh169dhQH+7rHBEq TykDI64e6DojmVekfYutEHrSpptCkPhH3gFzMB1uYGlfDhzA5G5S+HC90yBeCSrA5JDb 4BHR3gyKq6oE3pvMyJE81A7g9wzjXGNuHa8CQTMO0e08T87FMh5kkeHkyNYEwE7NPtbj JYj2FA+SPxVWAzKCskFuKeq6a4X2h7g2mWpIiMhiq2VUpaTZTI7KT6vjxaPiZrQWXYfR hLkb2sAUwO4npeH71lJWIxZGrpB9sy52lIT6qB3kbAy/wQ5mByPYd/lkSehE37xiIe5i eAZA== X-Gm-Message-State: AJaThX6Z3LOgwQMzQLkiMxTHZFIvjS3mZTwKCS5FlBGuQqxbdrTiTAjS 2AaHyBz+WNGUnstKKIi4a8uu0/nfUcSxql9e+Vl82A== X-Google-Smtp-Source: AGs4zMYoEWqJDxWntjbScL30UTicDH9OP4TI/wS2esqm55t2d9DQlha32diKFV6H6+EfaD+P2LB4J0BIaAmZYqba1Zs= X-Received: by 10.36.31.212 with SMTP id d203mr2741139itd.48.1510848376077; Thu, 16 Nov 2017 08:06:16 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.3 with HTTP; Thu, 16 Nov 2017 08:06:15 -0800 (PST) In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103B9B6346@shsmsx102.ccr.corp.intel.com> References: <20171116072700.11456-1-jian.j.wang@intel.com> <20171116072700.11456-2-jian.j.wang@intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B9B62E6@shsmsx102.ccr.corp.intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B9B6346@shsmsx102.ccr.corp.intel.com> From: Ard Biesheuvel Date: Thu, 16 Nov 2017 16:06:15 +0000 Message-ID: To: "Zeng, Star" Cc: "edk2-devel@lists.01.org" , Laszlo Ersek , "Yao, Jiewen" Subject: Re: [PATCH v6 1/2] MdeModulePkg/DxeCore: Filter out all paging capabilities X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Nov 2017 16:02:07 -0000 Content-Type: text/plain; charset="UTF-8" On 16 November 2017 at 09:48, Zeng, Star wrote: > As I remember UEFI 2.5 clarified this and added EFI_MEMORY_RO was because EFI_MEMORY_WP had been typically used for cache even before UEFI 2.5. > > And I do not think this patch should filter out EFI_MEMORY_WP since this patch is to filter out new paging bits caused by 14dde9e903bb9a719ebb8f3381da72b19509bc36 [MdeModulePkg/Core: Fix out-of-sync issue in GCD]. > Ah ok. If the bit does not get copied from the GCD map then there is no need to filter it here. > > Thanks, > Star > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Thursday, November 16, 2017 5:30 PM > To: Zeng, Star > Cc: Wang, Jian J ; Laszlo Ersek ; edk2-devel@lists.01.org; Yao, Jiewen > Subject: Re: [edk2] [PATCH v6 1/2] MdeModulePkg/DxeCore: Filter out all paging capabilities > > On 16 November 2017 at 09:28, Zeng, Star wrote: >> Ard, >> >> EFI_MEMORY_WP is for cache. >> >> UefiSpec.h >> // >> // Note: UEFI spec 2.5 and following: use EFI_MEMORY_RO as >> write-protected physical memory // protection attribute. Also, EFI_MEMORY_WP means cacheability attribute. >> // >> #define EFI_MEMORY_WP 0x0000000000001000ULL >> > > Yes, but that was a change in v2.5, before that it was a permission attribute. So it should be filtered from the OS visible memory map as well. > >> Thanks, >> Star >> -----Original Message----- >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >> Sent: Thursday, November 16, 2017 5:25 PM >> To: Wang, Jian J >> Cc: edk2-devel@lists.01.org; Yao, Jiewen ; Zeng, >> Star ; Laszlo Ersek >> Subject: Re: [PATCH v6 1/2] MdeModulePkg/DxeCore: Filter out all >> paging capabilities >> >> On 16 November 2017 at 07:26, Jian J Wang wrote: >>> Some OSs will treat EFI_MEMORY_DESCRIPTOR.Attribute as really set >>> attributes and change memory paging attribute accordingly. >>> But current EFI_MEMORY_DESCRIPTOR.Attribute is assigned by value from >>> Capabilities in GCD memory map. This might cause boot problems. >>> Clearing all paging related capabilities can workaround it. The code >>> added in this patch is supposed to be removed once the usage of >>> EFI_MEMORY_DESCRIPTOR.Attribute is clarified in UEFI spec and adopted >>> by both EDK-II Core and all supported OSs. >>> >>> Cc: Jiewen Yao >>> Cc: Star Zeng >>> Cc: Laszlo Ersek >>> Cc: Ard Biesheuvel >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Jian J Wang >>> --- >>> MdeModulePkg/Core/Dxe/Mem/Page.c | 17 +++++++++++++++++ >>> 1 file changed, 17 insertions(+) >>> >>> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c >>> b/MdeModulePkg/Core/Dxe/Mem/Page.c >>> index c9219cc068..783b576e35 100644 >>> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c >>> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c >>> @@ -1829,6 +1829,23 @@ CoreGetMemoryMap ( >>> // >>> BufferSize = ((UINT8 *)MemoryMap - (UINT8 *)MemoryMapStart); >>> >>> + // >>> + // WORKAROUND: Some OSs will treat EFI_MEMORY_DESCRIPTOR.Attribute as really >>> + // set attributes and change memory paging attribute accordingly. >>> + // But current EFI_MEMORY_DESCRIPTOR.Attribute is assigned by >>> + // value from Capabilities in GCD memory map. This might cause >>> + // boot problems. Clearing all paging related capabilities can >>> + // workaround it. Following code is supposed to be removed once >>> + // the usage of EFI_MEMORY_DESCRIPTOR.Attribute is clarified in >>> + // UEFI spec and adopted by both EDK-II Core and all supported >>> + // OSs. >>> + // >>> + while (MemoryMapStart < MemoryMap) { >>> + MemoryMapStart->Attribute &= ~(UINT64)(EFI_MEMORY_RP | EFI_MEMORY_RO | >>> + EFI_MEMORY_XP); >> >> Why is EFI_MEMORY_WP missing here? >> >>> + MemoryMapStart = NEXT_MEMORY_DESCRIPTOR(MemoryMapStart, Size); >>> + } >>> + >>> Status = EFI_SUCCESS; >>> >>> Done: >>> -- >>> 2.14.1.windows.1 >>> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel