From 0942d9a9553b251173fe1485c3ab6ce8f5204f0a Mon Sep 17 00:00:00 2001
From: Mike Leach <mike.leach@linaro.org>
Date: Wed, 23 Aug 2017 15:58:24 +0100
Subject: [PATCH] opencsd: etmv4: Bugfix - address val exact match packets.

Decoder needs to maintain a stack of 3 broadcast addresses to use
if a packet type called "exact match" is emitted - which has an index
into the stack.

Under certain circumstances this tracking was failing and resulting in
incorrect following address values being processed by the decoder.

Patch moves address match stack into the packet processor to ensure
correct following addresses are used after exact match packets.

Packet classes and some internal API changes & redundant code removal.

Additional updates to packet printing to output exact match in dump.

Signed-off-by: Mike Leach <mike.leach@linaro.org>
---
 decoder/include/etmv4/trc_pkt_decode_etmv4i.h     |   4 -
 decoder/include/etmv4/trc_pkt_elem_etmv4i.h       | 112 ++++++++++++++++++----
 decoder/source/etmv4/trc_etmv4_stack_elem.h       |  41 +-------
 decoder/source/etmv4/trc_pkt_decode_etmv4i.cpp    |  76 ++-------------
 decoder/source/etmv4/trc_pkt_elem_etmv4i.cpp      |   2 +
 decoder/source/etmv4/trc_pkt_proc_etmv4i_impl.cpp |  10 +-
 6 files changed, 110 insertions(+), 135 deletions(-)

diff --git a/decoder/include/etmv4/trc_pkt_decode_etmv4i.h b/decoder/include/etmv4/trc_pkt_decode_etmv4i.h
index 66a0d54..3621405 100644
--- a/decoder/include/etmv4/trc_pkt_decode_etmv4i.h
+++ b/decoder/include/etmv4/trc_pkt_decode_etmv4i.h
@@ -43,7 +43,6 @@
 
 #include <deque>
 
-class AddrValStack;
 class TrcStackElem;
 class TrcStackElemParam;
 class TrcStackElemCtxt;
@@ -100,9 +99,6 @@ private:
     // timestamping
     uint64_t m_timestamp;   // last broadcast global Timestamp.
 
-    // address values stack   
-    AddrValStack *m_pAddrRegs;
-
     // state and context 
     uint32_t m_context_id;              // most recent context ID
     uint32_t m_vmid_id;                 // most recent VMID
