On Wed, 3 Jun 2026 09:34:20 +0800 Junjie Cao <[email protected]> wrote:
> Two more debug-prone paths benefit from trace coverage. Event log > insertion and clearing are exercised on every error injection or > dynamic capacity event, but their interaction with the per-log > overflow counter is currently opaque. CFMWS address decoding > translates a host physical address through up to two HDM decoder > lookups (host bridge and optional switch USP). Again, two largely unrelated things - two patches please. > > Add five trace points: > > * cxl_event_insert - successful enqueue, log type and > resulting depth > * cxl_event_overflow - the artificial overflow threshold > was hit; an event was dropped > * cxl_event_clear - bulk clear, log type and request size > > * cxl_cfmws_lookup - entry to address decoding, with > the resolved fixed-window index > * cxl_cfmws_target_resolved - HDM decoder produced a target index > (fires for both the host-bridge and > switch USP lookups) > > In non-passthrough topologies, a cxl_cfmws_lookup without a following > cxl_cfmws_target_resolved indicates the host-bridge HDM decode > failed. In passthrough mode the lookup resolves via > pcie_find_port_first() without an HDM target step, so no > target_resolved event is expected. Ah. Good to see you have noted these don't apply to pass through case. I think for that we should probably have an event that makes it clear it is enabled and we won't see these. (Need's Ali's patches to go upstream first though). > > In cxl_event_insert(), cxl_event_count() is hoisted into a local > variable so its O(n) queue walk is performed once per insert rather > than twice (once for the trace point, once for the existing return- > value comparison). > > Signed-off-by: Junjie Cao <[email protected]> > --- > hw/cxl/cxl-events.c | 8 +++++++- > hw/cxl/cxl-host.c | 4 ++++ > hw/cxl/trace-events | 9 +++++++++ > 3 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/hw/cxl/cxl-events.c b/hw/cxl/cxl-events.c > index 5356dfb5b3..f4aa419953 100644 > --- a/hw/cxl/cxl-events.c > +++ b/hw/cxl/cxl-events.c > @@ -13,6 +13,7 @@ > #include "hw/pci/msix.h" > #include "hw/cxl/cxl.h" > #include "hw/cxl/cxl_events.h" > +#include "trace.h" > > /* Artificial limit on the number of events a log can hold */ > #define CXL_TEST_EVENT_OVERFLOW 8 > @@ -98,6 +99,7 @@ bool cxl_event_insert(CXLDeviceState *cxlds, > CXLEventLogType log_type, > uint64_t time; > CXLEventLog *log; > CXLEvent *entry; > + int depth; > > if (log_type >= CXL_EVENT_TYPE_MAX) { > return false; > @@ -115,6 +117,7 @@ bool cxl_event_insert(CXLDeviceState *cxlds, > CXLEventLogType log_type, > } > log->overflow_err_count++; > log->last_overflow_timestamp = time; > + trace_cxl_event_overflow(log_type, log->overflow_err_count); > return false; > } > > @@ -133,8 +136,10 @@ bool cxl_event_insert(CXLDeviceState *cxlds, > CXLEventLogType log_type, > QSIMPLEQ_INSERT_TAIL(&log->events, entry, node); > cxl_event_set_status(cxlds, log_type, true); > > + depth = cxl_event_count(log); Hmm. I wonder if we can name this more clearly. It is kind of queue depth, but I've not really seen that used with and people might think that's the maximum space. Maybe occupancy is better? (Assisted by claude ;) > + trace_cxl_event_insert(log_type, depth); > /* Count went from 0 to 1 */ > - return cxl_event_count(log) == 1; > + return depth == 1; > } > > void cxl_discard_all_event_records(CXLDeviceState *cxlds) > @@ -234,6 +239,7 @@ CXLRetCode cxl_event_clear_records(CXLDeviceState *cxlds, > entry = cxl_event_get_head(log); > } > > + trace_cxl_event_clear(log_type, pl->nr_recs); > return CXL_MBOX_SUCCESS; > } > > diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c > index a94b893e99..4157377a95 100644 > --- a/hw/cxl/cxl-host.c > +++ b/hw/cxl/cxl-host.c > @@ -21,6 +21,7 @@ > #include "hw/pci/pci_host.h" > #include "hw/pci/pcie_port.h" > #include "hw/pci-bridge/pci_expander_bridge.h" > +#include "trace.h" > > static void cxl_fixed_memory_window_config(CXLFixedMemoryWindowOptions > *object, > int index, Error **errp) > @@ -168,6 +169,7 @@ static PCIDevice *cxl_cfmws_find_device(CXLFixedWindow > *fw, hwaddr addr) > bool target_found; > PCIDevice *rp, *d; > > + trace_cxl_cfmws_lookup(addr, fw->index); > rb_index = (addr / cxl_decode_ig(fw->enc_int_gran)) % fw->num_targets; > hb = PCI_HOST_BRIDGE(fw->target_hbs[rb_index]->cxl_host_bridge); > if (!hb || !hb->bus || !pci_bus_is_cxl(hb->bus)) { > @@ -191,6 +193,7 @@ static PCIDevice *cxl_cfmws_find_device(CXLFixedWindow > *fw, hwaddr addr) > if (!target_found) { > return NULL; > } > + trace_cxl_cfmws_target_resolved(addr, target); > > rp = pcie_find_port_by_pn(hb->bus, target); > if (!rp) { > @@ -227,6 +230,7 @@ static PCIDevice *cxl_cfmws_find_device(CXLFixedWindow > *fw, hwaddr addr) > if (!target_found) { > return NULL; > } > + trace_cxl_cfmws_target_resolved(addr, target); > > d = pcie_find_port_by_pn(&PCI_BRIDGE(d)->sec_bus, target); > if (!d) { > diff --git a/hw/cxl/trace-events b/hw/cxl/trace-events > index 15977dce6e..e5f196787d 100644 > --- a/hw/cxl/trace-events > +++ b/hw/cxl/trace-events > @@ -10,3 +10,12 @@ cxl_mailbox_handler_return(uint16_t opcode, int ret, > size_t len_out) "opcode=0x% > # cxl-cdat.c > cxl_cdat_init(const char *source) "source=%s" > cxl_cdat_loaded(const char *filename, size_t length, int num_entries) > "filename=%s length=%zu entries=%d" > + > +# cxl-events.c > +cxl_event_insert(int log_type, int depth) "log_type=%d depth=%d" > +cxl_event_overflow(int log_type, uint16_t overflow_err_count) "log_type=%d > overflow_err_count=%u" > +cxl_event_clear(int log_type, int nr_recs) "log_type=%d nr_recs=%d" > + > +# cxl-host.c > +cxl_cfmws_lookup(uint64_t addr, int fw_index) "addr=0x%"PRIx64" fw=%d" > +cxl_cfmws_target_resolved(uint64_t addr, uint8_t target) "addr=0x%"PRIx64" > target=0x%02x"
