[Skiboot] [PATCH v2] libpore: Fix incorrect mtspr instruction generation

Cyril Bur cyril.bur at au1.ibm.com
Fri Nov 17 11:29:15 AEDT 2017


>From coverity defect 173758:
CID 173758 (#1 of 1): Unused value (UNUSED_VALUE)
assigned_value: Assigning value from (uint8_t)i_Rs << 21 to
mtsprInstOpcode here, but that stored value is overwritten before it can
be used.

This causes the generated mtspr to always move from register r0 as
opposed to the function parameter i_Rs.

Luckily the only call to getMtsprInstruction is:
getMtsprInstruction( 0, (uint16_t)i_regId );
the first parameter is the register so in an incredible stroke of luck,
the requirement is to generate a mtspr from r0.

Therefore no bug exists today, this is still a fairly important fix
because if anyone uses getMtsprInstruction() with a non zero first
parameter, it will cause them endless headache.

Fixes: CID 173758
---
 libpore/p9_stop_api.C | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/libpore/p9_stop_api.C b/libpore/p9_stop_api.C
index 26a14bbf..33a3bf39 100644
--- a/libpore/p9_stop_api.C
+++ b/libpore/p9_stop_api.C
@@ -255,12 +255,10 @@ static uint32_t getOrisInstruction( const uint16_t i_Rs, const uint16_t i_Ra,
  */
 static uint32_t getMtsprInstruction( const uint16_t i_Rs, const uint16_t i_Spr )
 {
-    uint32_t mtsprInstOpcode = 0;
-    uint32_t temp = (( i_Spr & 0x03FF ) << 11);
-    mtsprInstOpcode = (uint8_t)i_Rs << 21;
-    mtsprInstOpcode = ( temp  & 0x0000F800 ) << 5;
-    mtsprInstOpcode |= ( temp & 0x001F0000 ) >> 5;
-    mtsprInstOpcode |= MTSPR_BASE_OPCODE;
+    uint32_t mtsprInstOpcode = MTSPR_BASE_OPCODE;
+    uint32_t temp = ((i_Spr & 0x1F) << 5) | ((i_Spr & 0x8F) >> 5);
+
+    mtsprInstOpcode |= ((i_Rs & 0x1F) << 21) | ((temp & 0x03FF) << 11);
 
     return SWIZZLE_4_BYTE(mtsprInstOpcode);
 }
-- 
2.15.0



More information about the Skiboot mailing list