diff options
author | Thi Tran <thi@us.ibm.com> | 2011-08-29 15:55:50 -0500 |
---|---|---|
committer | Thi N. Tran <thi@us.ibm.com> | 2011-09-02 15:38:45 -0500 |
commit | 85005b2607a50aec3c2f4528871687bb7b334a7a (patch) | |
tree | 34f6cfca94d5d0ea539656bfa8722f9f15a85d94 /src | |
parent | 07580067764f07953570e6e6dddf7b9196b1c400 (diff) | |
download | talos-hostboot-85005b2607a50aec3c2f4528871687bb7b334a7a.tar.gz talos-hostboot-85005b2607a50aec3c2f4528871687bb7b334a7a.zip |
Fix XSCOM assert on non-sentinel chip
Change virt addr detect logic for better performance
Updated with review comments
Updated with review comments from set 3
Updated with review comments from set 4
Change-Id: I16828c8c325bb9e825683545d28c6bad2562efa9
Reviewed-on: http://gfw160.austin.ibm.com:8080/gerrit/288
Tested-by: Jenkins Server
Reviewed-by: A. Patrick Williams III <iawillia@us.ibm.com>
Reviewed-by: Nicholas E. Bofferding <bofferdn@us.ibm.com>
Diffstat (limited to 'src')
-rw-r--r-- | src/include/usr/xscom/xscomreasoncodes.H | 8 | ||||
-rw-r--r-- | src/usr/xscom/test/xscomtest.H | 6 | ||||
-rw-r--r-- | src/usr/xscom/xscom.C | 300 | ||||
-rw-r--r-- | src/usr/xscom/xscom.H | 31 |
4 files changed, 290 insertions, 55 deletions
diff --git a/src/include/usr/xscom/xscomreasoncodes.H b/src/include/usr/xscom/xscomreasoncodes.H index f66382a37..09fdb5d30 100644 --- a/src/include/usr/xscom/xscomreasoncodes.H +++ b/src/include/usr/xscom/xscomreasoncodes.H @@ -29,9 +29,10 @@ namespace XSCOM { enum xscomModuleId { - XSCOM_PERFORM_OP = 0x00, - XSCOM_SANITY_CHECK = 0x01, - XSCOM_TEST_XSCOM1 = 0x02, + XSCOM_PERFORM_OP = 0x00, + XSCOM_SANITY_CHECK = 0x01, + XSCOM_TEST_XSCOM1 = 0x02, + XSCOM_GET_TARGET_VIRT_ADDR = 0x03, }; enum xscomReasonCode @@ -40,6 +41,7 @@ namespace XSCOM XSCOM_INVALID_DATA_BUFFER = XSCOM_COMP_ID | 0x02, XSCOM_INVALID_OP_TYPE = XSCOM_COMP_ID | 0x03, XSCOM_DATA_UNMATCHED = XSCOM_COMP_ID | 0x04, + XSCOM_MMIO_UNMAP_ERR = XSCOM_COMP_ID | 0x05, }; }; diff --git a/src/usr/xscom/test/xscomtest.H b/src/usr/xscom/test/xscomtest.H index 235fb995a..6526de1b4 100644 --- a/src/usr/xscom/test/xscomtest.H +++ b/src/usr/xscom/test/xscomtest.H @@ -69,7 +69,11 @@ public: void testXscom1(void) { - TARGETING::Target* l_testTarget = MASTER_PROCESSOR_CHIP_TARGET_SENTINEL; + TARGETING::TargetService& l_targetService = TARGETING::targetService(); + TARGETING::Target* l_testTarget = NULL; + l_targetService.masterProcChipTargetHandle( l_testTarget ); + assert(l_testTarget != NULL); + size_t l_size = sizeof(uint64_t); // Loop thru table diff --git a/src/usr/xscom/xscom.C b/src/usr/xscom/xscom.C index fb962cb24..179e8dc31 100644 --- a/src/usr/xscom/xscom.C +++ b/src/usr/xscom/xscom.C @@ -32,6 +32,7 @@ #include <sys/mmio.h> #include <sys/task.h> #include <sys/sync.h> +#include <sys/misc.h> #include <string.h> #include <devicefw/driverif.H> #include <trace/interface.H> @@ -40,6 +41,7 @@ #include <targeting/targetservice.H> #include <xscom/xscomreasoncodes.H> #include "xscom.H" +#include <assert.h> // Trace definition trace_desc_t* g_trac_xscom = NULL; @@ -51,6 +53,10 @@ namespace XSCOM // Master processor virtual address uint64_t* g_masterProcVirtAddr = NULL; +// Max chip per node in this system +extern uint8_t getMaxChipsPerNode(); +static uint8_t g_xscomMaxChipsPerNode = getMaxChipsPerNode(); + // Register XSCcom access functions to DD framework DEVICE_REGISTER_ROUTE(DeviceFW::WILDCARD, DeviceFW::XSCOM, @@ -194,6 +200,218 @@ errlHndl_t xscomOpSanityCheck(const DeviceFW::OperationType i_opType, return l_err; } +/** + * @brief Returns maximum processors chip per node + * base on system type + * + * @return uint8_t + */ +uint8_t getMaxChipsPerNode() +{ + uint8_t l_numOfChips = 0; + + ProcessorCoreType l_coreType = cpu_core_type(); + //@todo - Need to verify if this number is correct + // for both Salerno and Venice + switch (l_coreType) + { + case CORE_POWER8_SALERNO: + case CORE_POWER8_VENICE: + case CORE_UNKNOWN: + default: + l_numOfChips = 8; + break; + } + return l_numOfChips; +} + +/** + * @brief Get the virtual address of the input target + * for an XSCOM access. + * + * Logic: + * + * If sentinel: + * If never XSCOM to sentinel + * Calculate virtual addr for sentinel + * Save it to g_masterProcVirtAddr for future XSCOM to sentinel + * Else + * Use virtual addr stored in g_masterProcVirtAddr + * End if + * Else (not sentinel) + * If never XSCOM to this chip: + * If this is a master processor object + * Use virtual addr stored for sentinel (g_masterProcVirtAddr) + * Else + * Call mmio_dev_map() to get virtual addr for this slave proc + * @todo: + * Currently virt addr attribute is not supported, so we + * must call unmap in xscomPerfomOp function once the + * xscom operation is done. + * When virt addr attribute is supported, the code that saves + * virt addr code in this function will be uncommented, + * and the mmio_dev_unmap() call in xscomPerformOp() + * function must be removed. + * End if + * Save virtual addr used to this chip's attribute + * Else + * Use virtual address stored in this chip's attributes. + * End if + * End if + * + * @param[in] i_target XSCom target + * @param[out] o_virtAddr Target's virtual address + * + * @return errlHndl_t + */ +errlHndl_t getTargetVirtualAddress(const TARGETING::Target* i_target, + uint64_t*& o_virtAddr) +{ + errlHndl_t l_err = NULL; + o_virtAddr = NULL; + XSComBase_t l_XSComBaseAddr = 0; + + do + { + // Sentinel + if (i_target == TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL) + { + // If never XSCOM to sentinel before, calculate + // sentinel's virtAddr and store it + + // Use atomic update instructions here to avoid + // race condition between different threads. + // Keep in mind that the mutex used in XSCOM is hardware mutex, + // not a mutex for the whole XSCOM logic. + if (__sync_bool_compare_and_swap(&g_masterProcVirtAddr, + NULL, NULL)) + { + l_XSComBaseAddr = MASTER_PROC_XSCOM_BASE_ADDR; + uint64_t* l_tempVirtAddr = static_cast<uint64_t*> + (mmio_dev_map(reinterpret_cast<void*>(l_XSComBaseAddr), + THIRTYTWO_GB)); + if (!__sync_bool_compare_and_swap(&g_masterProcVirtAddr, + NULL, l_tempVirtAddr)) + { + // If g_masterProcVirtAddr has already been updated by + // another thread, we need to unmap the dev_map we just + // called above. + int rc = 0; + rc = mmio_dev_unmap(reinterpret_cast<void*> + (l_tempVirtAddr)); + if (rc != 0) + { + /*@ + * @errortype + * @moduleid XSCOM_GET_TARGET_VIRT_ADDR + * @reasoncode XSCOM_MMIO_UNMAP_ERR + * @userdata1 Return Code + * @userdata2 Unmap address + * @devdesc mmio_dev_unmap() returns error + */ + l_err = new ERRORLOG::ErrlEntry( + ERRORLOG::ERRL_SEV_UNRECOVERABLE, + XSCOM_GET_TARGET_VIRT_ADDR, + XSCOM_MMIO_UNMAP_ERR, + rc, + reinterpret_cast<uint64_t>(l_tempVirtAddr)); + break; + } + } + } + + // Set virtual address to sentinel's value + o_virtAddr = g_masterProcVirtAddr; + } + + // Non-sentinel + else + { + // @todo: + // We (Nick/Patrick/Thi) agree to review the performance cost of + // map/unmap calls for each xscom to determine if it's justified + // to add virtual address as one of the chip's attributes. + // For now, call map/unmap to get virtual address. + // If virtual address attribute is implemented, call the target + // to get it + // Get the virtual addr value of the chip + // l_virtAddr = i_target->getAttr<TARGETING::<ATTR_VIRTUAL_ADDR>(); + + // If virtual addr value is NULL, need to calculate it + if (o_virtAddr == NULL) + { + // Get the target chip info + TARGETING::XscomChipInfo l_xscomChipInfo = {0}; + l_xscomChipInfo = + i_target->getAttr<TARGETING::ATTR_XSCOM_CHIP_INFO>(); + //@todo + // Save the node id of the master chip in a global as well and + // update it. For Rainer systems the node id of the master chip may + // not be 0 if it is on a second node. + + // Is this the master proc? + TARGETING::Target* l_masterProcTarget = NULL; + TARGETING::TargetService& l_targetService = + TARGETING::targetService(); + l_targetService.masterProcChipTargetHandle( l_masterProcTarget ); + + // Use sentinel's virtual address for Master processor + if (l_masterProcTarget == i_target) + { + // There's no way g_masterProcVirtAddr is NULL + // at this point. For safety + assert(g_masterProcVirtAddr != NULL); + o_virtAddr = g_masterProcVirtAddr; + } + + // Calculate virtual address for slave processors + else + { + // Get system XSCOM base address + // Note: can't call TARGETING code prior to PNOR being + // brought up. + TARGETING::TargetService& l_targetService = + TARGETING::targetService(); + TARGETING::Target* l_pTopLevel = NULL; + (void) l_targetService.getTopLevelTarget(l_pTopLevel); + assert(l_pTopLevel != NULL); + XSComBase_t l_systemBaseAddr = + l_pTopLevel->getAttr<TARGETING::ATTR_XSCOM_BASE_ADDRESS>(); + + // Target's XSCOM Base address + l_XSComBaseAddr = l_systemBaseAddr + + ( ( (g_xscomMaxChipsPerNode * l_xscomChipInfo.nodeId) + + l_xscomChipInfo.chipId ) * THIRTYTWO_GB); + + // Target's virtual address + o_virtAddr = static_cast<uint64_t*> + (mmio_dev_map(reinterpret_cast<void*>(l_XSComBaseAddr), + THIRTYTWO_GB)); + } + + // @todo - Save as an attribute if Virtual address attribute + // is implemented, + // Technically there is a race condition here. The mutex is + // a per-hardware thread mutex, not a mutex for the whole XSCOM + // logic. So there is possibility that this same thread is running + // on another thread at the exact same time. We can use atomic + // update instructions here. + // Comment for Nick: This is a good candidate for having a way + // to return a reference to the attribute instead of requiring + // to call setAttr. We currently have no way to SMP-safely update + // this attribute, where as if we had a reference to it we could use + // the atomic update functions (_sync_bool_compare_and_swap in + // this case. + // i_target->setAttr<ATTR_VIRTUAL_ADDR>(l_virtAddr); + } + } + + } while (0); + + return l_err; +} + + /////////////////////////////////////////////////////////////////////////////// /////////////////////////////////////////////////////////////////////////////// errlHndl_t xscomPerformOp(DeviceFW::OperationType i_opType, @@ -224,24 +442,6 @@ errlHndl_t xscomPerformOp(DeviceFW::OperationType i_opType, // Set to buffer len to 0 until successfully access io_buflen = 0; - // Setup the address - - // Init values are for master processor, as PNOR may not - // yet available - XSComBase_t l_XSComBaseAddr = MASTER_PROC_XSCOM_BASE_ADDR; - TARGETING::XscomChipInfo l_xscomChipInfo = {0}; - if (i_target != TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL) - { - l_XSComBaseAddr = - i_target->getAttr<TARGETING::ATTR_XSCOM_BASE_ADDRESS>(); - l_xscomChipInfo = - i_target->getAttr<TARGETING::ATTR_XSCOM_CHIP_INFO>(); - } - - // Build the XSCom address - XSComP8Address l_mmioAddr(l_addr, l_xscomChipInfo.nodeId, - l_xscomChipInfo.chipId, l_XSComBaseAddr); - // Re-init l_retry for loop l_retry = false; @@ -252,23 +452,23 @@ errlHndl_t xscomPerformOp(DeviceFW::OperationType i_opType, l_XSComMutex = mmio_xscom_mutex(); mutex_lock(l_XSComMutex); + // Get the target chip's virtual address uint64_t* l_virtAddr = NULL; - if (i_target == TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL) - { - if (g_masterProcVirtAddr == NULL) - { - g_masterProcVirtAddr = static_cast<uint64_t*> - (mmio_dev_map(reinterpret_cast<void*>(l_XSComBaseAddr), THIRTYTWO_GB)); - } - l_virtAddr = g_masterProcVirtAddr; - } - else + l_err = getTargetVirtualAddress(i_target, l_virtAddr); + if (l_err) { - //@todo - handle slave processors here + // Unlock + mutex_unlock(l_XSComMutex); + // Done, un-pin + task_affinity_unpin(); + break; } + // Build the XSCom address (relative to node 0, chip 0) + XSComP8Address l_mmioAddr(l_addr); + // Get the offset - uint64_t l_offset = l_mmioAddr.offset(l_XSComBaseAddr); + uint64_t l_offset = l_mmioAddr.offset(); // Keep MMIO access until XSCOM successfully done or error uint64_t l_data = 0; @@ -296,6 +496,46 @@ errlHndl_t xscomPerformOp(DeviceFW::OperationType i_opType, } while (l_hmer.mXSComStatus == HMER::XSCOM_BLOCKED); + + // @todo: this block of code is to un-map the slave devices. + // It should be removed if Virtual Addr attribute + // is supported (since we only map it once then cache + // the virtual addr value + if (i_target != TARGETING::MASTER_PROCESSOR_CHIP_TARGET_SENTINEL) + { + TARGETING::Target* l_masterProcTarget = NULL; + TARGETING::TargetService& l_targetService = + TARGETING::targetService(); + l_targetService.masterProcChipTargetHandle( l_masterProcTarget ); + if (l_masterProcTarget != i_target) + { + int rc = 0; + rc = mmio_dev_unmap(reinterpret_cast<void*>(l_virtAddr)); + if (rc != 0) + { + /*@ + * @errortype + * @moduleid XSCOM_PERFORM_OP + * @reasoncode XSCOM_MMIO_UNMAP_ERR + * @userdata1 Return Code + * @userdata2 Unmap address + * @devdesc mmio_dev_unmap() returns error + */ + l_err = new ERRORLOG::ErrlEntry(ERRORLOG::ERRL_SEV_UNRECOVERABLE, + XSCOM_PERFORM_OP, + XSCOM_MMIO_UNMAP_ERR, + rc, + reinterpret_cast<uint64_t>(l_virtAddr)); + + // Unlock + mutex_unlock(l_XSComMutex); + // Done, un-pin + task_affinity_unpin(); + break; + } + } + } + // Unlock mutex_unlock(l_XSComMutex); diff --git a/src/usr/xscom/xscom.H b/src/usr/xscom/xscom.H index 36fb323a3..86773120a 100644 --- a/src/usr/xscom/xscom.H +++ b/src/usr/xscom/xscom.H @@ -164,17 +164,10 @@ class XSComP8Address * @brief Constructor of XSComP8Address class * * @param[in] i_addr PCB address of the register being accessed - * @param[in] i_nodeId Node number where the processor chip resides - * @param[in] i_chipId Chip number of the processor chip that - * holds the register being accessed. - * @param[in] i_mXSComBase Base address of XSCom address range. * * @return None */ - ALWAYS_INLINE inline XSComP8Address(const XSComAddress_t i_addr, - const XSComNodeId_t i_nodeId, - const XSComChipId_t i_chipId, - const XSComBase_t i_mXSComBase); + ALWAYS_INLINE inline XSComP8Address(const XSComAddress_t i_addr); /** * @brief Conversion operator @@ -182,13 +175,11 @@ class XSComP8Address ALWAYS_INLINE inline operator uint64_t() const; /** - * @brief Return the address' 64-bit full offset from the input - * XSCOM base addr + * @brief Return the address' 64-bit full offset * * @return uint64_t */ - ALWAYS_INLINE inline uint64_t offset( - const uint64_t i_xscomBaseAddr) const; + ALWAYS_INLINE inline uint64_t offset(void) const; private: /** @@ -222,14 +213,12 @@ class XSComP8Address //////////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////////// -XSComP8Address::XSComP8Address(const XSComAddress_t i_addr, - const XSComNodeId_t i_nodeId, - const XSComChipId_t i_chipId, - const XSComBase_t i_mXSComBase) -:mMmioAddress(i_mXSComBase) +XSComP8Address::XSComP8Address(const XSComAddress_t i_addr) +:mMmioAddress(0) { - mAddressParts.mNodeId = i_nodeId; - mAddressParts.mChipId = i_chipId; + // Relative address of Node 0, chip 0 + // The chip's nodeId and chip id will be taken into account + // when calculating its XSCOM base address mAddressParts.mSComAddrHi = i_addr >> 4; // Get 27-bits mAddressParts.mSComAddrLo = i_addr; // Get 4 bits } @@ -243,9 +232,9 @@ XSComP8Address::operator uint64_t() const //////////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////////// -uint64_t XSComP8Address::offset(const uint64_t i_xscomBaseAddr) const +uint64_t XSComP8Address::offset(void) const { - return ((mMmioAddress - i_xscomBaseAddr) / sizeof(uint64_t)); + return (mMmioAddress / sizeof(uint64_t)); } }; |