Hi Star, I agree with both the use cases.
I think only 'gDS->FreeMemorySpace' need to be updated, currently it directly sets the 'ImageHandle = NULL' instead it should do below. - If the memory is not in use by drivers, do the cleanup and set the 'ImageHandle = NULL' - If the memory is in use by drivers, do not set the 'ImageHandle = NULL' and return error like 'Memory is in use'. I think no need to change anything for 'gDS->RemoveMemorySpace' call, as it returns error (EFI_ACCESS_DENIED) if the ImageHandle != NULL. So it will automatically pass for case1 and fails for case2. Regards, Wasim > -----Original Message----- > From: Zeng, Star [mailto:[email protected]] > Sent: Monday, January 08, 2018 7:16 PM > To: Wasim Khan <[email protected]>; [email protected] > Cc: Gao, Liming <[email protected]>; Zeng, Star <[email protected]> > Subject: RE: Memory space entry is not removed after calling FreeMemorySpace > and RemoveMemorySpace > > There are two cases here. > > Case 1: > 1. gDS->AddMemorySpace > The memory range is automatically allocated for use by the UEFI memory > services. > 2. No any driver allocates memory at this memory range by gBS- > >AllocatePages()/AllocatePool(). > 3. gDS->FreeMemorySpace > 4. gDS->RemoveMemorySpace > > Case 2: > 1. gDS->AddMemorySpace > The memory range is automatically allocated for use by the UEFI memory > services. > 2. There are driver(s) allocate memory at this memory range by gBS- > >AllocatePages()/AllocatePool. > 3. gDS->FreeMemorySpace > 4. gDS->RemoveMemorySpace > > For case 1, the memory range could be removed safely from UEFI memory map > at step 3, and step 3 and 4 could return success. > For case 2, the memory range could be not removed from UEFI memory map at > step3 as the memory range is still been used by drivers, then step 3 and 4 > should > return failure. > > > Thanks, > Star > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of > Wasim Khan > Sent: Monday, January 8, 2018 8:48 PM > To: Zeng, Star <[email protected]>; [email protected] > Cc: Gao, Liming <[email protected]> > Subject: Re: [edk2] Memory space entry is not removed after calling > FreeMemorySpace and RemoveMemorySpace > > Hi Star, > > I agree with you that 'gDS->AddMemorySpace' should always pass, but if these > functions are made to return the Status, we should ideally check the Status > and > take necessary actions upon failure. > > After going through the code, if a memory space for > EfiGcdMemoryTypeSystemMemory (or EfiGcdMemoryTypeMoreReliable) is > added then memory range is automatically allocated for use by the UEFI > memory services by adding a memory descriptor to gMemoryMap . > But when we call 'gDS->FreeMemorySpace' and ''gDS->RemoveMemorySpace' , > memory space is only removed from GCD map and it is not removed from UEFI > Services map. > > So ideally while when a call to gDS->AddMemorySpace (for memory type > EfiGcdMemoryTypeSystemMemory) automatically allocates it for use by UEFI > memory services. It should also free the memory when 'gDS- > >FreeMemorySpace' call is made. > > I have opened a ticket for this issue > (https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzill > a.tianocore.org%2Fshow_bug.cgi%3Fid%3D841&data=02%7C01%7Cwasim.kha > n%40nxp.com%7Cd45b6709b297491dba9b08d5569e20f7%7C686ea1d3bc2b4c6 > fa92cd99c5c301635%7C0%7C1%7C636510159548814134&sdata=B2yvF5RzoRJa > IeNJejOR8hedKCJygbkCCkYieTNFXzw%3D&reserved=0) . > > > Regards, > Wasim > > > -----Original Message----- > > From: Zeng, Star [mailto:[email protected]] > > Sent: Monday, January 08, 2018 6:37 AM > > To: Wasim Khan <[email protected]>; [email protected] > > Cc: Gao, Liming <[email protected]>; Zeng, Star > > <[email protected]> > > Subject: RE: Memory space entry is not removed after calling > > FreeMemorySpace and RemoveMemorySpace > > > > Hi Wasim, > > > > Got the point. > > > > Yes, gDS->AddMemorySpace should return success normally, you may can > > assume it. > > Or a little tricky method if you only want the memory space to be used > > by OS kernel, but not post, I think you may can hook the > > gBS->GetMemoryMap() with your own GetMemoryMap() function, it will > > call original GetMemoryMap() function, and then append one entry for > > the memory space you want to add for OS kernel. > > > > > > > > Thanks, > > Star > > -----Original Message----- > > From: Wasim Khan [mailto:[email protected]] > > Sent: Monday, January 8, 2018 3:03 AM > > To: Zeng, Star <[email protected]>; [email protected] > > Cc: Gao, Liming <[email protected]> > > Subject: RE: Memory space entry is not removed after calling > > FreeMemorySpace and RemoveMemorySpace > > > > Hi Star, > > > > Thank you for your reply. > > > > I cannot add memory space as EfiGcdMemoryTypeReserved because for my > > use case, i am exposing memory space of an extended DDR memory to > > kernel. If I add this memory space as EfiGcdMemoryTypeReserved, then > > linux is not able to allocate memory from this region to applications > > (a simple application requesting memory from this region fails). So I > > have to add the memory space as EfiGcdMemoryTypeSystemMemory only. > > > > > What is the usage that the memory space needs to be added first and > > > removed > > later? > > No specific use case, I am checking the status of > > 'gDS->AddMemorySpace' and 'gDS->SetMemorySpaceAttributes', if any of > > these calls fails, I want to remove the added memory space. Ideally it > > should > work. > > > > > > > > Regards, > > Wasim > > > > > > > > > -----Original Message----- > > > From: Zeng, Star [mailto:[email protected]] > > > Sent: Friday, January 05, 2018 4:35 PM > > > To: Wasim Khan <[email protected]>; [email protected] > > > Cc: Zeng, Star <[email protected]>; Gao, Liming > > > <[email protected]> > > > Subject: RE: Memory space entry is not removed after calling > > > FreeMemorySpace and RemoveMemorySpace > > > > > > PI spec has clear description below in AddMemorySpace(). > > > > > > "If the memory range specified by BaseAddress and Length is of type > > > EfiGcdMemoryTypeSystemMemory or EfiGcdMemoryTypeMoreReliable, > then > > the > > > memory range may be *automatically allocated for use by the UEFI > > > memory services*." > > > > > > But PI spec has no clear description about removing the use from the > > > UEFI memory services in FreeMemorySpace() and RemoveMemorySpace(). > > > I think it is very hard (may be not possible) as the added memory > > > space may have been used by UEFI memory (page) services for drivers > > > after the memory space is added by AddMemorySpace(). > > > > > > What is the usage that the memory space needs to be added first and > > > removed later? > > > Could the memory space be added as EfiGcdMemoryTypeReserved type > > > instead of EfiGcdMemoryTypeSystemMemory type? > > > > > > > > > Thanks, > > > Star > > > -----Original Message----- > > > From: edk2-devel [mailto:[email protected]] On Behalf > > > Of Wasim Khan > > > Sent: Friday, January 5, 2018 5:59 PM > > > To: [email protected] > > > Subject: [edk2] Memory space entry is not removed after calling > > > FreeMemorySpace and RemoveMemorySpace > > > > > > Hi All, > > > > > > I am facing a problem that if a add a memory space using 'gDS- > > > >AddMemorySpace' and then remove it using 'gDS->FreeMemorySpace' > > > followed by 'gDS->RemoveMemorySpace'. > > > I can still see the entry of added memory space in the table shown > > > by > > 'memmap' > > > command. > > > > > > I enabled DEBUG_GCD and as per logs , the entry is removed from > > > GcdMemorySpaceMap , but when I run 'memmap' command which gets the > > > memory map using 'gBS->GetMemoryMap', I can see that entry for the > > > added memory space is still present. Steps and Logs are below for > > > reference > > > > > > Do I need to perform any other steps in order to cleanly remove the > > > memory space entry ? > > > > > > Regards, > > > Wasim > > > > > > > > > > > > Below are the steps and GCD debug logs. > > > Steps 1 : Add a memory space . We can see that an entry from system > > > memory is added. > > > > > > Status = gDS->AddMemorySpace ( > > > EfiGcdMemoryTypeSystemMemory, > > > > > > FixedPcdGet64(PcdSystemMemoryExBase), > > > SystemMemoryExSize, > > > EFI_MEMORY_WC | EFI_MEMORY_WT > > > | > > > EFI_MEMORY_WB > > > ); > > > > > > Logs: > > > > > > > > > GCD:AddMemorySpace(Base=0000008080000000,Length=0000000380000000) > > > GcdMemoryType = SystemMem > > > Capabilities = 000000000000000E > > > CoreConvertSpace 774 > > > Status = Success > > > GCDMemType Range Capabilities Attributes > > > ========== ================================= ================ > > > ================ > > > NonExist 0000000000000000-000000009FFFFFFF 0000000000000000 > > > 0000000000000000 > > > SystemMem 00000000A0000000-00000000DFFFFFFF 800000000000000E > > > 0000000000000008* > > > NonExist 00000000E0000000-00000000E01BFFFF 0000000000000000 > > > 0000000000000000 > > > SystemMem 00000000E01C0000-00000000FFFFFFFF 800000000000000E > > > 0000000000000008* > > > NonExist 0000000100000000-000000807FFFFFFF 0000000000000000 > > > 0000000000000000 > > > SystemMem 0000008080000000-00000083FFFFFFFF 800000000000000E > > > 0000000000000000 > > > NonExist 0000008400000000-0000FFFFFFFFFFFF 0000000000000000 > > > 0000000000000000 > > > > > > > > > GCD:AllocateMemorySpace(Base=0000008080000000,Length=00000003800000 > > > 00) > > > GcdAllocateType = AtAddress > > > GcdMemoryType = SystemMem > > > Alignment = 0000000000001000 > > > ImageHandle = FED1FF98 > > > DeviceHandle = 0 > > > Status = Success (BaseAddress = 0000008080000000) > > > GCDMemType Range Capabilities Attributes > > > ========== ================================= ================ > > > ================ > > > NonExist 0000000000000000-000000009FFFFFFF 0000000000000000 > > > 0000000000000000 > > > SystemMem 00000000A0000000-00000000DFFFFFFF 800000000000000E > > > 0000000000000008* > > > NonExist 00000000E0000000-00000000E01BFFFF 0000000000000000 > > > 0000000000000000 > > > SystemMem 00000000E01C0000-00000000FFFFFFFF 800000000000000E > > > 0000000000000008* > > > NonExist 0000000100000000-000000807FFFFFFF 0000000000000000 > > > 0000000000000000 > > > SystemMem 0000008080000000-00000083FFFFFFFF 800000000000000E > > > 0000000000000000* > > > NonExist 0000008400000000-0000FFFFFFFFFFFF 0000000000000000 > > > 0000000000000000 > > > > > > > > > Step2: Free the memory space > > > Status = gDS->FreeMemorySpace ( > > > FixedPcdGet64(PcdSystemMemoryExBase), > > > SystemMemoryExSize > > > ); > > > > > > Logs: > > > > > > > > > GCD:FreeMemorySpace(Base=0000008080000000,Length=0000000380000000) > > > Status = Success > > > GCDMemType Range Capabilities Attributes > > > ========== ================================= ================ > > > ================ > > > NonExist 0000000000000000-000000009FFFFFFF 0000000000000000 > > > 0000000000000000 > > > SystemMem 00000000A0000000-00000000DFFFFFFF 800000000000000E > > > 0000000000000008* > > > NonExist 00000000E0000000-00000000E01BFFFF 0000000000000000 > > > 0000000000000000 > > > SystemMem 00000000E01C0000-00000000FFFFFFFF 800000000000000E > > > 0000000000000008* > > > NonExist 0000000100000000-000000807FFFFFFF 0000000000000000 > > > 0000000000000000 > > > SystemMem 0000008080000000-00000083FFFFFFFF 800000000000000E > > > 0000000000000000 > > > NonExist 0000008400000000-0000FFFFFFFFFFFF 0000000000000000 > > > 0000000000000000 > > > > > > > > > Step3: Remove the memory space , As we can see that entry 'SystemMem > > > 0000008080000000-00000083FFFFFFFF' is removed. > > > Status = gDS->RemoveMemorySpace ( > > > FixedPcdGet64(PcdSystemMemoryExBase), > > > SystemMemoryExSize > > > ); > > > > > > Logs: > > > > > > > > > GCD:RemoveMemorySpace(Base=0000008080000000,Length=00000003800000 > > > 00) > > > Status = Success > > > GCDMemType Range Capabilities Attributes > > > ========== ================================= ================ > > > ================ > > > NonExist 0000000000000000-000000009FFFFFFF 0000000000000000 > > > 0000000000000000 > > > SystemMem 00000000A0000000-00000000DFFFFFFF 800000000000000E > > > 0000000000000008* > > > NonExist 00000000E0000000-00000000E01BFFFF 0000000000000000 > > > 0000000000000000 > > > SystemMem 00000000E01C0000-00000000FFFFFFFF 800000000000000E > > > 0000000000000008* > > > NonExist 0000000100000000-0000FFFFFFFFFFFF 0000000000000000 > > > 0000000000000000 > > > > > > Step4 : Run the 'memmap' command. As we can see that entry is still > present. > > > Shell> memmap > > > Type Start End # Pages Attributes > > > Available 00000000A0000000-00000000DFFFFFFF 0000000000040000 > > > 000000000000000E Available 00000000E01C0000-00000000E53C0FFF > > > 0000000000005201 000000000000000E > > > BS_Data 00000000E53C1000-00000000E5D99FFF 00000000000009D9 > > > 000000000000000E > > > ... > > > Available 0000008080000000-00000083FFFFFFFF 0000000000380000 > > > 000000000000000E > > > ... > > > _______________________________________________ > > > edk2-devel mailing list > > > [email protected] > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fl > > > is > > > ts.01 > > > .org%2Fmailman%2Flistinfo%2Fedk2- > > > > > > devel&data=02%7C01%7Cwasim.khan%40nxp.com%7C2be1d44f45084e831b92 > > > > > > 08d5542c1b7f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636507 > > > > > > 470804351379&sdata=B4v0K9MGZEWiSNYinRtWPGB%2F85biiU5iqpHmAE3YxsQ > > > %3D&reserved=0 > _______________________________________________ > edk2-devel mailing list > [email protected] > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01 > .org%2Fmailman%2Flistinfo%2Fedk2- > devel&data=02%7C01%7Cwasim.khan%40nxp.com%7Cd45b6709b297491dba9b > 08d5569e20f7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C636510 > 159548814134&sdata=0evsd87geFVyGlX0rnzyLOXnIVrt4vbcLRgEQaZwWZw%3D > &reserved=0 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
