Hi Ard, > Thanks for the patch. But please consider carefully what you're doing: > My point is really that you can just remove lines 124 - 189 instead.
We want, OS to re-use firmware volume area. Therefore making firmware volume visible. You are right, removing lines 124-189 will do the same, and no need of adding splitting area with same attribute. Also, I am not sure, if we can remove this entirely, therefore we posted this as RFC:) Code comments says // 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 On our architecture, we start boot from DRAM. And we haven't seen any overwrite by DXE. This work perfectly well for us. Do you think, on other platforms it may add a risk of overwrite ? Regards Udit > -----Original Message----- > From: Ard Biesheuvel [mailto:[email protected]] > Sent: Saturday, September 16, 2017 4:25 AM > To: Leif Lindholm <[email protected]> > Cc: Meenakshi Aggarwal <[email protected]>; edk2- > [email protected]; Udit Kumar <[email protected]> > Subject: Re: [PATCH] RFC Inform UEFI memory to Linux > > On 15 September 2017 at 03:13, Leif Lindholm <[email protected]> > wrote: > > On Fri, Sep 15, 2017 at 08:02:34PM +0530, Meenakshi Aggarwal wrote: > >> From: Udit Kumar <[email protected]> > >> > >> 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: Meenakshi Aggarwal <[email protected]> > >> Signed-off-by: Udit Kumar <[email protected]> > > > > I will let Ard comment on the technical aspect, since you've been > > discussing this offline. > > > > However, there is something broken in your setup: the patch should > > look like this: > > --- > > iff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > > b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > > index 2feb11f21d..d3fa894244 100644 > > --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > > +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c > > @@ -150,7 +150,7 @@ MemoryPeim ( > > } else { > > // Create the System Memory HOB for the > > // firmware with the non-present attribute > > BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > > - ResourceAttributes & > ~EFI_RESOURCE_ATTRIBUTE_PRESENT, > > + ResourceAttributes, > > PcdGet64 (PcdFdBaseAddress), > > PcdGet32 (PcdFdSize)); > > > > @@ -161,7 +161,7 @@ MemoryPeim ( > > } else { > > // Create the System Memory HOB for the firmware > > // with the non-present attribute > > BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, > > - ResourceAttributes & > ~EFI_RESOURCE_ATTRIBUTE_PRESENT, > > + ResourceAttributes, > > PcdGet64 (PcdFdBaseAddress), > > PcdGet32 (PcdFdSize)); > > > > --- > > > > I am guessing you have inadvertently modified the line terminations > > from CRLF to LF (the evidence gets stripped out by SMTP). > > Please resubmit with this addressed. > > > > Thanks for the patch. But please consider carefully what you're doing: > in the original code, the entire DRAM region is added, and subsequently split > up > so the part that overlaps with the FD can be registered with different > attributes. > > After your patch, we carefully isolate the region that overlaps the FD, and > subsequently register it with the exact same attributes, which means we didn't > have to isolate it in the first place. > > My point is really that you can just remove lines 124 - 189 instead. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
