Hi Ard, Sorry for coming back late on this Are you ok to apply this patch in edk2
Thx Udit > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Sakar > Arora > Sent: Wednesday, September 20, 2017 1:50 PM > To: Ard Biesheuvel <ard.biesheu...@linaro.org> > Cc: edk2-devel@lists.01.org; leif.lindh...@linaro.org > Subject: Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux > > Thanks for the information. Seems my understanding was not correct in this > context. I have no other doubts on this change. > > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: Wednesday, September 20, 2017 12:02 PM > To: Sakar Arora <sakar.ar...@arm.com> > Cc: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>; edk2- > de...@lists.01.org; leif.lindh...@linaro.org > Subject: Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux > > On 19 September 2017 at 22:32, Sakar Arora <sakar.ar...@arm.com> wrote: > > The DXE core dispatcher relies on the available memory to allocate space for > loading the rest of the modules from the UEFI image. If we declare the UEFI > image memory space (from which DXE dispatcher reads the efi modules) open to > allocation, it might lead to data corruption, depending on the location of > UEFI > image and cumulative size of uncompressed EFI modules. > > > > Also, since UEFI allows unloading and loading of drivers at runtime, IMO, > > there > is a need to preserve the UEFI image even after all the modules have been > decompressed and loaded in the boot sequence. > > > > None of this is relevant. The uncompressed firmware volume containing DXE > core and everything else is preserved as before. The only thing that gets > discarded is the outer FV, which only contains the PrePi SEC module, and the > compressed FV, neither of which is ever touched again after DXE core has > started executing. So we should not register the FV in the first place, and > not > reserve the memory so we don't lose it. > > If you still think this may break anything, could you please elaborate? > > > > > -----Original Message----- > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > > Sent: Tuesday, September 19, 2017 6:18 PM > > To: Sakar Arora <sakar.ar...@arm.com> > > Cc: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>; > > edk2-devel@lists.01.org; leif.lindh...@linaro.org > > Subject: Re: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux > > > > On 19 September 2017 at 01:07, Sakar Arora <sakar.ar...@arm.com> wrote: > >> This change will create the possibility for memory space holding the UEFI > image to be over-written by the DXE core code, since this space will then be > available for allocation. > > > > Yes. But why does this matter after the entire payload has been decompressed > into memory already? > > > > > >> Any such change, if required, should be done just before booting the OS. > >> > >> -----Original Message----- > >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf > >> Of Meenakshi Aggarwal > >> Sent: Tuesday, September 19, 2017 6:02 PM > >> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org; > >> ard.biesheu...@linaro.org > >> Subject: [edk2] [PATCH v2] PeiLib : Inform UEFI memory to Linux > >> > >> While creating Hob list, ArmPlatformPkg is hiding UEFI memory. > >> whereas this memory can be used by OS. > >> > >> This patch, allows OS to use UEFI code area. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Udit Kumar <udit.ku...@nxp.com> > >> Signed-off-by: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com> > >> --- > >> ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 69 > >> ------------------------- > >> 1 file changed, 69 deletions(-) > >> > >> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > >> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > >> index 2feb11f..d03214b 100644 > >> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > >> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > >> @@ -70,11 +70,7 @@ MemoryPeim ( > >> { > >> ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable; > >> EFI_RESOURCE_ATTRIBUTE_TYPE ResourceAttributes; > >> - UINT64 ResourceLength; > >> EFI_PEI_HOB_POINTERS NextHob; > >> - EFI_PHYSICAL_ADDRESS FdTop; > >> - EFI_PHYSICAL_ADDRESS SystemMemoryTop; > >> - EFI_PHYSICAL_ADDRESS ResourceTop; > >> BOOLEAN Found; > >> > >> // Get Virtual Memory Map from the Platform Library @@ -121,71 +117,6 > @@ MemoryPeim ( > >> ); > >> } > >> > >> - // > >> - // Reserved the memory space occupied by the firmware volume > >> - // > >> - > >> - SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 > >> (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64 > >> (PcdSystemMemorySize); > >> - FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) + > >> (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize); > >> - > >> - // EDK2 does not have the concept of boot firmware copied into > >> DRAM. To avoid the DXE > >> - // core to overwrite this area we must mark the region with the > >> attribute non-present > >> - if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) > && (FdTop <= SystemMemoryTop)) { > >> - Found = FALSE; > >> - > >> - // Search for System Memory Hob that contains the firmware > >> - NextHob.Raw = GetHobList (); > >> - while ((NextHob.Raw = GetNextHob > (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) { > >> - if ((NextHob.ResourceDescriptor->ResourceType == > EFI_RESOURCE_SYSTEM_MEMORY) && > >> - (PcdGet64 (PcdFdBaseAddress) >= NextHob.ResourceDescriptor- > >PhysicalStart) && > >> - (FdTop <= NextHob.ResourceDescriptor->PhysicalStart + > NextHob.ResourceDescriptor->ResourceLength)) > >> - { > >> - ResourceAttributes = > >> NextHob.ResourceDescriptor->ResourceAttribute; > >> - ResourceLength = NextHob.ResourceDescriptor->ResourceLength; > >> - ResourceTop = NextHob.ResourceDescriptor->PhysicalStart + > ResourceLength; > >> - > >> - if (PcdGet64 (PcdFdBaseAddress) == NextHob.ResourceDescriptor- > >PhysicalStart) { > >> - if (SystemMemoryTop == FdTop) { > >> - NextHob.ResourceDescriptor->ResourceAttribute = > ResourceAttributes & ~EFI_RESOURCE_ATTRIBUTE_PRESENT; > >> - } else { > >> - // Create the System Memory HOB for the firmware with the non- > present attribute > >> - BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > >> - ResourceAttributes & > ~EFI_RESOURCE_ATTRIBUTE_PRESENT, > >> - PcdGet64 (PcdFdBaseAddress), > >> - PcdGet32 (PcdFdSize)); > >> - > >> - // Top of the FD is system memory available for UEFI > >> - NextHob.ResourceDescriptor->PhysicalStart += > >> PcdGet32(PcdFdSize); > >> - NextHob.ResourceDescriptor->ResourceLength -= > PcdGet32(PcdFdSize); > >> - } > >> - } else { > >> - // Create the System Memory HOB for the firmware with the non- > present attribute > >> - BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > >> - ResourceAttributes & > ~EFI_RESOURCE_ATTRIBUTE_PRESENT, > >> - PcdGet64 (PcdFdBaseAddress), > >> - PcdGet32 (PcdFdSize)); > >> - > >> - // Update the HOB > >> - NextHob.ResourceDescriptor->ResourceLength = PcdGet64 > (PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart; > >> - > >> - // If there is some memory available on the top of the FD then > >> create a > HOB > >> - if (FdTop < NextHob.ResourceDescriptor->PhysicalStart + > ResourceLength) { > >> - // Create the System Memory HOB for the remaining region (top > >> of > the FD) > >> - BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > >> - ResourceAttributes, > >> - FdTop, > >> - ResourceTop - FdTop); > >> - } > >> - } > >> - Found = TRUE; > >> - break; > >> - } > >> - NextHob.Raw = GET_NEXT_HOB (NextHob); > >> - } > >> - > >> - ASSERT(Found); > >> - } > >> - > >> // Build Memory Allocation Hob > >> InitMmu (MemoryTable); > >> > >> -- > >> 1.9.1 > >> > >> _______________________________________________ > >> edk2-devel mailing list > >> edk2-devel@lists.01.org > >> https://lists.01.org/mailman/listinfo/edk2-devel > >> IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, > please notify the sender immediately and do not disclose the contents to any > other person, use it for any purpose, or store or copy the information in any > medium. Thank you. > > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, > please notify the sender immediately and do not disclose the contents to any > other person, use it for any purpose, or store or copy the information in any > medium. Thank you. > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, > please notify the sender immediately and do not disclose the contents to any > other person, use it for any purpose, or store or copy the information in any > medium. Thank you. > _______________________________________________ > 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
