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.

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

Paged user TAs #1044

Merged
merged 2 commits into from
Sep 28, 2016
Merged

Paged user TAs #1044

merged 2 commits into from
Sep 28, 2016

Conversation

jenswi-linaro
Copy link
Contributor

Adds support for paged user TAs

@jenswi-linaro
Copy link
Contributor Author

Depends on OP-TEE/optee_test#124

@d3zd3z
Copy link
Contributor

d3zd3z commented Sep 19, 2016

There's quite a bit of code, so I wouldn't say my review has been exhaustive, but what I've seen looks good.

@jforissier
Copy link
Contributor

I have tested on HiKey, this is basically OK assuming the following are addressed:

  • The increase of emulated SRAM from 200 to 256 KiB is needed for HiKey, too (otherwise xtest is painfully slow). Would you like me to submit a separate PR, or do you want to add it to this series?
  • xtest 1016 Storage concurency was recently modified to apply to all filesystems. It is failing with TEEC_ERROR_OUT_OF_MEMORY. Should it be disabled when TA paging is enabled? (I could not find proper memory settings to make it pass).
  • Similarly, xtest 1005 Many sessions logs TEEC_ERROR_GENERIC, which I assume are out-of-memory underneath. Can be fixed with 320 KiB of SRAM.

@jenswi-linaro
Copy link
Contributor Author

The xtest 1016 Storage concurrency is a bit tricky. When I'm testing on latest on master branch with pager enabled it sometimes fails. I'm starting to suspect that the pager itself isn't the culprit, instead it just makes it easier for race conditions to be triggered as context switch are suddenly very likely at certain addresses in the code.

@jenswi-linaro
Copy link
Contributor Author

Rebased on master, squashed the [review] commit. Added a Hikey commit.

@jforissier
Copy link
Contributor

@jenswi-linaro it looks like HiKey needs 40 KiB of HEAP_SIZE to pass xtest 6016 reliably when pager is enabled.
There's also the checkpatch error and warning to be addressed.

@jbech-linaro
Copy link
Contributor

Am I'm the only one that get the feeling that we have been fiddling with increasing both stack and heap quite a few times the last year? Maybe time to do an overhaul for all devices we support and at the same time increase to something a bit bigger than we actually need? Also, it might be worth checking whether we really need all of it or if we're unnecessarily use too much of them?

@jenswi-linaro
Copy link
Contributor Author

Rebased with the memory fiddling patches removed as it's already dealt with on master now.

@jforissier
Copy link
Contributor

Working fine on HiKey.
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (HiKey)

@d3zd3z
Copy link
Contributor

d3zd3z commented Sep 28, 2016

Acked-by: David Brown <david.brown@linaro.org>

Replace the last 4 bytes alignment statements with 8 bytes alignment to
avoid implicit padding when linking the binary.

Implicit padding following the .data section doesn't work with the
pager.

Acked-by: David Brown <david.brown@linaro.org>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Enables support for paging of user TAs if CFG_PAGED_USER_TA is y

Acked-by: David Brown <david.brown@linaro.org>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (HiKey)
Tested-by: Jens Wiklander <jens.wiklander@linaro.org> (QEMU 7)
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
@jenswi-linaro
Copy link
Contributor Author

Checkpatch warnings/errors fixed, rebased and tags applied.

@jforissier jforissier merged commit a884c93 into OP-TEE:master Sep 28, 2016
@jenswi-linaro jenswi-linaro deleted the pgta branch September 28, 2016 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants