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

riscv: virt: Update the configurations #6432

Merged
merged 3 commits into from
Dec 8, 2023
Merged

riscv: virt: Update the configurations #6432

merged 3 commits into from
Dec 8, 2023

Conversation

gagachang
Copy link
Contributor

This commit updates the configurations for QEMU RISC-V virtual platform:

  1. Enable CFG_RISCV_S_MODE and CFG_RISCV_SBI to run OP-TEE on S-mode and utilize SBI to communicate with M-mode firmware.
  2. Do not force CFG_TEE_CORE_NB_CORE and CFG_NUM_THREADS to be 1, since we may run SMP system.
  3. Disable CFG_BOOT_SYNC_CPU and CFG_BOOT_SECONDARY_REQUEST, since the boot sequence is controlled by M-mode firmware.
  4. Enable CFG_WITH_STAT to build PTA for debug and statistics information.
  5. Enable CFG_DT to parse the external DTB passed by M-mode firmware.

@gagachang
Copy link
Contributor Author

Hello @maroueneboubakri
You may want to take a look at this PR

@jforissier
Copy link
Contributor

This commit updates the configurations for QEMU RISC-V virtual platform:

  1. Enable CFG_RISCV_S_MODE and CFG_RISCV_SBI to run OP-TEE on S-mode and utilize SBI to communicate with M-mode firmware.
  2. Do not force CFG_TEE_CORE_NB_CORE and CFG_NUM_THREADS to be 1, since we may run SMP system.
  3. Disable CFG_BOOT_SYNC_CPU and CFG_BOOT_SECONDARY_REQUEST, since the boot sequence is controlled by M-mode firmware.
  4. Enable CFG_WITH_STAT to build PTA for debug and statistics information.
  5. Enable CFG_DT to parse the external DTB passed by M-mode firmware.

If all these changes are independent (which I believe they are), it would be better to make one commit for each.

@gagachang
Copy link
Contributor Author

gagachang commented Nov 6, 2023 •
edited
Loading

If all these changes are independent (which I believe they are), it would be better to make one commit for each.

OK I will.

jforissier
jforissier reviewed Nov 7, 2023
View reviewed changes
Copy link
Contributor

@jforissier jforissier left a comment

Choose a reason for hiding this comment

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

The "if' in the first commit description is somewhat incorrect, I suggest "we enable CFG_RISCV_SBI so that OP-TEE utilizes SBI to communicate with other OS". In any case, LGTM. For all commits:

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>

@gagachang
Copy link
Contributor Author

The "if' in the first commit description is somewhat incorrect, I suggest "we enable CFG_RISCV_SBI so that OP-TEE utilizes SBI to communicate with other OS". In any case, LGTM. For all commits:

Acked-by: Jerome Forissier <jerome.forissier@linaro.org>

Apply the commit message and tag. Thanks for suggestion!

@maroueneboubakri
Copy link
Contributor

Hi @gagachang,

Thanks for these patches, is there any reason why CFG_WITH_STATS defaults to y ? This should be enabled to allow stats/debug info. Also this will increase the core footprint. If I need it I would enable the flag using command line.

Best
Maro

@gagachang
Copy link
Contributor Author

Hi @gagachang,

Thanks for these patches, is there any reason why CFG_WITH_STATS defaults to y ? This should be enabled to allow stats/debug info. Also this will increase the core footprint. If I need it I would enable the flag using command line.

Best Maro

OK I can remove that commit. We can enable it by command line.
Thanks! Would you provide the tags for review?

@jforissier
Copy link
Contributor

Ping @maroueneboubakri would you like to provide any review tag here? Thanks.

@maroueneboubakri
Copy link
Contributor

@jforissier forgot this one, @gagachang sorry for delay, please apply

Reviewed-by: Marouene Boubakri <marouene.boubakri@nxp.com>

@gagachang
riscv: virt: Enable configurations for S-mode execution
9f4ce42 
In RISC-V QEMU virtual platform, we run OP-TEE as S-mode. This commit
forcely enables CFG_RISCV_S_MODE and disables CFG_RISCV_M_MODE. Also, we
enable CFG_RISCV_SBI so that OP-TEE utilizes SBI to communicate with
other OS.

Signed-off-by: Alvin Chang <alvinga@andestech.com>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Marouene Boubakri <marouene.boubakri@nxp.com>
@gagachang
riscv: virt: Relax the configurations related to hart/thread number
21a1f77 
Do not force the CFG_TEE_CORE_NB_CORE and CFG_NUM_THREADS to be 1, since
we may run SMP system which has multiple harts and threads.

Signed-off-by: Alvin Chang <alvinga@andestech.com>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Marouene Boubakri <marouene.boubakri@nxp.com>
@gagachang
riscv: virt: Enable CFG_DT
b43ae3e 
Enable CFG_DT to parse the external DTB passed by previous boot stage.

Signed-off-by: Alvin Chang <alvinga@andestech.com>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Marouene Boubakri <marouene.boubakri@nxp.com>
@gagachang
Copy link
Contributor Author

Rebase and apply the tags.

@gagachang
Copy link
Contributor Author

Hi @jforissier
It seems something broken in Hafnium checking

@jforissier
Copy link
Contributor

Hi @jforissier It seems something broken in Hafnium checking

We have just updated Hafnium to v2.10 (OP-TEE/manifest#257) so the error is likely related. @jenswi-linaro is investigating this timeout in another PR (#6534 (comment)).

@jforissier jforissier merged commit 5798810 into OP-TEE:master Dec 8, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jforissier jforissier jforissier left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gagachang @jforissier @maroueneboubakri