From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (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 30EF921AF39BD for ; Mon, 5 Jun 2017 18:11:20 -0700 (PDT) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Jun 2017 18:12:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,303,1493708400"; d="scan'208";a="95940685" Received: from sjaworsk-mobl1.amr.corp.intel.com (HELO localhost) ([10.255.73.108]) by orsmga002.jf.intel.com with ESMTP; 05 Jun 2017 18:12:23 -0700 MIME-Version: 1.0 To: Brijesh Singh , Laszlo Ersek , edk2-devel@lists.01.org, Star Zeng , Eric Dong Message-ID: <149671154262.11907.18297341281786344033@jljusten-skl> From: Jordan Justen In-Reply-To: <661e46af-5e1c-733a-d027-1ae2e3052a28@amd.com> Cc: Thomas.Lendacky@amd.com, Liming Gao , leo.duran@amd.com, Jiewen Yao , Jeff Fan 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> User-Agent: alot/0.5.1 Date: Mon, 05 Jun 2017 18:12:22 -0700 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 01:11:20 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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 express= ion > > = > > always clear SEV mask for MMIO *and all rest* > > = > > (emphasis mine), which I think we may have missed *in combination with* > > the DxeIpl. > > = > > 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. > > = > > This is going to be harder than the current approach, because: > > = > > - 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). > > = > > - 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. > > = > > 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.) > > = > > Is this what you have in mind? > > = > = > 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. :) As far as I can see, this is an architecturally defined AMD feature. (Is this true, or is BaseMemcryptSevLib actually 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.) 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. I don't know that the owners of MdeModulePkg and UefiCpuPkg will agree with me though. > 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: > = > 1) add a new null DxeGcdCorePlatformHookLib > = > https://github.com/codomania/edk2/commit/171f816376b3b0677cbfb90271a94a92= 0d7ad72d > = > The library provides a function "DxeGcdCorePlatformHookReady" which can b= e called > by DxeMain just after it initializes the GcdServices (which will guarante= e 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.) -Jordan > 2) override DxeGcdCorePlatformHookLib inside the Ovmf to clear the C-bit = when > SEV is detected. > = > https://github.com/codomania/edk2/commit/914ce904ca1b7647c966562596ba53c9= 5949f659 > = > I've tested the approach and it seems to work. Is this something aligned = with your > thinking? > = > = > > Thanks > > Laszlo > > = > >> -Jordan > >> > >>> In second patch > >>> [2], Leo tried to introduce a new notify protocol to get MMIO add/rem= ove > >>> 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 requi= re > >>> adding any CPU specific code inside the Gcd. > >>> > >>> [1] https://lists.01.org/pipermail/edk2-devel/2017-March/008974.html > >>> [2] https://lists.01.org/pipermail/edk2-devel/2017-April/009852.html > >>> > > = > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel