On 03/20/17 15:08, Ard Biesheuvel wrote: > On 20 March 2017 at 11:38, Laszlo Ersek <ler...@redhat.com> wrote: >> On 03/20/17 12:20, Ard Biesheuvel wrote: >>> On 20 March 2017 at 11:16, Michael Zimmermann <sigmaepsilo...@gmail.com> >>> wrote: >>>> Ard, why is SetMSetMemorySpaceAttributes being called in first place? >>>> (ignoring the recent NX patch) >>>> Looking at the initial GCD, it looks like unused memory usually >>>> doesn't have any attributes set anyway. >>>> >>> >>> Originally, we added the new memory with >>> EFI_MEMORY_WB|EFI_MEMORY_WT|EFI_MEMORY_WC|EFI_MEMORY_UC capabilities, >>> and selected the EFI_MEMORY_WB attribute with the call to >>> gDS->SetMemorySpaceAttributes. Later, we removed all capabilities >>> expect EFI_MEMORY_WB, since the other ones cannot be supported under >>> virtualization with KVM. >>> >>> Whether that makes the SetMemorySpaceAttributes redundant is not >>> entirely clear to me, but it does appear the adding the memory does >>> the right thing wrt non-exec permissions if the policy is enabled. So >>> perhaps we can simply drop this call? >> >> Won't that turn off caching for the memory just added? >> > > I think it may not map the memory at all in this case, so we need to > do something. It looks like calling mCpu->SetMemoryAttributes() should > be sufficient here, and so I wonder whether we violate anything by > replacing gDS->SetMemorySpaceAttributes with mCpu->SetMemoryAttributes > here.
Earlier you mentioned that we cannot get some piece of information from the GCD map, which limits what we can do here. Looking at GetMemorySpaceDescriptor() and GetMemorySpaceMap(), they should return both Capabilities and Attributes. Also, looking at vol2 in PI 1.5, I find: * GetMemorySpaceDescriptor() returns EFI_NOT_AVAILABLE_YET if "The attributes cannot be set because CPU architectural protocol is not available yet." * 9.7.3.2 SetMemorySpaceAttributes() -- When the DXE Foundation is notified that the EFI_CPU_ARCH_PROTOCOL has been installed, then the DXE Service SetMemorySpaceAttributes() can be made available. The DXE Foundation can then use the SetMemoryAttributes() service of the EFI_CPU_ARCH_PROTOCOL to implement the DXE Service SetMemorySpaceAttributes(). * 9.7.3.3 GetMemorySpaceMap() -- When the DXE Foundation is notified that the EFI_CPU_ARCH_PROTOCOL has been installed, then the DXE Service GetMemorySpaceMap() is fully functional. This function is made available when the memory-based services are initialized. However, the Attributes field of the array of EFI_GCD_MEMORY_SPACE_DESCRIPTORs is not valid until the EFI_CPU_ARCH_PROTOCOL is installed. So, assuming that you have tested GetMemorySpaceMap() earlier, and have found Attributes useless for the protection (or other) purposes, may that have happened because the CPU arch protocol was not available yet? ... I guess my email is a bit confusing. My points are, (a) we should likely not mess directly with the CPU arch protocol if we know (and we do know) that the GCD services use them internally, (b) are we absolutely sure that the GCD services can't help us here? If nothing else works, I agree we can mess with the CPU arch protocol directly. Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
