Re: [PATCH v2 01/12] opencsd: Add decode statistics API to packet processor.

Just one comment below, otherwise looks good to me and produces reasonable stats on the files I tested with.

On 9/20/21 11:04 PM, Mike Leach wrote:

Adds a generic statistics API to the packet processor base class to allow protocol specifc decoders to initialise and use to count total bytes processed, unsynced bytes processed, packet header errors and packet sequence errors.

Signed-off-by: Mike Leach mike.leach@linaro.org

decoder/include/common/ocsd_dcd_tree.h | 26 ++++++++++- decoder/include/common/trc_pkt_proc_base.h | 44 ++++++++++++++++++- decoder/include/opencsd/c_api/opencsd_c_api.h | 30 ++++++++++++- decoder/include/opencsd/ocsd_if_types.h | 20 +++++++++ decoder/source/c_api/ocsd_c_api.cpp | 18 +++++++- decoder/source/ocsd_dcd_tree.cpp | 39 ++++++++++++++++ 6 files changed, 171 insertions(+), 6 deletions(-)

diff --git a/decoder/include/common/ocsd_dcd_tree.h b/decoder/include/common/ocsd_dcd_tree.h index e4e74f2..a7297b2 100644 --- a/decoder/include/common/ocsd_dcd_tree.h +++ b/decoder/include/common/ocsd_dcd_tree.h @@ -168,6 +168,30 @@ public: */ ocsd_err_t removeDecoder(const uint8_t CSID);

• /*!
• • Get the stats block for the channel indicated.
• • Caller must check p_stats_block->version to esure that the block
• • is filled in a compatible manner.
• • @param CSID : Configured CoreSight trace ID for the decoder.
• • @param p_stats_block: block pointer to set to reference the stats block.
• • @return ocsd_err_t : Library error code - OCSD_OK if valid block pointer returned,

• •                  OCSD_ERR_NOTINIT if decoder does not support stats counting.
    

• */
• ocsd_err_t getDecoderStats(const uint8_t CSID, ocsd_decode_stats_t **p_stats_block);
• /*!
• • Reset the stats block for the chosens decode channel.
• • stats block is reset independently of the decoder reset to allow counts across
• • multiple decode runs.
• • @param handle : Handle to decode tree.
• • @param CSID : Configured CoreSight trace ID for the decoder.
• • @return ocsd_err_t : Library error code - OCSD_OK if successful.
• */
• ocsd_err_t resetDecoderStats(const uint8_t CSID);

/* get decoder elements currently in use */ @@ -387,7 +411,7 @@ private: void destroyMemAccMapper(); ocsd_err_t initCallbackMemAcc(const ocsd_vaddr_t st_address, const ocsd_vaddr_t en_address, const ocsd_mem_space_acc_t mem_space, void *p_cb_func, bool IDfn, const void *p_context);

• TrcPktProcI *getPktProcI(const uint8_t CSID);

ocsd_dcd_tree_src_t m_dcd_tree_type; diff --git a/decoder/include/common/trc_pkt_proc_base.h b/decoder/include/common/trc_pkt_proc_base.h index 3098a3d..e7c43f5 100644 --- a/decoder/include/common/trc_pkt_proc_base.h +++ b/decoder/include/common/trc_pkt_proc_base.h @@ -43,6 +43,7 @@ #include "trc_component.h" #include "comp_attach_pt_t.h" +#include "opencsd/ocsd_if_version.h" /** @defgroup ocsd_pkt_proc OpenCSD Library : Packet Processors. @brief Classes providing Protocol Packet Processing capability. @@ -76,6 +77,8 @@ public: const uint8_t *pDataBlock, uint32_t *numBytesProcessed) = 0;

• virtual ocsd_err_t getStatsBlock(ocsd_decode_stats_t **pp_stats) = 0;
• virtual void resetStats() = 0;

protected: /* implementation packet processing interface */ @@ -155,6 +158,10 @@ public: //!< Get the configuration for the decoder. virtual const Pc *getProtocolConfig() const { return m_config; }; +/* stats block access - derived class must init stats for the block to be returned. */

• virtual ocsd_err_t getStatsBlock(ocsd_decode_stats_t **pp_stats);
• virtual void resetStats(); /* reset the counts - operates separately from decoder reset. */

protected: /* data output functions */ @@ -183,6 +190,14 @@ protected: const bool checkInit(); // return true if init (configured and at least one output sink attached), false otherwise.

