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>

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));
 }
 
 };