diff options
author | Yong Li <yong.b.li@linux.intel.com> | 2019-09-29 14:18:07 +0800 |
---|---|---|
committer | Vernon Mauery <vernon.mauery@linux.intel.com> | 2019-10-11 15:27:52 +0000 |
commit | 4dd71af3d5262064c8d8700825f8cb4d616b5fa7 (patch) | |
tree | 7c8f4cff5dfcdcba828099993a9787bffda70977 | |
parent | 449f2166f951a650bef8634b573d6306a6ed378c (diff) | |
download | phosphor-host-ipmid-4dd71af3d5262064c8d8700825f8cb4d616b5fa7.tar.gz phosphor-host-ipmid-4dd71af3d5262064c8d8700825f8cb4d616b5fa7.zip |
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
-rw-r--r-- | app/watchdog.cpp | 47 | ||||
-rw-r--r-- | app/watchdog.hpp | 8 | ||||
-rw-r--r-- | 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 = |