From 0a776030207cbff81530c9f9d345bec568391ab4 Mon Sep 17 00:00:00 2001 From: "Andre A. Marin" Date: Sat, 7 Mar 2020 06:06:22 -0500 Subject: [PATCH] Add unit tests for mss field_t and macro def mss::field get and set field API was missing explicit unit tests. Added new mss macro definitions for testing the most common assertions that are counter intuitive to the default provided from Catch.hpp when testing ReturnCode information. Change-Id: Ia340f901da293213257401c3e916b8dcdbef27aa Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/92715 Tested-by: FSP CI Jenkins Reviewed-by: Louis Stermole Reviewed-by: Mark Pizzutillo Tested-by: Jenkins Server Tested-by: PPE CI Tested-by: Hostboot CI Dev-Ready: ANDRE A MARIN Reviewed-by: Jennifer A Stofer Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/92726 Reviewed-by: RAJA DAS --- .../procedures/hwp/memory/lib/i2c/exp_i2c.H | 3 +- .../generic/memory/lib/utils/mss_field.H | 76 ++++++++----------- .../memory/lib/utils/mss_generic_check.H | 38 +++++++++- 3 files changed, 69 insertions(+), 48 deletions(-) diff --git a/src/import/chips/ocmb/explorer/procedures/hwp/memory/lib/i2c/exp_i2c.H b/src/import/chips/ocmb/explorer/procedures/hwp/memory/lib/i2c/exp_i2c.H index c1d8dec46..8f56ca64b 100644 --- a/src/import/chips/ocmb/explorer/procedures/hwp/memory/lib/i2c/exp_i2c.H +++ b/src/import/chips/ocmb/explorer/procedures/hwp/memory/lib/i2c/exp_i2c.H @@ -40,7 +40,6 @@ #include #include -// reused TARGTIDFORMAT defined in generic/memory/lib/utils/mss_generic_check.H #ifdef __PPE__ #include #include @@ -55,6 +54,8 @@ #define MSSTARGID mss::c_str(i_target) #endif +// reused TARGTIDFORMAT defined in generic/memory/lib/utils/mss_generic_check.H + namespace mss { namespace exp diff --git a/src/import/generic/memory/lib/utils/mss_field.H b/src/import/generic/memory/lib/utils/mss_field.H index 7ed4a291d..01d7f53db 100644 --- a/src/import/generic/memory/lib/utils/mss_field.H +++ b/src/import/generic/memory/lib/utils/mss_field.H @@ -165,8 +165,7 @@ inline bool conditional(const T i_field, /// @tparam E endian type /// @tparam F the field to extract /// @tparam T the fapi2 target type -/// @tparam IT data input type -/// @tparam OT data output type +/// @tparam OT output type /// @tparam FFDC ffdc code type /// @param[in] i_target the fapi2 target /// @param[in] i_data the data @@ -177,17 +176,13 @@ inline bool conditional(const T i_field, template< mss::endian E, const mss::field_t& F, fapi2::TargetType T, - typename IT, typename OT, typename FFDC > inline fapi2::ReturnCode get_field(const fapi2::Target& i_target, - const std::vector& i_data, + const std::vector& i_data, const FFDC i_ffdc_codes, OT& o_value) { - // Initializes the output value to 0, that way, we won't have any stale data in o_value - o_value = 0; - const size_t BYTE = F.get_byte(i_data); FAPI_ASSERT( BYTE < i_data.size(), @@ -202,9 +197,11 @@ inline fapi2::ReturnCode get_field(const fapi2::Target& i_target, TARGTID); { + // clear out stale state + o_value = 0; + // Extracting desired bits - const fapi2::buffer l_buffer(i_data[BYTE]); - l_buffer.template extractToRight(o_value); + fapi2::buffer(i_data[BYTE]).extractToRight(o_value); FAPI_DBG(TARGTIDFORMAT " data[%d] = 0x%02x. Field with start bit %d, bit len %d, has data 0x%02x.", TARGTID, @@ -227,8 +224,7 @@ fapi_try_exit: /// @tparam E endian type /// @tparam F the field to extract /// @tparam T the fapi2 target type -/// @tparam IT data input type -/// @tparam OT data output type +/// @tparam IT input type /// @tparam FFDC ffdc code type /// @param[in] i_target the fapi2 target /// @param[in] i_setting the setting to set @@ -240,12 +236,11 @@ template< mss::endian E, const mss::field_t& F, fapi2::TargetType T, typename IT, - typename OT, typename FFDC > inline fapi2::ReturnCode set_field(const fapi2::Target& i_target, const IT i_setting, const FFDC i_ffdc_codes, - std::vector& io_data) + std::vector& io_data) { const size_t BYTE = F.get_byte(io_data); @@ -260,28 +255,30 @@ inline fapi2::ReturnCode set_field(const fapi2::Target& i_target, io_data.size(), spd::c_str(i_target)); + FAPI_TRY(check::invalid_type_conversion(i_target, i_setting, i_ffdc_codes)); + { // Insert desired setting - fapi2::buffer l_buffer(io_data[BYTE]); + fapi2::buffer l_buffer(io_data[BYTE]); l_buffer.template insertFromRight(i_setting); - io_data[BYTE] = static_cast(l_buffer); - - FAPI_DBG("%s data[%d] = 0x%02x. Field with start bit %d, bit len %d, has data 0x%02x.", - spd::c_str(i_target), - BYTE, - io_data[BYTE], - F.get_start(), - F.get_length(), - i_setting); + // Safe to set since no implicit conversion errors will occur here + io_data[BYTE] = l_buffer; } + FAPI_DBG("%s data[%d] = 0x%02x. Field with start bit %d, bit len %d, has data 0x%02x.", + spd::c_str(i_target), + BYTE, + io_data[BYTE], + F.get_start(), + F.get_length(), + i_setting); + return fapi2::FAPI2_RC_SUCCESS; fapi_try_exit: return fapi2::current_err; } -#endif /// /// @brief byte field reader @@ -289,8 +286,6 @@ fapi_try_exit: /// @tparam F the byte field to read /// @tparam TT traits associated with F - required /// @tparam T the fapi2 target type -/// @tparam IT data input type -/// @tparam OT data output type /// @param[in] i_target the dimm target /// @param[in] i_data the data /// @param[in] i_ffdc_codes FFDC code @@ -301,15 +296,14 @@ template< mss::endian E, const mss::field_t& F, typename TT, fapi2::TargetType T, - typename IT, typename OT, typename FFDC > inline fapi2::ReturnCode get_field( const fapi2::Target& i_target, - const std::vector& i_data, + const std::vector& i_data, const FFDC i_ffdc_codes, OT& o_value ) { - IT l_temp = 0; + uint8_t l_temp = 0; FAPI_TRY( (get_field(i_target, i_data, i_ffdc_codes, l_temp)), "Failed get_field() for " TARGTIDFORMAT, TARGTID ); @@ -317,7 +311,7 @@ inline fapi2::ReturnCode get_field( const fapi2::Target& i_target, FAPI_TRY( check::invalid_value(i_target, conditional( l_temp, TT::COMPARISON_VAL, - typename TT::template COMPARISON_OP() ), + typename TT::template COMPARISON_OP() ), F.get_byte(i_data), l_temp, i_ffdc_codes, @@ -325,16 +319,8 @@ inline fapi2::ReturnCode get_field( const fapi2::Target& i_target, "%s failed check::invalid_value() for %s", TT::FIELD_STR, spd::c_str(i_target) ); - // Output should only change if data check passes + // Implicit (possible) promotion during conversion is safe o_value = static_cast(l_temp); - FAPI_ASSERT( o_value == l_temp, - fapi2::MSS_CONVERSION_ERROR() - .set_ORIGINAL_VAL(l_temp) - .set_CONVERTED_VAL(o_value) - .set_TARGET(i_target) - .set_FUNCTION(i_ffdc_codes), - "Conversion error between original %d to converted %d value for " TARGTIDFORMAT, - l_temp, o_value, TARGTID); FAPI_DBG("%s: 0x%02x for %s", TT::FIELD_STR, @@ -346,15 +332,13 @@ fapi_try_exit: return fapi2::current_err; } -#ifndef __PPE__ /// /// @brief byte field writer /// @tparam E endian type /// @tparam F the byte field to read /// @tparam TT traits associated with writer /// @tparam T the fapi2 target type -/// @tparam IT data input type -/// @tparam OT data output type +/// @tparam IT input type /// @tparam FFDC ffdc code type /// @param[in] i_target the dimm target /// @param[in] i_setting value to set for field @@ -367,12 +351,11 @@ template< mss::endian E, typename TT, fapi2::TargetType T, typename IT, - typename OT, typename FFDC > inline fapi2::ReturnCode set_field( const fapi2::Target& i_target, const IT i_setting, const FFDC i_ffdc_codes, - std::vector& io_data ) + std::vector& io_data ) { const size_t BYTE = F.get_byte(io_data); @@ -399,8 +382,9 @@ inline fapi2::ReturnCode set_field( const fapi2::Target& i_target, fapi_try_exit: return fapi2::current_err; } -#endif + +#endif // PPE }// mss -#endif +#endif // include guard diff --git a/src/import/generic/memory/lib/utils/mss_generic_check.H b/src/import/generic/memory/lib/utils/mss_generic_check.H index 9f4088754..bb2780b6d 100644 --- a/src/import/generic/memory/lib/utils/mss_generic_check.H +++ b/src/import/generic/memory/lib/utils/mss_generic_check.H @@ -247,8 +247,44 @@ inline fapi2::ReturnCode invalid_value(const fapi2::Target& i_target, fapi_try_exit: return fapi2::current_err; -} // fail_for_invalid_value +} // invalid_value +/// +/// @brief Checks conditional passes and implements traces & exits if it fails +/// @tparam T fapi2 target type +/// @tparam IT input data type +/// @tparam FFDC error callout code type +/// @param[in] i_target fapi2 target +/// @param[in] i_conditional conditional that we are testing against +/// @param[in] i_byte_index byte index +/// @param[in] i_data debug data +/// @param[in] i_ffdc_codes FFDC code +/// @param[in] i_err_str error string - defaulted to "" +/// @return FAPI2_RC_SUCCESS iff okay +/// +template< typename OT, fapi2::TargetType T, typename IT, typename FFDC > +inline fapi2::ReturnCode invalid_type_conversion(const fapi2::Target& i_target, + const IT& i_data, + const FFDC i_ffdc_codes, + const char* i_err_str = "") +{ + OT l_temp = static_cast(i_data); + + FAPI_ASSERT( i_data == l_temp, + fapi2::MSS_CONVERSION_ERROR() + .set_ORIGINAL_VAL(i_data) + .set_CONVERTED_VAL(l_temp) + .set_TARGET(i_target) + .set_FUNCTION(i_ffdc_codes), + "Conversion error between original %d to converted %d value for " TARGTIDFORMAT, + i_data, l_temp, TARGTID); + + return fapi2::FAPI2_RC_SUCCESS; + +fapi_try_exit: + return fapi2::current_err; + +} // invalid_type_conversion } // check }// mss