Attention is currently required from: Philipp Hug, Tim Wawrzynczak, ron minnich.
Maximilian Brune has posted comments on this change by Maximilian Brune. ( https://review.coreboot.org/c/coreboot/+/81082?usp=email )
Change subject: arch/riscv: Refactor SMP code ......................................................................
Patch Set 5:
(2 comments)
File src/arch/riscv/smp.c:
https://review.coreboot.org/c/coreboot/+/81082/comment/cc29ca5d_d98d57aa?usp... : PS5, Line 49: for (int i = 0; i < hart_count; i++) {
There seems to be an assumption that hartid are always contiguous from 0, is that the case?
good point. It is the case for all platforms I have encountered so far, but it is not mandated by the RISC-V spec. There are currently multiple places where we assume continues hart ids. `OTHER_HLS` macros and friends and our stack setup (at least partly). I guess at some point there will be a special snowflake SOC that doesn't have continuous hart ids. For this patch I would keep the assumption (since the assumption was present also before this patch). I would propose to do a separate patch that removes the assumption from the entire `arch/riscv` codebase.
https://review.coreboot.org/c/coreboot/+/81082/comment/fa71d59e_b6da3f62?usp... : PS5, Line 80: while (atomic_read(&OTHER_HLS(i)->entry.sync_a) != 0)
should there be a timeout ?
I currently assume the time between `smp_pause` and `smp_resume` is enough for a timeout right now. But I can also add a timeout if you want.