diff --git a/decoder/include/etmv4/trc_pkt_elem_etmv4i.h b/decoder/include/etmv4/trc_pkt_elem_etmv4i.h
index 890baf1..e0343c7 100644
--- a/decoder/include/etmv4/trc_pkt_elem_etmv4i.h
+++ b/decoder/include/etmv4/trc_pkt_elem_etmv4i.h
@@ -44,6 +44,55 @@
 @{*/
 
 /*!
+* @class Etmv4PktAddrStack
+* @brief ETMv4 Address packet values stack
+* @ingroup trc_pkts
+*
+*  This class represents a stack of recent broadcast address values - 
+*  used to fulfil the ExactMatch address type where no address is output.
+*
+*/
+class Etmv4PktAddrStack
+{
+public:
+    Etmv4PktAddrStack()
+    {
+        for (int i = 0; i < 3; i++)
+        {
+            m_v_addr[i].pkt_bits = 0;
+            m_v_addr[i].size = VA_64BIT;
+            m_v_addr[i].val = 0;
+            m_v_addr[i].valid_bits = 0;
+            m_v_addr_ISA[i] = 0;
+        }
+    }
+    ~Etmv4PktAddrStack() {};
+
+    void push(const ocsd_pkt_vaddr vaddr, const uint8_t isa)
+    {
+        m_v_addr[2] = m_v_addr[1];
+        m_v_addr[1] = m_v_addr[0];
+        m_v_addr[0] = vaddr;
+        m_v_addr_ISA[2] = m_v_addr_ISA[1];
+        m_v_addr_ISA[1] = m_v_addr_ISA[0];
+        m_v_addr_ISA[0] = isa;
+    }
+
+    void get_idx(const uint8_t idx, ocsd_pkt_vaddr &vaddr, uint8_t &isa)
+    {
+        if (idx < 3)
+        {
+            vaddr = m_v_addr[idx];
+            isa = m_v_addr_ISA[idx];
+        }
+    }
+
+private:
+    ocsd_pkt_vaddr m_v_addr[3];         //!< most recently broadcast address packet
+    uint8_t        m_v_addr_ISA[3];
+};
+
+/*!
  * @class EtmV4ITrcPacket   
  * @brief ETMv4 Instuction Trace Protocol Packet.
  * @ingroup trc_pkts
@@ -95,8 +144,8 @@ public:
 
     void setExceptionInfo(const uint16_t excep_type, const uint8_t addr_interp, const uint8_t m_fault_pending, const uint8_t m_type);
     
-    void set64BitAddress(const uint64_t addr, const uint8_t IS, const uint8_t update_bits);
-    void set32BitAddress(const uint32_t addr, const uint8_t IS, const uint8_t update_bits);
+    void set64BitAddress(const uint64_t addr, const uint8_t IS);
+    void set32BitAddress(const uint32_t addr, const uint8_t IS);
     void updateShortAddress(const uint32_t addr, const uint8_t IS, const uint8_t update_bits);
     void setAddressExactMatch(const uint8_t idx);
 
@@ -131,6 +180,7 @@ public:
     const uint8_t &getAddrMatch() const  { return addr_exact_match_idx; };
     const ocsd_vaddr_t &getAddrVal() const { return v_addr.val; };
     const uint8_t &getAddrIS() const { return v_addr_ISA; };
+    const bool getAddr64Bit() const { return v_addr.size == VA_64BIT; };
 
     // ts
     const uint64_t getTS() const { return pkt_valid.bits.ts_valid ? ts.timestamp : 0; };
@@ -151,6 +201,11 @@ private:
     void atomSeq(std::string &valStr) const;
     void addrMatchIdx(std::string &valStr) const;
     void exceptionInfo(std::string &valStr) const;
+
+    void push_vaddr();
+    void pop_vaddr_idx(const uint8_t idx);
+
+    Etmv4PktAddrStack m_addr_stack;
 };
 
 inline void  EtmV4ITrcPacket::updateErrType(const ocsd_etmv4_i_pkt_type err_pkt_type)
@@ -374,32 +429,35 @@ inline void EtmV4ITrcPacket::setExceptionInfo(const uint16_t excep_type, const u
     exception_info.m_type = m_type;
 }
 
-inline void EtmV4ITrcPacket::set64BitAddress(const uint64_t addr, const uint8_t IS, const uint8_t update_bits)
+inline void EtmV4ITrcPacket::set64BitAddress(const uint64_t addr, const uint8_t IS)
 {
-    uint64_t update_mask = (uint64_t)-1;
-    v_addr.pkt_bits = update_bits;
+    v_addr.pkt_bits = 64;
+    v_addr.valid_bits = 64;
     v_addr.size = VA_64BIT;
-    if(v_addr.valid_bits < update_bits)
-        v_addr.valid_bits = update_bits;
-
-    if(update_bits < 64) 
-    {
-        update_mask = ((uint64_t)1 << update_bits) - 1;
-    }
-    v_addr.val = (v_addr.val & ~update_mask) | (addr & update_mask);
+    v_addr.val = addr;
     v_addr_ISA = IS;
+    push_vaddr();
 }
 
-inline void EtmV4ITrcPacket::set32BitAddress(const uint32_t addr, const uint8_t IS, const uint8_t update_bits)
+inline void EtmV4ITrcPacket::set32BitAddress(const uint32_t addr, const uint8_t IS)
 {
-    ocsd_vaddr_t update_mask = OCSD_BIT_MASK(update_bits);
-    v_addr.pkt_bits = update_bits;
-    v_addr.size = VA_32BIT;
-    if((v_addr.valid_bits < update_bits) || (update_bits == 32) )  // account for dropping into AArch32
-        v_addr.valid_bits = update_bits;
+    uint64_t mask = OCSD_BIT_MASK(32);
+    v_addr.pkt_bits = 32;
 
-    v_addr.val = (v_addr.val & ~update_mask) | (addr & update_mask);
+    if (pkt_valid.bits.context_valid && context.SF)
+        v_addr.size = VA_64BIT;
+    else
+    {
+        v_addr.val &= 0xFFFFFFFF;   // ensure vaddr is only 32 bits if not 64 bit 
+        v_addr.size = VA_32BIT;
+    }
+
+    if (v_addr.valid_bits < 32) // may be 64 bit address so only set 32 if less
+        v_addr.valid_bits = 32;
+
+    v_addr.val = (v_addr.val & ~mask) | (addr & mask);
     v_addr_ISA = IS;
+    push_vaddr();
 }
 
 inline void EtmV4ITrcPacket::updateShortAddress(const uint32_t addr, const uint8_t IS, const uint8_t update_bits)
@@ -411,11 +469,14 @@ inline void EtmV4ITrcPacket::updateShortAddress(const uint32_t addr, const uint8
 
     v_addr.val = (v_addr.val & ~update_mask) | (addr & update_mask);
     v_addr_ISA = IS;
+    push_vaddr();
 }
 
 inline void EtmV4ITrcPacket::setAddressExactMatch(const uint8_t idx)
 {
-    addr_exact_match_idx = idx;    
+    addr_exact_match_idx = idx;   
+    pop_vaddr_idx(idx);
+    push_vaddr();
 }
 
 inline void EtmV4ITrcPacket::setDataSyncMarker(const uint8_t dsm_value)
@@ -442,6 +503,15 @@ inline const bool EtmV4ITrcPacket::isBadPacket() const
     return (type >= ETM4_PKT_I_BAD_SEQUENCE);
 }
 
+inline void  EtmV4ITrcPacket::push_vaddr()
+{
+    m_addr_stack.push(v_addr, v_addr_ISA);
+}
+
+inline void EtmV4ITrcPacket::pop_vaddr_idx(const uint8_t idx)
+{
+    m_addr_stack.get_idx(idx, v_addr, v_addr_ISA);
+}
 
 /** @}*/
 
diff --git a/decoder/source/etmv4/trc_etmv4_stack_elem.h b/decoder/source/etmv4/trc_etmv4_stack_elem.h
index a34d4ef..5335ba8 100644
--- a/decoder/source/etmv4/trc_etmv4_stack_elem.h
+++ b/decoder/source/etmv4/trc_etmv4_stack_elem.h
@@ -35,32 +35,6 @@
 
 #include "etmv4/trc_pkt_types_etmv4.h"
 
-
-
-
-// decoder must maintain stack of last 3 broadcast addresses.
-// used in "same address" packets.
-class AddrValStack {
-public:
-    AddrValStack() {};
-    ~AddrValStack() {};
-
-    void push(const etmv4_addr_val_t &val);
-
-    const etmv4_addr_val_t &get(const int N) const { return m_stack[N & 0x3]; };
-
-private:
-
-    etmv4_addr_val_t m_stack[4];  // only need 3, but 4th allows easy limit checking for N
-};
-
-inline void AddrValStack::push(const etmv4_addr_val_t &val)
-{
-    m_stack[2] = m_stack[1];
-    m_stack[1] = m_stack[0];
-    m_stack[0] = val;
-}
-
 /* ETMv4 I trace stack elements  
     Speculation requires that we stack certain elements till they are committed or 
     cancelled. (P0 elements + other associated parts.)
@@ -124,26 +98,15 @@ public:
     TrcStackElemAddr(const ocsd_etmv4_i_pkt_type root_pkt, const ocsd_trc_index_t root_index);
     virtual ~TrcStackElemAddr() {};
 
-    void setAddr(const etmv4_addr_val_t addr_val, const bool is64bit) { m_addr_val = addr_val; m_64bit = is64bit; };
-    void setAddrMatch(const int idx) { m_is_addr_match = true; m_addr_match_idx = idx; };
-
-    const bool is64bit() const { return m_64bit; };
+    void setAddr(const etmv4_addr_val_t addr_val) { m_addr_val = addr_val; };
     const etmv4_addr_val_t &getAddr() const { return m_addr_val; };
-    const bool isAddrMatch(int &idx) const { idx = m_addr_match_idx; return m_is_addr_match; };
-
 
 private:
     etmv4_addr_val_t m_addr_val;
-    bool m_64bit;
-    bool m_is_addr_match;
-    int  m_addr_match_idx;
 };
 
 inline TrcStackElemAddr::TrcStackElemAddr(const ocsd_etmv4_i_pkt_type root_pkt, const ocsd_trc_index_t root_index) :
-    TrcStackElem(P0_ADDR, false, root_pkt,root_index),
-        m_64bit(false),
-        m_is_addr_match(false),
-        m_addr_match_idx(0)
+    TrcStackElem(P0_ADDR, false, root_pkt,root_index)
 {
     m_addr_val.val = 0;
     m_addr_val.isa = 0;
diff --git a/decoder/source/etmv4/trc_pkt_decode_etmv4i.cpp b/decoder/source/etmv4/trc_pkt_decode_etmv4i.cpp
index 57e8519..bca204c 100644
--- a/decoder/source/etmv4/trc_pkt_decode_etmv4i.cpp
+++ b/decoder/source/etmv4/trc_pkt_decode_etmv4i.cpp
@@ -57,7 +57,6 @@ TrcPktDecodeEtmV4I::TrcPktDecodeEtmV4I(int instIDNum)
 
 TrcPktDecodeEtmV4I::~TrcPktDecodeEtmV4I()   
 {
-    delete m_pAddrRegs;
 }
 
 /*********************** implementation packet decoding interface */
@@ -211,9 +210,6 @@ void TrcPktDecodeEtmV4I::initDecoder()
     m_cond_key_max_incr = 0;
     m_IASize64 = false;
 
-    // set up the broadcast address stack
-    m_pAddrRegs = new (std::nothrow) AddrValStack();
-
     // reset decoder state to unsynced
     resetDecoder();
 }
