From 4dd71af3d5262064c8d8700825f8cb4d616b5fa7 Mon Sep 17 00:00:00 2001
From: Yong Li <yong.b.li@linux.intel.com>
Date: Sun, 29 Sep 2019 14:18:07 +0800
Subject: 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         | 47 +++++++++++++++++++++++++----------------------
 app/watchdog.hpp         |  8 ++++----
 app/watchdog_service.cpp |  2 ++
 3 files changed, 31 insertions(+), 26 deletions(-)

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 wdTimerUseResTimer1 = 0x0;
 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 @@ WatchdogService::TimerUse ipmiTimerUseToWdTimerUse(IpmiTimerUse ipmiTimerUse)
     }
 }
 
-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 @@ ipmi::RspType<> ipmiSetWatchdogTimer(
     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 @@ ipmi::RspType<> ipmiSetWatchdogTimer(
         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 @@ static constexpr uint8_t wd_running = 0x1 << 6;
  * - 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 @@ ipmi::RspType<uint8_t,  // timerUse
         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 @@ ipmi::RspType<uint8_t,  // timerUse
 
         if (wd_prop.enabled)
         {
-            timerUse |= wd_running;
             presentCountdown = htole16(wd_prop.timeRemaining / 100);
             expireFlags = 0;
         }
@@ -405,19 +406,21 @@ ipmi::RspType<uint8_t,  // timerUse
             {
                 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)
     {
diff --git a/app/watchdog.hpp b/app/watchdog.hpp
index d52b6e6..a7ff62e 100644
--- a/app/watchdog.hpp
+++ b/app/watchdog.hpp
@@ -35,10 +35,10 @@ ipmi::RspType<> ipmiSetWatchdogTimer(
  * - initialCountdown
  * - presentCountdown
  **/
-ipmi::RspType<uint8_t,  // timerUse
-              uint8_t,  // timerAction
-              uint8_t,  // pretimeout
-              uint8_t,  // expireFlags
+ipmi::RspType<uint3_t, uint3_t, bool, bool,       // timerUse
+              uint3_t, uint1_t, uint3_t, uint1_t, // timerAction
+              uint8_t,                            // pretimeout
+              uint8_t,                            // expireFlags
               uint16_t, // initial Countdown - Little Endian (deciseconds)
               uint16_t  // present Countdown - Little Endian (deciseconds)
               >
diff --git a/app/watchdog_service.cpp b/app/watchdog_service.cpp
index 0eedf1b..a929cb6 100644
--- a/app/watchdog_service.cpp
+++ b/app/watchdog_service.cpp
@@ -80,6 +80,8 @@ WatchdogService::Properties WatchdogService::getProperties()
             std::get<std::string>(properties.at("ExpireAction")));
         wd_prop.timerUse = Watchdog::convertTimerUseFromString(
             std::get<std::string>(properties.at("CurrentTimerUse")));
+        wd_prop.expiredTimerUse = Watchdog::convertTimerUseFromString(
+            std::get<std::string>(properties.at("ExpiredTimerUse")));
 
         wd_prop.interval = std::get<uint64_t>(properties.at("Interval"));
         wd_prop.timeRemaining =
-- 
cgit v1.2.1