On 12/6/2023 1:35 PM, Rob Herring wrote: > On Mon, Dec 04, 2023 at 10:54:03AM -0800, Oreoluwa Babatunde wrote: >> The reserved_mem array is used to store the data of the different >> reserved memory regions specified in the DT of a device. >> The array stores information such as the name, node, starting address, >> and size of a reserved memory region. >> >> The array is currently statically allocated with a size of >> MAX_RESERVED_REGIONS(64). This means that any system that specifies a >> number of reserved memory regions greater than MAX_RESERVED_REGIONS(64) >> will not have enough space to store the information for all the regions. >> >> Therefore, this series extends the use of a static array for >> reserved_mem, and introduces a dynamically allocated array using >> memblock_alloc() based on the number of reserved memory regions >> specified in the DT. >> >> Memory gotten from memblock_alloc() is only writable after paging_init() >> is called, but the reserved memory regions need to be reserved before >> then so that the system does not create page table mappings for them. >> >> Reserved memory regions can be divided into 2 groups. >> i) Statically-placed reserved memory regions >> i.e. regions defined in the DT using the @reg property. >> ii) Dynamically-placed reserved memory regions. >> i.e. regions specified in the DT using the @alloc_ranges >> and @size properties. >> >> It is possible to call memblock_reserve() and memblock_mark_nomap() on >> the statically-placed reserved memory regions and not need to save them >> to the array until after paging_init(), but this is not possible for the >> dynamically-placed reserved memory because the starting address of these >> regions need to be stored somewhere after they are allocated. >> >> Therefore, this series achieves the allocation and population of the >> reserved_mem array in two steps: >> >> 1. Before paging_init() >> Before paging_init() is called, iterate through the reserved_mem >> nodes in the DT and do the following: >> - Allocate memory for dynamically-placed reserved memory regions and >> store their starting address in the static allocated reserved_mem >> array. >> - Call memblock_reserve() and memblock_mark_nomap() on all the >> reserved memory regions as needed. >> - Count the total number of reserved_mem nodes in the DT. >> >> 2. After paging_init() >> After paging_init() is called: >> - Allocate new memory for the reserved_mem array based on the number >> of reserved memory nodes in the DT. >> - Transfer all the information that was stored in the static array >> into the new array. >> - Store the rest of the reserved_mem regions in the new array. >> i.e. the statically-placed regions. >> >> The static array is no longer needed after this point, but there is >> currently no obvious way to free the memory. Therefore, the size of the >> initial static array is now defined using a config option. > A config option is not going to work here. > >> Because the array is used only before paging_init() to store the >> dynamically-placed reserved memory regions, the required size can vary >> from device to device. Therefore, scaling it can help get some memory >> savings. >> >> A possible solution to freeing the memory for the static array will be >> to mark it as __initdata. This will automatically free the memory once >> the init process is done running. >> The reason why this is not pursued in this series is because of >> the possibility of a use-after-free. >> If the dynamic allocation of the reserved_mem array fails, then future >> accesses of the reserved_mem array will still be referencing the static >> array. When the init process ends and the memory is freed up, any >> further attempts to use the reserved_mem array will result in a >> use-after-free. > If memory allocation for the reserved_mem array fails so early in boot, > you've got much bigger problems. Use __initdata, and just WARN if > allocation fails and continue on (so hopefully the console is brought > up and someone can see the WARN). > > Rob Thanks for pointing that out! I'll move forward with implementing the use of __initdata to delete the static array. Regards, Oreoluwa
