Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34774 )
Change subject: lib: ramdetect: Register exception handlers for ARMv8 ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34774/1/src/lib/ramdetect.c File src/lib/ramdetect.c:
https://review.coreboot.org/c/coreboot/+/34774/1/src/lib/ramdetect.c@58 PS1, Line 58: probe_mb
I created 2 versions: […]
Well, now you essentially created a whole separate implementation for the whole thing in patch set 3, which isn't quite what I meant. But I agree that trying to do this with weak functions gets too confusion, and just having a few ENV_ARM64 checks in the main code is way simpler. So I'd also prefer the patch set 2 approach.
https://review.coreboot.org/c/coreboot/+/34774/2/src/lib/ramdetect.c File src/lib/ramdetect.c:
https://review.coreboot.org/c/coreboot/+/34774/2/src/lib/ramdetect.c@46 PS2, Line 46: struct exception_handler sync_el0; Like Raul said in the other patch set, this needs to be static or global. This is probably the reason for your very weird bug (you're using random garbage on the stack as your exception handler function pointer).
https://review.coreboot.org/c/coreboot/+/34774/2/src/lib/ramdetect.c@90 PS2, Line 90: if (ENV_ARM64) { This should be below the part with saved_result below. (It probably makes sense to put both of these right around the for loop.)