From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-74.mimecast.com (us-smtp-delivery-74.mimecast.com [216.205.24.74]) by mx.groups.io with SMTP id smtpd.web10.16537.1585155332982244014 for ; Wed, 25 Mar 2020 09:55:33 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=WDh9xKHh; spf=pass (domain: redhat.com, ip: 216.205.24.74, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1585155332; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=0VezS1wkb+VIfo0OhbUOIjHO7qCPOoim5dEcIITOcos=; b=WDh9xKHhYI/AiaXsAxvyGysosYU9P2b5h25AVZ5N82PGYSo43UK+dK2ykgQnLoDXTN7mvV +rKYU/kFok/SkD8e86+rubyYF/vFGrj3LPDOBF1M0XjmBrfbIgJcKQ5rfbKEL5R+I02TAN SGOc5cddvO8CHTtQeENU5zZdJlc8Ei8= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-205-U0cOOy3IMRy5acYq2cK5qA-1; Wed, 25 Mar 2020 12:54:56 -0400 X-MC-Unique: U0cOOy3IMRy5acYq2cK5qA-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B56A98010EE; Wed, 25 Mar 2020 16:54:54 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-153.ams2.redhat.com [10.36.113.153]) by smtp.corp.redhat.com (Postfix) with ESMTP id C59EE1001DD8; Wed, 25 Mar 2020 16:54:50 +0000 (UTC) Subject: Re: [edk2-devel] Questions about UEFI MAT / PcdPropertiesTableEnable To: "Ni, Ray" , "devel@edk2.groups.io" , "tigerliu@zhaoxin.com" References: <986b51e441804c3ba288a0af210d0f4f@zhaoxin.com> <4c1161fc-2f1a-3a18-fe7e-9a395a5a532c@redhat.com> <734D49CCEBEEF84792F5B80ED585239D5C4B26AD@SHSMSX104.ccr.corp.intel.com> From: "Laszlo Ersek" Cc: Michael Kinney , "Leif Lindholm (Nuvia address)" , Ard Biesheuvel , Jiewen Yao Message-ID: Date: Wed, 25 Mar 2020 17:54:49 +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: <734D49CCEBEEF84792F5B80ED585239D5C4B26AD@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Groupsio-MsgNum: 56329 Content-Language: en-US Content-Type: multipart/mixed; boundary="------------90AEA0B034D46DF3EAE46850" --------------90AEA0B034D46DF3EAE46850 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 03/25/20 06:17, Ni, Ray wrote: >> >> The properties table should not be used. It has been superseded by the memory attributes table, per spec. >> >> In edk2, the properties table is controlled by the PCD, regardless of the memory attributes table. >> >> In edk2, the memory attributes table is always produced, regardless of the properties table. >> >> Please see the discussion under: >> >> [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >> http://mid.mail-archive.com/1454069539-4056-1-git-send-email-jiewen.yao@intel.com >> > > Laszlo, > I cannot open the URL above. I'm really sorry about that! Unfortunately, the discussion thread linked above goes back to January / February 2016. At that time, we were using for the mailing list address. And *normally*, you would expect the mailing list archive at 01.org to contain the discussion, right? Except, a few months ago, the 01.org admins replaced the Mailman2 list software with Mailman3 / HyperKitty, on 01.org. As a result, the old list archive links into 01.org are all broken. :( However, that's still not the worst. Because the old 01.org archive is still exposed to the world, only through a new User Interface. And the new User Interface (called HyperKitty) is still searchable -- so if you know at least the subject line of a message or thread, you can still find it, here: https://lists.01.org/hyperkitty/list/edk2-devel@lists.01.org/ But then, what *is* the worst, is that the new Mailman3 / HyperKitty archive does not go back far enough in time. In other words, when the 01.org admins replaced Mailman2 with Mailman3 on 01.org, they didn't import *all* of the old archive. If you click the link above and select 2016 in the left sidebar, you can see the new archive only goes back to July 2016. And the subject discussion occurred in Jan/Feb 2016. This is why I *absolutely* insist on keeping our discussions on mailing lists, because mailing lists can be federated. If we hadn't had subscribe to the original 01.org-based edk2-devel mailing list, then now we wouldn't have *any* public record whatsoever, of the thread in question! In other words, I'm just explaining to you why the *only* link I have provided here is for (which you regrettably cannot open, from behind the Great Firewall). The reason is that there *is* no other link. Otherwise, if there are multiple links, I always provide at least two -- one into the official archive (which in this case would be 01.org), and another into a secondary archive (such as mail-archive.com). So... in this reply, let me attach the relevant part of the 2016 discussion for you (and others that cannot read ) -- 18 messages. > Do you think we could remove properties table? Yes, that's exactly what Ard requested, as soon as Jiewen posted the MAT series. Back then, Jiewen said that some production OSes were still using the properties table, and would need time to migrate to MAT. The agreement -- four years ago! -- seemed to be that the UEFI spec should drop the properties table definition in some time, and then edk2 could remove the reference implementation too. See the attached discussion. Given that the properties table had been deprecated in the UEFI spec even in Feb 2016, I think it's now high time to remove it altogether (both spec and edk2). > The existence of both is confusing. Yes, very much. Thanks Laszlo --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta02.collab.prod.int.phx2.redhat.com (LHLO zmta02.collab.prod.int.phx2.redhat.com) (10.5.81.9) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Fri, 29 Jan 2016 07:12:31 -0500 (EST) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by zmta02.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id CBCA0122785; Fri, 29 Jan 2016 07:12:31 -0500 (EST) Received: from mx1.redhat.com (ext-mx03.extmail.prod.ext.phx2.redhat.com [10.5.110.27]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0TCCVUx016532 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 29 Jan 2016 07:12:31 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id A32348E010; Fri, 29 Jan 2016 12:12:30 +0000 (UTC) Received: from ml01.vlan14.01.org (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 23BBC1A2580; Fri, 29 Jan 2016 04:12:29 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by ml01.01.org (Postfix) with ESMTP id A17291A257B for ; Fri, 29 Jan 2016 04:12:28 -0800 (PST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP; 29 Jan 2016 04:12:28 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,363,1449561600"; d="scan'208";a="643459809" Received: from unknown (HELO JYAO1-MOBL1.ccr.corp.intel.com) ([10.254.211.14]) by FMSMGA003.fm.intel.com with ESMTP; 29 Jan 2016 04:12:27 -0800 From: jiewen yao To: edk2-devel@ml01.01.org Date: Fri, 29 Jan 2016 20:12:12 +0800 Message-Id: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> Cc: "Yao, Jiewen" , "Gao, Liming" Subject: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.39 (BAYES_50,DCC_REPUT_00_12,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Scanned-By: MIMEDefang 2.76 on 10.5.110.27 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit This series patches add UEFI2.6 MemoryAttributesTable support. This table is used to retire old PropertiesTable. This is standalone table published by DxeCore, so there is no compatibility issue. The patch is validated with or without properties table. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: "Yao, Jiewen" Cc: "Gao, Liming" jiewen yao (7): MdePkg: Add UEFI2.6 MemoryAttributes Table definition. MdePkg: Add UEFI2.6 MemoryAttributesTable GUID MdeModulePkg: Add MemoryAttributesTable generation. MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header file. MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. MdePkg: Update DxeCore INF for MemoryAttributesTable. MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ MdePkg/MdePkg.dec | 11 +- 7 files changed, 362 insertions(+), 15 deletions(-) create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h -- 1.9.5.msysgit.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta02.collab.prod.int.phx2.redhat.com (LHLO zmta02.collab.prod.int.phx2.redhat.com) (10.5.81.9) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Fri, 29 Jan 2016 12:12:59 -0500 (EST) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by zmta02.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id A5131122796; Fri, 29 Jan 2016 12:12:59 -0500 (EST) Received: from mx1.redhat.com (ext-mx03.extmail.prod.ext.phx2.redhat.com [10.5.110.27]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0THCxNr002350 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 29 Jan 2016 12:12:59 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id 7EE208E010; Fri, 29 Jan 2016 17:12:58 +0000 (UTC) Received: from ml01.vlan14.01.org (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 439881A2495; Fri, 29 Jan 2016 09:12:57 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: 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 41C711A2485 for ; Fri, 29 Jan 2016 09:12:56 -0800 (PST) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id AC3E58E010; Fri, 29 Jan 2016 17:12:55 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-142.phx2.redhat.com [10.3.113.142]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0THCsXb002289; Fri, 29 Jan 2016 12:12:54 -0500 To: jiewen yao References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> From: Laszlo Ersek Message-ID: <56AB9D95.3070402@redhat.com> Date: Fri, 29 Jan 2016 18:12:53 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 Cc: edk2-devel@ml01.01.org, "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.39 (BAYES_50,DCC_REPUT_00_12,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.76 on 10.5.110.27 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Hello Jiewen, On 01/29/16 13:12, jiewen yao wrote: > This series patches add UEFI2.6 MemoryAttributesTable support. > This table is used to retire old PropertiesTable. > This is standalone table published by DxeCore, so there is no > compatibility issue. > > The patch is validated with or without properties table. Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE? I skimmed the commit messages, and it looks like the properties table is preserved, but now it serves only as foundation for the memory attributes table. In OVMF we set PcdPropertiesTableEnable to FALSE by default, but flip it dynamically to TRUE in PlatformPei, if the user requests the feature on the QEMU command line. This is being done for avoiding regressions with OSes that don't know about the properties table, and its impact on the UEFI memmap. However, if the memory attributes table is safe for OSes that don't know about it, I think we can eliminate the above "FALSE" default, and dynamism, from OVMF, and just inherit the PcdPropertiesTableEnable=TRUE setting from "MdeModulePkg.dec". Does it sound reasonable to you? Thanks! Laszlo > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: "Yao, Jiewen" > Cc: "Gao, Liming" > > jiewen yao (7): > MdePkg: Add UEFI2.6 MemoryAttributes Table definition. > MdePkg: Add UEFI2.6 MemoryAttributesTable GUID > MdeModulePkg: Add MemoryAttributesTable generation. > MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. > MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header > file. > MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. > MdePkg: Update DxeCore INF for MemoryAttributesTable. > > MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- > MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- > MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- > MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ > MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- > MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ > MdePkg/MdePkg.dec | 11 +- > 7 files changed, 362 insertions(+), 15 deletions(-) > create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c > create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta01.collab.prod.int.phx2.redhat.com (LHLO zmta01.collab.prod.int.phx2.redhat.com) (10.5.81.8) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Fri, 29 Jan 2016 19:31:55 -0500 (EST) Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by zmta01.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id 3C0B218584E; Fri, 29 Jan 2016 19:31:55 -0500 (EST) Received: from mx1.redhat.com (ext-mx08.extmail.prod.ext.phx2.redhat.com [10.5.110.32]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0U0VtY7013610 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 29 Jan 2016 19:31:55 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id 12C46C0BFBA5; Sat, 30 Jan 2016 00:31:54 +0000 (UTC) Received: from ml01.vlan14.01.org (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id AC6011A2587; Fri, 29 Jan 2016 16:31:52 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by ml01.01.org (Postfix) with ESMTP id 830441A2582 for ; Fri, 29 Jan 2016 16:31:51 -0800 (PST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga103.fm.intel.com with ESMTP; 29 Jan 2016 16:31:51 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,367,1449561600"; d="scan'208";a="38808187" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga004.fm.intel.com with ESMTP; 29 Jan 2016 16:31:51 -0800 Received: from fmsmsx111.amr.corp.intel.com (10.18.116.5) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 29 Jan 2016 16:31:51 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.110.15) by fmsmsx111.amr.corp.intel.com (10.18.116.5) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 29 Jan 2016 16:31:50 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.172]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.117]) with mapi id 14.03.0248.002; Sat, 30 Jan 2016 08:31:43 +0800 From: "Yao, Jiewen" To: Laszlo Ersek Thread-Topic: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. Thread-Index: AQHRWo5UfnwfB+DE8USx6LXBJH6a7Z8SNVqAgAD+OSA= Date: Sat, 30 Jan 2016 00:31:42 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> In-Reply-To: <56AB9D95.3070402@redhat.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNGI0NzMzMzUtODU1MS00MzRjLTlhZDYtY2NjNTI4MTEyOTNhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjQuMTAuMTkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiYkdoMVQ2UGJ4ekxoSU9Ib1JscEVvWGlVS093T3F1MmJrUkJNMndza2l6ST0ifQ== x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Cc: "edk2-devel@ml01.01.org" , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.39 (BAYES_50,DCC_REPUT_00_12,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Scanned-By: MIMEDefang 2.78 on 10.5.110.32 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Comment below: -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com] Sent: Saturday, January 30, 2016 1:13 AM To: Yao, Jiewen Cc: edk2-devel@ml01.01.org; Gao, Liming Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. Hello Jiewen, On 01/29/16 13:12, jiewen yao wrote: > This series patches add UEFI2.6 MemoryAttributesTable support. > This table is used to retire old PropertiesTable. > This is standalone table published by DxeCore, so there is no > compatibility issue. > > The patch is validated with or without properties table. Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE? [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to FALSE or TRUE, MemoryAttributesTable is always published. That is one compatibility we want to maintain. I skimmed the commit messages, and it looks like the properties table is preserved, but now it serves only as foundation for the memory attributes table. [Jiewen] Yes, the DxeCore global variable - properties table is always preserved. And the system UEFI configuration table for properties table is controlled by PcdPropertiestableEnable. No matter properties table or memory attributes table, we always need a flag to record if RT image is 4K aligned or not, so I just choose the original global variable. In OVMF we set PcdPropertiesTableEnable to FALSE by default, but flip it dynamically to TRUE in PlatformPei, if the user requests the feature on the QEMU command line. This is being done for avoiding regressions with OSes that don't know about the properties table, and its impact on the UEFI memmap. [Jiewen] That is good design. I believe most real platforms do similar thing. However, if the memory attributes table is safe for OSes that don't know about it, I think we can eliminate the above "FALSE" default, and dynamism, from OVMF, and just inherit the PcdPropertiesTableEnable=TRUE setting from "MdeModulePkg.dec". [Jiewen] I am sorry that I am not clear on your suggestion. What do you mean "eliminate the above "FALSE" default, and dynamism, from OVMF"? Do you mean you suggest to change PcdPropertiesTableEnable=FALSE as default value in MdeModulePkg.dec file? Or do you want to change OVMF.dsc file? Does it sound reasonable to you? Thanks! Laszlo > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: "Yao, Jiewen" > Cc: "Gao, Liming" > > jiewen yao (7): > MdePkg: Add UEFI2.6 MemoryAttributes Table definition. > MdePkg: Add UEFI2.6 MemoryAttributesTable GUID > MdeModulePkg: Add MemoryAttributesTable generation. > MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. > MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header > file. > MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. > MdePkg: Update DxeCore INF for MemoryAttributesTable. > > MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- > MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- > MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- > MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ > MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- > MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ > MdePkg/MdePkg.dec | 11 +- > 7 files changed, 362 insertions(+), 15 deletions(-) create mode > 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c > create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta01.collab.prod.int.phx2.redhat.com (LHLO zmta01.collab.prod.int.phx2.redhat.com) (10.5.81.8) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Fri, 29 Jan 2016 21:24:10 -0500 (EST) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by zmta01.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id 3502A18584E; Fri, 29 Jan 2016 21:24:10 -0500 (EST) Received: from mx1.redhat.com (ext-mx05.extmail.prod.ext.phx2.redhat.com [10.5.110.29]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0U2OAtU027260 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 29 Jan 2016 21:24:10 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id B11D141307C; Sat, 30 Jan 2016 02:24:08 +0000 (UTC) Received: from ml01.vlan14.01.org (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 57C161A2274; Fri, 29 Jan 2016 18:24:07 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: 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 D6BEB1A1FFF for ; Fri, 29 Jan 2016 18:24:05 -0800 (PST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 251763D7EED; Sat, 30 Jan 2016 02:24:05 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com ([10.3.113.2]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0U2O35h006335; Fri, 29 Jan 2016 21:24:03 -0500 To: "Yao, Jiewen" References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> From: Laszlo Ersek Message-ID: <56AC1EC2.4020700@redhat.com> Date: Sat, 30 Jan 2016 03:24:02 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 Cc: "edk2-devel@ml01.01.org" , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.39 (BAYES_50,DCC_REPUT_00_12,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.78 on 10.5.110.29 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 01/30/16 01:31, Yao, Jiewen wrote: > Comment below: > > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Saturday, January 30, 2016 1:13 AM > To: Yao, Jiewen > Cc: edk2-devel@ml01.01.org; Gao, Liming > Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. > > Hello Jiewen, > > On 01/29/16 13:12, jiewen yao wrote: >> This series patches add UEFI2.6 MemoryAttributesTable support. >> This table is used to retire old PropertiesTable. >> This is standalone table published by DxeCore, so there is no >> compatibility issue. >> >> The patch is validated with or without properties table. > > Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE? > [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to FALSE or TRUE, MemoryAttributesTable is always published. > That is one compatibility we want to maintain. > > > I skimmed the commit messages, and it looks like the properties table is preserved, but now it serves only as foundation for the memory attributes table. > [Jiewen] Yes, the DxeCore global variable - properties table is always preserved. And the system UEFI configuration table for properties table is controlled by PcdPropertiestableEnable. > No matter properties table or memory attributes table, we always need a flag to record if RT image is 4K aligned or not, so I just choose the original global variable. > > > In OVMF we set PcdPropertiesTableEnable to FALSE by default, but flip it dynamically to TRUE in PlatformPei, if the user requests the feature on the QEMU command line. This is being done for avoiding regressions with OSes that don't know about the properties table, and its impact on the UEFI memmap. > [Jiewen] That is good design. I believe most real platforms do similar thing. > > > However, if the memory attributes table is safe for OSes that don't > know about it, I think we can eliminate the above "FALSE" default, > and dynamism, from OVMF, and just inherit the > PcdPropertiesTableEnable=TRUE setting from "MdeModulePkg.dec". > [Jiewen] I am sorry that I am not clear on your suggestion. What do you mean "eliminate the above "FALSE" default, and dynamism, from OVMF"? > Do you mean you suggest to change PcdPropertiesTableEnable=FALSE as default value in MdeModulePkg.dec file? > Or do you want to change OVMF.dsc file? Sorry, I should have educated myself on the memory attributes table first, in UEFI 2.6. I have now carefully read the EFI_MEMORY_ATTRIBUTES_TABLE section, and I've also noticed the new last paragraph in the EFI_PROPERTIES_TABLE section. What I was missing when I asked my question is that EFI_MEMORY_ATTRIBUTES_TABLE was an *additional* feature. Your cover letter said "This table is used to retire old PropertiesTable". So I thought, the OS-visible properties table would be completely removed from edk2, and the DXE core internals would only be reused for computing the new memory attributes table. In other words, I thought that the memory attributes table would *replace* the properties table, and the original PcdPropertiesTableEnable PCD would now control the presence of the memory attributes table instead. This is why I asked if we should change OVMF to remove its FALSE default for the PCD, alongside the possibility for the QEMU user to change the PCD dynamically, on the command line. Because, the memory attributes table would *always* be safe to install, so we should just stick, in OVMF, with the TRUE default for the PCD, from MdeModulePkg.dec. I do understand now that the memory attributes table is an additional feature. It will always be exposed to the OS. *Independently*, the properties table will continue to behave the same as before -- if the PCD is TRUE, then it will be installed; if the PCD is FALSE, then it won't. In the end, there's nothing to do under OvmfPkg/. We'll let the QEMU user control the properties table same as before (defaulting to FALSE), and orthogonally, the new memory attributes table will always be installed (with your patches in place), because it's safe. Thanks, and sorry about the confusion. Laszlo > > > > Does it sound reasonable to you? > > Thanks! > Laszlo > >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: "Yao, Jiewen" >> Cc: "Gao, Liming" >> >> jiewen yao (7): >> MdePkg: Add UEFI2.6 MemoryAttributes Table definition. >> MdePkg: Add UEFI2.6 MemoryAttributesTable GUID >> MdeModulePkg: Add MemoryAttributesTable generation. >> MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. >> MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header >> file. >> MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. >> MdePkg: Update DxeCore INF for MemoryAttributesTable. >> >> MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- >> MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- >> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- >> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ >> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- >> MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ >> MdePkg/MdePkg.dec | 11 +- >> 7 files changed, 362 insertions(+), 15 deletions(-) create mode >> 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c >> create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h >> > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta01.collab.prod.int.phx2.redhat.com (LHLO zmta01.collab.prod.int.phx2.redhat.com) (10.5.81.8) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Fri, 29 Jan 2016 22:17:35 -0500 (EST) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by zmta01.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id A1CFC185826; Fri, 29 Jan 2016 22:17:35 -0500 (EST) Received: from mx1.redhat.com (ext-mx07.extmail.prod.ext.phx2.redhat.com [10.5.110.31]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0U3HZmi011640 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 29 Jan 2016 22:17:35 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id 74A16C0D78EA; Sat, 30 Jan 2016 03:17:34 +0000 (UTC) Received: from ml01.vlan14.01.org (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id DF7F71A23E1; Fri, 29 Jan 2016 19:17:32 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by ml01.01.org (Postfix) with ESMTP id 086DA1A23D6 for ; Fri, 29 Jan 2016 19:17:32 -0800 (PST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP; 29 Jan 2016 19:17:31 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,367,1449561600"; d="scan'208";a="872225056" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga001.jf.intel.com with ESMTP; 29 Jan 2016 19:17:32 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 29 Jan 2016 19:17:31 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.172]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.218]) with mapi id 14.03.0248.002; Sat, 30 Jan 2016 11:17:23 +0800 From: "Yao, Jiewen" To: Laszlo Ersek Thread-Topic: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. Thread-Index: AQHRWo5UfnwfB+DE8USx6LXBJH6a7Z8SNVqAgAD+OSD//5vFAIAAk1RQ Date: Sat, 30 Jan 2016 03:17:22 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> In-Reply-To: <56AC1EC2.4020700@redhat.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNGZmZTIyMmMtZGJlZi00YzhlLTkwNzAtNTU0ZjQxZDQ3YjZhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjQuMTAuMTkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiKzhXUHVxZ014ZWZDMmxqa1RlNVpMQUFCYm5DUWpCYzhqMDEyem5mWWRuOD0ifQ== x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Cc: "edk2-devel@ml01.01.org" , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.39 (BAYES_50,DCC_REPUT_00_12,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Scanned-By: MIMEDefang 2.78 on 10.5.110.31 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Thanks for the clarification. I think you are right. I said "This table is used to retire old PropertiesTable", because UEFI2.6 does not recommend using PropertiesTable to report RT information. The future BIOS/OS should always produce/consume MemoryAttributesTable as better solution. Thank you Yao Jiewen -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com] Sent: Saturday, January 30, 2016 10:24 AM To: Yao, Jiewen Cc: edk2-devel@ml01.01.org; Gao, Liming Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. On 01/30/16 01:31, Yao, Jiewen wrote: > Comment below: > > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Saturday, January 30, 2016 1:13 AM > To: Yao, Jiewen > Cc: edk2-devel@ml01.01.org; Gao, Liming > Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. > > Hello Jiewen, > > On 01/29/16 13:12, jiewen yao wrote: >> This series patches add UEFI2.6 MemoryAttributesTable support. >> This table is used to retire old PropertiesTable. >> This is standalone table published by DxeCore, so there is no >> compatibility issue. >> >> The patch is validated with or without properties table. > > Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE? > [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to FALSE or TRUE, MemoryAttributesTable is always published. > That is one compatibility we want to maintain. > > > I skimmed the commit messages, and it looks like the properties table is preserved, but now it serves only as foundation for the memory attributes table. > [Jiewen] Yes, the DxeCore global variable - properties table is always preserved. And the system UEFI configuration table for properties table is controlled by PcdPropertiestableEnable. > No matter properties table or memory attributes table, we always need a flag to record if RT image is 4K aligned or not, so I just choose the original global variable. > > > In OVMF we set PcdPropertiesTableEnable to FALSE by default, but flip it dynamically to TRUE in PlatformPei, if the user requests the feature on the QEMU command line. This is being done for avoiding regressions with OSes that don't know about the properties table, and its impact on the UEFI memmap. > [Jiewen] That is good design. I believe most real platforms do similar thing. > > > However, if the memory attributes table is safe for OSes that don't > know about it, I think we can eliminate the above "FALSE" default, and > dynamism, from OVMF, and just inherit the > PcdPropertiesTableEnable=TRUE setting from "MdeModulePkg.dec". > [Jiewen] I am sorry that I am not clear on your suggestion. What do you mean "eliminate the above "FALSE" default, and dynamism, from OVMF"? > Do you mean you suggest to change PcdPropertiesTableEnable=FALSE as default value in MdeModulePkg.dec file? > Or do you want to change OVMF.dsc file? Sorry, I should have educated myself on the memory attributes table first, in UEFI 2.6. I have now carefully read the EFI_MEMORY_ATTRIBUTES_TABLE section, and I've also noticed the new last paragraph in the EFI_PROPERTIES_TABLE section. What I was missing when I asked my question is that EFI_MEMORY_ATTRIBUTES_TABLE was an *additional* feature. Your cover letter said "This table is used to retire old PropertiesTable". So I thought, the OS-visible properties table would be completely removed from edk2, and the DXE core internals would only be reused for computing the new memory attributes table. In other words, I thought that the memory attributes table would *replace* the properties table, and the original PcdPropertiesTableEnable PCD would now control the presence of the memory attributes table instead. This is why I asked if we should change OVMF to remove its FALSE default for the PCD, alongside the possibility for the QEMU user to change the PCD dynamically, on the command line. Because, the memory attributes table would *always* be safe to install, so we should just stick, in OVMF, with the TRUE default for the PCD, from MdeModulePkg.dec. I do understand now that the memory attributes table is an additional feature. It will always be exposed to the OS. *Independently*, the properties table will continue to behave the same as before -- if the PCD is TRUE, then it will be installed; if the PCD is FALSE, then it won't. In the end, there's nothing to do under OvmfPkg/. We'll let the QEMU user control the properties table same as before (defaulting to FALSE), and orthogonally, the new memory attributes table will always be installed (with your patches in place), because it's safe. Thanks, and sorry about the confusion. Laszlo > > > > Does it sound reasonable to you? > > Thanks! > Laszlo > >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: "Yao, Jiewen" >> Cc: "Gao, Liming" >> >> jiewen yao (7): >> MdePkg: Add UEFI2.6 MemoryAttributes Table definition. >> MdePkg: Add UEFI2.6 MemoryAttributesTable GUID >> MdeModulePkg: Add MemoryAttributesTable generation. >> MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. >> MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header >> file. >> MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. >> MdePkg: Update DxeCore INF for MemoryAttributesTable. >> >> MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- >> MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- >> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- >> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ >> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- >> MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ >> MdePkg/MdePkg.dec | 11 +- >> 7 files changed, 362 insertions(+), 15 deletions(-) create mode >> 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c >> create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h >> > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta03.collab.prod.int.phx2.redhat.com (LHLO zmta03.collab.prod.int.phx2.redhat.com) (10.5.81.10) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Sat, 30 Jan 2016 05:25:25 -0500 (EST) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by zmta03.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id 4C8B4433DE; Sat, 30 Jan 2016 05:25:25 -0500 (EST) Received: from mx1.redhat.com (ext-mx05.extmail.prod.ext.phx2.redhat.com [10.5.110.29]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0UAPPIM010937 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Sat, 30 Jan 2016 05:25:25 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id 89F3919CBB2; Sat, 30 Jan 2016 10:25:23 +0000 (UTC) Received: from ml01.vlan14.01.org (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 2F73B1A234D; Sat, 30 Jan 2016 02:25:22 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mail-ig0-x233.google.com (mail-ig0-x233.google.com [IPv6:2607:f8b0:4001:c05::233]) (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 86C941A233A for ; Sat, 30 Jan 2016 02:25:20 -0800 (PST) Received: by mail-ig0-x233.google.com with SMTP id z14so5714370igp.1 for ; Sat, 30 Jan 2016 02:25:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=UUKGPwKnPbaraGai3THi8yHhQDKn+aMtRcXhxCSt+l4=; b=k4OU2xrD0tP0LcKKubaVR0iz8Kv/p7nykkJSMjEVRhEDXycQ1xC2DmYlQf/GCa0NBw K+UumIxnXI2hEa04sma5gOqg7++bwhrMoN1C0auz4hTaGOwXdDqTmdl91YYklO2WesZ4 Wk+pk6Fsr5uG1qSmAu15VzuLntQsMnvQ1OuVE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=UUKGPwKnPbaraGai3THi8yHhQDKn+aMtRcXhxCSt+l4=; b=RWDqI1OQTvoyvfgWo0LBG6pXyoJRQkuUAF7MN/xXUBSk6dWKQ6IxyJ8hn/Z1D/gx8b 1+LMPX1x+B4zz514SlwCxc04EdTeuDQ7Mv5RzQuT42SyUfr6QfqXzGg8I2ywkGahIZO6 IXmTXfAvoYwEKWi/EWIstEYXSrwl0u0+z/eZ2h7ZzArxiO+mjLyLRBQPKdvF5E9RspIY p+xqJrZS2KbXTQ9KQ0Vqg84XOtXHL2aX4nwi07tGgg7CJpT9eDJMeTsUgjt9ujOayTWr CwzPKUq85tqV/iB6snMH0xbCFF9mgZCOpdLAzNH4B7564XTjyTnNIucIcrcwo2EyosQo F3DQ== X-Gm-Message-State: AG10YORTL/qg1PKoEi1dx13QKDY1DRB6SFh97EbOkuUVGdAZohnlSarcHwsOGqSJa4/aXah/oSr74nf/fMUZEuC/ MIME-Version: 1.0 X-Received: by 10.50.141.226 with SMTP id rr2mr1477946igb.75.1454149519843; Sat, 30 Jan 2016 02:25:19 -0800 (PST) Received: by 10.36.29.6 with HTTP; Sat, 30 Jan 2016 02:25:19 -0800 (PST) In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> Date: Sat, 30 Jan 2016 11:25:19 +0100 Message-ID: From: Ard Biesheuvel To: "Yao, Jiewen" Cc: "edk2-devel@ml01.01.org" , Laszlo Ersek , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.5 (BAYES_50,DCC_REPUT_00_12,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,T_DKIM_INVALID) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Scanned-By: MIMEDefang 2.78 on 10.5.110.29 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 30 January 2016 at 04:17, Yao, Jiewen wrote: > Thanks for the clarification. I think you are right. > > I said "This table is used to retire old PropertiesTable", because UEFI2.6 does not recommend using PropertiesTable to report RT information. > The future BIOS/OS should always produce/consume MemoryAttributesTable as better solution. > I strongly feel we should remove the original PropertiesTable. It breaks backward compatibility, and was replaced by the MemoryAttributesTable for a good reason. The OS side will not be implemented in Linux (i.e., it will ignore the RO and XP attributes in the standard UEFI memory map), and the only reason the PropertiesTable was not removed entirely from the specification is because of business interests of one of the participating OS vendors. I understand that we need to keep the underlying machinery to produce either table. But we should remove the functionality that results in each PE/COFF image to be split into several regions marked RO or XP in the ordinary UEFI memory map. Regards, Ard. > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Saturday, January 30, 2016 10:24 AM > To: Yao, Jiewen > Cc: edk2-devel@ml01.01.org; Gao, Liming > Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. > > On 01/30/16 01:31, Yao, Jiewen wrote: >> Comment below: >> >> -----Original Message----- >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Saturday, January 30, 2016 1:13 AM >> To: Yao, Jiewen >> Cc: edk2-devel@ml01.01.org; Gao, Liming >> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >> >> Hello Jiewen, >> >> On 01/29/16 13:12, jiewen yao wrote: >>> This series patches add UEFI2.6 MemoryAttributesTable support. >>> This table is used to retire old PropertiesTable. >>> This is standalone table published by DxeCore, so there is no >>> compatibility issue. >>> >>> The patch is validated with or without properties table. >> >> Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE? >> [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to FALSE or TRUE, MemoryAttributesTable is always published. >> That is one compatibility we want to maintain. >> >> >> I skimmed the commit messages, and it looks like the properties table is preserved, but now it serves only as foundation for the memory attributes table. >> [Jiewen] Yes, the DxeCore global variable - properties table is always preserved. And the system UEFI configuration table for properties table is controlled by PcdPropertiestableEnable. >> No matter properties table or memory attributes table, we always need a flag to record if RT image is 4K aligned or not, so I just choose the original global variable. >> >> >> In OVMF we set PcdPropertiesTableEnable to FALSE by default, but flip it dynamically to TRUE in PlatformPei, if the user requests the feature on the QEMU command line. This is being done for avoiding regressions with OSes that don't know about the properties table, and its impact on the UEFI memmap. >> [Jiewen] That is good design. I believe most real platforms do similar thing. >> >> >> However, if the memory attributes table is safe for OSes that don't >> know about it, I think we can eliminate the above "FALSE" default, and >> dynamism, from OVMF, and just inherit the >> PcdPropertiesTableEnable=TRUE setting from "MdeModulePkg.dec". >> [Jiewen] I am sorry that I am not clear on your suggestion. What do you mean "eliminate the above "FALSE" default, and dynamism, from OVMF"? >> Do you mean you suggest to change PcdPropertiesTableEnable=FALSE as default value in MdeModulePkg.dec file? >> Or do you want to change OVMF.dsc file? > > Sorry, I should have educated myself on the memory attributes table first, in UEFI 2.6. I have now carefully read the EFI_MEMORY_ATTRIBUTES_TABLE section, and I've also noticed the new last paragraph in the EFI_PROPERTIES_TABLE section. > > What I was missing when I asked my question is that EFI_MEMORY_ATTRIBUTES_TABLE was an *additional* feature. > > Your cover letter said "This table is used to retire old PropertiesTable". So I thought, the OS-visible properties table would be completely removed from edk2, and the DXE core internals would only be reused for computing the new memory attributes table. In other words, I thought that the memory attributes table would *replace* the properties table, and the original PcdPropertiesTableEnable PCD would now control the presence of the memory attributes table instead. > > This is why I asked if we should change OVMF to remove its FALSE default for the PCD, alongside the possibility for the QEMU user to change the PCD dynamically, on the command line. Because, the memory attributes table would *always* be safe to install, so we should just stick, in OVMF, with the TRUE default for the PCD, from MdeModulePkg.dec. > > I do understand now that the memory attributes table is an additional feature. It will always be exposed to the OS. *Independently*, the properties table will continue to behave the same as before -- if the PCD is TRUE, then it will be installed; if the PCD is FALSE, then it won't. > > In the end, there's nothing to do under OvmfPkg/. We'll let the QEMU user control the properties table same as before (defaulting to FALSE), and orthogonally, the new memory attributes table will always be installed (with your patches in place), because it's safe. > > Thanks, and sorry about the confusion. > Laszlo > >> >> >> >> Does it sound reasonable to you? >> >> Thanks! >> Laszlo >> >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: "Yao, Jiewen" >>> Cc: "Gao, Liming" >>> >>> jiewen yao (7): >>> MdePkg: Add UEFI2.6 MemoryAttributes Table definition. >>> MdePkg: Add UEFI2.6 MemoryAttributesTable GUID >>> MdeModulePkg: Add MemoryAttributesTable generation. >>> MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. >>> MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header >>> file. >>> MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. >>> MdePkg: Update DxeCore INF for MemoryAttributesTable. >>> >>> MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- >>> MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- >>> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- >>> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ >>> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- >>> MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ >>> MdePkg/MdePkg.dec | 11 +- >>> 7 files changed, 362 insertions(+), 15 deletions(-) create mode >>> 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c >>> create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h >>> >> > > _______________________________________________ > 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 --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta02.collab.prod.int.phx2.redhat.com (LHLO zmta02.collab.prod.int.phx2.redhat.com) (10.5.81.9) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Sun, 31 Jan 2016 15:36:22 -0500 (EST) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by zmta02.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id CA54E1227E5; Sun, 31 Jan 2016 15:36:22 -0500 (EST) Received: from mx1.redhat.com (ext-mx06.extmail.prod.ext.phx2.redhat.com [10.5.110.30]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0VKaMKm014671 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Sun, 31 Jan 2016 15:36:22 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id 437635A55; Sun, 31 Jan 2016 20:36:21 +0000 (UTC) Received: from ml01.vlan14.01.org (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id F006B1A2466; Sun, 31 Jan 2016 12:36:19 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: 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 0081C1A2430 for ; Sun, 31 Jan 2016 12:36:18 -0800 (PST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 5C849C0AD1C9; Sun, 31 Jan 2016 20:36:18 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-66.phx2.redhat.com [10.3.113.66]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0VKaGcf013170; Sun, 31 Jan 2016 15:36:17 -0500 To: Ard Biesheuvel References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> From: Laszlo Ersek Message-ID: <56AE7040.4060004@redhat.com> Date: Sun, 31 Jan 2016 21:36:16 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 Cc: "edk2-devel@ml01.01.org" , "Yao, Jiewen" , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: -0.162 (BAYES_50,DCC_REPUT_00_12,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.78 on 10.5.110.30 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 01/30/16 11:25, Ard Biesheuvel wrote: > On 30 January 2016 at 04:17, Yao, Jiewen wrote: >> Thanks for the clarification. I think you are right. >> >> I said "This table is used to retire old PropertiesTable", because UEFI2.6 does not recommend using PropertiesTable to report RT information. >> The future BIOS/OS should always produce/consume MemoryAttributesTable as better solution. >> > > I strongly feel we should remove the original PropertiesTable. It > breaks backward compatibility, and was replaced by the > MemoryAttributesTable for a good reason. The OS side will not be > implemented in Linux (i.e., it will ignore the RO and XP attributes in > the standard UEFI memory map), and the only reason the PropertiesTable > was not removed entirely from the specification is because of business > interests of one of the participating OS vendors. > > I understand that we need to keep the underlying machinery to produce > either table. But we should remove the functionality that results in > each PE/COFF image to be split into several regions marked RO or XP in > the ordinary UEFI memory map. We can mitigate this by setting PcdPropertiesTableEnable to FALSE in "ArmVirt.dsc.inc" as well. (Hmmm, I forget why SVN r18140 and r18566 don't already result in a problematic memmap for Linux... Probably due to your kernel commit 0ce3cc008ec0.) Would you like me to submit a patch for the dsc include file? Thanks Laszlo > Regards, > Ard. > > >> -----Original Message----- >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Saturday, January 30, 2016 10:24 AM >> To: Yao, Jiewen >> Cc: edk2-devel@ml01.01.org; Gao, Liming >> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >> >> On 01/30/16 01:31, Yao, Jiewen wrote: >>> Comment below: >>> >>> -----Original Message----- >>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>> Sent: Saturday, January 30, 2016 1:13 AM >>> To: Yao, Jiewen >>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>> >>> Hello Jiewen, >>> >>> On 01/29/16 13:12, jiewen yao wrote: >>>> This series patches add UEFI2.6 MemoryAttributesTable support. >>>> This table is used to retire old PropertiesTable. >>>> This is standalone table published by DxeCore, so there is no >>>> compatibility issue. >>>> >>>> The patch is validated with or without properties table. >>> >>> Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE? >>> [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to FALSE or TRUE, MemoryAttributesTable is always published. >>> That is one compatibility we want to maintain. >>> >>> >>> I skimmed the commit messages, and it looks like the properties table is preserved, but now it serves only as foundation for the memory attributes table. >>> [Jiewen] Yes, the DxeCore global variable - properties table is always preserved. And the system UEFI configuration table for properties table is controlled by PcdPropertiestableEnable. >>> No matter properties table or memory attributes table, we always need a flag to record if RT image is 4K aligned or not, so I just choose the original global variable. >>> >>> >>> In OVMF we set PcdPropertiesTableEnable to FALSE by default, but flip it dynamically to TRUE in PlatformPei, if the user requests the feature on the QEMU command line. This is being done for avoiding regressions with OSes that don't know about the properties table, and its impact on the UEFI memmap. >>> [Jiewen] That is good design. I believe most real platforms do similar thing. >>> >>> >>> However, if the memory attributes table is safe for OSes that don't >>> know about it, I think we can eliminate the above "FALSE" default, and >>> dynamism, from OVMF, and just inherit the >>> PcdPropertiesTableEnable=TRUE setting from "MdeModulePkg.dec". >>> [Jiewen] I am sorry that I am not clear on your suggestion. What do you mean "eliminate the above "FALSE" default, and dynamism, from OVMF"? >>> Do you mean you suggest to change PcdPropertiesTableEnable=FALSE as default value in MdeModulePkg.dec file? >>> Or do you want to change OVMF.dsc file? >> >> Sorry, I should have educated myself on the memory attributes table first, in UEFI 2.6. I have now carefully read the EFI_MEMORY_ATTRIBUTES_TABLE section, and I've also noticed the new last paragraph in the EFI_PROPERTIES_TABLE section. >> >> What I was missing when I asked my question is that EFI_MEMORY_ATTRIBUTES_TABLE was an *additional* feature. >> >> Your cover letter said "This table is used to retire old PropertiesTable". So I thought, the OS-visible properties table would be completely removed from edk2, and the DXE core internals would only be reused for computing the new memory attributes table. In other words, I thought that the memory attributes table would *replace* the properties table, and the original PcdPropertiesTableEnable PCD would now control the presence of the memory attributes table instead. >> >> This is why I asked if we should change OVMF to remove its FALSE default for the PCD, alongside the possibility for the QEMU user to change the PCD dynamically, on the command line. Because, the memory attributes table would *always* be safe to install, so we should just stick, in OVMF, with the TRUE default for the PCD, from MdeModulePkg.dec. >> >> I do understand now that the memory attributes table is an additional feature. It will always be exposed to the OS. *Independently*, the properties table will continue to behave the same as before -- if the PCD is TRUE, then it will be installed; if the PCD is FALSE, then it won't. >> >> In the end, there's nothing to do under OvmfPkg/. We'll let the QEMU user control the properties table same as before (defaulting to FALSE), and orthogonally, the new memory attributes table will always be installed (with your patches in place), because it's safe. >> >> Thanks, and sorry about the confusion. >> Laszlo >> >>> >>> >>> >>> Does it sound reasonable to you? >>> >>> Thanks! >>> Laszlo >>> >>>> >>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>> Signed-off-by: "Yao, Jiewen" >>>> Cc: "Gao, Liming" >>>> >>>> jiewen yao (7): >>>> MdePkg: Add UEFI2.6 MemoryAttributes Table definition. >>>> MdePkg: Add UEFI2.6 MemoryAttributesTable GUID >>>> MdeModulePkg: Add MemoryAttributesTable generation. >>>> MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. >>>> MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header >>>> file. >>>> MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. >>>> MdePkg: Update DxeCore INF for MemoryAttributesTable. >>>> >>>> MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- >>>> MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- >>>> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- >>>> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ >>>> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- >>>> MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ >>>> MdePkg/MdePkg.dec | 11 +- >>>> 7 files changed, 362 insertions(+), 15 deletions(-) create mode >>>> 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c >>>> create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h >>>> >>> >> >> _______________________________________________ >> 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 --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta06.collab.prod.int.phx2.redhat.com (LHLO zmta06.collab.prod.int.phx2.redhat.com) (10.5.81.13) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Mon, 1 Feb 2016 01:27:53 -0500 (EST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by zmta06.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id 1459E1689B9; Mon, 1 Feb 2016 01:27:53 -0500 (EST) Received: from mx1.redhat.com (ext-mx01.extmail.prod.ext.phx2.redhat.com [10.5.110.25]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u116RqQ4013365 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Mon, 1 Feb 2016 01:27:52 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id 8D7ACE1B7B; Mon, 1 Feb 2016 06:27:51 +0000 (UTC) Received: from ml01.vlan14.01.org (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id EAD411A24E9; Sun, 31 Jan 2016 22:27:49 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mail-ig0-x235.google.com (mail-ig0-x235.google.com [IPv6:2607:f8b0:4001:c05::235]) (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 A0DA51A24E1 for ; Sun, 31 Jan 2016 22:27:48 -0800 (PST) Received: by mail-ig0-x235.google.com with SMTP id z14so28978749igp.0 for ; Sun, 31 Jan 2016 22:27:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=5ns6unh+c8ZbJAm/Te+XUMi+JaVJRgYH4gxFmb2j0j4=; b=DhMPs3a4uM7ECsKp7OQ2CGoQxILMCGFtqmTVdvf2fDoGt+pdK/9p8a9lE4ZiJjo6JZ QyBaW2sDP9Pf+qaquzlYAivdBtnpRnVLwPoF7jJ34jBgxa7SpMJkuzYJWaYIN9k9Kjz5 M72q5df/uZFPgChOv6hkpSwm+MwrWudvbBmKU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=5ns6unh+c8ZbJAm/Te+XUMi+JaVJRgYH4gxFmb2j0j4=; b=VjqStxoC6GZQx+99lN6RPDC++yB/cLvU2on6O+JrJW0uqVAxIOl507/RjNve0xRcgm Mv9TCLQVXKcbAaUu/79JVJWXjCSvasBPyECB+z3Px2k80t7Sa6w13UZMhrKg7qOCedS6 Jp4lqY+j0pwJ04hJVbWXKlBsLuOo/CEo8AbMBiTaW3qHE/kI/XnHzhT6BYZTXNY1NSMl Itf0zcsQKDSMjktbBgk1wjy8Wy5gl8DO4ZVkyA+mZz14aiMe/g2OUI0yrds3HlNhISd6 o/3gI0za4kjXO89mqaLFqmSlyXfZhZuYwtML4Xhm5yPQ9rZ8QmrgsRKRKGuubx/7F2ov UXxA== X-Gm-Message-State: AG10YOQWtEJMNB8IastrJ0LkknuuUCelQscxexe3s3y/yASJY2EG3skATYky6Wl8QhPYW46wtW8DRxLo7TpGsREY MIME-Version: 1.0 X-Received: by 10.50.33.81 with SMTP id p17mr8320920igi.75.1454308068002; Sun, 31 Jan 2016 22:27:48 -0800 (PST) Received: by 10.36.29.6 with HTTP; Sun, 31 Jan 2016 22:27:47 -0800 (PST) In-Reply-To: <56AE7040.4060004@redhat.com> References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> <56AE7040.4060004@redhat.com> Date: Mon, 1 Feb 2016 07:27:47 +0100 Message-ID: From: Ard Biesheuvel To: Laszlo Ersek Cc: "edk2-devel@ml01.01.org" , "Yao, Jiewen" , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: -0.044 (BAYES_50,DCC_REPUT_00_12,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,T_DKIM_INVALID) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Scanned-By: MIMEDefang 2.75 on 10.5.110.25 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 31 January 2016 at 21:36, Laszlo Ersek wrote: > On 01/30/16 11:25, Ard Biesheuvel wrote: >> On 30 January 2016 at 04:17, Yao, Jiewen wrote: >>> Thanks for the clarification. I think you are right. >>> >>> I said "This table is used to retire old PropertiesTable", because UEFI2.6 does not recommend using PropertiesTable to report RT information. >>> The future BIOS/OS should always produce/consume MemoryAttributesTable as better solution. >>> >> >> I strongly feel we should remove the original PropertiesTable. It >> breaks backward compatibility, and was replaced by the >> MemoryAttributesTable for a good reason. The OS side will not be >> implemented in Linux (i.e., it will ignore the RO and XP attributes in >> the standard UEFI memory map), and the only reason the PropertiesTable >> was not removed entirely from the specification is because of business >> interests of one of the participating OS vendors. >> >> I understand that we need to keep the underlying machinery to produce >> either table. But we should remove the functionality that results in >> each PE/COFF image to be split into several regions marked RO or XP in >> the ordinary UEFI memory map. > > We can mitigate this by setting PcdPropertiesTableEnable to FALSE in > "ArmVirt.dsc.inc" as well. > No, we should remove it. > (Hmmm, I forget why SVN r18140 and r18566 don't already result in a > problematic memmap for Linux... Probably due to your kernel commit > 0ce3cc008ec0.) > It will result in a problematic memmap for older Linux kernels but also for less recent versions of Windows 8, for instance. And the only OS version that actually consumes the properties table is Windows 10, which will we updated in the future to use the MemoryAttributesTable instead. >>> -----Original Message----- >>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>> Sent: Saturday, January 30, 2016 10:24 AM >>> To: Yao, Jiewen >>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>> >>> On 01/30/16 01:31, Yao, Jiewen wrote: >>>> Comment below: >>>> >>>> -----Original Message----- >>>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>>> Sent: Saturday, January 30, 2016 1:13 AM >>>> To: Yao, Jiewen >>>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>>> >>>> Hello Jiewen, >>>> >>>> On 01/29/16 13:12, jiewen yao wrote: >>>>> This series patches add UEFI2.6 MemoryAttributesTable support. >>>>> This table is used to retire old PropertiesTable. >>>>> This is standalone table published by DxeCore, so there is no >>>>> compatibility issue. >>>>> >>>>> The patch is validated with or without properties table. >>>> >>>> Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE? >>>> [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to FALSE or TRUE, MemoryAttributesTable is always published. >>>> That is one compatibility we want to maintain. >>>> >>>> >>>> I skimmed the commit messages, and it looks like the properties table is preserved, but now it serves only as foundation for the memory attributes table. >>>> [Jiewen] Yes, the DxeCore global variable - properties table is always preserved. And the system UEFI configuration table for properties table is controlled by PcdPropertiestableEnable. >>>> No matter properties table or memory attributes table, we always need a flag to record if RT image is 4K aligned or not, so I just choose the original global variable. >>>> >>>> >>>> In OVMF we set PcdPropertiesTableEnable to FALSE by default, but flip it dynamically to TRUE in PlatformPei, if the user requests the feature on the QEMU command line. This is being done for avoiding regressions with OSes that don't know about the properties table, and its impact on the UEFI memmap. >>>> [Jiewen] That is good design. I believe most real platforms do similar thing. >>>> >>>> >>>> However, if the memory attributes table is safe for OSes that don't >>>> know about it, I think we can eliminate the above "FALSE" default, and >>>> dynamism, from OVMF, and just inherit the >>>> PcdPropertiesTableEnable=TRUE setting from "MdeModulePkg.dec". >>>> [Jiewen] I am sorry that I am not clear on your suggestion. What do you mean "eliminate the above "FALSE" default, and dynamism, from OVMF"? >>>> Do you mean you suggest to change PcdPropertiesTableEnable=FALSE as default value in MdeModulePkg.dec file? >>>> Or do you want to change OVMF.dsc file? >>> >>> Sorry, I should have educated myself on the memory attributes table first, in UEFI 2.6. I have now carefully read the EFI_MEMORY_ATTRIBUTES_TABLE section, and I've also noticed the new last paragraph in the EFI_PROPERTIES_TABLE section. >>> >>> What I was missing when I asked my question is that EFI_MEMORY_ATTRIBUTES_TABLE was an *additional* feature. >>> >>> Your cover letter said "This table is used to retire old PropertiesTable". So I thought, the OS-visible properties table would be completely removed from edk2, and the DXE core internals would only be reused for computing the new memory attributes table. In other words, I thought that the memory attributes table would *replace* the properties table, and the original PcdPropertiesTableEnable PCD would now control the presence of the memory attributes table instead. >>> >>> This is why I asked if we should change OVMF to remove its FALSE default for the PCD, alongside the possibility for the QEMU user to change the PCD dynamically, on the command line. Because, the memory attributes table would *always* be safe to install, so we should just stick, in OVMF, with the TRUE default for the PCD, from MdeModulePkg.dec. >>> >>> I do understand now that the memory attributes table is an additional feature. It will always be exposed to the OS. *Independently*, the properties table will continue to behave the same as before -- if the PCD is TRUE, then it will be installed; if the PCD is FALSE, then it won't. >>> >>> In the end, there's nothing to do under OvmfPkg/. We'll let the QEMU user control the properties table same as before (defaulting to FALSE), and orthogonally, the new memory attributes table will always be installed (with your patches in place), because it's safe. >>> >>> Thanks, and sorry about the confusion. >>> Laszlo >>> >>>> >>>> >>>> >>>> Does it sound reasonable to you? >>>> >>>> Thanks! >>>> Laszlo >>>> >>>>> >>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>> Signed-off-by: "Yao, Jiewen" >>>>> Cc: "Gao, Liming" >>>>> >>>>> jiewen yao (7): >>>>> MdePkg: Add UEFI2.6 MemoryAttributes Table definition. >>>>> MdePkg: Add UEFI2.6 MemoryAttributesTable GUID >>>>> MdeModulePkg: Add MemoryAttributesTable generation. >>>>> MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. >>>>> MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header >>>>> file. >>>>> MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. >>>>> MdePkg: Update DxeCore INF for MemoryAttributesTable. >>>>> >>>>> MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- >>>>> MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- >>>>> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- >>>>> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ >>>>> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- >>>>> MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ >>>>> MdePkg/MdePkg.dec | 11 +- >>>>> 7 files changed, 362 insertions(+), 15 deletions(-) create mode >>>>> 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c >>>>> create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h >>>>> >>>> >>> >>> _______________________________________________ >>> 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 --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta05.collab.prod.int.phx2.redhat.com (LHLO zmta05.collab.prod.int.phx2.redhat.com) (10.5.81.12) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Sun, 14 Feb 2016 00:44:53 -0500 (EST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by zmta05.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id 84DDFF3451; Sun, 14 Feb 2016 00:44:53 -0500 (EST) Received: from mx1.redhat.com (ext-mx04.extmail.prod.ext.phx2.redhat.com [10.5.110.28]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1E5ir76021458 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Sun, 14 Feb 2016 00:44:53 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id BEF428F4E6; Sun, 14 Feb 2016 05:44:51 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id DBCD81A1DFD; Sat, 13 Feb 2016 21:44:50 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by ml01.01.org (Postfix) with ESMTP id B64EF1A1DFA for ; Sat, 13 Feb 2016 21:44:49 -0800 (PST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 13 Feb 2016 21:44:49 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,444,1449561600"; d="scan'208";a="914794153" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga002.fm.intel.com with ESMTP; 13 Feb 2016 21:44:49 -0800 Received: from fmsmsx155.amr.corp.intel.com (10.18.116.71) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sat, 13 Feb 2016 21:44:49 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.110.14) by FMSMSX155.amr.corp.intel.com (10.18.116.71) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sat, 13 Feb 2016 21:44:48 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.172]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.218]) with mapi id 14.03.0248.002; Sun, 14 Feb 2016 13:44:46 +0800 From: "Yao, Jiewen" To: Ard Biesheuvel , Laszlo Ersek Thread-Topic: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. Thread-Index: AQHRWo5UfnwfB+DE8USx6LXBJH6a7Z8SNVqAgAD+OSD//5vFAIAAk1RQ///zJICAAj0HAIAApUWAgBTkw9A= Date: Sun, 14 Feb 2016 05:44:45 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C5002759103@shsmsx102.ccr.corp.intel.com> References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> <56AE7040.4060004@redhat.com> In-Reply-To: Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzc0YjA1NzAtNGRlNS00MDQ2LWFjNDYtMzViYzliNDQyODc5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IlNTbnRqenY4dEI5VHpKdjJ4S2VUTXNxSGFkZW5PMk5STHV6Tk5oTVZQdWs9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Cc: "edk2-devel@ml01.01.org" , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.385 (BAYES_50,DCC_REPUT_00_12,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Scanned-By: MIMEDefang 2.75 on 10.5.110.28 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit HI Thanks to discuss this properties table issue. The purpose of this patch is to *add* UEFI2.6 memory attributes table. This patch does not handle any UEFI2.5 properties table. If we want to change EDKII code for properties table, I suggest we separate it from this patch. Is there any comment for adding UEFI2.6 memory attributes table? For UEFI2.5 properties table, let me summarize. Currently, we have several options: 0) Keep it as is. Current setting is PcdPropertiesTableEnable TRUE in MdeModulePkg.DEC. 1) Update PcdPropertiesTableEnable to FALSE in XXXPlatform.DSC file. (Platform choice. It can be static PCD or dynamic PCD.) 2) Update PcdPropertiesTableEnable to FALSE in MdeModulePkg.DEC file. (Disable Default. BIOS will not publish this table by default. If platform wants this table, it can set PCD to true.) 3) Remove PcdPropertiesTableEnable, and remove PropertiesTable support code in DxeCore. (PropertiesTable support is removed.) Personally, I do not suggest we choose 3), because this table is still defined in UEFI specification. We can remove it after UEFI spec removes it later. I do not have strong opinion for other options. Thank you Yao Jiewen -----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] Sent: Monday, February 01, 2016 2:28 PM To: Laszlo Ersek Cc: Yao, Jiewen; edk2-devel@ml01.01.org; Gao, Liming Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. On 31 January 2016 at 21:36, Laszlo Ersek wrote: > On 01/30/16 11:25, Ard Biesheuvel wrote: >> On 30 January 2016 at 04:17, Yao, Jiewen wrote: >>> Thanks for the clarification. I think you are right. >>> >>> I said "This table is used to retire old PropertiesTable", because UEFI2.6 does not recommend using PropertiesTable to report RT information. >>> The future BIOS/OS should always produce/consume MemoryAttributesTable as better solution. >>> >> >> I strongly feel we should remove the original PropertiesTable. It >> breaks backward compatibility, and was replaced by the >> MemoryAttributesTable for a good reason. The OS side will not be >> implemented in Linux (i.e., it will ignore the RO and XP attributes >> in the standard UEFI memory map), and the only reason the >> PropertiesTable was not removed entirely from the specification is >> because of business interests of one of the participating OS vendors. >> >> I understand that we need to keep the underlying machinery to produce >> either table. But we should remove the functionality that results in >> each PE/COFF image to be split into several regions marked RO or XP >> in the ordinary UEFI memory map. > > We can mitigate this by setting PcdPropertiesTableEnable to FALSE in > "ArmVirt.dsc.inc" as well. > No, we should remove it. > (Hmmm, I forget why SVN r18140 and r18566 don't already result in a > problematic memmap for Linux... Probably due to your kernel commit > 0ce3cc008ec0.) > It will result in a problematic memmap for older Linux kernels but also for less recent versions of Windows 8, for instance. And the only OS version that actually consumes the properties table is Windows 10, which will we updated in the future to use the MemoryAttributesTable instead. >>> -----Original Message----- >>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>> Sent: Saturday, January 30, 2016 10:24 AM >>> To: Yao, Jiewen >>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>> >>> On 01/30/16 01:31, Yao, Jiewen wrote: >>>> Comment below: >>>> >>>> -----Original Message----- >>>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>>> Sent: Saturday, January 30, 2016 1:13 AM >>>> To: Yao, Jiewen >>>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>>> >>>> Hello Jiewen, >>>> >>>> On 01/29/16 13:12, jiewen yao wrote: >>>>> This series patches add UEFI2.6 MemoryAttributesTable support. >>>>> This table is used to retire old PropertiesTable. >>>>> This is standalone table published by DxeCore, so there is no >>>>> compatibility issue. >>>>> >>>>> The patch is validated with or without properties table. >>>> >>>> Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE? >>>> [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to FALSE or TRUE, MemoryAttributesTable is always published. >>>> That is one compatibility we want to maintain. >>>> >>>> >>>> I skimmed the commit messages, and it looks like the properties table is preserved, but now it serves only as foundation for the memory attributes table. >>>> [Jiewen] Yes, the DxeCore global variable - properties table is always preserved. And the system UEFI configuration table for properties table is controlled by PcdPropertiestableEnable. >>>> No matter properties table or memory attributes table, we always need a flag to record if RT image is 4K aligned or not, so I just choose the original global variable. >>>> >>>> >>>> In OVMF we set PcdPropertiesTableEnable to FALSE by default, but flip it dynamically to TRUE in PlatformPei, if the user requests the feature on the QEMU command line. This is being done for avoiding regressions with OSes that don't know about the properties table, and its impact on the UEFI memmap. >>>> [Jiewen] That is good design. I believe most real platforms do similar thing. >>>> >>>> >>>> However, if the memory attributes table is safe for OSes that don't >>>> know about it, I think we can eliminate the above "FALSE" default, >>>> and dynamism, from OVMF, and just inherit the >>>> PcdPropertiesTableEnable=TRUE setting from "MdeModulePkg.dec". >>>> [Jiewen] I am sorry that I am not clear on your suggestion. What do you mean "eliminate the above "FALSE" default, and dynamism, from OVMF"? >>>> Do you mean you suggest to change PcdPropertiesTableEnable=FALSE as default value in MdeModulePkg.dec file? >>>> Or do you want to change OVMF.dsc file? >>> >>> Sorry, I should have educated myself on the memory attributes table first, in UEFI 2.6. I have now carefully read the EFI_MEMORY_ATTRIBUTES_TABLE section, and I've also noticed the new last paragraph in the EFI_PROPERTIES_TABLE section. >>> >>> What I was missing when I asked my question is that EFI_MEMORY_ATTRIBUTES_TABLE was an *additional* feature. >>> >>> Your cover letter said "This table is used to retire old PropertiesTable". So I thought, the OS-visible properties table would be completely removed from edk2, and the DXE core internals would only be reused for computing the new memory attributes table. In other words, I thought that the memory attributes table would *replace* the properties table, and the original PcdPropertiesTableEnable PCD would now control the presence of the memory attributes table instead. >>> >>> This is why I asked if we should change OVMF to remove its FALSE default for the PCD, alongside the possibility for the QEMU user to change the PCD dynamically, on the command line. Because, the memory attributes table would *always* be safe to install, so we should just stick, in OVMF, with the TRUE default for the PCD, from MdeModulePkg.dec. >>> >>> I do understand now that the memory attributes table is an additional feature. It will always be exposed to the OS. *Independently*, the properties table will continue to behave the same as before -- if the PCD is TRUE, then it will be installed; if the PCD is FALSE, then it won't. >>> >>> In the end, there's nothing to do under OvmfPkg/. We'll let the QEMU user control the properties table same as before (defaulting to FALSE), and orthogonally, the new memory attributes table will always be installed (with your patches in place), because it's safe. >>> >>> Thanks, and sorry about the confusion. >>> Laszlo >>> >>>> >>>> >>>> >>>> Does it sound reasonable to you? >>>> >>>> Thanks! >>>> Laszlo >>>> >>>>> >>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>> Signed-off-by: "Yao, Jiewen" >>>>> Cc: "Gao, Liming" >>>>> >>>>> jiewen yao (7): >>>>> MdePkg: Add UEFI2.6 MemoryAttributes Table definition. >>>>> MdePkg: Add UEFI2.6 MemoryAttributesTable GUID >>>>> MdeModulePkg: Add MemoryAttributesTable generation. >>>>> MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. >>>>> MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header >>>>> file. >>>>> MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. >>>>> MdePkg: Update DxeCore INF for MemoryAttributesTable. >>>>> >>>>> MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- >>>>> MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- >>>>> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- >>>>> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ >>>>> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- >>>>> MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ >>>>> MdePkg/MdePkg.dec | 11 +- >>>>> 7 files changed, 362 insertions(+), 15 deletions(-) create mode >>>>> 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c >>>>> create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h >>>>> >>>> >>> >>> _______________________________________________ >>> 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 --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta04.collab.prod.int.phx2.redhat.com (LHLO zmta04.collab.prod.int.phx2.redhat.com) (10.5.81.11) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Thu, 18 Feb 2016 12:44:36 -0500 (EST) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by zmta04.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id A710DD0751; Thu, 18 Feb 2016 12:44:36 -0500 (EST) Received: from mx1.redhat.com (ext-mx08.extmail.prod.ext.phx2.redhat.com [10.5.110.32]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1IHiaSV001422 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 18 Feb 2016 12:44:36 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id 0A8A8C057EC2; Thu, 18 Feb 2016 17:44:35 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id B70E01A1E2D; Thu, 18 Feb 2016 09:44:33 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mail-io0-x233.google.com (mail-io0-x233.google.com [IPv6:2607:f8b0:4001:c06::233]) (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 CC3A91A1E2A for ; Thu, 18 Feb 2016 09:44:31 -0800 (PST) Received: by mail-io0-x233.google.com with SMTP id g203so81818346iof.2 for ; Thu, 18 Feb 2016 09:44:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=JjJFlJQz7Sw85z3Yt8i0AwVBJFqvxSBLeUgNkQkm7g0=; b=A1Vaw8qZ9NUtmzB1mDGQ60s3slfJGF98jMuaLdYDH5/LkpQ0ZqQu2hQjJgRfErlCPr y5scs8hhueGilgg+4W7WWePczJjLdzvGlJFr05tyGELMjZLXRXkYBJXPu3nVv/2ZCPIx 8okxKOytlHJjpG4YHi2sCn+WGiORfrMWU5PU8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=JjJFlJQz7Sw85z3Yt8i0AwVBJFqvxSBLeUgNkQkm7g0=; b=fqh778wFE+SKP65ij3Py/L2oaHk9XlaZKKkJ1SvHTR+iXhFjdjkFnRbCfVUI7FXSwX eSQ/bxEluA7bE60V4kc0kgayyje6cKenSsScv85xI0P/nfvrGJycxoFese9dym6wF51H oZBtLI7AXKjWQyc1kTBsVBjpJ5AY1pN1M7MAzugdW+aQ7JYLAlFPNCCVbVAKKBThPdgy h058o9GTs3igJkIOU8TaG8DZpzACYEWt/sWwdQstW/a46/YbuYyaNlYr9rQBsRZHAcBF ByQPj8qrMJMFlYQnY20wTvuyXmOYh4dO0gI2RnGQ3iwNy4dAqfGNufDy3W07+ZmiYCeG PrzQ== X-Gm-Message-State: AG10YOQ9OoRbY6TLwbetD3Tj/77zTwfVYfx3tH3Db7+0jfJnAfnpdHGc6aV4jzPN9ONgBY/vuPWTb0Y4R2wT3NR1 MIME-Version: 1.0 X-Received: by 10.107.135.20 with SMTP id j20mr11975767iod.56.1455817471094; Thu, 18 Feb 2016 09:44:31 -0800 (PST) Received: by 10.36.29.6 with HTTP; Thu, 18 Feb 2016 09:44:30 -0800 (PST) In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C5002759103@shsmsx102.ccr.corp.intel.com> References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> <56AE7040.4060004@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002759103@shsmsx102.ccr.corp.intel.com> Date: Thu, 18 Feb 2016 18:44:30 +0100 Message-ID: From: Ard Biesheuvel To: "Yao, Jiewen" Cc: "edk2-devel@ml01.01.org" , Laszlo Ersek , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.495 (BAYES_50,DCC_REPUT_00_12,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,T_DKIM_INVALID) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Scanned-By: MIMEDefang 2.78 on 10.5.110.32 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 14 February 2016 at 06:44, Yao, Jiewen wrote: > HI > Thanks to discuss this properties table issue. > The purpose of this patch is to *add* UEFI2.6 memory attributes table. This patch does not handle any UEFI2.5 properties table. > If we want to change EDKII code for properties table, I suggest we separate it from this patch. > > Is there any comment for adding UEFI2.6 memory attributes table? > I am running into a problem with these patches. According to the spec, each entry in the Memory Attributes table shall have the same type as the region it was carved out of in the UEFI memory map. However, when I dump the table from Linux, it looks something like UEFI memory map: 0x00013c0e0000-0x00013c15ffff [Runtime Code |RUN| | | | | ] 0x00013c160000-0x00013c17ffff [Runtime Data |RUN| | | | | ] 0x00013c1a0000-0x00013c1dffff [Runtime Code |RUN| | | | | ] 0x00013c1e0000-0x00013c27ffff [Runtime Data |RUN| | | | | ] 0x00013c280000-0x00013c2cffff [Runtime Code |RUN| | | | | ] 0x00013c2d0000-0x00013c31ffff [Runtime Data |RUN| | | | | ] 0x00013c320000-0x00013c36ffff [Runtime Code |RUN| | | | | ] 0x00013f650000-0x00013f6dffff [Runtime Code |RUN| | | | | ] 0x00013f6f0000-0x00013f80ffff [Runtime Data |RUN| | | | | ] Memory Attributes Table: 0x00013c0e0000-0x00013c0effff [Runtime Data |RUN| |XP| | | ] 0x00013c0f0000-0x00013c0fffff [Runtime Code |RUN| | | | |RO] 0x00013c100000-0x00013c12ffff [Runtime Data |RUN| |XP| | | ] 0x00013c130000-0x00013c13ffff [Runtime Code |RUN| | | | |RO] 0x00013c140000-0x00013c15ffff [Runtime Data |RUN| |XP| | | ] 0x00013c160000-0x00013c17ffff [Runtime Data |RUN| |XP| | | ] 0x00013c1a0000-0x00013c1affff [Runtime Data |RUN| |XP| | | ] 0x00013c1b0000-0x00013c1bffff [Runtime Code |RUN| | | | |RO] 0x00013c1c0000-0x00013c1dffff [Runtime Data |RUN| |XP| | | ] 0x00013c1e0000-0x00013c27ffff [Runtime Data |RUN| |XP| | | ] 0x00013c280000-0x00013c28ffff [Runtime Data |RUN| |XP| | | ] 0x00013c290000-0x00013c29ffff [Runtime Code |RUN| | | | |RO] 0x00013c2a0000-0x00013c2cffff [Runtime Data |RUN| |XP| | | ] 0x00013c2d0000-0x00013c31ffff [Runtime Data |RUN| |XP| | | ] 0x00013c320000-0x00013c32ffff [Runtime Data |RUN| |XP| | | ] 0x00013c330000-0x00013c33ffff [Runtime Code |RUN| | | | |RO] 0x00013c340000-0x00013c36ffff [Runtime Data |RUN| |XP| | | ] 0x00013f650000-0x00013f65ffff [Runtime Data |RUN| |XP| | | ] 0x00013f660000-0x00013f66ffff [Runtime Code |RUN| | | | |RO] 0x00013f670000-0x00013f69ffff [Runtime Data |RUN| |XP| | | ] 0x00013f6a0000-0x00013f6affff [Runtime Code |RUN| | | | |RO] 0x00013f6b0000-0x00013f6dffff [Runtime Data |RUN| |XP| | | ] 0x00013f6f0000-0x00013f80ffff [Runtime Data |RUN| |XP| | | ] So what you see here is that the entry types in the Memory Attribute table are set according to the RO/XP attribute, and not according to the original region in the UEFI memory map. In other words, what I would expect here is that the first five entries are all 'Runtime Code', with the same permissions as are listed currently. Literally, the spec describes this requirement as follows: """ Irrespective of the memory protections implied by Attribute, the EFI_MEMORY_DESCRIPTOR.Type field should match the type of the memory in enclosing SetMemoryMap() entry. """ > > For UEFI2.5 properties table, let me summarize. Currently, we have several options: > 0) Keep it as is. Current setting is PcdPropertiesTableEnable TRUE in MdeModulePkg.DEC. > 1) Update PcdPropertiesTableEnable to FALSE in XXXPlatform.DSC file. (Platform choice. It can be static PCD or dynamic PCD.) > 2) Update PcdPropertiesTableEnable to FALSE in MdeModulePkg.DEC file. (Disable Default. BIOS will not publish this table by default. If platform wants this table, it can set PCD to true.) > 3) Remove PcdPropertiesTableEnable, and remove PropertiesTable support code in DxeCore. (PropertiesTable support is removed.) > > Personally, I do not suggest we choose 3), because this table is still defined in UEFI specification. We can remove it after UEFI spec removes it later. > I do not have strong opinion for other options. > > Thank you > Yao Jiewen > > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Monday, February 01, 2016 2:28 PM > To: Laszlo Ersek > Cc: Yao, Jiewen; edk2-devel@ml01.01.org; Gao, Liming > Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. > > On 31 January 2016 at 21:36, Laszlo Ersek wrote: >> On 01/30/16 11:25, Ard Biesheuvel wrote: >>> On 30 January 2016 at 04:17, Yao, Jiewen wrote: >>>> Thanks for the clarification. I think you are right. >>>> >>>> I said "This table is used to retire old PropertiesTable", because UEFI2.6 does not recommend using PropertiesTable to report RT information. >>>> The future BIOS/OS should always produce/consume MemoryAttributesTable as better solution. >>>> >>> >>> I strongly feel we should remove the original PropertiesTable. It >>> breaks backward compatibility, and was replaced by the >>> MemoryAttributesTable for a good reason. The OS side will not be >>> implemented in Linux (i.e., it will ignore the RO and XP attributes >>> in the standard UEFI memory map), and the only reason the >>> PropertiesTable was not removed entirely from the specification is >>> because of business interests of one of the participating OS vendors. >>> >>> I understand that we need to keep the underlying machinery to produce >>> either table. But we should remove the functionality that results in >>> each PE/COFF image to be split into several regions marked RO or XP >>> in the ordinary UEFI memory map. >> >> We can mitigate this by setting PcdPropertiesTableEnable to FALSE in >> "ArmVirt.dsc.inc" as well. >> > > No, we should remove it. > >> (Hmmm, I forget why SVN r18140 and r18566 don't already result in a >> problematic memmap for Linux... Probably due to your kernel commit >> 0ce3cc008ec0.) >> > > It will result in a problematic memmap for older Linux kernels but also for less recent versions of Windows 8, for instance. And the only OS version that actually consumes the properties table is Windows 10, which will we updated in the future to use the MemoryAttributesTable instead. > > >>>> -----Original Message----- >>>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>>> Sent: Saturday, January 30, 2016 10:24 AM >>>> To: Yao, Jiewen >>>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>>> >>>> On 01/30/16 01:31, Yao, Jiewen wrote: >>>>> Comment below: >>>>> >>>>> -----Original Message----- >>>>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>>>> Sent: Saturday, January 30, 2016 1:13 AM >>>>> To: Yao, Jiewen >>>>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>>>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>>>> >>>>> Hello Jiewen, >>>>> >>>>> On 01/29/16 13:12, jiewen yao wrote: >>>>>> This series patches add UEFI2.6 MemoryAttributesTable support. >>>>>> This table is used to retire old PropertiesTable. >>>>>> This is standalone table published by DxeCore, so there is no >>>>>> compatibility issue. >>>>>> >>>>>> The patch is validated with or without properties table. >>>>> >>>>> Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE? >>>>> [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to FALSE or TRUE, MemoryAttributesTable is always published. >>>>> That is one compatibility we want to maintain. >>>>> >>>>> >>>>> I skimmed the commit messages, and it looks like the properties table is preserved, but now it serves only as foundation for the memory attributes table. >>>>> [Jiewen] Yes, the DxeCore global variable - properties table is always preserved. And the system UEFI configuration table for properties table is controlled by PcdPropertiestableEnable. >>>>> No matter properties table or memory attributes table, we always need a flag to record if RT image is 4K aligned or not, so I just choose the original global variable. >>>>> >>>>> >>>>> In OVMF we set PcdPropertiesTableEnable to FALSE by default, but flip it dynamically to TRUE in PlatformPei, if the user requests the feature on the QEMU command line. This is being done for avoiding regressions with OSes that don't know about the properties table, and its impact on the UEFI memmap. >>>>> [Jiewen] That is good design. I believe most real platforms do similar thing. >>>>> >>>>> >>>>> However, if the memory attributes table is safe for OSes that don't >>>>> know about it, I think we can eliminate the above "FALSE" default, >>>>> and dynamism, from OVMF, and just inherit the >>>>> PcdPropertiesTableEnable=TRUE setting from "MdeModulePkg.dec". >>>>> [Jiewen] I am sorry that I am not clear on your suggestion. What do you mean "eliminate the above "FALSE" default, and dynamism, from OVMF"? >>>>> Do you mean you suggest to change PcdPropertiesTableEnable=FALSE as default value in MdeModulePkg.dec file? >>>>> Or do you want to change OVMF.dsc file? >>>> >>>> Sorry, I should have educated myself on the memory attributes table first, in UEFI 2.6. I have now carefully read the EFI_MEMORY_ATTRIBUTES_TABLE section, and I've also noticed the new last paragraph in the EFI_PROPERTIES_TABLE section. >>>> >>>> What I was missing when I asked my question is that EFI_MEMORY_ATTRIBUTES_TABLE was an *additional* feature. >>>> >>>> Your cover letter said "This table is used to retire old PropertiesTable". So I thought, the OS-visible properties table would be completely removed from edk2, and the DXE core internals would only be reused for computing the new memory attributes table. In other words, I thought that the memory attributes table would *replace* the properties table, and the original PcdPropertiesTableEnable PCD would now control the presence of the memory attributes table instead. >>>> >>>> This is why I asked if we should change OVMF to remove its FALSE default for the PCD, alongside the possibility for the QEMU user to change the PCD dynamically, on the command line. Because, the memory attributes table would *always* be safe to install, so we should just stick, in OVMF, with the TRUE default for the PCD, from MdeModulePkg.dec. >>>> >>>> I do understand now that the memory attributes table is an additional feature. It will always be exposed to the OS. *Independently*, the properties table will continue to behave the same as before -- if the PCD is TRUE, then it will be installed; if the PCD is FALSE, then it won't. >>>> >>>> In the end, there's nothing to do under OvmfPkg/. We'll let the QEMU user control the properties table same as before (defaulting to FALSE), and orthogonally, the new memory attributes table will always be installed (with your patches in place), because it's safe. >>>> >>>> Thanks, and sorry about the confusion. >>>> Laszlo >>>> >>>>> >>>>> >>>>> >>>>> Does it sound reasonable to you? >>>>> >>>>> Thanks! >>>>> Laszlo >>>>> >>>>>> >>>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>>> Signed-off-by: "Yao, Jiewen" >>>>>> Cc: "Gao, Liming" >>>>>> >>>>>> jiewen yao (7): >>>>>> MdePkg: Add UEFI2.6 MemoryAttributes Table definition. >>>>>> MdePkg: Add UEFI2.6 MemoryAttributesTable GUID >>>>>> MdeModulePkg: Add MemoryAttributesTable generation. >>>>>> MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. >>>>>> MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header >>>>>> file. >>>>>> MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. >>>>>> MdePkg: Update DxeCore INF for MemoryAttributesTable. >>>>>> >>>>>> MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- >>>>>> MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- >>>>>> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- >>>>>> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ >>>>>> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- >>>>>> MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ >>>>>> MdePkg/MdePkg.dec | 11 +- >>>>>> 7 files changed, 362 insertions(+), 15 deletions(-) create mode >>>>>> 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c >>>>>> create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h >>>>>> >>>>> >>>> >>>> _______________________________________________ >>>> 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 --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta04.collab.prod.int.phx2.redhat.com (LHLO zmta04.collab.prod.int.phx2.redhat.com) (10.5.81.11) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Thu, 18 Feb 2016 19:43:26 -0500 (EST) Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by zmta04.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id 5F7EED1623; Thu, 18 Feb 2016 19:43:26 -0500 (EST) Received: from mx1.redhat.com (ext-mx07.extmail.prod.ext.phx2.redhat.com [10.5.110.31]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1J0hQZf019927 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 18 Feb 2016 19:43:26 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id CA3E2C00FBA5; Fri, 19 Feb 2016 00:43:24 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 7F5B11A1E51; Thu, 18 Feb 2016 16:43:23 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by ml01.01.org (Postfix) with ESMTP id 0CB331A1E27 for ; Thu, 18 Feb 2016 16:43:22 -0800 (PST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga103.jf.intel.com with ESMTP; 18 Feb 2016 16:43:21 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,468,1449561600"; d="scan'208";a="749051242" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga003.jf.intel.com with ESMTP; 18 Feb 2016 16:43:21 -0800 Received: from fmsmsx151.amr.corp.intel.com (10.18.125.4) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 18 Feb 2016 16:43:21 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX151.amr.corp.intel.com (10.18.125.4) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 18 Feb 2016 16:43:20 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.172]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.4]) with mapi id 14.03.0248.002; Fri, 19 Feb 2016 08:43:12 +0800 From: "Yao, Jiewen" To: Ard Biesheuvel Thread-Topic: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. Thread-Index: AQHRWo5UfnwfB+DE8USx6LXBJH6a7Z8SNVqAgAD+OSD//5vFAIAAk1RQ///zJICAAj0HAIAApUWAgBTkw9CABo/xAIAA+pAw Date: Fri, 19 Feb 2016 00:43:12 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C500275CB21@shsmsx102.ccr.corp.intel.com> References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> <56AE7040.4060004@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002759103@shsmsx102.ccr.corp.intel.com> In-Reply-To: Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiY2FiYzU4Y2MtYjNjMS00YWZmLTk3NzAtMmE0ZjA1MzE5MTdjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6InFycko3V0dcL0RHZTNpaDJqSFwvWnE2aXQyc1RzcG13XC9LOGFyK0FlQTg1S009In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Cc: "Yao, Jiewen" , "edk2-devel@ml01.01.org" , Laszlo Ersek , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.385 (BAYES_50,DCC_REPUT_00_12,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Scanned-By: MIMEDefang 2.78 on 10.5.110.31 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Thanks. It seems my misunderstanding. Just want to double confirm: It is expected to see memory attribute table like below. Right? 0x00013c0e0000-0x00013c0effff [Runtime Code |RUN| |XP| | | ] 0x00013c0f0000-0x00013c0fffff [Runtime Code |RUN| | | | |RO] 0x00013c100000-0x00013c12ffff [Runtime Code |RUN| |XP| | | ] 0x00013c130000-0x00013c13ffff [Runtime Code |RUN| | | | |RO] 0x00013c140000-0x00013c15ffff [Runtime Code |RUN| |XP| | | ] 0x00013c160000-0x00013c17ffff [Runtime Data |RUN| |XP| | | ] 0x00013c1a0000-0x00013c1affff [Runtime Code |RUN| |XP| | | ] 0x00013c1b0000-0x00013c1bffff [Runtime Code |RUN| | | | |RO] 0x00013c1c0000-0x00013c1dffff [Runtime Code |RUN| |XP| | | ] 0x00013c1e0000-0x00013c27ffff [Runtime Data |RUN| |XP| | | ] 0x00013c280000-0x00013c28ffff [Runtime Code |RUN| |XP| | | ] 0x00013c290000-0x00013c29ffff [Runtime Code |RUN| | | | |RO] 0x00013c2a0000-0x00013c2cffff [Runtime Code |RUN| |XP| | | ] 0x00013c2d0000-0x00013c31ffff [Runtime Data |RUN| |XP| | | ] 0x00013c320000-0x00013c32ffff [Runtime Code |RUN| |XP| | | ] 0x00013c330000-0x00013c33ffff [Runtime Code |RUN| | | | |RO] 0x00013c340000-0x00013c36ffff [Runtime Code |RUN| |XP| | | ] 0x00013f650000-0x00013f65ffff [Runtime Code |RUN| |XP| | | ] 0x00013f660000-0x00013f66ffff [Runtime Code |RUN| | | | |RO] 0x00013f670000-0x00013f69ffff [Runtime Code |RUN| |XP| | | ] 0x00013f6a0000-0x00013f6affff [Runtime Code |RUN| | | | |RO] 0x00013f6b0000-0x00013f6dffff [Runtime Code |RUN| |XP| | | ] 0x00013f6f0000-0x00013f80ffff [Runtime Data |RUN| |XP| | | ] -----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] Sent: Friday, February 19, 2016 1:44 AM To: Yao, Jiewen Cc: Laszlo Ersek; edk2-devel@ml01.01.org; Gao, Liming Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. On 14 February 2016 at 06:44, Yao, Jiewen wrote: > HI > Thanks to discuss this properties table issue. > The purpose of this patch is to *add* UEFI2.6 memory attributes table. This patch does not handle any UEFI2.5 properties table. > If we want to change EDKII code for properties table, I suggest we separate it from this patch. > > Is there any comment for adding UEFI2.6 memory attributes table? > I am running into a problem with these patches. According to the spec, each entry in the Memory Attributes table shall have the same type as the region it was carved out of in the UEFI memory map. However, when I dump the table from Linux, it looks something like UEFI memory map: 0x00013c0e0000-0x00013c15ffff [Runtime Code |RUN| | | | | ] 0x00013c160000-0x00013c17ffff [Runtime Data |RUN| | | | | ] 0x00013c1a0000-0x00013c1dffff [Runtime Code |RUN| | | | | ] 0x00013c1e0000-0x00013c27ffff [Runtime Data |RUN| | | | | ] 0x00013c280000-0x00013c2cffff [Runtime Code |RUN| | | | | ] 0x00013c2d0000-0x00013c31ffff [Runtime Data |RUN| | | | | ] 0x00013c320000-0x00013c36ffff [Runtime Code |RUN| | | | | ] 0x00013f650000-0x00013f6dffff [Runtime Code |RUN| | | | | ] 0x00013f6f0000-0x00013f80ffff [Runtime Data |RUN| | | | | ] Memory Attributes Table: 0x00013c0e0000-0x00013c0effff [Runtime Data |RUN| |XP| | | ] 0x00013c0f0000-0x00013c0fffff [Runtime Code |RUN| | | | |RO] 0x00013c100000-0x00013c12ffff [Runtime Data |RUN| |XP| | | ] 0x00013c130000-0x00013c13ffff [Runtime Code |RUN| | | | |RO] 0x00013c140000-0x00013c15ffff [Runtime Data |RUN| |XP| | | ] 0x00013c160000-0x00013c17ffff [Runtime Data |RUN| |XP| | | ] 0x00013c1a0000-0x00013c1affff [Runtime Data |RUN| |XP| | | ] 0x00013c1b0000-0x00013c1bffff [Runtime Code |RUN| | | | |RO] 0x00013c1c0000-0x00013c1dffff [Runtime Data |RUN| |XP| | | ] 0x00013c1e0000-0x00013c27ffff [Runtime Data |RUN| |XP| | | ] 0x00013c280000-0x00013c28ffff [Runtime Data |RUN| |XP| | | ] 0x00013c290000-0x00013c29ffff [Runtime Code |RUN| | | | |RO] 0x00013c2a0000-0x00013c2cffff [Runtime Data |RUN| |XP| | | ] 0x00013c2d0000-0x00013c31ffff [Runtime Data |RUN| |XP| | | ] 0x00013c320000-0x00013c32ffff [Runtime Data |RUN| |XP| | | ] 0x00013c330000-0x00013c33ffff [Runtime Code |RUN| | | | |RO] 0x00013c340000-0x00013c36ffff [Runtime Data |RUN| |XP| | | ] 0x00013f650000-0x00013f65ffff [Runtime Data |RUN| |XP| | | ] 0x00013f660000-0x00013f66ffff [Runtime Code |RUN| | | | |RO] 0x00013f670000-0x00013f69ffff [Runtime Data |RUN| |XP| | | ] 0x00013f6a0000-0x00013f6affff [Runtime Code |RUN| | | | |RO] 0x00013f6b0000-0x00013f6dffff [Runtime Data |RUN| |XP| | | ] 0x00013f6f0000-0x00013f80ffff [Runtime Data |RUN| |XP| | | ] So what you see here is that the entry types in the Memory Attribute table are set according to the RO/XP attribute, and not according to the original region in the UEFI memory map. In other words, what I would expect here is that the first five entries are all 'Runtime Code', with the same permissions as are listed currently. Literally, the spec describes this requirement as follows: """ Irrespective of the memory protections implied by Attribute, the EFI_MEMORY_DESCRIPTOR.Type field should match the type of the memory in enclosing SetMemoryMap() entry. """ > > For UEFI2.5 properties table, let me summarize. Currently, we have several options: > 0) Keep it as is. Current setting is PcdPropertiesTableEnable TRUE in MdeModulePkg.DEC. > 1) Update PcdPropertiesTableEnable to FALSE in XXXPlatform.DSC file. > (Platform choice. It can be static PCD or dynamic PCD.) > 2) Update PcdPropertiesTableEnable to FALSE in MdeModulePkg.DEC file. > (Disable Default. BIOS will not publish this table by default. If > platform wants this table, it can set PCD to true.) > 3) Remove PcdPropertiesTableEnable, and remove PropertiesTable support > code in DxeCore. (PropertiesTable support is removed.) > > Personally, I do not suggest we choose 3), because this table is still defined in UEFI specification. We can remove it after UEFI spec removes it later. > I do not have strong opinion for other options. > > Thank you > Yao Jiewen > > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Monday, February 01, 2016 2:28 PM > To: Laszlo Ersek > Cc: Yao, Jiewen; edk2-devel@ml01.01.org; Gao, Liming > Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. > > On 31 January 2016 at 21:36, Laszlo Ersek wrote: >> On 01/30/16 11:25, Ard Biesheuvel wrote: >>> On 30 January 2016 at 04:17, Yao, Jiewen wrote: >>>> Thanks for the clarification. I think you are right. >>>> >>>> I said "This table is used to retire old PropertiesTable", because UEFI2.6 does not recommend using PropertiesTable to report RT information. >>>> The future BIOS/OS should always produce/consume MemoryAttributesTable as better solution. >>>> >>> >>> I strongly feel we should remove the original PropertiesTable. It >>> breaks backward compatibility, and was replaced by the >>> MemoryAttributesTable for a good reason. The OS side will not be >>> implemented in Linux (i.e., it will ignore the RO and XP attributes >>> in the standard UEFI memory map), and the only reason the >>> PropertiesTable was not removed entirely from the specification is >>> because of business interests of one of the participating OS vendors. >>> >>> I understand that we need to keep the underlying machinery to >>> produce either table. But we should remove the functionality that >>> results in each PE/COFF image to be split into several regions >>> marked RO or XP in the ordinary UEFI memory map. >> >> We can mitigate this by setting PcdPropertiesTableEnable to FALSE in >> "ArmVirt.dsc.inc" as well. >> > > No, we should remove it. > >> (Hmmm, I forget why SVN r18140 and r18566 don't already result in a >> problematic memmap for Linux... Probably due to your kernel commit >> 0ce3cc008ec0.) >> > > It will result in a problematic memmap for older Linux kernels but also for less recent versions of Windows 8, for instance. And the only OS version that actually consumes the properties table is Windows 10, which will we updated in the future to use the MemoryAttributesTable instead. > > >>>> -----Original Message----- >>>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>>> Sent: Saturday, January 30, 2016 10:24 AM >>>> To: Yao, Jiewen >>>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>>> >>>> On 01/30/16 01:31, Yao, Jiewen wrote: >>>>> Comment below: >>>>> >>>>> -----Original Message----- >>>>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>>>> Sent: Saturday, January 30, 2016 1:13 AM >>>>> To: Yao, Jiewen >>>>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>>>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>>>> >>>>> Hello Jiewen, >>>>> >>>>> On 01/29/16 13:12, jiewen yao wrote: >>>>>> This series patches add UEFI2.6 MemoryAttributesTable support. >>>>>> This table is used to retire old PropertiesTable. >>>>>> This is standalone table published by DxeCore, so there is no >>>>>> compatibility issue. >>>>>> >>>>>> The patch is validated with or without properties table. >>>>> >>>>> Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE? >>>>> [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to FALSE or TRUE, MemoryAttributesTable is always published. >>>>> That is one compatibility we want to maintain. >>>>> >>>>> >>>>> I skimmed the commit messages, and it looks like the properties table is preserved, but now it serves only as foundation for the memory attributes table. >>>>> [Jiewen] Yes, the DxeCore global variable - properties table is always preserved. And the system UEFI configuration table for properties table is controlled by PcdPropertiestableEnable. >>>>> No matter properties table or memory attributes table, we always need a flag to record if RT image is 4K aligned or not, so I just choose the original global variable. >>>>> >>>>> >>>>> In OVMF we set PcdPropertiesTableEnable to FALSE by default, but flip it dynamically to TRUE in PlatformPei, if the user requests the feature on the QEMU command line. This is being done for avoiding regressions with OSes that don't know about the properties table, and its impact on the UEFI memmap. >>>>> [Jiewen] That is good design. I believe most real platforms do similar thing. >>>>> >>>>> >>>>> However, if the memory attributes table is safe for OSes that >>>>> don't know about it, I think we can eliminate the above "FALSE" >>>>> default, and dynamism, from OVMF, and just inherit the >>>>> PcdPropertiesTableEnable=TRUE setting from "MdeModulePkg.dec". >>>>> [Jiewen] I am sorry that I am not clear on your suggestion. What do you mean "eliminate the above "FALSE" default, and dynamism, from OVMF"? >>>>> Do you mean you suggest to change PcdPropertiesTableEnable=FALSE as default value in MdeModulePkg.dec file? >>>>> Or do you want to change OVMF.dsc file? >>>> >>>> Sorry, I should have educated myself on the memory attributes table first, in UEFI 2.6. I have now carefully read the EFI_MEMORY_ATTRIBUTES_TABLE section, and I've also noticed the new last paragraph in the EFI_PROPERTIES_TABLE section. >>>> >>>> What I was missing when I asked my question is that EFI_MEMORY_ATTRIBUTES_TABLE was an *additional* feature. >>>> >>>> Your cover letter said "This table is used to retire old PropertiesTable". So I thought, the OS-visible properties table would be completely removed from edk2, and the DXE core internals would only be reused for computing the new memory attributes table. In other words, I thought that the memory attributes table would *replace* the properties table, and the original PcdPropertiesTableEnable PCD would now control the presence of the memory attributes table instead. >>>> >>>> This is why I asked if we should change OVMF to remove its FALSE default for the PCD, alongside the possibility for the QEMU user to change the PCD dynamically, on the command line. Because, the memory attributes table would *always* be safe to install, so we should just stick, in OVMF, with the TRUE default for the PCD, from MdeModulePkg.dec. >>>> >>>> I do understand now that the memory attributes table is an additional feature. It will always be exposed to the OS. *Independently*, the properties table will continue to behave the same as before -- if the PCD is TRUE, then it will be installed; if the PCD is FALSE, then it won't. >>>> >>>> In the end, there's nothing to do under OvmfPkg/. We'll let the QEMU user control the properties table same as before (defaulting to FALSE), and orthogonally, the new memory attributes table will always be installed (with your patches in place), because it's safe. >>>> >>>> Thanks, and sorry about the confusion. >>>> Laszlo >>>> >>>>> >>>>> >>>>> >>>>> Does it sound reasonable to you? >>>>> >>>>> Thanks! >>>>> Laszlo >>>>> >>>>>> >>>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>>> Signed-off-by: "Yao, Jiewen" >>>>>> Cc: "Gao, Liming" >>>>>> >>>>>> jiewen yao (7): >>>>>> MdePkg: Add UEFI2.6 MemoryAttributes Table definition. >>>>>> MdePkg: Add UEFI2.6 MemoryAttributesTable GUID >>>>>> MdeModulePkg: Add MemoryAttributesTable generation. >>>>>> MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. >>>>>> MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header >>>>>> file. >>>>>> MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. >>>>>> MdePkg: Update DxeCore INF for MemoryAttributesTable. >>>>>> >>>>>> MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- >>>>>> MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- >>>>>> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- >>>>>> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ >>>>>> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- >>>>>> MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ >>>>>> MdePkg/MdePkg.dec | 11 +- >>>>>> 7 files changed, 362 insertions(+), 15 deletions(-) create mode >>>>>> 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c >>>>>> create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h >>>>>> >>>>> >>>> >>>> _______________________________________________ >>>> 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 --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta04.collab.prod.int.phx2.redhat.com (LHLO zmta04.collab.prod.int.phx2.redhat.com) (10.5.81.11) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Fri, 19 Feb 2016 01:27:06 -0500 (EST) Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by zmta04.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id B34D9DA0F1; Fri, 19 Feb 2016 01:27:06 -0500 (EST) Received: from mx1.redhat.com (ext-mx07.extmail.prod.ext.phx2.redhat.com [10.5.110.31]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1J6R6FV021710 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 19 Feb 2016 01:27:06 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id C5651C0006F8; Fri, 19 Feb 2016 06:27:04 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 9D8931A1E69; Thu, 18 Feb 2016 22:27:03 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mail-io0-x22c.google.com (mail-io0-x22c.google.com [IPv6:2607:f8b0:4001:c06::22c]) (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 B34501A1E66 for ; Thu, 18 Feb 2016 22:27:01 -0800 (PST) Received: by mail-io0-x22c.google.com with SMTP id 9so99552318iom.1 for ; Thu, 18 Feb 2016 22:27:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=zxZXImo9WXIKhOZN4t1otu1UM5ZBzjAYCN9dgzd1zZY=; b=boA0Hv/rP2/sZ+OD8o4DVn4GFxbInc6HdAm1paGWTDi5tPjT89/uuH0/jmypefDmQU yZs4DBfwfqhs4vChJiRgocz53/VtNeIXwYEkdNGlXVo2slKKIEJkJuWUpSMfvrLoVN9T Iw6Lj+lOl8Xf+VhWNYBaW99yCj9fUtdkB0U44= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=zxZXImo9WXIKhOZN4t1otu1UM5ZBzjAYCN9dgzd1zZY=; b=OqOQrtLeDCQMkfsSw+Dk1S7fjcTCyxyL9PeGiRAD+IQksh9I4vzrRyxfi90veK0JV8 1lZwqiDHmMyVqdDLKbwefLmJJRVv1v0CvzRoyyFRmA2L5R/iJieAVy7C3wnmiDQ9DISB FaYRnUe9TJjBmocfs6SVMnrKEdSv9dxs4ReaeEgnoeidgWJdUcJLqAqwb/q6oxQEs7b3 IEKRVhPvGqKSinxu82tSXC6XOzl7veju9ynwPf/N6e7Vj1GF6ZuX4HGHcc9fU63Bl6K0 ZiChLhULOnp0qsPfSmLJzGZ67I2E8e2up8GEQZPkh15sPT4npIj9RBVIGqiz7VBKVIax d6Zg== X-Gm-Message-State: AG10YOQz/n2x7cWKjImL4Ia4gVeMvymXITRRevsT3q7rMsM+3nItA/9qNTrBiFaRqn/HHpSBq97TtgnamA74HKFb MIME-Version: 1.0 X-Received: by 10.107.18.199 with SMTP id 68mr12655676ios.130.1455863221075; Thu, 18 Feb 2016 22:27:01 -0800 (PST) Received: by 10.36.29.6 with HTTP; Thu, 18 Feb 2016 22:27:00 -0800 (PST) In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C500275CB21@shsmsx102.ccr.corp.intel.com> References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> <56AE7040.4060004@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002759103@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C500275CB21@shsmsx102.ccr.corp.intel.com> Date: Fri, 19 Feb 2016 07:27:00 +0100 Message-ID: From: Ard Biesheuvel To: "Yao, Jiewen" Cc: "edk2-devel@ml01.01.org" , Laszlo Ersek , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.495 (BAYES_50,DCC_REPUT_00_12,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,T_DKIM_INVALID) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Scanned-By: MIMEDefang 2.78 on 10.5.110.31 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 19 February 2016 at 01:43, Yao, Jiewen wrote: > Thanks. It seems my misunderstanding. > > Just want to double confirm: It is expected to see memory attribute table like below. Right? > Correct. > 0x00013c0e0000-0x00013c0effff [Runtime Code |RUN| |XP| | | ] > 0x00013c0f0000-0x00013c0fffff [Runtime Code |RUN| | | | |RO] > 0x00013c100000-0x00013c12ffff [Runtime Code |RUN| |XP| | | ] > 0x00013c130000-0x00013c13ffff [Runtime Code |RUN| | | | |RO] > 0x00013c140000-0x00013c15ffff [Runtime Code |RUN| |XP| | | ] > > 0x00013c160000-0x00013c17ffff [Runtime Data |RUN| |XP| | | ] > > 0x00013c1a0000-0x00013c1affff [Runtime Code |RUN| |XP| | | ] > 0x00013c1b0000-0x00013c1bffff [Runtime Code |RUN| | | | |RO] > 0x00013c1c0000-0x00013c1dffff [Runtime Code |RUN| |XP| | | ] > > 0x00013c1e0000-0x00013c27ffff [Runtime Data |RUN| |XP| | | ] > > 0x00013c280000-0x00013c28ffff [Runtime Code |RUN| |XP| | | ] > 0x00013c290000-0x00013c29ffff [Runtime Code |RUN| | | | |RO] > 0x00013c2a0000-0x00013c2cffff [Runtime Code |RUN| |XP| | | ] > > 0x00013c2d0000-0x00013c31ffff [Runtime Data |RUN| |XP| | | ] > > 0x00013c320000-0x00013c32ffff [Runtime Code |RUN| |XP| | | ] > 0x00013c330000-0x00013c33ffff [Runtime Code |RUN| | | | |RO] > 0x00013c340000-0x00013c36ffff [Runtime Code |RUN| |XP| | | ] > > 0x00013f650000-0x00013f65ffff [Runtime Code |RUN| |XP| | | ] > 0x00013f660000-0x00013f66ffff [Runtime Code |RUN| | | | |RO] > 0x00013f670000-0x00013f69ffff [Runtime Code |RUN| |XP| | | ] > 0x00013f6a0000-0x00013f6affff [Runtime Code |RUN| | | | |RO] > 0x00013f6b0000-0x00013f6dffff [Runtime Code |RUN| |XP| | | ] > > 0x00013f6f0000-0x00013f80ffff [Runtime Data |RUN| |XP| | | ] > > > > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Friday, February 19, 2016 1:44 AM > To: Yao, Jiewen > Cc: Laszlo Ersek; edk2-devel@ml01.01.org; Gao, Liming > Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. > > On 14 February 2016 at 06:44, Yao, Jiewen wrote: >> HI >> Thanks to discuss this properties table issue. >> The purpose of this patch is to *add* UEFI2.6 memory attributes table. This patch does not handle any UEFI2.5 properties table. >> If we want to change EDKII code for properties table, I suggest we separate it from this patch. >> >> Is there any comment for adding UEFI2.6 memory attributes table? >> > > I am running into a problem with these patches. According to the spec, each entry in the Memory Attributes table shall have the same type as the region it was carved out of in the UEFI memory map. However, when I dump the table from Linux, it looks something like > > UEFI memory map: > 0x00013c0e0000-0x00013c15ffff [Runtime Code |RUN| | | | | ] > > 0x00013c160000-0x00013c17ffff [Runtime Data |RUN| | | | | ] > > 0x00013c1a0000-0x00013c1dffff [Runtime Code |RUN| | | | | ] > > 0x00013c1e0000-0x00013c27ffff [Runtime Data |RUN| | | | | ] > > 0x00013c280000-0x00013c2cffff [Runtime Code |RUN| | | | | ] > > 0x00013c2d0000-0x00013c31ffff [Runtime Data |RUN| | | | | ] > > 0x00013c320000-0x00013c36ffff [Runtime Code |RUN| | | | | ] > > 0x00013f650000-0x00013f6dffff [Runtime Code |RUN| | | | | ] > > 0x00013f6f0000-0x00013f80ffff [Runtime Data |RUN| | | | | ] > > > Memory Attributes Table: > 0x00013c0e0000-0x00013c0effff [Runtime Data |RUN| |XP| | | ] > 0x00013c0f0000-0x00013c0fffff [Runtime Code |RUN| | | | |RO] > 0x00013c100000-0x00013c12ffff [Runtime Data |RUN| |XP| | | ] > 0x00013c130000-0x00013c13ffff [Runtime Code |RUN| | | | |RO] > 0x00013c140000-0x00013c15ffff [Runtime Data |RUN| |XP| | | ] > > 0x00013c160000-0x00013c17ffff [Runtime Data |RUN| |XP| | | ] > > 0x00013c1a0000-0x00013c1affff [Runtime Data |RUN| |XP| | | ] > 0x00013c1b0000-0x00013c1bffff [Runtime Code |RUN| | | | |RO] > 0x00013c1c0000-0x00013c1dffff [Runtime Data |RUN| |XP| | | ] > > 0x00013c1e0000-0x00013c27ffff [Runtime Data |RUN| |XP| | | ] > > 0x00013c280000-0x00013c28ffff [Runtime Data |RUN| |XP| | | ] > 0x00013c290000-0x00013c29ffff [Runtime Code |RUN| | | | |RO] > 0x00013c2a0000-0x00013c2cffff [Runtime Data |RUN| |XP| | | ] > > 0x00013c2d0000-0x00013c31ffff [Runtime Data |RUN| |XP| | | ] > > 0x00013c320000-0x00013c32ffff [Runtime Data |RUN| |XP| | | ] > 0x00013c330000-0x00013c33ffff [Runtime Code |RUN| | | | |RO] > 0x00013c340000-0x00013c36ffff [Runtime Data |RUN| |XP| | | ] > > 0x00013f650000-0x00013f65ffff [Runtime Data |RUN| |XP| | | ] > 0x00013f660000-0x00013f66ffff [Runtime Code |RUN| | | | |RO] > 0x00013f670000-0x00013f69ffff [Runtime Data |RUN| |XP| | | ] > 0x00013f6a0000-0x00013f6affff [Runtime Code |RUN| | | | |RO] > 0x00013f6b0000-0x00013f6dffff [Runtime Data |RUN| |XP| | | ] > > 0x00013f6f0000-0x00013f80ffff [Runtime Data |RUN| |XP| | | ] > > So what you see here is that the entry types in the Memory Attribute table are set according to the RO/XP attribute, and not according to the original region in the UEFI memory map. > In other words, what I would expect here is that the first five entries are all 'Runtime Code', with the same permissions as are listed currently. > > Literally, the spec describes this requirement as follows: > """ > Irrespective of the memory protections implied by Attribute, the EFI_MEMORY_DESCRIPTOR.Type field should match the type of the memory in enclosing SetMemoryMap() entry. > """ > > > >> >> For UEFI2.5 properties table, let me summarize. Currently, we have several options: >> 0) Keep it as is. Current setting is PcdPropertiesTableEnable TRUE in MdeModulePkg.DEC. >> 1) Update PcdPropertiesTableEnable to FALSE in XXXPlatform.DSC file. >> (Platform choice. It can be static PCD or dynamic PCD.) >> 2) Update PcdPropertiesTableEnable to FALSE in MdeModulePkg.DEC file. >> (Disable Default. BIOS will not publish this table by default. If >> platform wants this table, it can set PCD to true.) >> 3) Remove PcdPropertiesTableEnable, and remove PropertiesTable support >> code in DxeCore. (PropertiesTable support is removed.) >> >> Personally, I do not suggest we choose 3), because this table is still defined in UEFI specification. We can remove it after UEFI spec removes it later. >> I do not have strong opinion for other options. >> >> Thank you >> Yao Jiewen >> >> -----Original Message----- >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >> Sent: Monday, February 01, 2016 2:28 PM >> To: Laszlo Ersek >> Cc: Yao, Jiewen; edk2-devel@ml01.01.org; Gao, Liming >> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >> >> On 31 January 2016 at 21:36, Laszlo Ersek wrote: >>> On 01/30/16 11:25, Ard Biesheuvel wrote: >>>> On 30 January 2016 at 04:17, Yao, Jiewen wrote: >>>>> Thanks for the clarification. I think you are right. >>>>> >>>>> I said "This table is used to retire old PropertiesTable", because UEFI2.6 does not recommend using PropertiesTable to report RT information. >>>>> The future BIOS/OS should always produce/consume MemoryAttributesTable as better solution. >>>>> >>>> >>>> I strongly feel we should remove the original PropertiesTable. It >>>> breaks backward compatibility, and was replaced by the >>>> MemoryAttributesTable for a good reason. The OS side will not be >>>> implemented in Linux (i.e., it will ignore the RO and XP attributes >>>> in the standard UEFI memory map), and the only reason the >>>> PropertiesTable was not removed entirely from the specification is >>>> because of business interests of one of the participating OS vendors. >>>> >>>> I understand that we need to keep the underlying machinery to >>>> produce either table. But we should remove the functionality that >>>> results in each PE/COFF image to be split into several regions >>>> marked RO or XP in the ordinary UEFI memory map. >>> >>> We can mitigate this by setting PcdPropertiesTableEnable to FALSE in >>> "ArmVirt.dsc.inc" as well. >>> >> >> No, we should remove it. >> >>> (Hmmm, I forget why SVN r18140 and r18566 don't already result in a >>> problematic memmap for Linux... Probably due to your kernel commit >>> 0ce3cc008ec0.) >>> >> >> It will result in a problematic memmap for older Linux kernels but also for less recent versions of Windows 8, for instance. And the only OS version that actually consumes the properties table is Windows 10, which will we updated in the future to use the MemoryAttributesTable instead. >> >> >>>>> -----Original Message----- >>>>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>>>> Sent: Saturday, January 30, 2016 10:24 AM >>>>> To: Yao, Jiewen >>>>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>>>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>>>> >>>>> On 01/30/16 01:31, Yao, Jiewen wrote: >>>>>> Comment below: >>>>>> >>>>>> -----Original Message----- >>>>>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>>>>> Sent: Saturday, January 30, 2016 1:13 AM >>>>>> To: Yao, Jiewen >>>>>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>>>>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>>>>> >>>>>> Hello Jiewen, >>>>>> >>>>>> On 01/29/16 13:12, jiewen yao wrote: >>>>>>> This series patches add UEFI2.6 MemoryAttributesTable support. >>>>>>> This table is used to retire old PropertiesTable. >>>>>>> This is standalone table published by DxeCore, so there is no >>>>>>> compatibility issue. >>>>>>> >>>>>>> The patch is validated with or without properties table. >>>>>> >>>>>> Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE? >>>>>> [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to FALSE or TRUE, MemoryAttributesTable is always published. >>>>>> That is one compatibility we want to maintain. >>>>>> >>>>>> >>>>>> I skimmed the commit messages, and it looks like the properties table is preserved, but now it serves only as foundation for the memory attributes table. >>>>>> [Jiewen] Yes, the DxeCore global variable - properties table is always preserved. And the system UEFI configuration table for properties table is controlled by PcdPropertiestableEnable. >>>>>> No matter properties table or memory attributes table, we always need a flag to record if RT image is 4K aligned or not, so I just choose the original global variable. >>>>>> >>>>>> >>>>>> In OVMF we set PcdPropertiesTableEnable to FALSE by default, but flip it dynamically to TRUE in PlatformPei, if the user requests the feature on the QEMU command line. This is being done for avoiding regressions with OSes that don't know about the properties table, and its impact on the UEFI memmap. >>>>>> [Jiewen] That is good design. I believe most real platforms do similar thing. >>>>>> >>>>>> >>>>>> However, if the memory attributes table is safe for OSes that >>>>>> don't know about it, I think we can eliminate the above "FALSE" >>>>>> default, and dynamism, from OVMF, and just inherit the >>>>>> PcdPropertiesTableEnable=TRUE setting from "MdeModulePkg.dec". >>>>>> [Jiewen] I am sorry that I am not clear on your suggestion. What do you mean "eliminate the above "FALSE" default, and dynamism, from OVMF"? >>>>>> Do you mean you suggest to change PcdPropertiesTableEnable=FALSE as default value in MdeModulePkg.dec file? >>>>>> Or do you want to change OVMF.dsc file? >>>>> >>>>> Sorry, I should have educated myself on the memory attributes table first, in UEFI 2.6. I have now carefully read the EFI_MEMORY_ATTRIBUTES_TABLE section, and I've also noticed the new last paragraph in the EFI_PROPERTIES_TABLE section. >>>>> >>>>> What I was missing when I asked my question is that EFI_MEMORY_ATTRIBUTES_TABLE was an *additional* feature. >>>>> >>>>> Your cover letter said "This table is used to retire old PropertiesTable". So I thought, the OS-visible properties table would be completely removed from edk2, and the DXE core internals would only be reused for computing the new memory attributes table. In other words, I thought that the memory attributes table would *replace* the properties table, and the original PcdPropertiesTableEnable PCD would now control the presence of the memory attributes table instead. >>>>> >>>>> This is why I asked if we should change OVMF to remove its FALSE default for the PCD, alongside the possibility for the QEMU user to change the PCD dynamically, on the command line. Because, the memory attributes table would *always* be safe to install, so we should just stick, in OVMF, with the TRUE default for the PCD, from MdeModulePkg.dec. >>>>> >>>>> I do understand now that the memory attributes table is an additional feature. It will always be exposed to the OS. *Independently*, the properties table will continue to behave the same as before -- if the PCD is TRUE, then it will be installed; if the PCD is FALSE, then it won't. >>>>> >>>>> In the end, there's nothing to do under OvmfPkg/. We'll let the QEMU user control the properties table same as before (defaulting to FALSE), and orthogonally, the new memory attributes table will always be installed (with your patches in place), because it's safe. >>>>> >>>>> Thanks, and sorry about the confusion. >>>>> Laszlo >>>>> >>>>>> >>>>>> >>>>>> >>>>>> Does it sound reasonable to you? >>>>>> >>>>>> Thanks! >>>>>> Laszlo >>>>>> >>>>>>> >>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>>>> Signed-off-by: "Yao, Jiewen" >>>>>>> Cc: "Gao, Liming" >>>>>>> >>>>>>> jiewen yao (7): >>>>>>> MdePkg: Add UEFI2.6 MemoryAttributes Table definition. >>>>>>> MdePkg: Add UEFI2.6 MemoryAttributesTable GUID >>>>>>> MdeModulePkg: Add MemoryAttributesTable generation. >>>>>>> MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. >>>>>>> MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header >>>>>>> file. >>>>>>> MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. >>>>>>> MdePkg: Update DxeCore INF for MemoryAttributesTable. >>>>>>> >>>>>>> MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- >>>>>>> MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- >>>>>>> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- >>>>>>> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ >>>>>>> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- >>>>>>> MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ >>>>>>> MdePkg/MdePkg.dec | 11 +- >>>>>>> 7 files changed, 362 insertions(+), 15 deletions(-) create mode >>>>>>> 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c >>>>>>> create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h >>>>>>> >>>>>> >>>>> >>>>> _______________________________________________ >>>>> 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 --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta01.collab.prod.int.phx2.redhat.com (LHLO zmta01.collab.prod.int.phx2.redhat.com) (10.5.81.8) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Sun, 14 Feb 2016 01:51:48 -0500 (EST) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by zmta01.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id BD1CF185AC4; Sun, 14 Feb 2016 01:51:48 -0500 (EST) Received: from mx1.redhat.com (ext-mx04.extmail.prod.ext.phx2.redhat.com [10.5.110.28]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1E6pmha017463 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Sun, 14 Feb 2016 01:51:48 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id 3B08F8EA20; Sun, 14 Feb 2016 06:51:47 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id B51A01A1DFC; Sat, 13 Feb 2016 22:51:45 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: 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 0B7151A1DF9 for ; Sat, 13 Feb 2016 22:51:44 -0800 (PST) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 596CD804E5; Sun, 14 Feb 2016 06:51:43 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-49.phx2.redhat.com [10.3.113.49]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1E6pdvI024554; Sun, 14 Feb 2016 01:51:40 -0500 To: "Yao, Jiewen" , Ard Biesheuvel References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> <56AE7040.4060004@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002759103@shsmsx102.ccr.corp.intel.com> From: Laszlo Ersek Message-ID: <56C023FB.60904@redhat.com> Date: Sun, 14 Feb 2016 07:51:39 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C5002759103@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 Cc: "edk2-devel@ml01.01.org" , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.385 (BAYES_50,DCC_REPUT_00_12,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.75 on 10.5.110.28 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 02/14/16 06:44, Yao, Jiewen wrote: > HI > Thanks to discuss this properties table issue. > The purpose of this patch is to *add* UEFI2.6 memory attributes table. This patch does not handle any UEFI2.5 properties table. > If we want to change EDKII code for properties table, I suggest we separate it from this patch. > > Is there any comment for adding UEFI2.6 memory attributes table? Not from my side, thanks. > For UEFI2.5 properties table, let me summarize. Currently, we have several options: > 0) Keep it as is. Current setting is PcdPropertiesTableEnable TRUE in MdeModulePkg.DEC. > 1) Update PcdPropertiesTableEnable to FALSE in XXXPlatform.DSC file. (Platform choice. It can be static PCD or dynamic PCD.) > 2) Update PcdPropertiesTableEnable to FALSE in MdeModulePkg.DEC file. (Disable Default. BIOS will not publish this table by default. If platform wants this table, it can set PCD to true.) > 3) Remove PcdPropertiesTableEnable, and remove PropertiesTable support code in DxeCore. (PropertiesTable support is removed.) > > Personally, I do not suggest we choose 3), because this table is still defined in UEFI specification. We can remove it after UEFI spec removes it later. > I do not have strong opinion for other options. I agree the implementation should follow the spec -- at least offer the feature as long as the spec defines it. My vote would be (2). Thanks Laszlo > > Thank you > Yao Jiewen > > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Monday, February 01, 2016 2:28 PM > To: Laszlo Ersek > Cc: Yao, Jiewen; edk2-devel@ml01.01.org; Gao, Liming > Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. > > On 31 January 2016 at 21:36, Laszlo Ersek wrote: >> On 01/30/16 11:25, Ard Biesheuvel wrote: >>> On 30 January 2016 at 04:17, Yao, Jiewen wrote: >>>> Thanks for the clarification. I think you are right. >>>> >>>> I said "This table is used to retire old PropertiesTable", because UEFI2.6 does not recommend using PropertiesTable to report RT information. >>>> The future BIOS/OS should always produce/consume MemoryAttributesTable as better solution. >>>> >>> >>> I strongly feel we should remove the original PropertiesTable. It >>> breaks backward compatibility, and was replaced by the >>> MemoryAttributesTable for a good reason. The OS side will not be >>> implemented in Linux (i.e., it will ignore the RO and XP attributes >>> in the standard UEFI memory map), and the only reason the >>> PropertiesTable was not removed entirely from the specification is >>> because of business interests of one of the participating OS vendors. >>> >>> I understand that we need to keep the underlying machinery to produce >>> either table. But we should remove the functionality that results in >>> each PE/COFF image to be split into several regions marked RO or XP >>> in the ordinary UEFI memory map. >> >> We can mitigate this by setting PcdPropertiesTableEnable to FALSE in >> "ArmVirt.dsc.inc" as well. >> > > No, we should remove it. > >> (Hmmm, I forget why SVN r18140 and r18566 don't already result in a >> problematic memmap for Linux... Probably due to your kernel commit >> 0ce3cc008ec0.) >> > > It will result in a problematic memmap for older Linux kernels but also for less recent versions of Windows 8, for instance. And the only OS version that actually consumes the properties table is Windows 10, which will we updated in the future to use the MemoryAttributesTable instead. > > >>>> -----Original Message----- >>>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>>> Sent: Saturday, January 30, 2016 10:24 AM >>>> To: Yao, Jiewen >>>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>>> >>>> On 01/30/16 01:31, Yao, Jiewen wrote: >>>>> Comment below: >>>>> >>>>> -----Original Message----- >>>>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>>>> Sent: Saturday, January 30, 2016 1:13 AM >>>>> To: Yao, Jiewen >>>>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>>>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>>>> >>>>> Hello Jiewen, >>>>> >>>>> On 01/29/16 13:12, jiewen yao wrote: >>>>>> This series patches add UEFI2.6 MemoryAttributesTable support. >>>>>> This table is used to retire old PropertiesTable. >>>>>> This is standalone table published by DxeCore, so there is no >>>>>> compatibility issue. >>>>>> >>>>>> The patch is validated with or without properties table. >>>>> >>>>> Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE? >>>>> [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to FALSE or TRUE, MemoryAttributesTable is always published. >>>>> That is one compatibility we want to maintain. >>>>> >>>>> >>>>> I skimmed the commit messages, and it looks like the properties table is preserved, but now it serves only as foundation for the memory attributes table. >>>>> [Jiewen] Yes, the DxeCore global variable - properties table is always preserved. And the system UEFI configuration table for properties table is controlled by PcdPropertiestableEnable. >>>>> No matter properties table or memory attributes table, we always need a flag to record if RT image is 4K aligned or not, so I just choose the original global variable. >>>>> >>>>> >>>>> In OVMF we set PcdPropertiesTableEnable to FALSE by default, but flip it dynamically to TRUE in PlatformPei, if the user requests the feature on the QEMU command line. This is being done for avoiding regressions with OSes that don't know about the properties table, and its impact on the UEFI memmap. >>>>> [Jiewen] That is good design. I believe most real platforms do similar thing. >>>>> >>>>> >>>>> However, if the memory attributes table is safe for OSes that don't >>>>> know about it, I think we can eliminate the above "FALSE" default, >>>>> and dynamism, from OVMF, and just inherit the >>>>> PcdPropertiesTableEnable=TRUE setting from "MdeModulePkg.dec". >>>>> [Jiewen] I am sorry that I am not clear on your suggestion. What do you mean "eliminate the above "FALSE" default, and dynamism, from OVMF"? >>>>> Do you mean you suggest to change PcdPropertiesTableEnable=FALSE as default value in MdeModulePkg.dec file? >>>>> Or do you want to change OVMF.dsc file? >>>> >>>> Sorry, I should have educated myself on the memory attributes table first, in UEFI 2.6. I have now carefully read the EFI_MEMORY_ATTRIBUTES_TABLE section, and I've also noticed the new last paragraph in the EFI_PROPERTIES_TABLE section. >>>> >>>> What I was missing when I asked my question is that EFI_MEMORY_ATTRIBUTES_TABLE was an *additional* feature. >>>> >>>> Your cover letter said "This table is used to retire old PropertiesTable". So I thought, the OS-visible properties table would be completely removed from edk2, and the DXE core internals would only be reused for computing the new memory attributes table. In other words, I thought that the memory attributes table would *replace* the properties table, and the original PcdPropertiesTableEnable PCD would now control the presence of the memory attributes table instead. >>>> >>>> This is why I asked if we should change OVMF to remove its FALSE default for the PCD, alongside the possibility for the QEMU user to change the PCD dynamically, on the command line. Because, the memory attributes table would *always* be safe to install, so we should just stick, in OVMF, with the TRUE default for the PCD, from MdeModulePkg.dec. >>>> >>>> I do understand now that the memory attributes table is an additional feature. It will always be exposed to the OS. *Independently*, the properties table will continue to behave the same as before -- if the PCD is TRUE, then it will be installed; if the PCD is FALSE, then it won't. >>>> >>>> In the end, there's nothing to do under OvmfPkg/. We'll let the QEMU user control the properties table same as before (defaulting to FALSE), and orthogonally, the new memory attributes table will always be installed (with your patches in place), because it's safe. >>>> >>>> Thanks, and sorry about the confusion. >>>> Laszlo >>>> >>>>> >>>>> >>>>> >>>>> Does it sound reasonable to you? >>>>> >>>>> Thanks! >>>>> Laszlo >>>>> >>>>>> >>>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>>> Signed-off-by: "Yao, Jiewen" >>>>>> Cc: "Gao, Liming" >>>>>> >>>>>> jiewen yao (7): >>>>>> MdePkg: Add UEFI2.6 MemoryAttributes Table definition. >>>>>> MdePkg: Add UEFI2.6 MemoryAttributesTable GUID >>>>>> MdeModulePkg: Add MemoryAttributesTable generation. >>>>>> MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. >>>>>> MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header >>>>>> file. >>>>>> MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. >>>>>> MdePkg: Update DxeCore INF for MemoryAttributesTable. >>>>>> >>>>>> MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- >>>>>> MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- >>>>>> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- >>>>>> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ >>>>>> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- >>>>>> MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ >>>>>> MdePkg/MdePkg.dec | 11 +- >>>>>> 7 files changed, 362 insertions(+), 15 deletions(-) create mode >>>>>> 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c >>>>>> create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h >>>>>> >>>>> >>>> >>>> _______________________________________________ >>>> 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 --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta03.collab.prod.int.phx2.redhat.com (LHLO zmta03.collab.prod.int.phx2.redhat.com) (10.5.81.10) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Sun, 14 Feb 2016 04:22:09 -0500 (EST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by zmta03.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id CB43F43561; Sun, 14 Feb 2016 04:22:09 -0500 (EST) Received: from mx1.redhat.com (ext-mx02.extmail.prod.ext.phx2.redhat.com [10.5.110.26]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1E9M9HG020333 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Sun, 14 Feb 2016 04:22:09 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id 4369D8E36D; Sun, 14 Feb 2016 09:22:08 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id DB2241A1DFC; Sun, 14 Feb 2016 01:22:06 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mail-io0-x236.google.com (mail-io0-x236.google.com [IPv6:2607:f8b0:4001:c06::236]) (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 F27431A1DF9 for ; Sun, 14 Feb 2016 01:22:04 -0800 (PST) Received: by mail-io0-x236.google.com with SMTP id g203so105939807iof.2 for ; Sun, 14 Feb 2016 01:22:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=NbBswMltz41TfGV8+vwZ1l6SszNGSwiNbHXL9kGKKjA=; b=fKSP07FXYBpwWeXEXPRVNe5wNu2ULOH+c+ifk0Id2AaEtO5kgZt9wNOwa0Lv3H2MVC +D9j9C+X/wAqhb58w4wrWX5N7QniCIkk10zsC2krBrAzVB0kbypKwPkOUnp2IrcJty4c pMufPnRYAIAIX/hJksZSE4CIKT88kHbgAe97I= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=NbBswMltz41TfGV8+vwZ1l6SszNGSwiNbHXL9kGKKjA=; b=GmVXhl9kKYVKdHcfciCi8GApMGXNRhvC0b79uk3sUHbLQU9e30jbxAHUX4/c7NiEma LVyzKPgoVIpOI/T8Yw/Wb26P/lfyYZ1KwfctGxX+UpvrQimxgGC+2eaJbEy/inMTg0aP bw/18+pNQz2Wv7pIejUTH+5chg/UKY1jAaGlg9hEqtJ/zjj/NTRKKTH8faP/yUakFPs/ vntfGE++UshOMdkFXpXlnH2NA7gPKQ2Jsv+Ptfs7xMId+dQj+OZNRm0Rq7kAJNKFHWYR VWbZtfQswIX5zdTo4DEiOp7qHKaFKz799XjupOypsXNUMRoK9dNL7O098RULaAkzXoU3 RXDQ== X-Gm-Message-State: AG10YORLnd6w09rilxli/mEY//zemTUAH6F5i3yvV9RbqPG34PFvm5FVq2CkA3L0f0/r9HbhbJ9cHYdE6Kqbc0hQ MIME-Version: 1.0 X-Received: by 10.107.135.20 with SMTP id j20mr13546556iod.56.1455441724134; Sun, 14 Feb 2016 01:22:04 -0800 (PST) Received: by 10.36.29.6 with HTTP; Sun, 14 Feb 2016 01:22:04 -0800 (PST) In-Reply-To: <56C023FB.60904@redhat.com> References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> <56AE7040.4060004@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002759103@shsmsx102.ccr.corp.intel.com> <56C023FB.60904@redhat.com> Date: Sun, 14 Feb 2016 10:22:04 +0100 Message-ID: From: Ard Biesheuvel To: Laszlo Ersek Cc: "edk2-devel@ml01.01.org" , "Yao, Jiewen" , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.495 (BAYES_50,DCC_REPUT_00_12,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,T_DKIM_INVALID) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Scanned-By: MIMEDefang 2.75 on 10.5.110.26 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 14 February 2016 at 07:51, Laszlo Ersek wrote: > On 02/14/16 06:44, Yao, Jiewen wrote: >> HI >> Thanks to discuss this properties table issue. >> The purpose of this patch is to *add* UEFI2.6 memory attributes table. This patch does not handle any UEFI2.5 properties table. >> If we want to change EDKII code for properties table, I suggest we separate it from this patch. >> >> Is there any comment for adding UEFI2.6 memory attributes table? > > Not from my side, thanks. > >> For UEFI2.5 properties table, let me summarize. Currently, we have several options: >> 0) Keep it as is. Current setting is PcdPropertiesTableEnable TRUE in MdeModulePkg.DEC. >> 1) Update PcdPropertiesTableEnable to FALSE in XXXPlatform.DSC file. (Platform choice. It can be static PCD or dynamic PCD.) >> 2) Update PcdPropertiesTableEnable to FALSE in MdeModulePkg.DEC file. (Disable Default. BIOS will not publish this table by default. If platform wants this table, it can set PCD to true.) >> 3) Remove PcdPropertiesTableEnable, and remove PropertiesTable support code in DxeCore. (PropertiesTable support is removed.) >> >> Personally, I do not suggest we choose 3), because this table is still defined in UEFI specification. We can remove it after UEFI spec removes it later. >> I do not have strong opinion for other options. > > I agree the implementation should follow the spec -- at least offer the > feature as long as the spec defines it. My vote would be (2). > I disagree. Not only was the PropertiesTable superseded by the MemoryAttributes table in 2.6, the recommendation not to use the PropertiesTable has also been added to 2.5 errata A, while it was originally defined in 2.5 This means effectively that it has been retracted, and so it does not matter if you claim to implement UEFI 2.4, 2.5 or 2.6, in none of these cases should the PropertiesTable be published. The reason that the definition remains in the text is for the OS side, not for the reference implementation of the firmware side, which should steer clear from it entirely. -- Ard. >> -----Original Message----- >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >> Sent: Monday, February 01, 2016 2:28 PM >> To: Laszlo Ersek >> Cc: Yao, Jiewen; edk2-devel@ml01.01.org; Gao, Liming >> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >> >> On 31 January 2016 at 21:36, Laszlo Ersek wrote: >>> On 01/30/16 11:25, Ard Biesheuvel wrote: >>>> On 30 January 2016 at 04:17, Yao, Jiewen wrote: >>>>> Thanks for the clarification. I think you are right. >>>>> >>>>> I said "This table is used to retire old PropertiesTable", because UEFI2.6 does not recommend using PropertiesTable to report RT information. >>>>> The future BIOS/OS should always produce/consume MemoryAttributesTable as better solution. >>>>> >>>> >>>> I strongly feel we should remove the original PropertiesTable. It >>>> breaks backward compatibility, and was replaced by the >>>> MemoryAttributesTable for a good reason. The OS side will not be >>>> implemented in Linux (i.e., it will ignore the RO and XP attributes >>>> in the standard UEFI memory map), and the only reason the >>>> PropertiesTable was not removed entirely from the specification is >>>> because of business interests of one of the participating OS vendors. >>>> >>>> I understand that we need to keep the underlying machinery to produce >>>> either table. But we should remove the functionality that results in >>>> each PE/COFF image to be split into several regions marked RO or XP >>>> in the ordinary UEFI memory map. >>> >>> We can mitigate this by setting PcdPropertiesTableEnable to FALSE in >>> "ArmVirt.dsc.inc" as well. >>> >> >> No, we should remove it. >> >>> (Hmmm, I forget why SVN r18140 and r18566 don't already result in a >>> problematic memmap for Linux... Probably due to your kernel commit >>> 0ce3cc008ec0.) >>> >> >> It will result in a problematic memmap for older Linux kernels but also for less recent versions of Windows 8, for instance. And the only OS version that actually consumes the properties table is Windows 10, which will we updated in the future to use the MemoryAttributesTable instead. >> >> >>>>> -----Original Message----- >>>>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>>>> Sent: Saturday, January 30, 2016 10:24 AM >>>>> To: Yao, Jiewen >>>>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>>>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>>>> >>>>> On 01/30/16 01:31, Yao, Jiewen wrote: >>>>>> Comment below: >>>>>> >>>>>> -----Original Message----- >>>>>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>>>>> Sent: Saturday, January 30, 2016 1:13 AM >>>>>> To: Yao, Jiewen >>>>>> Cc: edk2-devel@ml01.01.org; Gao, Liming >>>>>> Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. >>>>>> >>>>>> Hello Jiewen, >>>>>> >>>>>> On 01/29/16 13:12, jiewen yao wrote: >>>>>>> This series patches add UEFI2.6 MemoryAttributesTable support. >>>>>>> This table is used to retire old PropertiesTable. >>>>>>> This is standalone table published by DxeCore, so there is no >>>>>>> compatibility issue. >>>>>>> >>>>>>> The patch is validated with or without properties table. >>>>>> >>>>>> Do you mean, tested with PcdPropertiesTableEnable set to FALSE vs. TRUE? >>>>>> [Jiewen] Yes, you are right. No matter PcdPropertiestableEnable set to FALSE or TRUE, MemoryAttributesTable is always published. >>>>>> That is one compatibility we want to maintain. >>>>>> >>>>>> >>>>>> I skimmed the commit messages, and it looks like the properties table is preserved, but now it serves only as foundation for the memory attributes table. >>>>>> [Jiewen] Yes, the DxeCore global variable - properties table is always preserved. And the system UEFI configuration table for properties table is controlled by PcdPropertiestableEnable. >>>>>> No matter properties table or memory attributes table, we always need a flag to record if RT image is 4K aligned or not, so I just choose the original global variable. >>>>>> >>>>>> >>>>>> In OVMF we set PcdPropertiesTableEnable to FALSE by default, but flip it dynamically to TRUE in PlatformPei, if the user requests the feature on the QEMU command line. This is being done for avoiding regressions with OSes that don't know about the properties table, and its impact on the UEFI memmap. >>>>>> [Jiewen] That is good design. I believe most real platforms do similar thing. >>>>>> >>>>>> >>>>>> However, if the memory attributes table is safe for OSes that don't >>>>>> know about it, I think we can eliminate the above "FALSE" default, >>>>>> and dynamism, from OVMF, and just inherit the >>>>>> PcdPropertiesTableEnable=TRUE setting from "MdeModulePkg.dec". >>>>>> [Jiewen] I am sorry that I am not clear on your suggestion. What do you mean "eliminate the above "FALSE" default, and dynamism, from OVMF"? >>>>>> Do you mean you suggest to change PcdPropertiesTableEnable=FALSE as default value in MdeModulePkg.dec file? >>>>>> Or do you want to change OVMF.dsc file? >>>>> >>>>> Sorry, I should have educated myself on the memory attributes table first, in UEFI 2.6. I have now carefully read the EFI_MEMORY_ATTRIBUTES_TABLE section, and I've also noticed the new last paragraph in the EFI_PROPERTIES_TABLE section. >>>>> >>>>> What I was missing when I asked my question is that EFI_MEMORY_ATTRIBUTES_TABLE was an *additional* feature. >>>>> >>>>> Your cover letter said "This table is used to retire old PropertiesTable". So I thought, the OS-visible properties table would be completely removed from edk2, and the DXE core internals would only be reused for computing the new memory attributes table. In other words, I thought that the memory attributes table would *replace* the properties table, and the original PcdPropertiesTableEnable PCD would now control the presence of the memory attributes table instead. >>>>> >>>>> This is why I asked if we should change OVMF to remove its FALSE default for the PCD, alongside the possibility for the QEMU user to change the PCD dynamically, on the command line. Because, the memory attributes table would *always* be safe to install, so we should just stick, in OVMF, with the TRUE default for the PCD, from MdeModulePkg.dec. >>>>> >>>>> I do understand now that the memory attributes table is an additional feature. It will always be exposed to the OS. *Independently*, the properties table will continue to behave the same as before -- if the PCD is TRUE, then it will be installed; if the PCD is FALSE, then it won't. >>>>> >>>>> In the end, there's nothing to do under OvmfPkg/. We'll let the QEMU user control the properties table same as before (defaulting to FALSE), and orthogonally, the new memory attributes table will always be installed (with your patches in place), because it's safe. >>>>> >>>>> Thanks, and sorry about the confusion. >>>>> Laszlo >>>>> >>>>>> >>>>>> >>>>>> >>>>>> Does it sound reasonable to you? >>>>>> >>>>>> Thanks! >>>>>> Laszlo >>>>>> >>>>>>> >>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>>>> Signed-off-by: "Yao, Jiewen" >>>>>>> Cc: "Gao, Liming" >>>>>>> >>>>>>> jiewen yao (7): >>>>>>> MdePkg: Add UEFI2.6 MemoryAttributes Table definition. >>>>>>> MdePkg: Add UEFI2.6 MemoryAttributesTable GUID >>>>>>> MdeModulePkg: Add MemoryAttributesTable generation. >>>>>>> MdeModulePkg: Update PropertiesTable for MemoryAttributesTable. >>>>>>> MdeModulePkg: Add CoreInitializeMemoryAttributesTable() to header >>>>>>> file. >>>>>>> MdePkg: Call CoreInitializeMemoryAttributesTable() in DXE Entrypoint. >>>>>>> MdePkg: Update DxeCore INF for MemoryAttributesTable. >>>>>>> >>>>>>> MdeModulePkg/Core/Dxe/DxeMain.h | 11 +- >>>>>>> MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +- >>>>>>> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +- >>>>>>> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 214 +++++++++++++++++++++ >>>>>>> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 100 +++++++++- >>>>>>> MdePkg/Include/Guid/MemoryAttributesTable.h | 34 ++++ >>>>>>> MdePkg/MdePkg.dec | 11 +- >>>>>>> 7 files changed, 362 insertions(+), 15 deletions(-) create mode >>>>>>> 100644 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c >>>>>>> create mode 100644 MdePkg/Include/Guid/MemoryAttributesTable.h >>>>>>> >>>>>> >>>>> >>>>> _______________________________________________ >>>>> 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 --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta05.collab.prod.int.phx2.redhat.com (LHLO zmta05.collab.prod.int.phx2.redhat.com) (10.5.81.12) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Sun, 14 Feb 2016 05:52:38 -0500 (EST) Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by zmta05.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id E1E11F3BE5; Sun, 14 Feb 2016 05:52:38 -0500 (EST) Received: from mx1.redhat.com (ext-mx07.extmail.prod.ext.phx2.redhat.com [10.5.110.31]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1EAqcVV024181 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Sun, 14 Feb 2016 05:52:38 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id E48A8C0AD401; Sun, 14 Feb 2016 10:52:37 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 865161A1DFC; Sun, 14 Feb 2016 02:52:36 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: 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 C241A1A1DF9 for ; Sun, 14 Feb 2016 02:52:35 -0800 (PST) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 372B1C0C2373; Sun, 14 Feb 2016 10:52:35 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-49.phx2.redhat.com [10.3.113.49]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1EAqXBK018295; Sun, 14 Feb 2016 05:52:33 -0500 To: Ard Biesheuvel References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> <56AE7040.4060004@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002759103@shsmsx102.ccr.corp.intel.com> <56C023FB.60904@redhat.com> From: Laszlo Ersek Message-ID: <56C05C70.5000700@redhat.com> Date: Sun, 14 Feb 2016 11:52:32 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 Cc: "edk2-devel@ml01.01.org" , "Yao, Jiewen" , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.385 (BAYES_50,DCC_REPUT_00_12,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.78 on 10.5.110.31 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 02/14/16 10:22, Ard Biesheuvel wrote: > On 14 February 2016 at 07:51, Laszlo Ersek wrote: >> On 02/14/16 06:44, Yao, Jiewen wrote: >>> HI >>> Thanks to discuss this properties table issue. >>> The purpose of this patch is to *add* UEFI2.6 memory attributes table. This patch does not handle any UEFI2.5 properties table. >>> If we want to change EDKII code for properties table, I suggest we separate it from this patch. >>> >>> Is there any comment for adding UEFI2.6 memory attributes table? >> >> Not from my side, thanks. >> >>> For UEFI2.5 properties table, let me summarize. Currently, we have several options: >>> 0) Keep it as is. Current setting is PcdPropertiesTableEnable TRUE in MdeModulePkg.DEC. >>> 1) Update PcdPropertiesTableEnable to FALSE in XXXPlatform.DSC file. (Platform choice. It can be static PCD or dynamic PCD.) >>> 2) Update PcdPropertiesTableEnable to FALSE in MdeModulePkg.DEC file. (Disable Default. BIOS will not publish this table by default. If platform wants this table, it can set PCD to true.) >>> 3) Remove PcdPropertiesTableEnable, and remove PropertiesTable support code in DxeCore. (PropertiesTable support is removed.) >>> >>> Personally, I do not suggest we choose 3), because this table is still defined in UEFI specification. We can remove it after UEFI spec removes it later. >>> I do not have strong opinion for other options. >> >> I agree the implementation should follow the spec -- at least offer the >> feature as long as the spec defines it. My vote would be (2). >> > > I disagree. Not only was the PropertiesTable superseded by the > MemoryAttributes table in 2.6, the recommendation not to use the > PropertiesTable has also been added to 2.5 errata A, while it was > originally defined in 2.5 This means effectively that it has been > retracted, and so it does not matter if you claim to implement UEFI > 2.4, 2.5 or 2.6, in none of these cases should the PropertiesTable be > published. Well, why not drop the text from the spec completely then? > The reason that the definition remains in the text is for the OS side, > not for the reference implementation of the firmware side, which > should steer clear from it entirely. Okay, but in that case, I sort of think of the edk2 code as something that would let you test an operating system's support for this (mis)feature. Anyway, I don't disagree with (3) either. If that is what gets implemented, I'll be fine with it. As long as there's a way to disable the (mis)feature -- by way of PCD or complete removal --, I'm okay. Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta03.collab.prod.int.phx2.redhat.com (LHLO zmta03.collab.prod.int.phx2.redhat.com) (10.5.81.10) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Mon, 15 Feb 2016 19:51:23 -0500 (EST) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by zmta03.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id C5075435F3; Mon, 15 Feb 2016 19:51:23 -0500 (EST) Received: from mx1.redhat.com (ext-mx08.extmail.prod.ext.phx2.redhat.com [10.5.110.32]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1G0pN0j014573 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Mon, 15 Feb 2016 19:51:23 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id 81BAFC001260; Tue, 16 Feb 2016 00:51:22 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 391431A1DF0; Mon, 15 Feb 2016 16:51:21 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by ml01.01.org (Postfix) with ESMTP id 0BBA91A1DEE for ; Mon, 15 Feb 2016 16:51:20 -0800 (PST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP; 15 Feb 2016 16:51:19 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,453,1449561600"; d="scan'208";a="652628680" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by FMSMGA003.fm.intel.com with ESMTP; 15 Feb 2016 16:51:19 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 15 Feb 2016 16:51:19 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.172]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.196]) with mapi id 14.03.0248.002; Tue, 16 Feb 2016 08:51:17 +0800 From: "Yao, Jiewen" To: Laszlo Ersek , Ard Biesheuvel Thread-Topic: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. Thread-Index: AQHRWo5UfnwfB+DE8USx6LXBJH6a7Z8SNVqAgAD+OSD//5vFAIAAk1RQ///zJICAAj0HAIAApUWAgBTkw9D//5A2gAAFQPIAAAMo1QAAYECTAA== Date: Tue, 16 Feb 2016 00:51:17 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C5002759A22@shsmsx102.ccr.corp.intel.com> References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> <56AE7040.4060004@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002759103@shsmsx102.ccr.corp.intel.com> <56C023FB.60904@redhat.com> <56C05C70.5000700@redhat.com> In-Reply-To: <56C05C70.5000700@redhat.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMDQ1ZmEwMGYtNzNkOC00MjMzLWIyMDEtMWIzYzhmZDMzNzRlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Ik5vZjBkcFlEN0h6T0hzNVRxR2p5dkplU1c3WVlJNUtBN1d0ZGJsMDdMNjA9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Cc: "edk2-devel@ml01.01.org" , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.385 (BAYES_50,DCC_REPUT_00_12,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Scanned-By: MIMEDefang 2.78 on 10.5.110.32 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Thanks. I think it is easy to just choose 2). For 3), we need start evaluating if there is any impact to any production. Thank you Yao Jiewen -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com] Sent: Sunday, February 14, 2016 6:53 PM To: Ard Biesheuvel Cc: Yao, Jiewen; edk2-devel@ml01.01.org; Gao, Liming Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. On 02/14/16 10:22, Ard Biesheuvel wrote: > On 14 February 2016 at 07:51, Laszlo Ersek wrote: >> On 02/14/16 06:44, Yao, Jiewen wrote: >>> HI >>> Thanks to discuss this properties table issue. >>> The purpose of this patch is to *add* UEFI2.6 memory attributes table. This patch does not handle any UEFI2.5 properties table. >>> If we want to change EDKII code for properties table, I suggest we separate it from this patch. >>> >>> Is there any comment for adding UEFI2.6 memory attributes table? >> >> Not from my side, thanks. >> >>> For UEFI2.5 properties table, let me summarize. Currently, we have several options: >>> 0) Keep it as is. Current setting is PcdPropertiesTableEnable TRUE in MdeModulePkg.DEC. >>> 1) Update PcdPropertiesTableEnable to FALSE in XXXPlatform.DSC file. >>> (Platform choice. It can be static PCD or dynamic PCD.) >>> 2) Update PcdPropertiesTableEnable to FALSE in MdeModulePkg.DEC >>> file. (Disable Default. BIOS will not publish this table by default. >>> If platform wants this table, it can set PCD to true.) >>> 3) Remove PcdPropertiesTableEnable, and remove PropertiesTable >>> support code in DxeCore. (PropertiesTable support is removed.) >>> >>> Personally, I do not suggest we choose 3), because this table is still defined in UEFI specification. We can remove it after UEFI spec removes it later. >>> I do not have strong opinion for other options. >> >> I agree the implementation should follow the spec -- at least offer >> the feature as long as the spec defines it. My vote would be (2). >> > > I disagree. Not only was the PropertiesTable superseded by the > MemoryAttributes table in 2.6, the recommendation not to use the > PropertiesTable has also been added to 2.5 errata A, while it was > originally defined in 2.5 This means effectively that it has been > retracted, and so it does not matter if you claim to implement UEFI > 2.4, 2.5 or 2.6, in none of these cases should the PropertiesTable be > published. Well, why not drop the text from the spec completely then? > The reason that the definition remains in the text is for the OS side, > not for the reference implementation of the firmware side, which > should steer clear from it entirely. Okay, but in that case, I sort of think of the edk2 code as something that would let you test an operating system's support for this (mis)feature. Anyway, I don't disagree with (3) either. If that is what gets implemented, I'll be fine with it. As long as there's a way to disable the (mis)feature -- by way of PCD or complete removal --, I'm okay. Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta04.collab.prod.int.phx2.redhat.com (LHLO zmta04.collab.prod.int.phx2.redhat.com) (10.5.81.11) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Wed, 17 Feb 2016 07:22:51 -0500 (EST) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by zmta04.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id 64A00D0310; Wed, 17 Feb 2016 07:22:51 -0500 (EST) Received: from mx1.redhat.com (ext-mx02.extmail.prod.ext.phx2.redhat.com [10.5.110.26]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1HCMpoV021801 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 17 Feb 2016 07:22:51 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id 6B6FB2589B; Wed, 17 Feb 2016 12:22:50 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 299521A1E04; Wed, 17 Feb 2016 04:22:49 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by ml01.01.org (Postfix) with ESMTP id 8D3861A1E02 for ; Wed, 17 Feb 2016 04:22:47 -0800 (PST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 17 Feb 2016 04:22:48 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,460,1449561600"; d="scan'208";a="916854742" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga002.fm.intel.com with ESMTP; 17 Feb 2016 04:22:47 -0800 Received: from fmsmsx157.amr.corp.intel.com (10.18.116.73) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 17 Feb 2016 04:22:46 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX157.amr.corp.intel.com (10.18.116.73) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 17 Feb 2016 04:22:46 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.172]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.84]) with mapi id 14.03.0248.002; Wed, 17 Feb 2016 20:22:44 +0800 From: "Yao, Jiewen" To: "Yao, Jiewen" , Laszlo Ersek , Ard Biesheuvel Thread-Topic: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. Thread-Index: AQHRWo5UfnwfB+DE8USx6LXBJH6a7Z8SNVqAgAD+OSD//5vFAIAAk1RQ///zJICAAj0HAIAApUWAgBTkw9D//5A2gAAFQPIAAAMo1QAAYECTAABKG9EQ Date: Wed, 17 Feb 2016 12:22:44 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C500275A94E@shsmsx102.ccr.corp.intel.com> References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> <56AE7040.4060004@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002759103@shsmsx102.ccr.corp.intel.com> <56C023FB.60904@redhat.com> <56C05C70.5000700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002759A22@shsmsx102.ccr.corp.intel.com> In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C5002759A22@shsmsx102.ccr.corp.intel.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMDQ1ZmEwMGYtNzNkOC00MjMzLWIyMDEtMWIzYzhmZDMzNzRlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Ik5vZjBkcFlEN0h6T0hzNVRxR2p5dkplU1c3WVlJNUtBN1d0ZGJsMDdMNjA9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Cc: "edk2-devel@ml01.01.org" , "Yao, Jiewen" , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.385 (BAYES_50,DCC_REPUT_00_12,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Scanned-By: MIMEDefang 2.75 on 10.5.110.26 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit I collected Intel internal feedback. 1) We observed the shipping production still using this Properties Table, and UEFI 2.6 specification still keeps the text. So we want to keep code to give production a way to enable this feature. 2) Since UEFI specification marks this as "not recommended", we agree to change default value to FALSE in DEC declaration. Intel vote #2, at current point of time. We vote #3 as long term plan, after UEFI 2.x specification removes the text later. Thank you Yao Jiewen -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen Sent: Tuesday, February 16, 2016 8:51 AM To: Laszlo Ersek; Ard Biesheuvel Cc: edk2-devel@ml01.01.org; Gao, Liming Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. Thanks. I think it is easy to just choose 2). For 3), we need start evaluating if there is any impact to any production. Thank you Yao Jiewen -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com] Sent: Sunday, February 14, 2016 6:53 PM To: Ard Biesheuvel Cc: Yao, Jiewen; edk2-devel@ml01.01.org; Gao, Liming Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. On 02/14/16 10:22, Ard Biesheuvel wrote: > On 14 February 2016 at 07:51, Laszlo Ersek wrote: >> On 02/14/16 06:44, Yao, Jiewen wrote: >>> HI >>> Thanks to discuss this properties table issue. >>> The purpose of this patch is to *add* UEFI2.6 memory attributes table. This patch does not handle any UEFI2.5 properties table. >>> If we want to change EDKII code for properties table, I suggest we separate it from this patch. >>> >>> Is there any comment for adding UEFI2.6 memory attributes table? >> >> Not from my side, thanks. >> >>> For UEFI2.5 properties table, let me summarize. Currently, we have several options: >>> 0) Keep it as is. Current setting is PcdPropertiesTableEnable TRUE in MdeModulePkg.DEC. >>> 1) Update PcdPropertiesTableEnable to FALSE in XXXPlatform.DSC file. >>> (Platform choice. It can be static PCD or dynamic PCD.) >>> 2) Update PcdPropertiesTableEnable to FALSE in MdeModulePkg.DEC >>> file. (Disable Default. BIOS will not publish this table by default. >>> If platform wants this table, it can set PCD to true.) >>> 3) Remove PcdPropertiesTableEnable, and remove PropertiesTable >>> support code in DxeCore. (PropertiesTable support is removed.) >>> >>> Personally, I do not suggest we choose 3), because this table is still defined in UEFI specification. We can remove it after UEFI spec removes it later. >>> I do not have strong opinion for other options. >> >> I agree the implementation should follow the spec -- at least offer >> the feature as long as the spec defines it. My vote would be (2). >> > > I disagree. Not only was the PropertiesTable superseded by the > MemoryAttributes table in 2.6, the recommendation not to use the > PropertiesTable has also been added to 2.5 errata A, while it was > originally defined in 2.5 This means effectively that it has been > retracted, and so it does not matter if you claim to implement UEFI > 2.4, 2.5 or 2.6, in none of these cases should the PropertiesTable be > published. Well, why not drop the text from the spec completely then? > The reason that the definition remains in the text is for the OS side, > not for the reference implementation of the firmware side, which > should steer clear from it entirely. Okay, but in that case, I sort of think of the edk2 code as something that would let you test an operating system's support for this (mis)feature. Anyway, I don't disagree with (3) either. If that is what gets implemented, I'll be fine with it. As long as there's a way to disable the (mis)feature -- by way of PCD or complete removal --, I'm okay. Thanks Laszlo _______________________________________________ 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 --------------90AEA0B034D46DF3EAE46850 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Return-Path: edk2-devel-bounces@ml01.01.org Received: from zmta04.collab.prod.int.phx2.redhat.com (LHLO zmta04.collab.prod.int.phx2.redhat.com) (10.5.81.11) by zmail17.collab.prod.int.phx2.redhat.com with LMTP; Wed, 17 Feb 2016 07:24:23 -0500 (EST) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by zmta04.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id 93D9ED0751; Wed, 17 Feb 2016 07:24:23 -0500 (EST) Received: from mx1.redhat.com (ext-mx08.extmail.prod.ext.phx2.redhat.com [10.5.110.32]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1HCONVV015906 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 17 Feb 2016 07:24:23 -0500 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx1.redhat.com (Postfix) with ESMTPS id 34353C00F5C4; Wed, 17 Feb 2016 12:24:22 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 836FB1A1E04; Wed, 17 Feb 2016 04:24:21 -0800 (PST) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mail-io0-x22a.google.com (mail-io0-x22a.google.com [IPv6:2607:f8b0:4001:c06::22a]) (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 7E8781A1E02 for ; Wed, 17 Feb 2016 04:24:19 -0800 (PST) Received: by mail-io0-x22a.google.com with SMTP id l127so34405103iof.3 for ; Wed, 17 Feb 2016 04:24:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=5KoEMIBQQFGMDc1XdOJ1qAh5sMZY8d8vkJHB2//P6sw=; b=VXap705sd8ueM3xTQHUAccnQQUz0FgS7y7B6rUjyIbYaCHv2scJK4Jwx/VrSWEkSTA W23Nw9C8ZVDjto+YKyHum6ucGgxnYYdhYuoe+GSsN12JINUVQI8LG8PDcX6bn9s3P54S nMzPbHt+WV+wcJBcmoaOkh66et57r5B8nn1TI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=5KoEMIBQQFGMDc1XdOJ1qAh5sMZY8d8vkJHB2//P6sw=; b=lfW2Lwm4ix48c4PMOlLGYulAY9sjJxUax7byorutoqGExUmZABleNmBjLoMwRUTIy3 JqVeCJBdXM2T8L2KOCkvYz0cVswaGp2vz6PdDC521cmhDTmfoPYMHKXa0LMKHIw+9/Qd gGW3Wm8Cd/sSvQjlS2X9rN8AoyhOWmY7LbsF5QynXBgqGVlh9bGTqZXUhbo5VFF2ujsk sepQbVszwl77fWURPNTAaPgJChEOq2hbQA8KyhGnwR1+ohIVaADafoKBAjJNojibtOiH yGlM0SOJWROg9zs2WW0NlUyYmqwiLMbh/LrMtgC5aiKp0PcLxXX5QHbkBWfcBqSpU429 fvSQ== X-Gm-Message-State: AG10YOSK6UyzfyNkuL/7fseDAjguC5LMB6fdKsaFeXf37/dSrV4wfqikUqpBEs5SWUNQdAfrqW3s3den+xKrdQVo MIME-Version: 1.0 X-Received: by 10.107.135.20 with SMTP id j20mr3661942iod.56.1455711858925; Wed, 17 Feb 2016 04:24:18 -0800 (PST) Received: by 10.36.29.6 with HTTP; Wed, 17 Feb 2016 04:24:18 -0800 (PST) In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C500275A94E@shsmsx102.ccr.corp.intel.com> References: <1454069539-4056-1-git-send-email-jiewen.yao@intel.com> <56AB9D95.3070402@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C500275227B@shsmsx102.ccr.corp.intel.com> <56AC1EC2.4020700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002752311@shsmsx102.ccr.corp.intel.com> <56AE7040.4060004@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002759103@shsmsx102.ccr.corp.intel.com> <56C023FB.60904@redhat.com> <56C05C70.5000700@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C5002759A22@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C500275A94E@shsmsx102.ccr.corp.intel.com> Date: Wed, 17 Feb 2016 13:24:18 +0100 Message-ID: From: Ard Biesheuvel To: "Yao, Jiewen" Cc: "edk2-devel@ml01.01.org" , Laszlo Ersek , "Gao, Liming" Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: edk2-devel-bounces@ml01.01.org Sender: "edk2-devel" X-RedHat-Spam-Score: 0.495 (BAYES_50,DCC_REPUT_00_12,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD,T_DKIM_INVALID) 198.145.21.10 ml01.01.org 198.145.21.10 ml01.01.org X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Scanned-By: MIMEDefang 2.78 on 10.5.110.32 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 17 February 2016 at 13:22, Yao, Jiewen wrote: > I collected Intel internal feedback. > > 1) We observed the shipping production still using this Properties Table, and UEFI 2.6 specification still keeps the text. So we want to keep code to give production a way to enable this feature. > 2) Since UEFI specification marks this as "not recommended", we agree to change default value to FALSE in DEC declaration. > > Intel vote #2, at current point of time. > We vote #3 as long term plan, after UEFI 2.x specification removes the text later. > OK, that is fine by me. As long as we agree that it is something that is on its way to be removed, rather than a valid alternative for the MemoryAttributes table. Thanks, Ard. > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen > Sent: Tuesday, February 16, 2016 8:51 AM > To: Laszlo Ersek; Ard Biesheuvel > Cc: edk2-devel@ml01.01.org; Gao, Liming > Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. > > Thanks. I think it is easy to just choose 2). > For 3), we need start evaluating if there is any impact to any production. > > Thank you > Yao Jiewen > > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Sunday, February 14, 2016 6:53 PM > To: Ard Biesheuvel > Cc: Yao, Jiewen; edk2-devel@ml01.01.org; Gao, Liming > Subject: Re: [edk2] [patch 0/7] Add UEFI2.6 MemoryAttributesTable support. > > On 02/14/16 10:22, Ard Biesheuvel wrote: >> On 14 February 2016 at 07:51, Laszlo Ersek wrote: >>> On 02/14/16 06:44, Yao, Jiewen wrote: >>>> HI >>>> Thanks to discuss this properties table issue. >>>> The purpose of this patch is to *add* UEFI2.6 memory attributes table. This patch does not handle any UEFI2.5 properties table. >>>> If we want to change EDKII code for properties table, I suggest we separate it from this patch. >>>> >>>> Is there any comment for adding UEFI2.6 memory attributes table? >>> >>> Not from my side, thanks. >>> >>>> For UEFI2.5 properties table, let me summarize. Currently, we have several options: >>>> 0) Keep it as is. Current setting is PcdPropertiesTableEnable TRUE in MdeModulePkg.DEC. >>>> 1) Update PcdPropertiesTableEnable to FALSE in XXXPlatform.DSC file. >>>> (Platform choice. It can be static PCD or dynamic PCD.) >>>> 2) Update PcdPropertiesTableEnable to FALSE in MdeModulePkg.DEC >>>> file. (Disable Default. BIOS will not publish this table by default. >>>> If platform wants this table, it can set PCD to true.) >>>> 3) Remove PcdPropertiesTableEnable, and remove PropertiesTable >>>> support code in DxeCore. (PropertiesTable support is removed.) >>>> >>>> Personally, I do not suggest we choose 3), because this table is still defined in UEFI specification. We can remove it after UEFI spec removes it later. >>>> I do not have strong opinion for other options. >>> >>> I agree the implementation should follow the spec -- at least offer >>> the feature as long as the spec defines it. My vote would be (2). >>> >> >> I disagree. Not only was the PropertiesTable superseded by the >> MemoryAttributes table in 2.6, the recommendation not to use the >> PropertiesTable has also been added to 2.5 errata A, while it was >> originally defined in 2.5 This means effectively that it has been >> retracted, and so it does not matter if you claim to implement UEFI >> 2.4, 2.5 or 2.6, in none of these cases should the PropertiesTable be >> published. > > Well, why not drop the text from the spec completely then? > >> The reason that the definition remains in the text is for the OS side, >> not for the reference implementation of the firmware side, which >> should steer clear from it entirely. > > Okay, but in that case, I sort of think of the edk2 code as something that would let you test an operating system's support for this (mis)feature. > > Anyway, I don't disagree with (3) either. If that is what gets implemented, I'll be fine with it. As long as there's a way to disable the (mis)feature -- by way of PCD or complete removal --, I'm okay. > > Thanks > Laszlo > _______________________________________________ > 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 --------------90AEA0B034D46DF3EAE46850--