Modeling Best Practices: Expert Code Review Checklist

When reviewing production SystemC code, "it compiles and simulates" is not enough. SystemC's flexibility means poor architectural choices might simulate correctly today but deadlock tomorrow under a different kernel scheduler order, or cripple the performance of an entire SoC integration.

Use this LRM-compliant checklist to evaluate IP blocks before they are merged into a shared library.

1. Core Kernel Semantics

Rule: Ensure predictable scheduling and memory safety.

• Hierarchy Lifetimes: No sc_object (ports, modules, signals) is created as a local stack variable inside a constructor.
• Elaboration Phase: Ports and exports are fully bound before end_of_elaboration.
• Method Restrictions: SC_METHOD processes must never call wait().
• Thread Yielding: SC_THREAD processes must contain a wait() statement inside their infinite loop (otherwise the simulation hangs forever).
• Delta-Cycle Safety: If a module relies on zero-time delays (SC_ZERO_TIME), is it explicitly documented why combinational loops won't occur?
• Writer Policies: If SC_MANY_WRITERS is used on a signal, is it justified? (Usually, it shouldn't be).

2. TLM-2.0 Correctness

Rule: Ensure strict adherence to the TLM-2.0 protocol phases.

• Response Status: Every target socket MUST set a response status (set_response_status()) before returning from a blocking transport call.
• Unsupported Commands: If a target doesn't support TLM_WRITE_COMMAND, does it correctly return TLM_COMMAND_ERROR_RESPONSE instead of silently ignoring it?
• Byte Enables: If byte enables are not supported, does the target check get_byte_enable_ptr() != nullptr and return TLM_BYTE_ENABLE_ERROR_RESPONSE?
• Data Length: Does the target validate get_data_length() against its internal register sizes to prevent buffer overflows (segfaults)?
• Debug Transport: Does transport_dbg guarantee zero side-effects? (It must never advance time, clear interrupts, or pop FIFOs).

3. Configuration & Virtual Platform Quality

Rule: Ensure the IP is reusable and configurable by a top-level architect.

• Parameters: Are magic numbers replaced by CCI-compliant parameters (cci::cci_param)?
• Abstraction: Is the timing abstraction clear? (e.g., "This timer fires accurately, but register read delays are assumed to be 0").
• Reporting: Do SC_REPORT messages use a hierarchical msg_type (e.g., "/VENDOR/IP/ERROR")? Do error messages include context (Address, value, internal state)?

Complete Example: The Reviewer's Sandbox

This complete sc_main acts as a "Reviewer's Sandbox", demonstrating a module that passes the checklist (Good IP) and a module that fails several checks (Bad IP).

#include <systemc>
#include <tlm>
#include <tlm_utils/simple_target_socket.h>
#include <iostream>
 
// ---------------------------------------------------------
// BAD IP: Fails the Code Review
// ---------------------------------------------------------
SC_MODULE(BadIP) {
    tlm_utils::simple_target_socket<BadIP> socket{"socket"};
    
    SC_CTOR(BadIP) {
        socket.register_b_transport(this, &BadIP::b_transport);
        // Fails Checklist: SC_METHOD calling wait()! (Will cause runtime crash)
        // SC_METHOD(bad_method);
    }
 
    void b_transport(tlm::tlm_generic_payload& trans, sc_core::sc_time& delay) {
        // Fails Checklist: Ignores command type!
        // Fails Checklist: Ignores byte enables!
        // Fails Checklist: NEVER SETS RESPONSE STATUS! (Initiator will panic)
        std::cout << "[BadIP] Received transaction.\n";
    }
};
 
// ---------------------------------------------------------
// GOOD IP: Passes the Code Review
// ---------------------------------------------------------
SC_MODULE(GoodIP) {
    tlm_utils::simple_target_socket<GoodIP> socket{"socket"};
    uint32_t internal_reg;
 
    SC_CTOR(GoodIP) : internal_reg(0) {
        socket.register_b_transport(this, &GoodIP::b_transport);
    }
 
    void b_transport(tlm::tlm_generic_payload& trans, sc_core::sc_time& delay) {
        tlm::tlm_command cmd = trans.get_command();
        uint64_t         adr = trans.get_address();
        unsigned char*   ptr = trans.get_data_ptr();
        unsigned int     len = trans.get_data_length();
        unsigned char*   byt = trans.get_byte_enable_ptr();
 
        // Passes Checklist: Validates Byte Enables
        if (byt != nullptr) {
            trans.set_response_status(tlm::TLM_BYTE_ENABLE_ERROR_RESPONSE);
            return;
        }
 
        // Passes Checklist: Validates Data Length
        if (len != 4) {
            trans.set_response_status(tlm::TLM_BURST_ERROR_RESPONSE);
            return;
        }
 
        // Passes Checklist: Handles Commands properly
        if (cmd == tlm::TLM_READ_COMMAND) {
            memcpy(ptr, &internal_reg, 4);
        } else if (cmd == tlm::TLM_WRITE_COMMAND) {
            memcpy(&internal_reg, ptr, 4);
        }
 
        // Passes Checklist: Sets Successful Response Status
        trans.set_response_status(tlm::TLM_OK_RESPONSE);
        
        // Annotate abstract delay
        delay += sc_core::sc_time(10, sc_core::SC_NS);
        
        std::cout << "[GoodIP] Successfully processed " 
                  << (cmd == tlm::TLM_READ_COMMAND ? "READ" : "WRITE") << ".\n";
    }
};
 
int sc_main(int argc, char* argv[]) {
    GoodIP good("good_ip");
    BadIP bad("bad_ip");
 
    // We don't simulate here as the sockets are unconnected, 
    // but this code compiles and demonstrates the architectural differences.
    std::cout << "Code Review Sandbox Compiled Successfully.\n";
    
    return 0;
}

Explanation of the Execution

The BadIP module is a prime example of code that might compile but violates the TLM-2.0 standard. If an initiator sends a payload to BadIP, the initiator's check of trans.get_response_status() will show TLM_INCOMPLETE_RESPONSE, causing the simulation to abort or fail a verification check.

The GoodIP module defensively checks inputs, handles the payload safely, annotates timing accurately, and guarantees a response status is set. This is the quality expected of production virtual platform IP.