Closed alwin-joshy closed 7 months ago
With 1162 in mind, I think it would be good to test whether code actually gets executed instead of only checking if the instruction pointer advances, so instead of nops just add a value to a register and check if it's correct at the second label.
With 1162 in mind, I think it would be good to test whether code actually gets executed instead of only checking if the instruction pointer advances, so instead of nops just add a value to a register and check if it's correct at the second label.
Good point. In that case, I'll port the equivalent x86 test, which seems to be more robust.
With 1162 in mind, I think it would be good to test whether code actually gets executed instead of only checking if the instruction pointer advances, so instead of nops just add a value to a register and check if it's correct at the second label.
Good point. In that case, I'll port the equivalent x86 test, which seems to be more robust.
At first glance I don't really see anything x86 specific in there, perhaps make that test cross-platform with some arch specific helper functions or defines to paper over the differences? That definitely seems to be the current intention. You'd need to move the test to some common place and make sure it still gets executed on x86.
With 1162 in mind, I think it would be good to test whether code actually gets executed instead of only checking if the instruction pointer advances, so instead of nops just add a value to a register and check if it's correct at the second label.
Good point. In that case, I'll port the equivalent x86 test, which seems to be more robust.
At first glance I don't really see anything x86 specific in there, perhaps make that test cross-platform with some arch specific helper functions or defines to paper over the differences? That definitely seems to be the current intention. You'd need to move the test to some common place and make sure it still gets executed on x86.
Yeah, should be simple. I do recall seeing a comment somewhere in the code about something being missing or not ready in ARM, but I'll see if I run into any issues. Didn't spot anything that immediately stood out.
@Indanz I moved the x86 single-stepping test into the arch-independent breakpoints file and it just worked.
It looks like the hw-test
trigger isn't working properly on this repo. I'll investigate..
Edit: it doesn't seem to be working on the seL4 repo any more either. Looks like something changed in the infrastructure.
Edit 2: looking more closely, it did actually work. It's just not showing them in the PR summary panel. It is showing them in the "Check" tab of the PR.
The Sabre clang fails with:
All is well in the universe
console_run returned 0
0.05user 0.02system 2:48.06elapsed 0%CPU (0avgtext+0avgdata 7936maxresident)k
0inputs+336outputs (0major+8920minor)pagefaults 0swaps
+++ python3 ../projects/seL4_libs/libsel4test/tools/extract_results.py -q results.xml parsed_results.xml
No log to check boot failure on.
+++ mq.sh sem -signal sabre2 -k seL4/sel4test-seL4Test HW-7619487774-hw-run-5
Error parsing parsed_results.xml
-----------[ end test SABRE_debug_MCS_clang_32 ]-----------
SABRE_debug_MCS_clang_32 FAILED
I verified that it still runs and passes on x86 and some 32 bit ARMs, it doesn't yet run on Aarch64 though. I think we should remove the lines around https://github.com/seL4/seL4/blob/cc3205ea486f6ce3da3a64d8a99e166f047b8419/src/arch/arm/config.cmake#L226.
So do we want to remove KernelHardwareDebugAPIUnsupported
from aarch64 before merging this, as the aarch64 code isn't tested yet, or merge this first and remove the define later?
@alwin-joshy, did you test this on aarch64 yet?
I verified that it still runs and passes on x86 and some 32 bit ARMs, it doesn't yet run on Aarch64 though. I think we should remove the lines around https://github.com/seL4/seL4/blob/cc3205ea486f6ce3da3a64d8a99e166f047b8419/src/arch/arm/config.cmake#L226.
Yep, that will be necessary for getting the test to run on AARCH64 eventually but there are other substantial changes required for this. I've been working on this and am close to done (https://github.com/alwin-joshy/seL4/pull/7) but there are still a few problems related to ARMv8 32-bit and ARM hyp mode that I want to iron out before submitting a proper PR.
So do we want to remove
KernelHardwareDebugAPIUnsupported
from aarch64 before merging this, as the aarch64 code isn't tested yet, or merge this first and remove the define later?@alwin-joshy, did you test this on aarch64 yet?
I think to merge this first and remove the KernelHardwareDebugAPIUnsupported in the PR when I have the hardware debug API fully ported to aarch64
seL4's hardware debug API is currently only implemented for ARMv7 and x86. I am currently implementing it for AARCH64 and as a part of this, have ported the software breakpoint test to AARCH64.
I have also implemented a new test to check that single stepping works on ARM. In doing so, I believe I have found a bug in the implementation which I will make another pull request for soon. This test is simpler than the existing x86 equivalent but I think it should be sufficient. If not, I can port the existing x86 test for single stepping to ARM.
Test with: https://github.com/seL4/seL4/pull/1162