kfdtest: Re-factor dynamic packet allocation into BasePacket

Avoids unnecessary and error-prone duplication of dynamic packet
allocation in many packet classes.

Change-Id: Icec981ae2cd7a3d88cdf9213298940d85627f853
Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>

tests/kfdtest/src/BasePacket.cpp

@@ -25,10 +25,15 @@
 #include "KFDTestUtil.hpp"
 #include "KFDBaseComponentTest.hpp"
 
-BasePacket::BasePacket(void) {
+BasePacket::BasePacket(void): m_packetAllocation(NULL) {
     m_FamilyId = g_baseTest->GetFamilyIdFromDefaultNode();
 }
 
+BasePacket::~BasePacket(void) {
+    if (m_packetAllocation)
+        free(m_packetAllocation);
+}
+
 void BasePacket::Dump() const {
     unsigned int size = SizeInDWords();
     const HSAuint32 *packet = (const HSAuint32 *)GetPacket();
@@ -40,3 +45,16 @@ void BasePacket::Dump() const {
         log << " " << std::setw(8) << std::setfill('0') << packet[i];
     log << std::endl;
 }
+
+void *BasePacket::AllocPacket(void) {
+    unsigned int size = SizeInBytes();
+
+    EXPECT_NE(0, size);
+    if (!size)
+        return NULL;
+
+    m_packetAllocation = calloc(1, size);
+    EXPECT_NOTNULL(m_packetAllocation);
+
+    return m_packetAllocation;
+}

tests/kfdtest/src/BasePacket.hpp

@@ -38,7 +38,7 @@ enum PACKETTYPE {
 class BasePacket {
  public:
     BasePacket(void);
-    virtual ~BasePacket(void) {}
+    virtual ~BasePacket(void);
 
     // @returns Packet type
     virtual PACKETTYPE PacketType() const = 0;
@@ -52,7 +52,10 @@ class BasePacket {
     void Dump() const;
 
  protected:
-     unsigned int m_FamilyId;
+    unsigned int m_FamilyId;
+    void *m_packetAllocation;
+
+    void *AllocPacket(void);
 };
 
 #endif  // __KFD_BASE_PACKET__H__

tests/kfdtest/src/PM4Packet.cpp

@@ -43,19 +43,12 @@ void PM4Packet::InitPM4Header(PM4_TYPE_3_HEADER &header, it_opcode_type opCode)
     header.reserved1             = 0;
 }
 
-PM4WriteDataPacket::~PM4WriteDataPacket(void) {
-    if (m_pPacketData)
-        free(m_pPacketData);
-}
-
 unsigned int PM4WriteDataPacket::SizeInBytes() const {
     return (offsetof(PM4WRITE_DATA_CI, data) + m_ndw*sizeof(uint32_t));
 }
 
 void PM4WriteDataPacket::InitPacket(unsigned int *destBuf, void *data) {
-    m_pPacketData = reinterpret_cast<PM4WRITE_DATA_CI *>(calloc(1, SizeInBytes()));
-    // Verify that the memory is allocated successfully, cannot use assert here
-    EXPECT_NOTNULL(m_pPacketData);
+    m_pPacketData = reinterpret_cast<PM4WRITE_DATA_CI *>(AllocPacket());
 
     InitPM4Header(m_pPacketData->header, IT_WRITE_DATA);
 
@@ -90,9 +83,8 @@ void PM4ReleaseMemoryPacket::InitPacketCI(bool isPolling, uint64_t address,
     PM4_RELEASE_MEM_CI *pkt;
 
     m_packetSize = sizeof(PM4_RELEASE_MEM_CI);
-    pkt = reinterpret_cast<PM4_RELEASE_MEM_CI *>(calloc(1, m_packetSize));
+    pkt = reinterpret_cast<PM4_RELEASE_MEM_CI *>(AllocPacket());
     m_pPacketData = pkt;
-    EXPECT_NOTNULL(m_pPacketData);
 
     InitPM4Header(pkt->header, IT_RELEASE_MEM);
 
@@ -155,9 +147,8 @@ void PM4ReleaseMemoryPacket::InitPacketAI(bool isPolling, uint64_t address,
     PM4MEC_RELEASE_MEM_AI *pkt;
 
     m_packetSize = sizeof(PM4MEC_RELEASE_MEM_AI);
-    pkt = reinterpret_cast<PM4MEC_RELEASE_MEM_AI *>(calloc(1, m_packetSize));
+    pkt = reinterpret_cast<PM4MEC_RELEASE_MEM_AI *>(AllocPacket());
     m_pPacketData = pkt;
-    EXPECT_NOTNULL(m_pPacketData);
 
     InitPM4Header(pkt->header, IT_RELEASE_MEM);
 
@@ -201,9 +192,8 @@ void PM4ReleaseMemoryPacket::InitPacketNV(bool isPolling, uint64_t address,
     PM4MEC_RELEASE_MEM_NV *pkt;
 
     m_packetSize = sizeof(PM4_MEC_RELEASE_MEM_NV);
-    pkt = reinterpret_cast<PM4_MEC_RELEASE_MEM_NV *>(calloc(1, m_packetSize));
+    pkt = reinterpret_cast<PM4_MEC_RELEASE_MEM_NV *>(AllocPacket());
     m_pPacketData = pkt;
-    EXPECT_NOTNULL(m_pPacketData);
 
     InitPM4Header(pkt->header, IT_RELEASE_MEM);
 
@@ -277,9 +267,8 @@ void PM4AcquireMemoryPacket::InitPacketAI(void) {
 
     PM4ACQUIRE_MEM *pkt;
     m_packetSize = sizeof(PM4ACQUIRE_MEM);
-    pkt = reinterpret_cast<PM4ACQUIRE_MEM*>(calloc(1, m_packetSize));
+    pkt = reinterpret_cast<PM4ACQUIRE_MEM*>(AllocPacket());
     m_pPacketData = pkt;
-    EXPECT_NOTNULL(m_pPacketData);
 
     InitPM4Header(pkt->header,  IT_ACQUIRE_MEM);
     pkt->bitfields2.coher_cntl     = 0x28c00000;  // copied from the way the HSART does this.
@@ -293,9 +282,9 @@ void PM4AcquireMemoryPacket::InitPacketAI(void) {
 void PM4AcquireMemoryPacket::InitPacketNV(void) {
     PM4ACQUIRE_MEM_NV *pkt;
     m_packetSize = sizeof(PM4ACQUIRE_MEM_NV);
-    pkt = reinterpret_cast<PM4ACQUIRE_MEM_NV*>(calloc(1, m_packetSize));
+    pkt = reinterpret_cast<PM4ACQUIRE_MEM_NV*>(AllocPacket());
     m_pPacketData = pkt;
-    EXPECT_NOTNULL(m_pPacketData);
+
     InitPM4Header(pkt->header,  IT_ACQUIRE_MEM);
     pkt->coher_size                = 0xFFFFFFFF;
     pkt->bitfields3.coher_size_hi  = 0;
@@ -309,21 +298,14 @@ void PM4AcquireMemoryPacket::InitPacketNV(void) {
     pkt->bitfields6.gcr_cntl = (1<<14|1<<9|1<<8|1<<7|1);
 }
 
-PM4SetShaderRegPacket::PM4SetShaderRegPacket(void)
-    : m_packetDataAllocated(false) {
+PM4SetShaderRegPacket::PM4SetShaderRegPacket(void) {
 }
 
 PM4SetShaderRegPacket::PM4SetShaderRegPacket(unsigned int baseOffset, const unsigned int regValues[],
-                                             unsigned int numRegs)
-    : m_packetDataAllocated(false) {
+                                             unsigned int numRegs) {
     InitPacket(baseOffset, regValues, numRegs);
 }
 
-PM4SetShaderRegPacket::~PM4SetShaderRegPacket(void) {
-    if (m_packetDataAllocated)
-        free(m_pPacketData);
-}
-
 void PM4SetShaderRegPacket::InitPacket(unsigned int baseOffset, const unsigned int regValues[],
                                        unsigned int numRegs) {
     // 1st register is a part of the packet struct.
@@ -332,11 +314,7 @@ void PM4SetShaderRegPacket::InitPacket(unsigned int baseOffset, const unsigned i
     /* Allocating the size of the packet, since the packet is assembled from a struct
      * followed by an additional dword data
      */
-    m_pPacketData = reinterpret_cast<PM4SET_SH_REG *>(malloc(m_packetSize));
-
-    EXPECT_NOTNULL(m_pPacketData);
-
-    m_packetDataAllocated = true;
+    m_pPacketData = reinterpret_cast<PM4SET_SH_REG *>(AllocPacket());
 
     memset(m_pPacketData, 0, m_packetSize);
 

tests/kfdtest/src/PM4Packet.hpp

@@ -59,7 +59,7 @@ class PM4WriteDataPacket : public PM4Packet {
         InitPacket(destBuf, data);
     }
 
-    virtual ~PM4WriteDataPacket(void);
+    virtual ~PM4WriteDataPacket(void) {}
     // @returns Packet size in bytes
     virtual unsigned int SizeInBytes() const;
     // @returns Pointer to the packet
@@ -91,7 +91,7 @@ class PM4ReleaseMemoryPacket : public PM4Packet {
     PM4ReleaseMemoryPacket(unsigned int familyId, bool isPolling, uint64_t address, uint64_t data,
                            bool is64bit = false, bool isTimeStamp = false);
 
-    virtual ~PM4ReleaseMemoryPacket(void) {if (m_pPacketData)free(m_pPacketData);}
+    virtual ~PM4ReleaseMemoryPacket(void) {}
     // @returns Packet size in bytes
     virtual unsigned int SizeInBytes() const { return m_packetSize; }
     // @returns Pointer to the packet
@@ -135,7 +135,7 @@ class PM4IndirectBufPacket : public PM4Packet {
 class PM4AcquireMemoryPacket : public PM4Packet {
  public:
     PM4AcquireMemoryPacket(unsigned int familyId);
-    virtual ~PM4AcquireMemoryPacket(void) {if (m_pPacketData)free(m_pPacketData);}
+    virtual ~PM4AcquireMemoryPacket(void) {}
 
     // @returns the packet size in bytes
     virtual unsigned int SizeInBytes() const { return m_packetSize; }
@@ -156,7 +156,7 @@ class PM4SetShaderRegPacket : public PM4Packet {
 
     PM4SetShaderRegPacket(unsigned int baseOffset, const unsigned int regValues[], unsigned int numRegs);
 
-    virtual ~PM4SetShaderRegPacket(void);
+    virtual ~PM4SetShaderRegPacket(void) {}
 
     // @returns Packet size in bytes
     virtual unsigned int SizeInBytes() const { return m_packetSize; }
@@ -167,7 +167,6 @@ class PM4SetShaderRegPacket : public PM4Packet {
 
  private:
     unsigned int m_packetSize;
-    bool m_packetDataAllocated;
     // PM4SET_SH_REG struct contains all the packet's data
     PM4SET_SH_REG  *m_pPacketData;
 };

tests/kfdtest/src/SDMAPacket.cpp

@@ -45,16 +45,11 @@ SDMAWriteDataPacket::SDMAWriteDataPacket(unsigned int familyId, void* destAddr,
     InitPacket(destAddr, ndw, data);
 }
 
-SDMAWriteDataPacket::~SDMAWriteDataPacket(void) {
-    if (packetData)
-        free(packetData);
-}
-
 void SDMAWriteDataPacket::InitPacket(void* destAddr, unsigned int ndw,
                                      void *data) {
     packetSize = sizeof(SDMA_PKT_WRITE_UNTILED) +
         (ndw - 1) * sizeof(unsigned int);
-    packetData = reinterpret_cast<SDMA_PKT_WRITE_UNTILED *>(calloc(1, packetSize));
+    packetData = reinterpret_cast<SDMA_PKT_WRITE_UNTILED *>(AllocPacket());
 
     packetData->HEADER_UNION.op = SDMA_OP_WRITE;
     packetData->HEADER_UNION.sub_op = SDMA_SUBOP_WRITE_LINEAR;
@@ -69,10 +64,6 @@ void SDMAWriteDataPacket::InitPacket(void* destAddr, unsigned int ndw,
 
 #define BITS (21)
 #define TWO_MEG (1 << BITS)
-SDMACopyDataPacket::~SDMACopyDataPacket(void) {
-    free(packetData);
-}
-
 SDMACopyDataPacket::SDMACopyDataPacket(unsigned int familyId,
                         void *const dsts[], void *src, int n, unsigned int surfsize) {
     int32_t size = 0, i;
@@ -88,7 +79,7 @@ SDMACopyDataPacket::SDMACopyDataPacket(unsigned int familyId,
 
     packetSize = ((surfsize + TWO_MEG - 1) >> BITS) * singlePacketSize;
 
-    SDMA_PKT_COPY_LINEAR *pSDMA = reinterpret_cast<SDMA_PKT_COPY_LINEAR *>(malloc(packetSize));
+    SDMA_PKT_COPY_LINEAR *pSDMA = reinterpret_cast<SDMA_PKT_COPY_LINEAR *>(AllocPacket());
     packetData = pSDMA;
 
     while (surfsize > 0) {
@@ -125,10 +116,6 @@ SDMACopyDataPacket::SDMACopyDataPacket(unsigned int familyId, void* dst, void *s
     new (this)SDMACopyDataPacket(familyId, &dst, src, 1, surfsize);
 }
 
-SDMAFillDataPacket::~SDMAFillDataPacket() {
-    free(m_PacketData);
-}
-
 SDMAFillDataPacket::SDMAFillDataPacket(unsigned int familyId, void *dst, unsigned int data, unsigned int size) {
     unsigned int copy_size;
     SDMA_PKT_CONSTANT_FILL *pSDMA;
@@ -136,7 +123,7 @@ SDMAFillDataPacket::SDMAFillDataPacket(unsigned int familyId, void *dst, unsigne
     m_FamilyId = familyId;
     /* SDMA support maximum 0x3fffe0 byte in one copy. Use 2M copy_size */
     m_PacketSize = ((size + TWO_MEG - 1) >> BITS) * sizeof(SDMA_PKT_CONSTANT_FILL);
-    pSDMA = reinterpret_cast<SDMA_PKT_CONSTANT_FILL *>(calloc(1, m_PacketSize));
+    pSDMA = reinterpret_cast<SDMA_PKT_CONSTANT_FILL *>(AllocPacket());
     m_PacketData = pSDMA;
 
     while (size > 0) {
@@ -249,13 +236,9 @@ void SDMATimePacket::InitPacket(void *destaddr) {
 
 SDMANopPacket::SDMANopPacket(unsigned int count) {
     packetSize = count * sizeof(unsigned int);
-    packetData = reinterpret_cast<SDMA_PKT_NOP *>(calloc(count, sizeof(unsigned int)));
+    packetData = reinterpret_cast<SDMA_PKT_NOP *>(AllocPacket());
 
     packetData->HEADER_UNION.op = SDMA_OP_NOP;
     packetData->HEADER_UNION.sub_op = 0;
     packetData->HEADER_UNION.count = count - 1;
 }
-
-SDMANopPacket::~SDMANopPacket() {
-    free(packetData);
-}

tests/kfdtest/src/SDMAPacket.hpp

@@ -42,7 +42,7 @@ class SDMAWriteDataPacket : public SDMAPacket {
     SDMAWriteDataPacket(unsigned int familyId, void* destAddr, unsigned int data);
     SDMAWriteDataPacket(unsigned int familyId, void* destAddr, unsigned int ndw, void *data);
 
-    virtual ~SDMAWriteDataPacket(void);
+    virtual ~SDMAWriteDataPacket(void) {}
 
     // @returns Pointer to the packet
     virtual const void *GetPacket() const  { return packetData; }
@@ -63,7 +63,7 @@ class SDMACopyDataPacket : public SDMAPacket {
     SDMACopyDataPacket(unsigned int familyId, void *dest, void *src, unsigned int size);
     SDMACopyDataPacket(unsigned int familyId, void *const dst[], void *src, int n, unsigned int surfsize);
 
-    virtual ~SDMACopyDataPacket(void);
+    virtual ~SDMACopyDataPacket(void) {}
 
     // @returns Pointer to the packet
     virtual const void *GetPacket() const  { return packetData; }
@@ -83,7 +83,7 @@ class SDMAFillDataPacket : public SDMAPacket {
     // This contructor will also init the packet, no need for additional calls
     SDMAFillDataPacket(unsigned int familyId, void *dest, unsigned int data, unsigned int size);
 
-    virtual ~SDMAFillDataPacket(void);
+    virtual ~SDMAFillDataPacket(void) {}
 
     // @returns Pointer to the packet
     virtual const void *GetPacket() const  { return m_PacketData; }
@@ -161,7 +161,7 @@ class SDMATimePacket : public SDMAPacket {
 class SDMANopPacket : public SDMAPacket {
  public:
     SDMANopPacket(unsigned int count = 1);
-    virtual ~SDMANopPacket(void);
+    virtual ~SDMANopPacket(void) {}
 
     // @returns Pointer to the packet
     virtual const void *GetPacket() const { return packetData; }