commit 4dd71af3d5262064c8d8700825f8cb4d616b5fa7
author Yong Li <yong.b.li@linux.intel.com> Sun Sep 29 14:18:07 2019 +0800
committer Vernon Mauery <vernon.mauery@linux.intel.com> Fri Oct 11 15:27:52 2019 +0000
tree 7c8f4cff5dfcdcba828099993a9787bffda70977
parent 449f2166f951a650bef8634b573d6306a6ed378c

Data checking fix for watchdog set/get commands

data length check for timeout action and byte 1 reserved field check
are missing, causing not to throw the error;
Log flags is on bit 7, also needs to right-shift this flag;
This commit fixes these issues;

Tested:
Set different timer use/actions:
ipmitool raw 0x06 0x24 0x85 0x0 0x0 0x0 0x64 0x00
ipmitool raw 0x06 0x24 0x84 0x1 0x0 0x0 0x64 0x00

Check the settings are correct:
ipmitool raw 0x06 0x25

Signed-off-by: Yong Li <yong.b.li@linux.intel.com>
Change-Id: Ia4226bb4597d2c670f93522aa763e43d15eb6cf1

app/watchdog.cpp

diff --git a/app/watchdog.cpp b/app/watchdog.cpp
index ff07b02..47eacf0 100644
--- a/app/watchdog.cpp
+++ b/app/watchdog.cpp
@@ -86,7 +86,7 @@
 static constexpr uint8_t wdTimerUseResTimer2 = 0x6;
 static constexpr uint8_t wdTimerUseResTimer3 = 0x7;
 
-static constexpr uint8_t wdTimeoutActionTimer = 0x40;
+static constexpr uint8_t wdTimeoutActionMax = 3;
 static constexpr uint8_t wdTimeoutInterruptTimer = 0x04;
 
 enum class IpmiAction : uint8_t
@@ -173,9 +173,9 @@
     }
 }
 
-static uint8_t timerLogFlags = 0;
-static uint8_t timerActions = 0;
+static bool timerNotLogFlags = false;
 static uint8_t timerUseExpirationFlags = 0;
+static uint3_t timerPreTimeoutInterrupt = 0;
 
 /**@brief The Set Watchdog Timer ipmi command.
  *
@@ -199,9 +199,9 @@
     if ((timerUse == wdTimerUseResTimer1) ||
         (timerUse == wdTimerUseResTimer2) ||
         (timerUse == wdTimerUseResTimer3) ||
-        (timeoutAction == wdTimeoutActionTimer) ||
+        (timeoutAction > wdTimeoutActionMax) ||
         (preTimeoutInterrupt == wdTimeoutInterruptTimer) ||
-        (reserved1 | reserved2 | reserved3 | reserved4))
+        (reserved | reserved1 | reserved2 | reserved3 | reserved4))
     {
         return ipmi::responseInvalidFieldRequest();
     }
@@ -211,9 +211,8 @@
         return ipmi::responseInvalidFieldRequest();
     }
 
-    timerLogFlags = static_cast<uint8_t>(dontLog);
-    timerActions &= static_cast<uint8_t>(timeoutAction) |
-                    static_cast<uint8_t>(preTimeoutInterrupt) << 4;
+    timerNotLogFlags = dontLog;
+    timerPreTimeoutInterrupt = preTimeoutInterrupt;
 
     try
     {
@@ -354,8 +353,16 @@
  * - initialCountdown
  * - presentCountdown
  **/
-ipmi::RspType<uint8_t,  // timerUse
-              uint8_t,  // timerAction
+ipmi::RspType<uint3_t, // timerUse - timer use
+              uint3_t, // timerUse - reserved
+              bool,    // timerUse - timer is started
+              bool,    // timerUse - don't log
+
+              uint3_t, // timerAction - timeout action
+              uint1_t, // timerAction - reserved
+              uint3_t, // timerAction - pre-timeout interrupt
+              uint1_t, // timerAction - reserved
+
               uint8_t,  // pretimeout
               uint8_t,  // expireFlags
               uint16_t, // initial Countdown - Little Endian (deciseconds)
@@ -373,11 +380,6 @@
         WatchdogService::Properties wd_prop = wd_service.getProperties();
 
         // Build and return the response
-        uint8_t timerUse = 0;
-        timerUse |= timerLogFlags;
-
-        uint8_t timerAction = timerActions;
-
         // Interval and timeRemaining need converted from milli -> deci seconds
         uint16_t initialCountdown = htole16(wd_prop.interval / 100);
 
@@ -390,7 +392,6 @@
 
         if (wd_prop.enabled)
         {
-            timerUse |= wd_running;
             presentCountdown = htole16(wd_prop.timeRemaining / 100);
             expireFlags = 0;
         }
@@ -405,19 +406,21 @@
             {
                 presentCountdown = 0;
                 expireFlags = timerUseExpirationFlags;
+                // Automatically clear it whenever a timer expiration occurs.
+                timerNotLogFlags = false;
             }
         }
 
-        timerUse |=
-            static_cast<uint8_t>(wdTimerUseToIpmiTimerUse(wd_prop.timerUse));
-
         // TODO: Do something about having pretimeout support
         pretimeout = 0;
 
         lastCallSuccessful = true;
-        return ipmi::responseSuccess(timerUse, timerAction, pretimeout,
-                                     expireFlags, initialCountdown,
-                                     presentCountdown);
+        return ipmi::responseSuccess(
+            static_cast<uint3_t>(wdTimerUseToIpmiTimerUse(wd_prop.timerUse)), 0,
+            wd_prop.enabled, timerNotLogFlags,
+            static_cast<uint3_t>(wdActionToIpmiAction(wd_prop.expireAction)), 0,
+            timerPreTimeoutInterrupt, 0, pretimeout, expireFlags,
+            initialCountdown, presentCountdown);
     }
     catch (const InternalFailure& e)
     {