Hi Mathieu,
Thanks for looking at this, a few comments inline....
Mike
On 12 June 2016 at 18:49, Mathieu Poirier mathieu.poirier@linaro.org wrote:
On 3 June 2016 at 02:26, Mike Leach mike.leach@linaro.org wrote:
Please provide a description of the changes presented this patch.
Signed-off-by: Mike Leach mike.leach@linaro.org
.../ref_trace_decode_lib.vcxproj | 2 + .../ref_trace_decode_lib.vcxproj.filters | 6 + decoder/include/common/ocsd_code_follower.h | 225
+++++++++++++++++++++
decoder/include/common/ocsd_pe_context.h | 4 +- decoder/include/ocsd_if_types.h | 2 + decoder/source/ocsd_code_follower.cpp | 155 ++++++++++++++ 6 files changed, 392 insertions(+), 2 deletions(-) create mode 100644 decoder/include/common/ocsd_code_follower.h create mode 100644 decoder/source/ocsd_code_follower.cpp
diff --git
a/decoder/build/win/ref_trace_decode_lib/ref_trace_decode_lib.vcxproj b/decoder/build/win/ref_trace_decode_lib/ref_trace_decode_lib.vcxproj
index 07aad23..2dc5f0c 100644 --- a/decoder/build/win/ref_trace_decode_lib/ref_trace_decode_lib.vcxproj +++ b/decoder/build/win/ref_trace_decode_lib/ref_trace_decode_lib.vcxproj @@ -304,6 +304,7 @@
<ItemGroup> <ClInclude
Include="......\include\common\comp_attach_notifier_i.h" />
<ClInclude Include="..\..\..\include\common\comp_attach_pt_t.h" />
<ClInclude Include="..\..\..\include\common\ocsd_code_follower.h" /> <ClInclude Include="..\..\..\include\common\ocsd_dcd_tree.h" /> <ClInclude Include="..\..\..\include\common\ocsd_dcd_tree_elem.h" /> <ClInclude Include="..\..\..\include\common\ocsd_error.h" />
@@ -390,6 +391,7 @@ <ClCompile Include="..\..\..\source\mem_acc\trc_mem_acc_cb.cpp" /> <ClCompile Include="..\..\..\source\mem_acc\trc_mem_acc_file.cpp" /> <ClCompile Include="......\source\mem_acc\trc_mem_acc_mapper.cpp"
/>
<ClCompile Include="..\..\..\source\ocsd_code_follower.cpp" /> <ClCompile Include="..\..\..\source\ocsd_dcd_tree.cpp" /> <ClCompile Include="..\..\..\source\ocsd_error.cpp" /> <ClCompile Include="..\..\..\source\ocsd_error_logger.cpp" />
diff --git
a/decoder/build/win/ref_trace_decode_lib/ref_trace_decode_lib.vcxproj.filters b/decoder/build/win/ref_trace_decode_lib/ref_trace_decode_lib.vcxproj.filters
index 34a7737..c221d2f 100644
a/decoder/build/win/ref_trace_decode_lib/ref_trace_decode_lib.vcxproj.filters
+++
b/decoder/build/win/ref_trace_decode_lib/ref_trace_decode_lib.vcxproj.filters
@@ -284,6 +284,9 @@ <ClInclude Include="..\..\..\include\common\ocsd_pe_context.h"> <Filter>Header Files\common</Filter> </ClInclude>
<ClInclude Include="..\..\..\include\common\ocsd_code_follower.h">
<Filter>Header Files\common</Filter></ClInclude> </ItemGroup> <ItemGroup> <ClCompile Include="..\..\..\source\trc_component.cpp">
@@ -388,5 +391,8 @@ <ClCompile Include="..\..\..\source\ocsd_version.cpp"> <Filter>Source Files</Filter> </ClCompile>
<ClCompile Include="..\..\..\source\ocsd_code_follower.cpp">
<Filter>Source Files</Filter></ClCompile> </ItemGroup>
</Project> \ No newline at end of file diff --git a/decoder/include/common/ocsd_code_follower.h
b/decoder/include/common/ocsd_code_follower.h
new file mode 100644 index 0000000..f05ad5b --- /dev/null +++ b/decoder/include/common/ocsd_code_follower.h @@ -0,0 +1,225 @@ +/*
- \file ocsd_code_follower.h
- \brief OpenCSD : Code follower for instruction trace decode
- \copyright Copyright (c) 2016, ARM Limited. All Rights Reserved.
- */
+/*
- Redistribution and use in source and binary forms, with or without
modification,
- are permitted provided that the following conditions are met:
- Redistributions of source code must retain the above copyright
notice,
- this list of conditions and the following disclaimer.
- Redistributions in binary form must reproduce the above copyright
notice,
- this list of conditions and the following disclaimer in the
documentation
- and/or other materials provided with the distribution.
- Neither the name of the copyright holder nor the names of its
contributors
- may be used to endorse or promote products derived from this
software without
- specific prior written permission.
- THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
'AS IS' AND
- ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
THE IMPLIED
- WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
ARE DISCLAIMED.
- IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR
ANY DIRECT,
- INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
- (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
SERVICES;
- LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
CAUSED AND
- ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
OR TORT
- (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
USE OF THIS
- SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
+#ifndef ARM_OCSD_CODE_FOLLOWER_H_INCLUDED +#define ARM_OCSD_CODE_FOLLOWER_H_INCLUDED
+#include "ocsd_if_types.h" +#include "trc_pkt_types.h" +#include "comp_attach_pt_t.h" +#include "interfaces/trc_tgt_mem_access_i.h" +#include "interfaces/trc_instr_decode_i.h"
+/*!
- @class OcsdCodeFollower
- @brief The code follower looks for waypoints or addresses.
- Code follower used to determine the trace ranges for Atom or other
waypoint
- elements. Uses memory accessor and I decoder to follow the code
path.
- */
+class OcsdCodeFollower +{ +public:
- OcsdCodeFollower();
- ~OcsdCodeFollower();
+//*********** setup API
- void initInterfaces(componentAttachPt<ITargetMemAccess>
*pMemAccess, componentAttachPt<IInstrDecode> *pIDecode);
+// set information for decode operation - static or occasionally
changing settings
+// per decode values are passed as parameters into the decode API calls.
- void setCoreProfile(const ocsd_arch_profile_t profile);
//!< core profile
- void setMemSpaceAccess(const ocsd_mem_space_acc_t mem_acc_rule);
//!< memory space to use for access (filtered by S/NS, EL etc).
- void setMemSpaceCSID(const uint8_t csid);
//!< memory spaces might be partitioned by CSID
- void setISA(const ocsd_isa isa); //!< set the ISA for the decode.
- void setDSBDMBasWP(); //!< DSB and DMB can be treated as WP in
some archs.
+//********** code following API
- // standard WP search - for program flow trace
- //ocsd_err_t followToAtomWP(idec_res_t &op_result, const
ocsd_vaddr_t addrStart, const ocsd_atm_val A);
- // PTM exception code may require follow to an address
- //ocsd_err_t followToAddress(idec_res_t &op_result, const
ocsd_vaddr_t addrStart, const ocsd_atm_val A, const ocsd_vaddr_t addrMatch);
- // single instruction atom format such as ETMv3
- ocsd_err_t followSingleAtom(const ocsd_vaddr_t addrStart, const
ocsd_atm_val A);
- // follow N instructions
- // ocsd_err_t followNInstructions(idec_res_t &op_result) // ETMv4 Q
elements
+//*********************** results API
- const ocsd_vaddr_t getRangeSt() const; //!< inclusive start
address of decoded range (value passed in)
- const ocsd_vaddr_t getRangeEn() const; //!< exclusive end address
of decoded range (first instruction _not_ executed / potential next instruction).
- const bool hasNextAddr() const; //!< we have calulated the
next address - otherwise this is needed from trace packets.
- const ocsd_vaddr_t getNextAddr() const; //!< next address - valid
if hasNextAddr() true.
- // information on last instruction executed in range.
- const ocsd_instr_type getInstrType() const; //!< last
instruction type
- const ocsd_instr_subtype getInstrSubType() const; //!< last
instruction sub-type
- const bool isCondInstr() const; //!< is a
conditional instruction
- const bool isLink() const; //!< is a link
(branch with link etc)
- const bool ISAChanged() const; //!< next ISA
different from input ISA.
- const ocsd_isa nextISA() const; //!< ISA for
next instruction
- // information on error conditions
- const bool isNacc() const; //!< true if Memory Not
Accessible (nacc) error occurred
- void clearNacc(); //!< clear the nacc
error flag
- const ocsd_vaddr_t getNaccAddr() const; //!< get the nacc error
address.
+private:
- bool initFollowerState(); //!< clear all the o/p data and
flags, check init valid.
- ocsd_err_t decodeSingleOpCode(); //!< decode single opcode
address from current m_inst_info packet
- ocsd_instr_info m_instr_info;
- ocsd_vaddr_t m_st_range_addr; //!< start of excuted range -
inclusive address.
- ocsd_vaddr_t m_en_range_addr; //!< end of executed range -
exclusive address.
- ocsd_vaddr_t m_next_addr; //!< calcuated next address (could
be eo range of branch address, not set for indirect branches)
- bool m_b_next_valid; //!< true if next address valid,
false if need address from trace packets.
- //! memory space rule to use when accessing memory.
- ocsd_mem_space_acc_t m_mem_acc_rule;
- //! memory space csid to use when accessing memory.
- uint8_t m_mem_space_csid;
- ocsd_vaddr_t m_nacc_address; //!< memory address that was
inaccessible - failed read @ start, or during follow operation
- bool m_b_nacc_err; //!< memory NACC error - required
address was unavailable.
- //! pointers to the memory access and i decode interfaces.
- componentAttachPt<ITargetMemAccess> *m_pMemAccess;
- componentAttachPt<IInstrDecode> *m_pIDecode;
+};
+#endif // ARM_OCSD_CODE_FOLLOWER_H_INCLUDED
+//*********** setup API +inline void OcsdCodeFollower::setCoreProfile(const ocsd_arch_profile_t
profile)
+{
- m_instr_info.pe_type = profile;
+}
+inline void OcsdCodeFollower::setMemSpaceAccess(const
ocsd_mem_space_acc_t mem_acc_rule)
+{
- m_mem_acc_rule = mem_acc_rule;
+}
+inline void OcsdCodeFollower::setMemSpaceCSID(const uint8_t csid) +{
- m_mem_space_csid = csid;
+}
+inline void OcsdCodeFollower::setISA(const ocsd_isa isa) +{
- m_instr_info.isa = isa;
+}
+inline void OcsdCodeFollower::setDSBDMBasWP() +{
- m_instr_info.dsb_dmb_waypoints = 1;
+}
+//**************************************** results API +inline const ocsd_vaddr_t OcsdCodeFollower::getRangeSt() const +{
- return m_st_range_addr;
+}
+inline const ocsd_vaddr_t OcsdCodeFollower::getRangeEn() const +{
- return m_en_range_addr;
+}
+inline const bool OcsdCodeFollower::hasNextAddr() const +{
- return m_b_next_valid;
+}
+inline const ocsd_vaddr_t OcsdCodeFollower::getNextAddr() const +{
- return m_next_addr;
+}
+// information on last instruction executed in range. +inline const ocsd_instr_type OcsdCodeFollower::getInstrType() const +{
- return m_instr_info.type;
+}
+inline const ocsd_instr_subtype OcsdCodeFollower::getInstrSubType()
const
+{
- return m_instr_info.sub_type;
+}
+inline const bool OcsdCodeFollower::isCondInstr() const +{
- return (bool)(m_instr_info.is_conditional == 1);
+}
+inline const bool OcsdCodeFollower::isLink() const +{
- return (bool)(m_instr_info.is_link == 1);
+}
+inline const bool OcsdCodeFollower::ISAChanged() const +{
- return (bool)(m_instr_info.isa != m_instr_info.next_isa);
+}
+inline const ocsd_isa OcsdCodeFollower::nextISA() const +{
- return m_instr_info.next_isa;
+}
+// information on error conditions +inline const bool OcsdCodeFollower::isNacc() const +{
- return m_b_nacc_err;
+}
+inline void OcsdCodeFollower::clearNacc() +{
- m_b_nacc_err = false;
+}
+inline const ocsd_vaddr_t OcsdCodeFollower::getNaccAddr() const +{
- return m_nacc_address;
+}
+/* End of File ocsd_code_follower.h */ diff --git a/decoder/include/common/ocsd_pe_context.h
b/decoder/include/common/ocsd_pe_context.h
index 37764d9..fe06e90 100644 --- a/decoder/include/common/ocsd_pe_context.h +++ b/decoder/include/common/ocsd_pe_context.h @@ -51,7 +51,7 @@ public: void resetCtxt();
void setSecLevel(const ocsd_sec_level sl) {m_context.security_level = sl; };
- void setEL(const ocsd_ex_level el) { m_context.exception_level =
el; m_context.el_valid = 1; };
- void setEL(const ocsd_ex_level el) { m_context.exception_level =
el; m_context.el_valid = el > ocsd_EL_unknown ? 1 : 0; };
void setCtxtID(const uint32_t id) { m_context.context_id = id;m_context.ctxt_id_valid = 1; };
void setVMID(const uint32_t id) { m_context.vmid = id;m_context.vmid_valid = 1; };
void set64bit(const bool is64bit) { m_context.bits64 = is64bit ? 1: 0; };
@@ -83,7 +83,7 @@ inline void OcsdPeContext::resetCtxt() m_context.context_id = 0; m_context.ctxt_id_valid = 0; m_context.el_valid = 0;
- m_context.exception_level = ocsd_EL0;
- m_context.exception_level = ocsd_EL_unknown; m_context.security_level = ocsd_sec_secure; m_context.vmid = 0; m_context.vmid_valid = 0;
diff --git a/decoder/include/ocsd_if_types.h
b/decoder/include/ocsd_if_types.h
index 3fa3032..9e4b051 100644 --- a/decoder/include/ocsd_if_types.h +++ b/decoder/include/ocsd_if_types.h @@ -110,6 +110,7 @@ typedef enum _ocsd_err_t { OCSD_ERR_UNSUPP_DECODE_PKT, /**< Packet not supported in
decoder */
OCSD_ERR_BAD_DECODE_PKT, /**< reserved or unknown packetin decoder. */
OCSD_ERR_COMMIT_PKT_OVERRUN, /**< overrun in commit packetstack - tried to commit more than available */
- OCSD_ERR_MEM_NACC, /**< unable to access required
memory address */
/* decode tree errors */ OCSD_ERR_DCDT_NO_FORMATTER, /**< No formatter in use -operation not valid. */
/* target memory access errors */@@ -330,6 +331,7 @@ typedef enum _ocsd_sec_level */ typedef enum _ocsd_ex_level {
- ocsd_EL_unknown = -1, /**< EL unknown / unsupported in trace */
Can I suggest ocsd_ELERR ?
EL unknown is most appropriate here - this is a genuine unknown, not an error situation.
ETMv4 traces ELs explicitly - EL will be unknown until we see the first context packet. ETMv3.5 traces hyp bit - EL will be EL2 if hyp bit active, unknown otherwise. ETMv3.4 and below, PTM - no explicit EL tracing - EL unknown all the time.
ocsd_EL0 = 0, /**< EL0 */ ocsd_EL1, /**< EL1 */ ocsd_EL2, /**< EL2 */diff --git a/decoder/source/ocsd_code_follower.cpp
b/decoder/source/ocsd_code_follower.cpp
new file mode 100644 index 0000000..b52704e --- /dev/null +++ b/decoder/source/ocsd_code_follower.cpp @@ -0,0 +1,155 @@ +/*
- \file ocsd_code_follower.cpp
- \brief OpenCSD : Instruction Code path follower.
- \copyright Copyright (c) 2016, ARM Limited. All Rights Reserved.
- */
+/*
- Redistribution and use in source and binary forms, with or without
modification,
- are permitted provided that the following conditions are met:
- Redistributions of source code must retain the above copyright
notice,
- this list of conditions and the following disclaimer.
- Redistributions in binary form must reproduce the above copyright
notice,
- this list of conditions and the following disclaimer in the
documentation
- and/or other materials provided with the distribution.
- Neither the name of the copyright holder nor the names of its
contributors
- may be used to endorse or promote products derived from this
software without
- specific prior written permission.
- THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
'AS IS' AND
- ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
THE IMPLIED
- WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
ARE DISCLAIMED.
- IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR
ANY DIRECT,
- INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
- (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
SERVICES;
- LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
CAUSED AND
- ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
OR TORT
- (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
USE OF THIS
- SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
+#include "common/ocsd_code_follower.h"
+OcsdCodeFollower::OcsdCodeFollower() +{
- m_instr_info.pe_type.arch = ARCH_UNKNOWN;
- m_instr_info.pe_type.profile = profile_Unknown;
- m_instr_info.isa = ocsd_isa_unknown;
- m_instr_info.dsb_dmb_waypoints = 0;
- m_instr_info.instr_addr = 0;
- m_instr_info.opcode = 0;
- m_pMemAccess = 0;
- m_pIDecode = 0;
- m_mem_space_csid = 0;
- m_st_range_addr = m_en_range_addr = m_next_addr = 0;
- m_b_next_valid = false;
- m_b_nacc_err = false;
+}
+OcsdCodeFollower::~OcsdCodeFollower() +{ +}
+void
OcsdCodeFollower::initInterfaces(componentAttachPt<ITargetMemAccess> *pMemAccess, componentAttachPt<IInstrDecode> *pIDecode)
+{
- m_pMemAccess = pMemAccess;
- m_pIDecode = pIDecode;
+}
+bool OcsdCodeFollower::initFollowerState() +{
- bool initDone = false;
- // reset per follow flags
- m_b_next_valid = false;
- m_b_nacc_err = false;
- // set range addresses
- m_en_range_addr = m_next_addr = m_st_range_addr;
+// check initialisation is valid.
- // must have attached memory access and i-decode objects
- if(m_pMemAccess && m_pIDecode)
- {
initDone = (m_pMemAccess->hasAttachedAndEnabled() &&m_pIDecode->hasAttachedAndEnabled());
- }
- return initDone;
+}
+/*!
- Decodes an instruction at a single location, calculates the next
address
- if possible according to the instruction type and atom.
- @param addrStart : Address of the instruction
- @param A : Atom value - E or N
- @return ocsd_err_t : OCSD_OK - decode correct, check flags for next
address
: OCSD_ERR_MEM_NACC - unable to access memoryarea @ address - need new address in trace packet stream.
: OCSD_ERR_NOT_INIT - not initialised - fatal.
: OCSD_<other> - other error occured - fatal.- */
+ocsd_err_t OcsdCodeFollower::followSingleAtom(const ocsd_vaddr_t
addrStart, const ocsd_atm_val A)
+{
- ocsd_err_t err = OCSD_ERR_NOT_INIT;
- if(initFollowerState())
if (!initFollowerState) return err;
- {
m_st_range_addr = m_instr_info.instr_addr = addrStart;err = decodeSingleOpCode();if(err == OCSD_OK)if (err != OCSD_OK) return err;
That way you avoid the imbrication.
{// set end range - always after the instruction executed.m_en_range_addr = m_instr_info.instr_addr +m_instr_info.instr_size;
// assume next addr is the instruction afterm_next_addr = m_en_range_addr;m_b_next_valid = true;// case when next address is differentswitch(m_instr_info.type){case OCSD_INSTR_BR:if(A == ATOM_E) // executed the direct branchm_next_addr = m_instr_info.branch_addr;break;case OCSD_INSTR_BR_INDIRECT:if(A == ATOM_E) // executed indirect branchm_b_next_valid = false;break;}}- }
- return err;
+}
+ocsd_err_t OcsdCodeFollower::decodeSingleOpCode() +{
- ocsd_err_t err = OCSD_OK;
- uint32_t bytesReq = 4;
This initialisation isn't necessary.
It really rather is - it is the buffer size passed into ReadTargetMemory(). A comment will be added though
- uint32_t opcode;
- err =
m_pMemAccess->first()->ReadTargetMemory(m_instr_info.instr_addr,m_mem_space_csid,m_mem_acc_rule,&bytesReq,(uint8_t *)&opcode);
- if(err == OCSD_OK)
- {
if(bytesReq == 4) // got memory.Please explain the hard coded value.
Check on number of bytes returned by memory access - will add comment
{m_instr_info.opcode = opcode;err = m_pIDecode->first()->DecodeInstruction(&m_instr_info);}else // memory unavailable.{m_b_nacc_err = true;m_nacc_address = m_instr_info.instr_addr;err = OCSD_ERR_MEM_NACC;}- }
- return err;
+}
+/* End of File ocsd_code_follower.cpp */
1.9.1
CoreSight mailing list CoreSight@lists.linaro.org https://lists.linaro.org/mailman/listinfo/coresight