Page MenuHomeFreeBSD

hwpmc: use kstack_contains()
ClosedPublic

Authored by mhorne on May 1 2023, 7:39 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 25, 10:58 PM
Unknown Object (File)
Wed, Nov 20, 5:23 PM
Unknown Object (File)
Wed, Nov 20, 4:10 PM
Unknown Object (File)
Wed, Nov 20, 4:09 PM
Unknown Object (File)
Nov 8 2024, 5:22 PM
Unknown Object (File)
Oct 3 2024, 7:22 PM
Unknown Object (File)
Sep 20 2024, 2:24 PM
Unknown Object (File)
Sep 20 2024, 5:16 AM

Details

Summary

This existing helper function is preferable to the hand-rolled
calculation of the kstack bounds.

Make some small style improvements while here. Notably, rename every
instance of "r", the return address, to "ra". Tidy the includes in the
affected files.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/dev/hwpmc/hwpmc_arm64_md.c
79–81

Could we use unwind_frame here? If not we'll need to add ADDR_MAKE_CANONICAL to make pc a valid kernel address when PAC is in use.

jkoshy added inline comments.
sys/amd64/include/pmc_mdep.h
111–112

Please consider keeping the existing macro name (PMC_IN_KERNEL_STACK)? I find it a little clearer than the abbreviation _KSTACK.

While here, the sole macro parameter S could be renamed to STACK_ADDRESS so that the macro becomes self-documenting.

sys/arm/include/pmc_mdep.h
55

Please consider keeping the existing name (PMC_IN_KERNEL_STACK) for clarity. The sole macro parameter could also be renamed to say STACK_ADDRESS so that the macro becomes self-documenting.

This revision is now accepted and ready to land.May 2 2023, 9:43 AM

Restore name to PMC_IN_KERNEL_STACK. Use va as the argument name, for brevity, and because it matches other existing macros.

Rebase on top of D39934.

This revision now requires review to proceed.May 2 2023, 4:15 PM
mhorne added inline comments.
sys/dev/hwpmc/hwpmc_arm64_md.c
79–81

Done in D39934.

This revision is now accepted and ready to land.May 2 2023, 6:04 PM
This revision was automatically updated to reflect the committed changes.
mhorne marked an inline comment as done.
arichardson added inline comments.
sys/amd64/include/pmc_mdep.h
112

Shouldn't this be passing 1 as the size argument to match the current logic (although I doubt it would ever make a difference).

sys/amd64/include/pmc_mdep.h
112

Yes they are different, but it won't matter in this context. There is a single instance in which the additional bounds checking provided by the size argument has any meaning at all, and that is when the debugger attempts to unwind after a kstack overflow. If the stack frame itself partially overflows the end of the stack, then this extra bit of checking could avoid a recursive panic.