On Tue, 30 Sep 2025, Jonathan Cameron wrote:
On Mon, 29 Sep 2025 20:21:49 -0700
Davidlohr Bueso <[email protected]> wrote:
PCIe Flit Mode, introduced with the PCIe 6.0 specification, is a
fundamental change in how data is transmitted over the bus to
improve transfer rates. It shifts from variable-sized Transaction
Layer Packets (TLPs) to fixed 256-byte Flow Control Units (FLITs).
As with the link speed and width training, have ad-hoc property for
setting the flit mode and allow CXL components to make use of it.
For the CXL root port and dsp cases, always report flit mode but
the actual value after 'training' will depend on the downstream
device configuration.
Suggested-by: Jonathan Cameron <[email protected]>
Signed-off-by: Davidlohr Bueso <[email protected]>
Hi Davidlohr,
A few comments inline. Main one is that I think we need the CAP to still
say we support 68B flits on the CXL devices and just mess with the
status.
---
hw/mem/cxl_type3.c | 9 ++++++---
hw/pci-bridge/cxl_downstream.c | 11 +++++++----
hw/pci-bridge/cxl_root_port.c | 11 +++++++----
hw/pci-bridge/cxl_upstream.c | 19 +++++++++++--------
hw/pci-bridge/gen_pcie_root_port.c | 1 +
hw/pci/pcie.c | 23 +++++++++++++++++++----
include/hw/cxl/cxl_device.h | 1 +
include/hw/pci-bridge/cxl_upstream_port.h | 1 +
include/hw/pci/pcie.h | 2 +-
include/hw/pci/pcie_port.h | 1 +
10 files changed, 55 insertions(+), 24 deletions(-)
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index c4658e0955d5..891b75618892 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -533,9 +533,10 @@ static void build_dvsecs(CXLType3Dev *ct3d)
GPF_DEVICE_DVSEC_REVID, dvsec);
dvsec = (uint8_t *)&(CXLDVSECPortFlexBus){
- .cap = 0x26, /* 68B, IO, Mem, non-MLD */
+ /* 68B (maybe), IO, Mem, non-MLD */
+ .cap = ct3d->flitmode ? 0x6 : 0x26,
Do we need this capability bit change? The bit says the device is capable of
doing it,
not that we currently are. I think from a spec point of view we aren't
allowed to
not support this. Applies int the various other places as well.
I agree, we should leave it alone, for which when in flitmode lspci shows:
FBCap: Cache- IO+ Mem+ 68BFlit+ MltLogDev- 256BFlit- PBRFlit-
... noting that that misleading 256BFlit- output is dependent on bit 13 (as
reported
by Ravi), but that in fact is the latency optimized version, which is not
mandatory.
.ctrl = 0x02, /* IO always enabled */
- .status = 0x26, /* same as capabilities */
+ .status = ct3d->flitmode ? 0x6 : 0x26, /* same */
This is correct but if we do leave teh cap alone we probably need to note
that flitmode means we aren't in the old 68B flit mode.
FBSta: Cache- IO+ Mem+ SynHdrByp- DrftBuf- 68BFlit- MltLogDev-
256BFlit- PBRFlit-
... which that 68BFlit- in fact implies regular 256B flit mode.
Thanks,
Davidlohr
.rcvd_mod_ts_data_phase1 = 0xef, /* WTF? */
};
cxl_component_create_dvsec(cxl_cstate, CXL2_TYPE3_DEVICE,
@@ -1501,7 +1502,8 @@ void ct3d_reset(DeviceState *dev)
uint32_t *reg_state = ct3d->cxl_cstate.crb.cache_mem_registers;
uint32_t *write_msk = ct3d->cxl_cstate.crb.cache_mem_regs_write_mask;
- pcie_cap_fill_link_ep_usp(PCI_DEVICE(dev), ct3d->width, ct3d->speed);
+ pcie_cap_fill_link_ep_usp(PCI_DEVICE(dev), ct3d->width, ct3d->speed,
+ ct3d->flitmode);
cxl_component_register_init_common(reg_state, write_msk,
CXL2_TYPE3_DEVICE);
cxl_device_register_init_t3(ct3d, CXL_T3_MSIX_MBOX);
@@ -1540,6 +1542,7 @@ static const Property ct3_props[] = {
speed, PCIE_LINK_SPEED_32),
DEFINE_PROP_PCIE_LINK_WIDTH("x-width", CXLType3Dev,
width, PCIE_LINK_WIDTH_16),
+ DEFINE_PROP_BOOL("x-256b-flit", CXLType3Dev, flitmode, false),
DEFINE_PROP_UINT16("chmu-port", CXLType3Dev, cxl_dstate.chmu[0].port, 0),
};
diff --git a/hw/pci-bridge/cxl_downstream.c b/hw/pci-bridge/cxl_downstream.c
index 6aa8586f0161..f8d64263ac08 100644
--- a/hw/pci-bridge/cxl_downstream.c
+++ b/hw/pci-bridge/cxl_downstream.c
static void gen_rp_dev_class_init(ObjectClass *klass, const void *data)
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index eaeb68894e6e..cc8e7c3cbf3f 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -1106,6 +1111,8 @@ void pcie_sync_bridge_lnk(PCIDevice *bridge_dev)
if (!target || !target->exp.exp_cap) {
lnksta = lnkcap;
} else {
+ uint16_t lnksta2;
+
lnksta = target->config_read(target,
target->exp.exp_cap + PCI_EXP_LNKSTA,
sizeof(lnksta));
@@ -1119,6 +1126,14 @@ void pcie_sync_bridge_lnk(PCIDevice *bridge_dev)
lnksta &= ~PCI_EXP_LNKSTA_CLS;
lnksta |= lnkcap & PCI_EXP_LNKCAP_SLS;
}
+
+ lnksta2 = target->config_read(target,
+ target->exp.exp_cap + PCI_EXP_LNKSTA2,
+ sizeof(lnksta2));
+ pci_word_test_and_clear_mask(exp_cap + PCI_EXP_LNKSTA2,
+ PCI_EXP_LNKSTA2_FLIT);
+ pci_word_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA2, lnksta2 &
+ PCI_EXP_LNKSTA2_FLIT);
Odd linewrap. Kind of makes some sense for the linksta one I guess you based
this
on but here, we can fit the whole parameter on one line.
}
if (!(lnksta & PCI_EXP_LNKSTA_NLW)) {
