diff options
author | Dan Crowell <dcrowell@us.ibm.com> | 2012-03-29 16:03:59 -0500 |
---|---|---|
committer | A. Patrick Williams III <iawillia@us.ibm.com> | 2012-03-30 15:50:50 -0500 |
commit | a2cf819c913c65fb9a019b8e62c8e77b8964929a (patch) | |
tree | 103843cd2c48b1a8af4f17de3da1bc23627b8836 /src/usr/devicefw | |
parent | df3648d7cd33ee146de3041d3f0d93a713075e26 (diff) | |
download | talos-hostboot-a2cf819c913c65fb9a019b8e62c8e77b8964929a.tar.gz talos-hostboot-a2cf819c913c65fb9a019b8e62c8e77b8964929a.zip |
Prevent double registration in device framework
The device framework code will log an error if more than one
function is registered for the same operation/access/target
combination.
Change-Id: Id5136c389250ed26d7b62ff3b71116bba54ceb89
RTC: 38760
Reviewed-on: http://gfw160.austin.ibm.com:8080/gerrit/805
Tested-by: Jenkins Server
Reviewed-by: A. Patrick Williams III <iawillia@us.ibm.com>
Diffstat (limited to 'src/usr/devicefw')
-rw-r--r-- | src/usr/devicefw/assoccontain.H | 2 | ||||
-rw-r--r-- | src/usr/devicefw/associator.C | 193 | ||||
-rw-r--r-- | src/usr/devicefw/associator.H | 23 | ||||
-rw-r--r-- | src/usr/devicefw/driverif.C | 14 | ||||
-rw-r--r-- | src/usr/devicefw/test/associatortest.H | 76 |
5 files changed, 216 insertions, 92 deletions
diff --git a/src/usr/devicefw/assoccontain.H b/src/usr/devicefw/assoccontain.H index 127057698..1d500e07a 100644 --- a/src/usr/devicefw/assoccontain.H +++ b/src/usr/devicefw/assoccontain.H @@ -41,7 +41,7 @@ namespace DeviceFW * @brief Data about a particular association. * * Typically this is a offset (pointer) to another association and an - * optional flag for the associatior to use. This allows a user to + * optional flag for the associator to use. This allows a user to * construct a light-weight multi-dimensional sparse array. * * The purpose of the flag field is left to the user. diff --git a/src/usr/devicefw/associator.C b/src/usr/devicefw/associator.C index 60a0895e0..7bc3ac28b 100644 --- a/src/usr/devicefw/associator.C +++ b/src/usr/devicefw/associator.C @@ -50,10 +50,10 @@ namespace DeviceFW TRACFCOMP(g_traceBuffer, EXIT_MRK "Associator::~Associator"); } - void Associator::registerRoute(int64_t i_opType, - int64_t i_accType, - int64_t i_targetType, - deviceOp_t i_regRoute) + errlHndl_t Associator::registerRoute(int64_t i_opType, + int64_t i_accType, + int64_t i_targetType, + deviceOp_t i_regRoute) { TRACFCOMP(g_traceBuffer, "Device route registered for (%d, %d, %d)", i_opType, i_accType, i_targetType); @@ -63,6 +63,43 @@ namespace DeviceFW // No assert-checks will be done here. mutex_lock(&iv_mutex); + + // Make sure we aren't doing a double registration + for( OperationType optype = FIRST_OP_TYPE; + optype < LAST_OP_TYPE; + optype = static_cast<OperationType>(optype+1) ) + { + if( (WILDCARD == i_opType) || (optype == i_opType) ) + { + deviceOp_t l_devRoute = findDeviceRoute( optype, + static_cast<TARGETING::TYPE>(i_targetType), + i_accType ); + if( l_devRoute != NULL ) + { + TRACFCOMP(g_traceBuffer, "Double registration attempted : i_opType=%d, i_accType=%d, i_targetType=0x%X, existing function=%p", i_opType, i_accType, i_targetType, l_devRoute ); + /*@ + * @errortype + * @moduleid DEVFW_MOD_ASSOCIATOR + * @reasoncode DEVFW_RC_DOUBLE_REGISTRATION + * @userdata1[0:31] OpType + * @userdata1[32:63] AccessType + * @userdata2 TargetType + * + * @devdesc A double registration was attempted + * with the routing framework. + */ + errlHndl_t l_errl = + new ErrlEntry(ERRL_SEV_INFORMATIONAL, + DEVFW_MOD_ASSOCIATOR, + DEVFW_RC_DOUBLE_REGISTRATION, + TWO_UINT32_TO_UINT64(i_opType, i_accType), + TO_UINT64(i_targetType) + ); + mutex_unlock(&iv_mutex); + return l_errl; + } + } + } size_t ops = 0; AssociationData targets = AssociationData(); @@ -71,7 +108,7 @@ namespace DeviceFW ops = iv_associations[iv_routeMap][i_accType].offset; if (0 == ops) { - // space for LAST_OP_TYPE plus WILDCARD(-1). + // space for LAST_OP_TYPE plus WILDCARD(-1). ops = iv_associations.allocate(LAST_OP_TYPE + 1) + 1; iv_associations[iv_routeMap][i_accType].offset = ops; } @@ -93,31 +130,6 @@ namespace DeviceFW } iv_associations[ops][i_opType] = targets; } - // Ensure the right block size was allocated previously for this - // target-type (wildcard vs non-wildcard). - if(((targets.flag) && (i_targetType != WILDCARD)) || - ((!targets.flag) && (i_targetType == WILDCARD))) - { - TRACFCOMP(g_traceBuffer, "Invalid registration type was given to register a device : i_opType=%d, i_accType=%d, i_targetType=%d", i_opType, i_accType, i_targetType ); - /*@ - * @errortype - * @moduleid DEVFW_MOD_ASSOCIATOR - * @reasoncode DEVFW_RC_INVALID_REGISTRATION - * @userdata1 (OpType << 32) | (AccessType) - * @userdata2 TargetType - * - * @devdesc An invalid registration type was given to - * register a device with the routing framework. - */ - errlHndl_t l_errl = - new ErrlEntry(ERRL_SEV_INFORMATIONAL, - DEVFW_MOD_ASSOCIATOR, - DEVFW_RC_INVALID_REGISTRATION, - TWO_UINT32_TO_UINT64(i_opType, i_accType), - TO_UINT64(i_targetType) - ); - errlCommit(l_errl,DEVFW_COMP_ID); - } // Index offset to proper target type. This is now lowest level of map. targets.offset += (i_targetType == WILDCARD ? 0 : i_targetType); @@ -134,14 +146,14 @@ namespace DeviceFW opLocation = iv_operations.end() - 1; } - // TODO: Implement std::distance algorithm and change to: - // std::distance(iv_operations.begin(), opLocation); - size_t opLoc = opLocation - iv_operations.begin(); + size_t opLoc = std::distance(iv_operations.begin(), opLocation); // Set function offset into map. True flag indicates valid. (*iv_associations[targets.offset]) = AssociationData(true, opLoc); mutex_unlock(&iv_mutex); + + return NULL; } errlHndl_t Associator::performOp(OperationType i_opType, @@ -179,15 +191,60 @@ namespace DeviceFW TRACDCOMP(g_traceBuffer, "Device op requested for (%d, %d, %d)", i_opType, i_accessType, l_devType); + mutex_lock(&iv_mutex); + + // Function pointer found for this route request. + deviceOp_t l_devRoute = findDeviceRoute( i_opType, + l_devType, + i_accessType ); + + mutex_unlock(&iv_mutex); + + // Call function if one was found, create error otherwise. + if (NULL == l_devRoute) + { + TRACFCOMP(g_traceBuffer, "A device driver operation was attempted for which no driver has been registered : i_opType=%d, i_accessType=%d, l_devType=%d", i_opType, i_accessType, l_devType ); + /*@ + * @errortype + * @moduleid DEVFW_MOD_ASSOCIATOR + * @reasoncode DEVFW_RC_NO_ROUTE_FOUND + * @userdata1 (OpType << 32) | (AccessType) + * @userdata2 TargetType + * + * @devdesc A device driver operation was attempted for + * which no driver has been registered. + */ + l_errl = new ErrlEntry(ERRL_SEV_INFORMATIONAL, + DEVFW_MOD_ASSOCIATOR, + DEVFW_RC_NO_ROUTE_FOUND, + TWO_UINT32_TO_UINT64(i_opType, i_accessType), + TO_UINT64(l_devType) + ); + } + else + { + l_errl = (*l_devRoute)(i_opType, i_target, + io_buffer, io_buflen, + i_accessType, i_addr); + } + + return l_errl; + } + + + deviceOp_t Associator::findDeviceRoute( OperationType i_opType, + TARGETING::TYPE i_devType, + int64_t i_accessType ) + { // The ranges of the parameters should all be verified by the - // compiler due to the template specializations in driverif.H. + // compiler due to the template specializations in driverif.H. + // e.g. i_accessType can never be negative // No assert-checks will be done here. - - mutex_lock(&iv_mutex); - // Function pointer found for this route request. + // Function pointer found for this route request. deviceOp_t l_devRoute = NULL; - // Pointer to root of the map. + + // Pointer to root of the map. const AssociationData* routeMap = iv_associations[iv_routeMap]; do @@ -199,7 +256,7 @@ namespace DeviceFW } const AssociationData* ops = - iv_associations[routeMap[i_accessType].offset]; + iv_associations[routeMap[i_accessType].offset]; // Check op type = WILDCARD registrations. if (0 != ops[WILDCARD].offset) @@ -208,23 +265,23 @@ namespace DeviceFW if (ops[WILDCARD].flag) { l_devRoute = - iv_operations[ + iv_operations[ iv_associations[ops[WILDCARD].offset]->offset]; break; } - + // Check access type = i_target->type registrations. const AssociationData* targets = - iv_associations[ops[WILDCARD].offset]; - if (targets[l_devType].flag) + iv_associations[ops[WILDCARD].offset]; + if (targets[i_devType].flag) { l_devRoute = - iv_operations[ - targets[l_devType].offset]; + iv_operations[ + targets[i_devType].offset]; break; } } - + // Check op type = i_opType registrations. if (0 != ops[i_opType].offset) { @@ -232,55 +289,25 @@ namespace DeviceFW if(ops[i_opType].flag) { l_devRoute = - iv_operations[ + iv_operations[ iv_associations[ops[i_opType].offset]->offset]; break; } - + // Check access type = i_target->type registrations. const AssociationData* targets = - iv_associations[ops[i_opType].offset]; - if (targets[l_devType].flag) + iv_associations[ops[i_opType].offset]; + if (targets[i_devType].flag) { l_devRoute = - iv_operations[ - targets[l_devType].offset]; + iv_operations[ + targets[i_devType].offset]; break; } } } while(0); - mutex_unlock(&iv_mutex); - - // Call function if one was found, create error otherwise. - if (NULL == l_devRoute) - { - TRACFCOMP(g_traceBuffer, "A device driver operation was attempted for which no driver has been registered : i_opType=%d, i_accessType=%d, l_devType=%d", i_opType, i_accessType, l_devType ); - /*@ - * @errortype - * @moduleid DEVFW_MOD_ASSOCIATOR - * @reasoncode DEVFW_RC_NO_ROUTE_FOUND - * @userdata1 (OpType << 32) | (AccessType) - * @userdata2 TargetType - * - * @devdesc A device driver operation was attempted for - * which no driver has been registered. - */ - l_errl = new ErrlEntry(ERRL_SEV_INFORMATIONAL, - DEVFW_MOD_ASSOCIATOR, - DEVFW_RC_NO_ROUTE_FOUND, - TWO_UINT32_TO_UINT64(i_opType, i_accessType), - TO_UINT64(l_devType) - ); - } - else - { - l_errl = (*l_devRoute)(i_opType, i_target, - io_buffer, io_buflen, - i_accessType, i_addr); - } - - return l_errl; + return l_devRoute; } } diff --git a/src/usr/devicefw/associator.H b/src/usr/devicefw/associator.H index f803926c2..7d462e537 100644 --- a/src/usr/devicefw/associator.H +++ b/src/usr/devicefw/associator.H @@ -66,16 +66,31 @@ namespace DeviceFW ~Associator(); /** Register routing interface. See deviceRegisterRoute. */ - void registerRoute(int64_t i_opType, - int64_t i_accType, - int64_t i_targetType, - deviceOp_t i_regRoute); + errlHndl_t registerRoute(int64_t i_opType, + int64_t i_accType, + int64_t i_targetType, + deviceOp_t i_regRoute); /** Perform routing. See deviceOp. */ errlHndl_t performOp(OperationType i_opType, TARGETING::Target* i_target, void* io_buffer, size_t& io_buflen, int64_t i_accessType, va_list i_addr); + + private: + /** + * @brief Find an associated function for the given operation + * + * @param[in] i_opType Enumeration specifying the operation type + * @param[in] i_accessType Enumeration specifying the access type + * @param[in] i_devType Enumeration specifying the target type + * + * @return NULL if none found, else a function pointer + */ + deviceOp_t findDeviceRoute( OperationType i_opType, + TARGETING::TYPE i_devType, + int64_t i_accessType ); + private: typedef std::vector<deviceOp_t> opVector_t; diff --git a/src/usr/devicefw/driverif.C b/src/usr/devicefw/driverif.C index 89aeb4ca0..2536bddf0 100644 --- a/src/usr/devicefw/driverif.C +++ b/src/usr/devicefw/driverif.C @@ -25,6 +25,7 @@ */ #include <devicefw/driverif.H> #include <util/singleton.H> +#include <errl/errlmanager.H> #include "associator.H" @@ -42,10 +43,15 @@ namespace DeviceFW int64_t i_targetType, deviceOp_t i_regRoute) { - Singleton<Associator>::instance().registerRoute(i_opType, - i_accessType, - i_targetType, - i_regRoute); + errlHndl_t errhdl = + Singleton<Associator>::instance().registerRoute(i_opType, + i_accessType, + i_targetType, + i_regRoute); + if( errhdl ) + { + errlCommit(errhdl,DEVFW_COMP_ID); + } } // deviceRegisterRoute: diff --git a/src/usr/devicefw/test/associatortest.H b/src/usr/devicefw/test/associatortest.H index c3ed5ab30..f54734858 100644 --- a/src/usr/devicefw/test/associatortest.H +++ b/src/usr/devicefw/test/associatortest.H @@ -113,6 +113,82 @@ public: } /** + * @test Verify we catch double registration. + */ + void testDoubleRegistration() + { + errlHndl_t l_errl = NULL; + + // Register non-sensical SCOM-READ to Nodes + Associator as; + l_errl = as.registerRoute(READ, + SCOM, + TYPE_NODE, + &performOperation); + if (l_errl) + { + TS_FAIL("testDoubleRegistration> Error received from registerRoute (1)."); + } + + // Register the exact same thing again + l_errl = as.registerRoute(READ, + SCOM, + TYPE_NODE, + &performOperation); + + if (l_errl) + { + // error log is expected case, delete it + delete l_errl; + } + else + { + TS_FAIL("testDoubleRegistration> No error from duplicate registration."); + } + + // Register a wildcard that overlaps + l_errl = as.registerRoute(WILDCARD, + SCOM, + TYPE_NODE, + &performOperation); + + if (l_errl) + { + // error log is expected case, delete it + delete l_errl; + } + else + { + TS_FAIL("testDoubleRegistration> No error from wildcard registration."); + } + + // Reverse the wildcard test + l_errl = as.registerRoute(WILDCARD, + PNOR, + TYPE_NODE, + &performOperation); + if (l_errl) + { + TS_FAIL("testDoubleRegistration> Error received from registerRoute (2)."); + } + l_errl = as.registerRoute(WRITE, + PNOR, + TYPE_NODE, + &performOperation); + if (l_errl) + { + // error log is expected case, delete it + delete l_errl; + } + else + { + TS_FAIL("testDoubleRegistration> No error from registration after wildcard."); + } + + + } + + /** * @test Verify registration with an operator as a WILDCARD. */ void testOpWildcard() |