On Mon, Aug 19, 2024 at 12:23 PM Andy Shevchenko <andy@xxxxxxxxxxxxxxxxxx> wrote: > > On Fri, Aug 09, 2024 at 11:48:12AM -0700, Oreoluwa Babatunde wrote: > > The reserved_mem array is used to store data for the different > > reserved memory regions defined in the DT of a device. The array > > stores information such as region name, node reference, start-address, > > and size of the different reserved memory regions. > > > > 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. > > > > This can be fixed by making the reserved_mem array a dynamically sized > > array which is allocated using memblock_alloc() based on the exact > > number of reserved memory regions defined in the DT. > > > > On architectures such as arm64, memblock allocated memory is not > > writable until after the page tables have been setup. > > This is an issue because the current implementation initializes the > > reserved memory regions and stores their information in the array before > > the page tables are setup. Hence, dynamically allocating the > > reserved_mem array and attempting to write information to it at this > > point will fail. > > > > Therefore, the allocation of the reserved_mem array will need to be done > > after the page tables have been setup, which means that the reserved > > memory regions will also need to wait until after the page tables have > > been setup to be stored in the array. > > > > When processing the reserved memory regions defined in the DT, these > > regions are marked as reserved by calling memblock_reserve(base, size). > > Where: base = base address of the reserved region. > > size = the size of the reserved memory region. > > > > Depending on if that region is defined using the "no-map" property, > > memblock_mark_nomap(base, size) is also called. > > > > The "no-map" property is used to indicate to the operating system that a > > mapping of the specified region must NOT be created. This also means > > that no access (including speculative accesses) is allowed on this > > region of memory except when it is coming from the device driver that > > this region of memory is being reserved for.[1] > > > > Therefore, it is important to call memblock_reserve() and > > memblock_mark_nomap() on all the reserved memory regions before the > > system sets up the page tables so that the system does not unknowingly > > include any of the no-map reserved memory regions in the memory map. > > > > There are two ways to define how/where a reserved memory region is > > placed in memory: > > i) Statically-placed reserved memory regions > > i.e. regions defined with a set start address and size using the > > "reg" property in the DT. > > ii) Dynamically-placed reserved memory regions. > > i.e. regions defined by specifying a range of addresses where they can > > be placed in memory using the "alloc_ranges" and "size" properties > > in the DT. > > > > The dynamically-placed reserved memory regions get assigned a start > > address only at runtime. And this needs to be done before the page > > tables are setup so that memblock_reserve() and memblock_mark_nomap() > > can be called on the allocated region as explained above. > > Since the dynamically allocated reserved_mem array can only be > > available after the page tables have been setup, the information for > > the dynamically-placed reserved memory regions needs to be stored > > somewhere temporarily until the reserved_mem array is available. > > > > Therefore, this series makes use of a temporary static array to store > > the information of the dynamically-placed reserved memory regions until > > the reserved_mem array is allocated. > > Once the reserved_mem array is available, the information is copied over > > from the temporary array into the reserved_mem array, and the memory for > > the temporary array is freed back to the system. > > > > The information for the statically-placed reserved memory regions does > > not need to be stored in a temporary array because their starting > > address is already stored in the devicetree. > > Once the reserved_mem array is allocated, the information for the > > statically-placed reserved memory regions is added to the array. > > > > Note: > > Because of the use of a temporary array to store the information of the > > dynamically-placed reserved memory regions, there still exists a > > limitation of 64 for this particular kind of reserved memory regions. > > >From my observation, these regions are typically small in number and > > hence I expect this to not be an issue for now. > > > This series (in particular the first patch) broke boot on Intel Meteor > Lake-P. Taking Linux next of 20240819 with these being reverted makes > things work again. Looks like this provides some detail: https://lore.kernel.org/all/202408192157.8d8fe8a9-oliver.sang@xxxxxxxxx/ I've dropped the patches for now. > Taking into account bisectability issue (that's how I noticed the issue > in the first place) I think it would be nice to have no such patches at > all in the respective subsystem tree. On my side I may help with testing > whatever solution or next version provides. I don't follow what you are asking for? That the patches should be bisectable? Well, yes, of course, but I don't verify that typically. Patch 1 builds fine for m, so I'm not sure what issue you see. Rob