• /* stats block updates - called by derived protocol specific decoder */
• void statsAddTotalCount(const uint64_t count) { m_stats.channel_total += count; };
• void statsAddUnsyncCount(const uint64_t count) { m_stats.channel_unsynced += count; };
• void statsAddBadSeqCount(const uint32_t count) { m_stats.bad_sequence_errs += count; };
• void statsAddBadHdrCount(const uint32_t count) { m_stats.bad_header_errs += count; };
• void statsInit() { m_stats_init = true; }; /* mark stats as in use */

private: /* decode control */ ocsd_datapath_resp_t Reset(const ocsd_trc_index_t index); @@ -195,20 +210,29 @@ private: componentAttachPt<ITrcPktIndexer<Pt>> m_pkt_indexer_i; bool m_b_is_init;

• /* decode statistics block */
• ocsd_decode_stats_t m_stats;
• bool m_stats_init; /*< true if the specific decoder is using the stats */

}; template<class P,class Pt, class Pc> TrcPktProcBase<P, Pt, Pc>::TrcPktProcBase(const char *component_name) : TrcPktProcI(component_name), m_config(0),

• m_b_is_init(false)

• m_b_is_init(false),
• m_stats_init(false)

{

• resetStats();

} template<class P,class Pt, class Pc> TrcPktProcBase<P, Pt, Pc>::TrcPktProcBase(const char *component_name, int instIDNum) : TrcPktProcI(component_name, instIDNum), m_config(0),

• m_b_is_init(false)

• m_b_is_init(false),
• m_stats_init(false)

{

• resetStats();

} template<class P,class Pt, class Pc> TrcPktProcBase<P, Pt, Pc>::~TrcPktProcBase() @@ -405,6 +429,22 @@ template<class P,class Pt, class Pc> const bool TrcPktProcBase<P, Pt, Pc>::check return m_b_is_init; } +template<class P,class Pt, class Pc> ocsd_err_t TrcPktProcBase<P, Pt, Pc>::getStatsBlock(ocsd_decode_stats_t **pp_stats) +{

• *pp_stats = &m_stats;
• return m_stats_init ? OCSD_OK : OCSD_ERR_NOT_INIT;

+}

+template<class P,class Pt, class Pc> void TrcPktProcBase<P, Pt, Pc>::resetStats() +{

• m_stats.version = OCSD_VER_NUM;
• m_stats.channel_total = 0;
• m_stats.channel_unsynced = 0;
• m_stats.bad_header_errs = 0;
• m_stats.bad_sequence_errs = 0;

+}

/** @}*/ #endif // ARM_TRC_PKT_PROC_BASE_H_INCLUDED diff --git a/decoder/include/opencsd/c_api/opencsd_c_api.h b/decoder/include/opencsd/c_api/opencsd_c_api.h index 25e9487..ebbba87 100644 --- a/decoder/include/opencsd/c_api/opencsd_c_api.h +++ b/decoder/include/opencsd/c_api/opencsd_c_api.h @@ -210,10 +210,36 @@ OCSD_C_API ocsd_err_t ocsd_dt_attach_packet_callback( const dcd_tree_handle_t h const void *p_context); +/*!

• • Get the stats block for the channel indicated.
• • Caller must check p_stats_block->version to esure that the block
• • is filled in a compatible manner.
• • @param handle : Handle to decode tree.
• • @param CSID : Configured CoreSight trace ID for the decoder.
• • @param p_stats_block: block pointer to set to reference the stats block.
• • @return ocsd_err_t : Library error code - OCSD_OK if valid block pointer returned,

• •                  OCSD_ERR_NOTINIT if decoder does not support stats counting.
    

• */

+OCSD_C_API ocsd_err_t ocsd_dt_get_decode_stats( const dcd_tree_handle_t handle,

•                                            const unsigned char CSID,
  

•                                            ocsd_decode_stats_t **p_stats_block);
  

+/*!

• • Reset the stats block for the chosens decode channel.
• • stats block is reset independently of the decoder reset to allow counts across
• • multiple decode runs.
• • @param handle : Handle to decode tree.
• • @param CSID : Configured CoreSight trace ID for the decoder.
• • @return ocsd_err_t : Library error code - OCSD_OK if successful.
• */

+OCSD_C_API ocsd_err_t ocsd_dt_reset_decode_stats( const dcd_tree_handle_t handle,

•                                              const unsigned char CSID);
  

/** @}*/ /*---------------------- Memory Access for traced opcodes ----------------------------------------------------------------------------------*/ /** @name Library Memory Accessor configuration on decode tree. diff --git a/decoder/include/opencsd/ocsd_if_types.h b/decoder/include/opencsd/ocsd_if_types.h index 2550f96..df6339b 100644 --- a/decoder/include/opencsd/ocsd_if_types.h +++ b/decoder/include/opencsd/ocsd_if_types.h @@ -633,6 +633,26 @@ typedef struct _ocsd_swt_info { /** @}*/ +/** @name Decode statistics

• Contains statistics for bytes decoded by the packet decoder, if statistics are supported.
• Stats block instantiated in the base class - derived protocol specific decoder must initialise and
• use as required.

+@{*/

