I was not tracking this thread. Jiewen will help give comments about the potential change in MdeModulePkg.
Thanks, Star -----Original Message----- From: Justen, Jordan L Sent: Tuesday, June 6, 2017 9:12 AM To: Brijesh Singh <[email protected]>; Laszlo Ersek <[email protected]>; [email protected]; Zeng, Star <[email protected]>; Dong, Eric <[email protected]> Cc: [email protected]; Gao, Liming <[email protected]>; [email protected]; Yao, Jiewen <[email protected]>; Fan, Jeff <[email protected]> Subject: Re: [edk2] [PATCH v6 00/17] x86: Secure Encrypted Virtualization (AMD) 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 > > > > 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/171f816376b3b0677cbfb90271a94 > a920d7ad72d > > 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.) -Jordan > 2) override DxeGcdCorePlatformHookLib inside the Ovmf to clear the C-bit when > SEV is detected. > > https://github.com/codomania/edk2/commit/914ce904ca1b7647c966562596ba5 > 3c95949f659 > > 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/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. > >>> > >>> [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 > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
