DHowett
commented
1 year ago This, along with the hx30 variant in #23, is in review upstream at https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/4114450. It is trending towards merging. 🙂
The code in espi_msvw[12]_interrupt relies on undefined behavior today. __builtin_ctz is specified as returning values in the range [0, 31], but we are checking for 32.
This behavior may be unexpected compared to the CTZ/CLZ instruction on ARM, which use the value 32 to indicate that there are no ones in the provided input.
GCC 11+ optimizes the two loops below into infinite loops, as it can see that the condition will never be met.
After this change, the disassembly of espi_mswv1_interrupt can be confirmed to contain an exit behind a branch.
... // r4 is loaded with girq24_result and has bits successively cleared 1a: b90c cbnz r4, 20 <espi_mswv1_interrupt+0x20> 1c: e8bd 81f0 ldmia.w sp!, {r4, r5, r6, r7, r8, pc} 20: fa94 f5a4 rbit r5, r4 ...
BUG=EmbeddedController#21 BRANCH=hx20-hx30 TEST=Examined the disassembly for espi_msvw[12]_interrupt; see above
Signed-off-by: Dustin L. Howett dustin@howett.net