Re: Memory space entry is not removed after calling FreeMemorySpace and RemoveMemorySpace

[Wasim Khan <wasim.khan@nxp.com>]

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:star.zeng@intel.com]
> Sent: Monday, January 08, 2018 7:16 PM
> To: Wasim Khan <wasim.khan@nxp.com>; edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
> 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:edk2-devel-bounces@lists.01.org] On Behalf Of
> Wasim Khan
> Sent: Monday, January 8, 2018 8:48 PM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> Cc: Gao, Liming <liming.gao@intel.com>
> 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:star.zeng@intel.com]
> > Sent: Monday, January 08, 2018 6:37 AM
> > To: Wasim Khan <wasim.khan@nxp.com>; edk2-devel@lists.01.org
> > Cc: Gao, Liming <liming.gao@intel.com>; Zeng, Star
> > <star.zeng@intel.com>
> > 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:wasim.khan@nxp.com]
> > Sent: Monday, January 8, 2018 3:03 AM
> > To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> > Cc: Gao, Liming <liming.gao@intel.com>
> > 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:star.zeng@intel.com]
> > > Sent: Friday, January 05, 2018 4:35 PM
> > > To: Wasim Khan <wasim.khan@nxp.com>; edk2-devel@lists.01.org
> > > Cc: Zeng, Star <star.zeng@intel.com>; Gao, Liming
> > > <liming.gao@intel.com>
> > > 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:edk2-devel-bounces@lists.01.org] On Behalf
> > > Of Wasim Khan
> > > Sent: Friday, January 5, 2018 5:59 PM
> > > To: edk2-devel@lists.01.org
> > > 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
> > > edk2-devel@lists.01.org
> > > 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
> edk2-devel@lists.01.org
> 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