-
Notifications
You must be signed in to change notification settings - Fork 93
ltimer: indicate no support for more platforms #196
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
Conversation
|
Moving the knowledge from sel4test to util_libs makes a lot of sense. But please don't use negated terms, use something like It would be nice if we could add a define to each timer.h driver file to set a HAVE_TIMER define instead of hard-coding it in CMake, but it looks like the code structure won't allow it. That would automatically be up-to-date with the code. Oh well. |
As suggested by Indan to be clearer to understand. seL4#196 (comment) Signed-off-by: julia <git.ts@trainwit.ch>
As suggested by Indan to be clearer to understand. seL4#196 (comment) Signed-off-by: julia <git.ts@trainwit.ch>
As suggested by Indan to be clearer to understand. seL4/util_libs#196 (comment) Signed-off-by: julia <git.ts@trainwit.ch>
As suggested by Indan to be clearer to understand. seL4/util_libs#196 (comment) Signed-off-by: julia <git.ts@trainwit.ch>
As suggested by Indan to be clearer to understand. seL4#196 (comment) Signed-off-by: julia <git.ts@trainwit.ch>
As suggested by Indan to be clearer to understand. seL4#196 (comment) Signed-off-by: julia <git.ts@trainwit.ch>
As suggested by Indan to be clearer to understand. seL4#196 (comment) Signed-off-by: julia <git.ts@trainwit.ch>
As suggested by Indan to be clearer to understand. seL4#196 (comment) Signed-off-by: julia <git.ts@trainwit.ch>
As suggested by Indan to be clearer to understand. #196 (comment) Signed-off-by: julia <git.ts@trainwit.ch>
As suggested by Indan to be clearer to understand. seL4/util_libs#196 (comment) Signed-off-by: julia <git.ts@trainwit.ch>
As suggested by Indan to be clearer to understand. seL4#196 (comment) Signed-off-by: julia <git.ts@trainwit.ch>
As suggested by Indan to be clearer to understand. seL4#196 (comment) Signed-off-by: julia <git.ts@trainwit.ch>
As suggested by Indan to be clearer to understand. seL4/util_libs#196 (comment) Signed-off-by: julia <git.ts@trainwit.ch>
39360b5 to
d85d4f1
Compare
Clang didn't have `__attribute__((error(msg)))` until version 14, but we support older versions of Clang. Both GCC and clang support `__has_attribute(error)`; GCC since v5 (2015) and Clang seemingly always. This fixes builds that were failing due to 'unknown attribute error'. We are not relying on the presence of this attribute for correct output, but purely as a debug message which fails before the linker. If we don't support the message failing at link-time is OK too. Signed-off-by: julia <git.ts@trainwit.ch>
These were taken from the sel4test Sel4testHaveTimer config option. Signed-off-by: julia <git.ts@trainwit.ch>
d85d4f1 to
23cd7ba
Compare
|
To be fully portable you would need an |
| OR KernelPlatformRocketchip | ||
| OR KernelPlatformRocketchipZCU102 | ||
| OR KernelPlatformCheshire | ||
| OR (SIMULATION AND (KernelArchRiscV OR KernelArchARM)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@midnightveil for some reason this line breaks the camkes timer component build. For all Arm platforms we're getting:
/github/workspace/projects/global-components/components/TimeServer/src/time_server.c: In function ‘post_init’:
/github/workspace/projects/global-components/components/TimeServer/src/time_server.c:230:13: error: call to ‘ltimer_default_init’ declared with attribute error: no ltimer support for this platform
230 | error = ltimer_default_init(<imer, io_ops, time_server_ltimer_handle, NULL);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
(e.g. https://github.com/seL4/camkes-tool/actions/runs/15923333523/job/45177979627)
If I remove this line, everything builds and works fine. Either SIMULATION does not mean what it says or it is on for those builds even though they are hardware images. It does seem to work if I explicitly provide -DSIMULATION=OFF on the command line.
Are we actually sure that it is true for all Arm platforms that qemu does not provide an ltimer device? I thought that some of them have fairly extensive device simulation (but maybe not timer, I have not checked).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SIMULATE does just seem to be on for those builds. It is actually generating a simulate script (which then says the platform is not supported for simulation).
So the right fix seems to be to make sure SIMULATE is not on for those builds, but it makes me wonder if we're braking other builds without intending to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so it does intentionally switch on SIMULATION by default in easy-settings.cmake
set(SIMULATION ON CACHE BOOL "Try and use simulable features")
So I'm not entirely sure how to proceed. Do we actually need to guard SIMULATION here in util_libs, or is it just to fail early?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was copied from sel4test: https://github.com/seL4/sel4test/pull/142/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What the issue is probably that it was previously an
if (qemu arm virt)
if (timers) enable
else disable
else (check this bunch of other conditions)
disable
Now it is
if (qemu arm virt OR these other conditions)
disable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| OR (SIMULATION AND (KernelArchRiscV OR KernelArchARM)) | |
| OR ((NOT KernelPlatformQEMUArmVirt) AND SIMULATION AND (KernelArchRiscV OR KernelArchARM)) |
I think would be equivalent?
EDIT: Actually, no, it's still different. I'm going to go back to the original double if I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These were taken from the sel4test Sel4testHaveTimer config option.
Do not merge until #195 has been merged (as it will break sel4test for more platforms)ok now