Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

improve RISC-V multi core boot #132

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

improve RISC-V multi core boot #132

wants to merge 6 commits into from

Conversation

axel-h
Copy link
Member

@axel-h axel-h commented Nov 13, 2021

  • add much more comments
  • fix SBI HSM register naming quirk
  • Ensure DTB is always passed to primary core boot
  • check SBI HSM error when bringing up secondary harts
  • Stop wrong boot hart via HSM if possible
  • unify stack definitions for all cores
  • add multicore helper functions
  • Drop variable hsm_exists, pass information as parameter
  • Print more log messages

@axel-h axel-h added the cleanup label Nov 13, 2021
@axel-h axel-h requested a review from yyshen November 13, 2021 06:46
@axel-h axel-h force-pushed the patch-axel-7 branch 5 times, most recently from ff35350 to 7c43455 Compare November 16, 2021 04:23
@axel-h axel-h mentioned this pull request Feb 2, 2022
Closed
@axel-h axel-h force-pushed the patch-axel-7 branch 5 times, most recently from 09be73e to 12c88f1 Compare February 6, 2022 21:49
@axel-h axel-h marked this pull request as draft February 6, 2022 21:59
@axel-h axel-h force-pushed the patch-axel-7 branch 7 times, most recently from e773b34 to a3ffa7c Compare February 7, 2022 01:53
@axel-h axel-h mentioned this pull request Feb 18, 2022
Closed
@axel-h axel-h force-pushed the patch-axel-7 branch 3 times, most recently from a0204aa to 8eec928 Compare February 18, 2022 05:01
@axel-h axel-h force-pushed the patch-axel-7 branch 3 times, most recently from d238b57 to e097abf Compare March 1, 2022 17:26
@axel-h axel-h force-pushed the patch-axel-7 branch 2 times, most recently from 2b1c483 to 29fc94b Compare October 19, 2022 22:37
@axel-h axel-h force-pushed the patch-axel-7 branch 2 times, most recently from 024584f to 34cb600 Compare June 15, 2023 00:15
@yyshen
Copy link
Contributor

yyshen commented Dec 2, 2023

@axel-h thanks for the changes. I just want to double check if there are any bugs fixed in the PR.

yyshen
yyshen reviewed Dec 2, 2023
View reviewed changes
elfloader-tool/src/arch-riscv/boot.c
@@ -176,17 +176,52 @@ int secondary_go = 0;
int next_logical_core_id = 1; /* incremented by assembly code */
int mutex = 0;
int core_ready[CONFIG_MAX_NUM_NODES] = { 0 };

static void acquire_multicore_lock(void)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that this is used inside this file only, maybe smp_lock and smp_unlock are shorter. just a suggestion.

yyshen
yyshen reviewed Dec 2, 2023
View reviewed changes
elfloader-tool/src/arch-riscv/boot.c
__atomic_store_n(&mutex, 0, __ATOMIC_RELEASE);
}

static void set_secondary_cores_go(void)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unleash_secondary_cores?

yyshen
yyshen reviewed Dec 2, 2023
View reviewed changes
elfloader-tool/src/arch-riscv/crt0.S

_start1: /* a0 must hold current hard ID passed by bootloader */
/* a1 must hold dtb address passed by bootloader */
/*----------------------------------------------------------------------------*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this needed?

yyshen
yyshen reviewed Dec 2, 2023
View reviewed changes
elfloader-tool/src/arch-riscv/boot.c
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this basically has no code logic changes? In this case, maybe we should state it in the commit message clearly.

yyshen
yyshen reviewed Dec 2, 2023
View reviewed changes
elfloader-tool/src/arch-riscv/boot.c Outdated
@@ -220,6 +220,7 @@ static int run_elfloader(UNUSED word_t hart_id, void *bootloader_dtb)
}

#if CONFIG_MAX_NUM_NODES > 1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are the additional empty lines?

@axel-h axel-h added the hw-build enable all sel4test hardware builds label Jan 15, 2024
@axel-h
elfloader/risc-v: clarify parameters and types
295458b 
- Use word_t to adapt to architecture
- sbi_hart_start() takes a custom argument.
- hart ID and custom argument is passed when starting a secondary hart.

Signed-off-by: Axel Heider <axelheider@gmx.de>
@axel-h
elfloader/risc-v: no need to use t0
1b7be5b 
This also makes the comment about a1 correct again.

Signed-off-by: Axel Heider <axelheider@gmx.de>
@axel-h
elfloader/risc-v: unify multicore stacks
740bc9b 
- declare stack fully in C code
- use one array that holds all stack
- use CONFIG_KERNEL_STACK_BITS for size

Signed-off-by: Axel Heider <axelheider@gmx.de>
@axel-h
elfloader/risc-v: improve comments
003dd16 
Signed-off-by: Axel Heider <axelheider@gmx.de>
@axel-h
elfloader/risc-v: Improve code readability
aae131a 
Reorder code blocks and improve comments.

Signed-off-by: Axel Heider <axelheider@gmx.de>
@axel-h
elfloader/risc-v: add multi core helper functions
3344a19 
Use wrapper function to Improve code readability.

Signed-off-by: Axel Heider <axelheider@gmx.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yyshen yyshen yyshen left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
cleanup hw-build enable all sel4test hardware builds
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@axel-h @yyshen