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
AArch64: Hardware Debug API port #1206
base: master
Are you sure you want to change the base?
AArch64: Hardware Debug API port #1206
Conversation
Signed-off-by: Alwin Joshy <joshyalwin@gmail.com>
Signed-off-by: Alwin Joshy <joshyalwin@gmail.com>
…_hw_debug_aarch32_attempt_iz
Signed-off-by: Alwin Joshy <joshyalwin@gmail.com>
Changing the debug register fields to what they are referred to as in the ARMv7 and ARMv8 manuals. Mostly a cosmetic change, but improves clarity. Signed-off-by: Alwin Joshy <joshyalwin@gmail.com>
On aarch32, single stepping is configured by setting an instruction breakpoint to mismatch mode. The way that it is checked whether a given breakpoint is being used as a normal breakpoint or for single-stepping is by checking if it has been configured to mismatch. This works because the HW debug API does not currently provide a way to otherwise configure mismatch breakpoints, but seems like an unsatisfactory solution. Whether single-stepping is enabled, and the breakpoint that is being used for it is already stored in the TCB of a thread, and this commit changes checks related to single-stepping to use this information instead. Signed-off-by: Alwin Joshy <joshyalwin@gmail.com>
Adding support for the hardware debug API to aarch64. Signed-off-by: Alwin Joshy <joshyalwin@gmail.com>
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.
There seem to be a number of changes to code that is visible to verification. See the diffs in the failed preprocess tests -- they show what changes the verified configurations see. Whitespace and comment changes there can be ignored, but there should be no new functions declarations or new instructions visible, which they are.
If this PR only concerns the hardware debug API, there should ideally be no such changes visible. If they are, this will need funding for re-verification.
The file platforms.yml in the ci-actions repo has a list which armv8 platforms support 32bit seL4 images, e.g. the IMX8MM_EVK:
arch: arm
modes: [32, 64]
smp: [64]
platform: imx8mm-evk
req: [imx8mm]
march: armv8a |
#define MAKE_DBGBVR(num) "DBGBVR" #num "_EL1" | ||
#define MAKE_DBGBCR(num) "DBGBCR" #num "_EL1" | ||
#define MAKE_DBGWVR(num) "DBGWVR" #num "_EL1" | ||
#define MAKE_DBGWCR(num) "DBGWCR" #num "_EL1" |
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.
Shouldn't this be _EL2
when HYP is enabled?
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.
I'm pretty sure those registers don't exist.
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.
I should have checked. About EL2, the ARM manual says:
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.
Yep, that's done in the setHDCRTrapDebugExceptionState
function and I've tested that the breakpoint and single-stepping tests still pass when running the kernel with ARM_HYP enabled.
|
||
/** Generates read functions for the CP14 control and value registers. | ||
*/ | ||
#define DEBUG_GENERATE_READ_FN(_name, _reg) \ |
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.
I hate these macros and don't understand why they're needed. I honestly think writing all the used cases out would be less code and clearer than this indirect macro stuff.
That 32-bit debug code has this is no excuse to copy it, but if you do, at least share the macros: The only difference is MRS versus MRC, easily defined away...
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.
Yeah I don't really like the macros either, but I thought that duplicating them would at least make it slightly less horrible than adding another level of pre-processing in there with extra ifdefs and more macros.
The DCO check failing is bogus: I just redid @alwin-joshy's commits in a way that preserves history, so the sign-off should be in his name. |
Happy to set this to "pass". It'll have to be re-set when force-pushed, but that's fine. |
faeea53
to
3ec84bf
Compare
passes seL4test. |
Signed-off-by: Alwin Joshy <joshyalwin@gmail.com>
3ec84bf
to
8299bb6
Compare
This pull request relates to the porting of the hardware debug API to aarch64. The API exposes hardware debugging features to userspace and enables the development of debugging tools on top of seL4.
The first four commits are to do with splitting arm/machine/debug.c, which contains the current implementation of the hardware debug API for aarch32, into mode-dependent and mode-independent files while retaining their git history.
aarch32: change dbg register fields renames some fields in the aarch32 implementation of the hardware debug API to enable cleaner reuse of code and makes fields better correspond with their names in the ARMv7 and ARMv8 manuals.
hw debug api: aarch32 - modify single stepping checks improves a check on aarch32 related to the configuration of single stepping (more details can be found in the commit message).
hw debug api: aarch64 port finally ports the hardware debug API to aarch64.
The code has been tested to work still pass seL4test on ARMv7 boards (sabre, odroidxu4) as well as on AARCH64 (odroidc4/odroidc2/qemu-arm-virt). I have not been able to confirm that it works with AARCH32 (armv8 32-bit), because I have been unable to run a 32-bit seL4test image (even unmodified) on an ARMv8 board.