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

AArch64: Hardware Debug API port #1206

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

alwin-joshy
Copy link
Contributor

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.

Indanz and others added 7 commits February 18, 2024 10:19
Signed-off-by: Alwin Joshy <joshyalwin@gmail.com>
Signed-off-by: Alwin Joshy <joshyalwin@gmail.com>
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>
Copy link
Member

@lsf37 lsf37 left a 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.

@lsf37 lsf37 added hw-test sel4test hardware builds + runs for this PR hw-bench run sel4bench on this PR labels Feb 26, 2024
@lsf37
Copy link
Member

lsf37 commented Feb 26, 2024

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.

The file platforms.yml in the ci-actions repo has a list which armv8 platforms support 32bit seL4 images, e.g. the imx8mm-evk:

  IMX8MM_EVK:
    arch: arm
    modes: [32, 64]
    smp: [64]
    platform: imx8mm-evk
    req: [imx8mm]
    march: armv8a

@Indanz Indanz marked this pull request as draft February 26, 2024 10:07
include/api/syscall.h Outdated Show resolved Hide resolved
#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"
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

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) \
Copy link
Contributor

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...

Copy link
Contributor Author

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.

include/arch/arm/arch/64/mode/machine/registerset.h Outdated Show resolved Hide resolved
include/arch/arm/arch/machine/debug.h Outdated Show resolved Hide resolved
include/arch/arm/arch/machine/debug.h Show resolved Hide resolved
src/arch/arm/c_traps.c Outdated Show resolved Hide resolved
src/arch/arm/machine/debug.c Outdated Show resolved Hide resolved
src/arch/arm/machine/debug.c Outdated Show resolved Hide resolved
@Indanz
Copy link
Contributor

Indanz commented Feb 26, 2024

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.

@lsf37
Copy link
Member

lsf37 commented Feb 26, 2024

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.

@alwin-joshy alwin-joshy force-pushed the refactor_hw_debug_aarch32_attempt_iz branch 2 times, most recently from faeea53 to 3ec84bf Compare February 27, 2024 01:40
@alwin-joshy
Copy link
Contributor Author

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.

The file platforms.yml in the ci-actions repo has a list which armv8 platforms support 32bit seL4 images, e.g. the imx8mm-evk:

  IMX8MM_EVK:
    arch: arm
    modes: [32, 64]
    smp: [64]
    platform: imx8mm-evk
    req: [imx8mm]
    march: armv8a
../init-build.sh -DPLATFORM=imx8mm-evk -DAARCH32=ON -DHardwareDebugAPI=ON

passes seL4test.

@alwin-joshy alwin-joshy changed the title AARCH64: Hardware Debug API port AArch64: Hardware Debug API port Feb 27, 2024
Signed-off-by: Alwin Joshy <joshyalwin@gmail.com>
@alwin-joshy alwin-joshy force-pushed the refactor_hw_debug_aarch32_attempt_iz branch from 3ec84bf to 8299bb6 Compare February 28, 2024 01:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hw-bench run sel4bench on this PR hw-test sel4test hardware builds + runs for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants