From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-in4.apple.com (mail-out4.apple.com [17.151.62.26]) (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 5803121AE30D0 for ; Tue, 6 Jun 2017 08:23:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; d=apple.com; s=mailout2048s; c=relaxed/simple; q=dns/txt; i=@apple.com; t=1496762682; h=From:Sender:Reply-To:Subject:Date:Message-id:To:Cc:MIME-version:Content-type: Content-transfer-encoding:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-reply-to:References:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=XfgWPdlhRJd8J7VexCVaPRwKBQ+FctmJd9DF/40/1Rc=; b=G3F6bhKMqmM8YEgrAHDNeJ9IrL85VmBac9VxVGmx+b7OrwhqpSEPsQdCVoXx/Uwh bRKpKtX73LboWMUDlRsLWkwme2J7N6Qk29KwdbKe6jaH+uUcevirN5hzzhbAj122 gq5qXgJHRtxQRG/izlW0u2kbJ9m8GTd4okR7N2L7HeNc0SAOWet7FebubjkWo318 0en7vu8iV4Qal5uiGWyazpiC+2WwujSCWxLTclU2hzvwXeIoErZxq1vJ+A8Z+N/T 1C2qOaYTqvhxnukTNRBmvF2ADui2Eyei+6QDsojFQhYXCnNhe5DY0QVe2sdRjFHe jLsq0rFRdfA0lg+5aVwoPw==; Received: from relay3.apple.com (relay3.apple.com [17.128.113.83]) (using TLS with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mail-in4.apple.com (Apple Secure Mail Relay) with SMTP id 2C.22.01052.939C6395; Tue, 6 Jun 2017 08:24:42 -0700 (PDT) X-AuditID: 11973e12-24ffb7000000041c-7f-5936c939a37e Received: from nwk-mmpp-sz10.apple.com (nwk-mmpp-sz10.apple.com [17.128.115.122]) by relay3.apple.com (Apple SCV relay) with SMTP id 51.FB.15148.839C6395; Tue, 6 Jun 2017 08:24:40 -0700 (PDT) MIME-version: 1.0 Received: from [17.153.85.153] (unknown [17.153.85.153]) by nwk-mmpp-sz10.apple.com (Oracle Communications Messaging Server 8.0.1.2.20170210 64bit (built Feb 10 2017)) with ESMTPSA id <0OR4002BATH39230@nwk-mmpp-sz10.apple.com>; Tue, 06 Jun 2017 08:24:40 -0700 (PDT) Sender: afish@apple.com From: Andrew Fish In-reply-to: <74D8A39837DF1E4DA445A8C0B3885C503A964542@shsmsx102.ccr.corp.intel.com> Date: Tue, 06 Jun 2017 08:24:38 -0700 Cc: Brijesh Singh , "Zeng, Star" , Jordan Justen , Laszlo Ersek , "edk2-devel@lists.01.org" , "Dong, Eric" , "Thomas.Lendacky@amd.com" , "leo.duran@amd.com" , "Fan, Jeff" , "Gao, Liming" Message-id: <89CEBE18-D16B-4D1E-8E51-263C4375FDB2@apple.com> References: <1495809845-32472-1-git-send-email-brijesh.singh@amd.com> <149583274037.25973.13062338567511386932@jljusten-skl> <6ecd0138-454e-6a6e-d034-beaf63466120@redhat.com> <149609029319.5770.13917390389219314003@jljusten-skl> <14301d64-9fa3-8231-42c1-52c2dcd9f96f@amd.com> <149630284935.10663.16670660897918560882@jljusten-skl> <661e46af-5e1c-733a-d027-1ae2e3052a28@amd.com> <149671154262.11907.18297341281786344033@jljusten-skl> <0C09AFA07DD0434D9E2A0C6AEB0483103B8E3C1D@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A964542@shsmsx102.ccr.corp.intel.com> To: "Yao, Jiewen" X-Mailer: Apple Mail (2.3273) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrHLMWRmVeSWpSXmKPExsUi2FAYrGt10izS4O5cZouZm/oYLfYcOsps sflFsMXJ9UsYLdZ99LDYca2fxaL7+Ul2i2XHdrBYrLi3gd1iX6+1xZEp+1gduD1aL/1l81i8 5yWTR/fsfywe7/ddZQtgieKySUnNySxLLdK3S+DKePJyDUvBtMCKxWeTGxgnOnUxcnJICJhI 9Hd8YOxi5OIQEljNJHG2/ygrTGJl5ys2iMQhRokv07vYQRK8AoISPybfY+li5OBgFlCXmDIl FyQsJDCRSeLMZD4QW1hAXOLdmU3MELa/xJmfV8BsNgFliRXzP4CN4RQIkziybiYLiM0ioCpx /P4sJpBdzAJLmSXWtc8DSzALaEs8eXeBFWQXr4CNxM2DshD3bGeVWLj2KtihIgIaEifmz2GG OFpW4tbsS8wgRRICv9kkrl+6zz6BUXgWkrtnIdw9C8mKBYzMqxiFchMzc3Qz80z0EgsKclL1 kvNzNzGCImi6ndAOxlOrrA4xCnAwKvHwZuwxixRiTSwrrsw9xCjNwaIkznvjoVGkkEB6Yklq dmpqQWpRfFFpTmrxIUYmDk6pBsbSgPm3N08OdDtUwewSqPBKWOzWdKNX9xmCmCZKzpXUq3yx bvdFj+2ZhZ/fB+lcfd136deNcw6Bgb6btqsKvdSIW1pcr1JkFu3Vw/4+QbBsj/vKWv792zqW Vr+Pk3vWc1jpwk7J5798vMxCLDWeT3j01W2GxM5b/Qb7AlNzTroHiXxMfeTfUqHEUpyRaKjF XFScCABlSxiJgQIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrAIsWRmVeSWpSXmKPExsUi2FBcpWt50izSoLOV0WLmpj5Giz2HjjJb bH4RbHFy/RJGi3UfPSx2XOtnseh+fpLdYtmxHSwWK+5tYLfY12ttcWTKPlYHbo/WS3/ZPBbv ecnk0T37H4vH+31X2QJYorhsUlJzMstSi/TtErgynrxcw1IwLbBi8dnkBsaJTl2MnBwSAiYS KztfsXUxcnEICRxilPgyvYsdJMErICjxY/I9li5GDg5mAXWJKVNyQcJCAhOZJM5M5gOxhQXE Jd6d2cQMYftLnPl5BcxmE1CWWDH/A9gYToEwiSPrZrKA2CwCqhLH789iAtnFLLCUWWJd+zyw BLOAtsSTdxdYQXbxCthI3DwoC3HPdlaJhWuvsoLUiAhoSJyYP4cZ4mhZiVuzLzFPYBSYheTU WQinzkIydQEj8ypGgaLUnMRKY73EgoKcVL3k/NxNjOCALwzewfhnmdUhRgEORiUeXoFdZpFC rIllxZW5wLDgYFYS4b21BijEm5JYWZValB9fVJqTWnyIsQromYnMUqLJ+cBozCuJNzQxMTAx NjYzNjY3MaeKsJI4r/F+o0ghgfTEktTs1NSC1CKY5UwcnFINjFK2SoufzJ8068ii6wtWFV+8 eOmxRLeaTsKUZcXT7UIWeDSx2Xo9uGcxY8NVxycxGX2z712JSRZQ5mxtSDTt81Jyy1J6Mmd/ 3wLGn/5/PW7Wsf8Uu7hrp2vMOxmFJ7kPjq27UBsx5YaNj+qsKn6WxkNeB2KXVB2KWXSseG7f yXOrujc/W1n61l+JpTgj0VCLuag4EQD/enPU0wIAAA== Subject: Re: [PATCH v6 00/17] x86: Secure Encrypted Virtualization (AMD) X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Jun 2017 15:23:35 -0000 Content-type: text/plain; charset=utf-8 Content-transfer-encoding: quoted-printable > On Jun 6, 2017, at 7:54 AM, Yao, Jiewen wrote: >=20 > Hi > It takes me some time to read all email below. I believe all of us = have a clean understanding on what problem we have now and the possible = solutions to clear C bit are below >=20 > 1) In DxeIpl, when it builds page table. >=20 > 2) In DxeCore >=20 > a) By use CpuArch >=20 > b) By use page table lib >=20 > c) By use a GCD update callback >=20 > d) By use PlatformHook lib >=20 > 3) In a standalone AmdSev driver. >=20 > Here is my thought: > 2.a) is not possible, per Leo=E2=80=99s investigation. > 2.b) is not a good design, because we do not introduce any Cpu = Specific thing to DxeCore so far. > 2.c) and 2.d) are same. I do not suggest we add a private interface to = the core just to support one specific feature. >=20 > 1) is one possible solution, I suggested before. But if Leo/Laszlo = think it is too hard to implement, I am OK. >=20 > If 1) cannot be chosen, I still think 3) is the best idea. > It makes the code very clean by introducing a standalone driver to = resolve the problem. > Zero impact on existing platform. > If this feature is not needed, just remove the driver. >=20 > I do not see any issue on using a priori, because: A) =E2=80=9Ca = priori=E2=80=9D is clearly defined in PI spec, B) =E2=80=9Ca priori=E2=80=9D= has already been widely used in current platform in EDKII open source, = as well as close source platform. >=20 Jiewen, I agree that "a priori" is part of the architecture so it is OK to use = it, but "a priori" was never really intended as a way to add basic = features. it was more for debugging and work arounds. It seems like a = feature like this should not require a work around.... 'So I think it is OK to accept this patch to get the feature enabled, = but we need to look at the GCD implementation and PI architecture to = figure out why there is not a cleaner way to add this feature. Maybe we = need to change the implementation, and/or the PI Spec? Thanks, Andrew Fish > Thank you > Yao Jiewen >=20 >=20 >=20 > From: Brijesh Singh [mailto:brijesh.singh@amd.com] > Sent: Tuesday, June 6, 2017 11:51 AM > To: Zeng, Star ; Justen, Jordan L = ; Laszlo Ersek ; = edk2-devel@lists.01.org; Dong, Eric ; Yao, Jiewen = > Cc: Thomas.Lendacky@amd.com; Gao, Liming ; = leo.duran@amd.com; Fan, Jeff > Subject: Re: [edk2] [PATCH v6 00/17] x86: Secure Encrypted = Virtualization (AMD) >=20 > Hi Jordan, >=20 >=20 > On 6/5/17 9:08 PM, Zeng, Star wrote: >> I was not tracking this thread. >> Jiewen will help give comments about the potential change in = MdeModulePkg. >>=20 >> Thanks, >> Star >> -----Original Message----- >> From: Justen, Jordan L >> Sent: Tuesday, June 6, 2017 9:12 AM >> To: Brijesh Singh = >; Laszlo Ersek = >; = edk2-devel@lists.01.org; Zeng, Star = >; Dong, Eric = > >> Cc: Thomas.Lendacky@amd.com; Gao, = Liming >; = leo.duran@amd.com; Yao, Jiewen = >; Fan, Jeff = > >> Subject: Re: [edk2] [PATCH v6 00/17] x86: Secure Encrypted = Virtualization (AMD) >>=20 >> On 2017-06-05 14:56:04, Brijesh Singh wrote: >>> On 06/01/2017 04:10 AM, Laszlo Ersek wrote: >>>> On 06/01/17 09:40, Jordan Justen wrote: >>>>> In = https://lists.01.org/pipermail/edk2-devel/2017-April/009883.html >>>>> Leo said that DxeIpl won't work because new I/O ranges might be = added. >>>>> I don't understand this, because isn't DxeIpl and an early APRIORI >>>>> entry are roughly equivalent in the boot sequence? >>>> I think you are right. I believe a patch for this exact idea hasn't >>>> been posted yet. Jiewen's message that you linked above contains = the >>>> expression >>>>=20 >>>> always clear SEV mask for MMIO *and all rest* >>>>=20 >>>> (emphasis mine), which I think we may have missed *in combination >>>> with* the DxeIpl. >>>>=20 >>>> So the idea would be to iterate over all the HOBs in the DxeIpl = PEIM. >>>> Keep the C bit set for system memory regions. Clear the C bit for >>>> MMIO regions that are known from the HOB list. Also clear the C bit >>>> everywhere else in the address space (known from the CPU HOB) where >>>> no coverage is provided by any memory resource descriptor HOB. >>>>=20 >>>> This is going to be harder than the current approach, because: >>>>=20 >>>> - The current approach can work off of the GCD memory space map, >>>> which provides explicit NonExistent entries, covering the entire >>>> address space (according to the CPU HOB). >>>>=20 >>>> - However, the DxeIpl method would take place before entering DXE, >>>> so no GCD memory space map would be available -- the "NonExistent" >>>> entries would have to be synthesized manually from the address = space >>>> size (known from the CPU HOB) and the lack of coverage by memory >>>> resource descriptor HOBs. >>>>=20 >>>> Basically, in order to move the current GCD memory space map >>>> traversal from early DXE to late PEI, the memory space map building >>>> logic of the DXE Core would have to be duplicated in the DxeIpl >>>> PEIM. If I understand correctly. (The DxeIpl PEIM may already >>>> contain very similar code, for the page table building, which might >>>> not be difficult to extend like this -- I haven't looked.) >>>>=20 >>>> Is this what you have in mind? >>>>=20 >>> Do you have any further thought on this? >> Regarding Laszlo's feedback, I'm not convinced that it would be = excessively difficult to accomplish this in DxeIpl. (I'm not saying that = I couldn't be convinced. :) >>=20 >> As far as I can see, this is an architecturally defined AMD feature. >> (Is this true, or is BaseMemcryptSevLib actually OVMF specific?) >=20 > Yes, SEV is AMD-V architecture extension and its applicable to > virtualization platform only (we can says BaseMemEncryptSevLib is OVMF > specific). >> You've asserted that it should work (SEV would not be detected) with = any Intel processor as well. Therefore, I don't see a good reason that = we shouldn't be able to support it in modules that already have >> IA32/X64 specific code. (I'm recalling >> 881813d7a93d9009c873515b043c41c4554779e4.) >>=20 >> Since DxeIpl builds the IA32/X64 page tables, and you need to modify = the page tables for this feature (correct?), I think we should try to = support the feature there if it is feasible. I can understand the = argument that this doesn't apply to all non-VM platforms, so I think we = could add a PCD which disables this support by default. >>=20 >> I don't know that the owners of MdeModulePkg and UefiCpuPkg will = agree with me though. >=20 > I am flexible to implement APRIORI or Platform hooks Lib. But one = thing > I want to highlight is: I'll prefer clearing C-bit through > BaseMemEncryptSevLib functions. One of the main reason for doing so - = In > future when we add migration support for the SEV guest then we will be > required to notify the unencrypted page range to hypevisor ( through > hypercall). During migration phase, Hypervisor will use this = information > to make decision on whether to invoke the SEV firmware to encrypt the > memory region for transport purposes. If clearing C-bit logic is > contained inside BaseMemEncryptSevLib then it will make life much = easier. >=20 >>> In meantime, I have been looking into MdeModule/Core/Dxe/DxeMain to >>> see if I can invoke a platform dependent library to clear C-bit = before >>> DxeMain finishes its execution. As Laszlo pointed, current approach = is >>> using GCD memory space map to get MMIO and NonExistent entries. I = have >>> pushed two patches in my development branch to show what I have been = doing: >>>=20 >>> 1) add a new null DxeGcdCorePlatformHookLib >>>=20 >>> = https://github.com/codomania/edk2/commit/171f816376b3b0677cbfb90271a94 >>> a920d7ad72d >>>=20 >>> The library provides a function "DxeGcdCorePlatformHookReady" which >>> can be called by DxeMain just after it initializes the GcdServices >>> (which will guarantee that Gcd memory space map is available). >> Regarding hooking into DxeCore, I don't think it is the best = approach, but it is better than APRIORI. I wonder if the MdeModulePkg = owners could jump in with an opinion. (Hopefully besides just pushing = the problem away via APRIORI.) >=20 > Jiewen, any comments ? >=20 >> -Jordan >>=20 >>> 2) override DxeGcdCorePlatformHookLib inside the Ovmf to clear the = C-bit when >>> SEV is detected. >>>=20 >>> = https://github.com/codomania/edk2/commit/914ce904ca1b7647c966562596ba5 >>> 3c95949f659 >>>=20 >>> I've tested the approach and it seems to work. Is this something >>> aligned with your thinking? >>>=20 >>>=20 >>>> Thanks >>>> Laszlo >>>>=20 >>>>> -Jordan >>>>>=20 >>>>>> In second patch >>>>>> [2], Leo tried to introduce a new notify protocol to get MMIO >>>>>> add/remove events. During discussion Jiewen suggested to look = into >>>>>> adding a new platform driver into APRIORI to avoid the need for >>>>>> any modifications inside the Gcdcore - this seems workable >>>>>> solution which did not require adding any CPU specific code = inside the Gcd. >>>>>>=20 >>>>>> [1] >>>>>> https://lists.01.org/pipermail/edk2-devel/2017-March/008974.html >>>>>> [2] >>>>>> https://lists.01.org/pipermail/edk2-devel/2017-April/009852.html >>>>>>=20 >>> _______________________________________________ >>> 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