search

FrameworkComputer / EmbeddedController

Embedded Controller firmware for the Framework Laptop
BSD 3-Clause "New" or "Revised" License
950 stars 64 forks source link

[hx30] mchp: Remove undefined behavior in espi msvw handlers #23

Closed DHowett closed 1 year ago

DHowett commented 1 year ago

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=hx30 TEST=Examined the disassembly for espi_msvw[12]_interrupt; see above

Signed-off-by: Dustin L. Howett dustin@howett.net

kiram9 commented 1 year ago

Closing this as we are moving to 1 common branch for both platforms. I updated the branch in https://github.com/FrameworkComputer/EmbeddedController/pull/22