On Wed, 21 Aug 2024 at 10:59, Sughosh Ganu sughosh.ganu@linaro.org wrote:
On Wed, 21 Aug 2024 at 07:41, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Tue, 20 Aug 2024 at 02:17, Sughosh Ganu sughosh.ganu@linaro.org wrote:
On Fri, 16 Aug 2024 at 02:04, Simon Glass sjg@chromium.org wrote:
Hi Sughosh,
On Wed, 14 Aug 2024 at 12:01, Sughosh Ganu sughosh.ganu@linaro.org wrote:
Introduce a function lmb_add_memory() to add available memory to the LMB memory map. Call this function during board init once the LMB data structures have been initialised.
Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org
Changes since V1:
- Call the lmb_add_memory() from lmb_init() instead of lmb_mem_regions_init().
include/lmb.h | 10 ++++++++++ lib/lmb.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
But this should not be weak.
This is being made weak, as there would be lmb_add_memory() definitions added for powerpc and x86 arch's in the EFI part of my patches. Moreover, the lmb_add_memory() function would be called even in the SPL stage when LMB is enabled for that stage. So I am not sure how do we get around this. You can check the relevant branch [1] on my github to check for the specific commits [2][3] that I am referring to. Thanks.
This is really strange.
The e820 is different on each x86 board. I'm not sure we want that in the lmb. What is the purpose of that? It is somewhat circular in most cases, since U-Boot sets it up itself. Where it comes from coreboot, it looks like we are mirroring it in the EFI memory map. I'm not sure I understand all this very well.
Yes, me neither. And I want to keep the behaviour same as before the patches. You would know that the function efi_add_known_memory() gets the memory map from a function install_e820_map() which includes conventional memory, which is the ram memory. And I am basically now doing this as part of the lmb_add_memory() function instead. Are you sure that we can do away with this function, and instead use the ram_base and ram_top values from the global data structure instead? I believe you have boards which exercise this code? So it will be great if you can test this if I remove the function for the e820 module.
For fsl, perhaps copy the #ifdef and handle arch.resv_ram in your code?
This is for adding ram to the lmb memory map, but yes, I can check by putting an ifdef in the function. Although the function might look ugly and hackish. Thanks.
I have taken an alternative approach to this, whereby boards/archs can still define their own memory map addition without using the weak attribute. Please check the relevant branch [1] on my github, and these commits [2][3][4] specifically. Thanks.
-sughosh
[1] - https://github.com/sughoshg/u-boot/tree/lmb_efi_sep_apis_nrfc_next_noweak_v3 [2] - https://github.com/u-boot/u-boot/commit/0b5dbaf07a3615230b9df06296e40267e838... [3] - https://github.com/u-boot/u-boot/commit/916a36dcb5cc66d801067924607fd94d8bad... [4] - https://github.com/u-boot/u-boot/commit/9da75a20e9a6af00cfaac71f09e4a1f89f10...
-sughosh
[..] Regards, Simon
[1] - https://github.com/sughoshg/u-boot/tree/lmb_efi_sep_apis_nrfc_next_v3 [2] - https://github.com/u-boot/u-boot/commit/077ced7aaa6d495b1b87b324fb1c60658c20... [3] - https://github.com/u-boot/u-boot/commit/d0fa3a89865b796f3bbebffebbe4f7b5b048...