psci: Teach the arm SMCCC code about KMSAN
Needs ReviewPublic
Actions

Authored by andrew on Mon, Oct 28, 5:12 PM.

Details

Reviewers

markj
manu

Summary

We write to a data structure in assembly. Because of this we need to
annotate the function manually for KMSAN to know the contents have been
written otherwise it picks up later reads as loading uninitialised data.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 60260
Build 57144: arc lint + arc unit

Event Timeline

andrew created this revision.Mon, Oct 28, 5:12 PM

Herald added subscribers: emaste, imp. · View Herald TranscriptMon, Oct 28, 5:12 PM

andrew requested review of this revision.Mon, Oct 28, 5:12 PM

Harbormaster completed remote builds in B60227: Diff 145550.Mon, Oct 28, 5:12 PM

This feels over-engineered: why not just make always-on C wrappers for the assembly routines and handle marking there? I don't see a need for a separate file and the extra indirection.

sys/dev/psci/smccc_kmsan.c
28 ↗	(On Diff #145550)	The purpose of these guards is to prevent recursion in the bits of the KMSAN runtime which actually need to use intercepted symbols, but that's not a danger for the SMCCC interfaces.

Move kmsan calls to asm
Add kmsan_check and kmsan_mark to arm_smccc_1_2_*

Herald added a reviewer: manu. · View Herald TranscriptWed, Oct 30, 1:56 PM

Harbormaster completed remote builds in B60260: Diff 145670.Wed, Oct 30, 1:57 PM

Why can't this be done in C? e.g., have asm _arm_smccc_1_2_hvc() which is called by the C function arm_smccc_1_2_hvc().

It seems like an unneeded level of indirection in the non-KMSAN case

sys/dev/psci/smccc_kmsan.c
28 ↗	(On Diff #145550)	I don't understand, these use the intercepted symbols, e.g. `kmsan_arm_smccc_hvc` calls `arm_smccc_hvc`.

In D47317#1079638, @andrew wrote:

It seems like an unneeded level of indirection in the non-KMSAN case

These aren't performance-critical in any way, so I'm not sure why it matters.

I don't object to handling everything in asm, but it really seems like it'd be simpler and cleaner to handle KMSAN marking in C.

sys/dev/psci/smccc_kmsan.c
28 ↗	(On Diff #145550)	Sorry, my comment wasn't right. I really just meant that this machinery isn't needed for routines which are only called from one file.