@@ -241,7 +237,6 @@ void TrcPktDecodeEtmV4I::resetDecoder()
     addr.isa = 0;
     addr.val = 0;
     
-    m_pAddrRegs->push(addr);    // preload first value with 0x0
     m_P0_stack.clear();
     m_output_elem.init();
     m_excep_proc = EXCEP_POP;
@@ -257,7 +252,6 @@ ocsd_datapath_resp_t TrcPktDecodeEtmV4I::decodePacket(bool &Complete)
     Complete = true;
     bool is_addr = false;
     bool is_except = false;
-    bool is_64L = false;
     
     switch(m_curr_packet_in->getType())
     {
@@ -328,7 +322,12 @@ ocsd_datapath_resp_t TrcPktDecodeEtmV4I::decodePacket(bool &Complete)
         TrcStackElemAddr *pElem = new (std::nothrow) TrcStackElemAddr(m_curr_packet_in->getType(), m_index_curr_pkt);
         if(pElem)
         {
-            pElem->setAddrMatch(m_curr_packet_in->getAddrMatch());  // must wait till speculation is resolved before we know the rigth address / index match
+            etmv4_addr_val_t addr;
+
+            // address match - just grab whatever the current value is...
+            addr.val = m_curr_packet_in->getAddrVal();
+            addr.isa = m_curr_packet_in->getAddrIS();
+            pElem->setAddr(addr);
             m_P0_stack.push_front(pElem);
         }
         else
@@ -339,7 +338,6 @@ ocsd_datapath_resp_t TrcPktDecodeEtmV4I::decodePacket(bool &Complete)
 
     case ETM4_PKT_I_ADDR_CTXT_L_64IS0:
     case ETM4_PKT_I_ADDR_CTXT_L_64IS1:
-        is_64L = true;
     case ETM4_PKT_I_ADDR_CTXT_L_32IS0:
     case ETM4_PKT_I_ADDR_CTXT_L_32IS1:    
         {
@@ -356,9 +354,6 @@ ocsd_datapath_resp_t TrcPktDecodeEtmV4I::decodePacket(bool &Complete)
     case ETM4_PKT_I_ADDR_L_32IS1:
     case ETM4_PKT_I_ADDR_L_64IS0:
     case ETM4_PKT_I_ADDR_L_64IS1:
-        if((m_curr_packet_in->getType() == ETM4_PKT_I_ADDR_L_64IS0) ||
-           (m_curr_packet_in->getType() == ETM4_PKT_I_ADDR_L_64IS1))
-           is_64L = true;
     case ETM4_PKT_I_ADDR_S_IS0:
     case ETM4_PKT_I_ADDR_S_IS1:
         {
@@ -369,7 +364,7 @@ ocsd_datapath_resp_t TrcPktDecodeEtmV4I::decodePacket(bool &Complete)
 
                 addr.val = m_curr_packet_in->getAddrVal();
                 addr.isa = m_curr_packet_in->getAddrIS();
-                pElem->setAddr(addr,is_64L);
+                pElem->setAddr(addr);
                 m_P0_stack.push_front(pElem);
             }
             else
@@ -585,39 +580,11 @@ ocsd_datapath_resp_t TrcPktDecodeEtmV4I::commitElements(bool &Complete)
 
             case P0_ADDR:
                 {
-                etmv4_addr_val_t new_addr;
                 TrcStackElemAddr *pAddrElem = dynamic_cast<TrcStackElemAddr *>(pElem);
                 m_return_stack.clear_pop_pending(); // address removes the need to pop the indirect address target from the stack
                 if(pAddrElem)
                 {
-                    int match_idx = 0;
-                    if(pAddrElem->isAddrMatch(match_idx))
-                    {
-                        new_addr.val = m_pAddrRegs->get(match_idx).val;
-                        new_addr.isa = m_pAddrRegs->get(match_idx).isa;
-                        SetInstrInfoInAddrISA(m_pAddrRegs->get(match_idx).val,m_pAddrRegs->get(match_idx).isa);
-                        m_pAddrRegs->push(new_addr);
-                    }
-                    else
-                    {
-                        // if 64 bit value, or IAsize is 32 bit only...
-                        if(!m_IASize64 || pAddrElem->is64bit())
-                        {
-                            // use value immediately
-                            SetInstrInfoInAddrISA(pAddrElem->getAddr().val,pAddrElem->getAddr().isa);
-                            m_pAddrRegs->push(pAddrElem->getAddr());
-                        }
-                        else
-                        {
-                            // otherwise 32 bit values must add in the top 32 from the stack
-                            new_addr = m_pAddrRegs->get(0);
-                            new_addr.val &= ~((ocsd_vaddr_t)0xFFFFFFFF);
-                            new_addr.val |= (pAddrElem->getAddr().val & 0xFFFFFFFF);
-                            new_addr.isa = pAddrElem->getAddr().isa;
-                            SetInstrInfoInAddrISA(new_addr.val,new_addr.isa);
-                            m_pAddrRegs->push(new_addr);
-                        }                                                                                                
-                    }
+                    SetInstrInfoInAddrISA(pAddrElem->getAddr().val, pAddrElem->getAddr().isa);
                     m_need_addr = false;
                 }
                 }
@@ -1000,28 +967,8 @@ ocsd_datapath_resp_t  TrcPktDecodeEtmV4I::processException()
         {
             // extract address
             pAddressElem = static_cast<TrcStackElemAddr *>(pElem);
-            int match_idx;
-            if(pAddressElem->isAddrMatch(match_idx))
-            {
-                m_excep_addr = m_pAddrRegs->get(match_idx);
-            }
-            else
-            {
-                // if 64 bit value, or IAsize is 32 bit only...
-                if(!m_IASize64 || pAddressElem->is64bit())
-                {
-                    // use value immediately
-                    m_excep_addr = pAddressElem->getAddr();                
-                }
-                else
-                {
-                    // otherwise 32 bit values must add in the top 32 from the stack
-                    m_excep_addr = m_pAddrRegs->get(0);
-                    m_excep_addr.val &= ~((ocsd_vaddr_t)0xFFFFFFFF);
-                    m_excep_addr.val |= (pAddressElem->getAddr().val & 0xFFFFFFFF);
-                    m_excep_addr.isa = pAddressElem->getAddr().isa;
-                }                                                                                                
-            }
+
+            m_excep_addr = pAddressElem->getAddr();                
 
             // if we have context, get that.
             if(pCtxtElem)
@@ -1033,9 +980,6 @@ ocsd_datapath_resp_t  TrcPktDecodeEtmV4I::processException()
             // see if there is an implied P0 element on the exception.
             excep_implied_P0 = pExceptElem->getPrevSame();
 
-            // save the address.
-            m_pAddrRegs->push(m_excep_addr);
-
             // save the trace index.
             m_excep_index = pExceptElem->getRootIndex();
 
diff --git a/decoder/source/etmv4/trc_pkt_elem_etmv4i.cpp b/decoder/source/etmv4/trc_pkt_elem_etmv4i.cpp
index 082198e..e848276 100644
--- a/decoder/source/etmv4/trc_pkt_elem_etmv4i.cpp
+++ b/decoder/source/etmv4/trc_pkt_elem_etmv4i.cpp
@@ -127,6 +127,8 @@ void EtmV4ITrcPacket::toString(std::string &str) const
     case ETM4_PKT_I_ADDR_MATCH:
         addrMatchIdx(valStr);
         str += ", " + valStr;
+        trcPrintableElem::getValStr(valStr, (v_addr.size == VA_64BIT) ? 64 : 32, v_addr.valid_bits, v_addr.val, true);
+        str += "; Addr=" + valStr + "; " + ctxtStr;
         break;
 
     case ETM4_PKT_I_ATOM_F1:
diff --git a/decoder/source/etmv4/trc_pkt_proc_etmv4i_impl.cpp b/decoder/source/etmv4/trc_pkt_proc_etmv4i_impl.cpp
index 41d3fbb..868b7a6 100644
--- a/decoder/source/etmv4/trc_pkt_proc_etmv4i_impl.cpp
+++ b/decoder/source/etmv4/trc_pkt_proc_etmv4i_impl.cpp
@@ -896,13 +896,13 @@ void EtmV4IPktProcImpl::iPktAddrCtxt()
             {
                 uint64_t val64;
                 st_idx+=extract64BitLongAddr(m_currPacketData,st_idx,m_addrIS,val64);
-                m_curr_packet.set64BitAddress(val64,m_addrIS,64);
+                m_curr_packet.set64BitAddress(val64,m_addrIS);
             }
             else
             {
                 uint32_t val32;
                 st_idx+=extract32BitLongAddr(m_currPacketData,st_idx,m_addrIS,val32);
-                m_curr_packet.set32BitAddress(val32,m_addrIS,32);
+                m_curr_packet.set32BitAddress(val32,m_addrIS);
             }
             extractAndSetContextInfo(m_currPacketData,st_idx);
             m_process_state = SEND_PKT;
@@ -986,13 +986,13 @@ void EtmV4IPktProcImpl::iPktLongAddr()
         {
             uint64_t val64;
             st_idx+=extract64BitLongAddr(m_currPacketData,st_idx,m_addrIS,val64);
-            m_curr_packet.set64BitAddress(val64,m_addrIS,64);
+            m_curr_packet.set64BitAddress(val64,m_addrIS);
         }
         else
         {
             uint32_t val32;
             st_idx+=extract32BitLongAddr(m_currPacketData,st_idx,m_addrIS,val32);
-            m_curr_packet.set32BitAddress(val32,m_addrIS,32);
+            m_curr_packet.set32BitAddress(val32,m_addrIS);
         }
         m_process_state = SEND_PKT;
     }
@@ -1094,7 +1094,7 @@ void EtmV4IPktProcImpl::iPktQ()
             else
             {
                 idx+=extract32BitLongAddr(m_currPacketData,idx,m_addrIS,q_addr);
-                m_curr_packet.set32BitAddress(q_addr,m_addrIS,32);
+                m_curr_packet.set32BitAddress(q_addr,m_addrIS);
             }
         }
 
-- 
2.12.2.windows.1