+typedef struct _ocsd_decode_stats {

• uint32_t version; /**< library version number - defines decode stat struct format */

I guess the version is there so that we can warn, or error out if the program using this was made for older version. If this is the intent, we would have to update users of this API every time there is a release of OpenCSD. It would probably be better to have a separate versioning that only changes when the stats structure changes.

• uint64_t channel_total; /**< total bytes processed for this channel */
• uint64_t channel_unsynced; /**< number of unsynced bytes processed on this channel */
• uint32_t bad_header_errs; /**< number of bad packet header errors */
• uint32_t bad_sequence_errs; /**< number of bad packet sequence errors */

+} ocsd_decode_stats_t;

+/** @}*/

/** @}*/ #endif // ARM_OCSD_IF_TYPES_H_INCLUDED diff --git a/decoder/source/c_api/ocsd_c_api.cpp b/decoder/source/c_api/ocsd_c_api.cpp index 66bfce8..750c847 100644 --- a/decoder/source/c_api/ocsd_c_api.cpp +++ b/decoder/source/c_api/ocsd_c_api.cpp @@ -234,8 +234,24 @@ OCSD_C_API ocsd_err_t ocsd_dt_attach_packet_callback( const dcd_tree_handle_t h return err; } -/*** Decode tree set element output */ +OCSD_C_API ocsd_err_t ocsd_dt_get_decode_stats(const dcd_tree_handle_t handle,

•                                           const unsigned char CSID,                                               
  

•                                           ocsd_decode_stats_t **p_stats_block)
  

+{

• DecodeTree *pDT = static_cast<DecodeTree *>(handle);
• return pDT->getDecoderStats(CSID, p_stats_block);

+}

+OCSD_C_API ocsd_err_t ocsd_dt_reset_decode_stats(const dcd_tree_handle_t handle,

•                                             const unsigned char CSID)
  

+{

• DecodeTree *pDT = static_cast<DecodeTree *>(handle);

• return pDT->resetDecoderStats(CSID);

+}

+/*** Decode tree set element output */ OCSD_C_API ocsd_err_t ocsd_dt_set_gen_elem_outfn(const dcd_tree_handle_t handle, FnTraceElemIn pFn, const void *p_context) { diff --git a/decoder/source/ocsd_dcd_tree.cpp b/decoder/source/ocsd_dcd_tree.cpp index b8c27a0..88e675c 100644 --- a/decoder/source/ocsd_dcd_tree.cpp +++ b/decoder/source/ocsd_dcd_tree.cpp @@ -486,6 +486,45 @@ ocsd_err_t DecodeTree::removeDecoder(const uint8_t CSID) return err; } +ocsd_err_t DecodeTree::getDecoderStats(const uint8_t CSID, ocsd_decode_stats_t **p_stats_block) +{

• TrcPktProcI *pPktProc = getPktProcI(CSID);
• if (!pPktProc)

•    return OCSD_ERR_INVALID_PARAM_VAL;
  

• return pPktProc->getStatsBlock(p_stats_block);

+}

+ocsd_err_t DecodeTree::resetDecoderStats(const uint8_t CSID) +{

• TrcPktProcI *pPktProc = getPktProcI(CSID);
• if (!pPktProc)

•    return OCSD_ERR_INVALID_PARAM_VAL;
  

• pPktProc->resetStats();
• return OCSD_OK;

+}

+TrcPktProcI *DecodeTree::getPktProcI(const uint8_t CSID) +{

• TrcPktProcI *pPktProc = 0;
• TraceComponent *pComp, *pAssoc;
• DecodeTreeElement *pElem = getDecoderElement(CSID);
• if (pElem)
• {

•    pComp = pElem->getDecoderHandle();
  

•    if (pComp)
  

•    {
  

•        /* if this is a full decoder then the associated component is the packet processor */
  

•        pAssoc = pComp->getAssocComponent();
  

•        if (pAssoc)
  

•            pPktProc = dynamic_cast<TrcPktProcI *>(pAssoc);
  

•        else
  

•            pPktProc = dynamic_cast<TrcPktProcI *>(pComp);
  

•    }
  

• }
• return pPktProc;

+}

DecodeTreeElement * DecodeTree::getDecoderElement(const uint8_t CSID) const { DecodeTreeElement *ret_elem = 0;