* [PATCH 1/5] hw/pcie: Support enabling flit mode
2025-09-30 3:21 [PATCH v3 -qemu 0/5] hw/cxl: Support Back-Invalidate Davidlohr Bueso
@ 2025-09-30 3:21 ` Davidlohr Bueso
2025-09-30 15:19 ` Jonathan Cameron via
2025-09-30 3:21 ` [PATCH 2/5] hw/cxl: Refactor component register initialization Davidlohr Bueso
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Davidlohr Bueso @ 2025-09-30 3:21 UTC (permalink / raw)
To: jonathan.cameron
Cc: ira.weiny, lucerop, a.manzanares, mst, marcel.apfelbaum, armbru,
linux-cxl, qemu-devel, dave, Jonathan Cameron
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 <Jonathan.Cameron@huawei.com>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
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,
.ctrl = 0x02, /* IO always enabled */
- .status = 0x26, /* same as capabilities */
+ .status = ct3d->flitmode ? 0x6 : 0x26, /* same */
.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
@@ -116,8 +116,9 @@ static void cxl_dsp_reset(DeviceState *qdev)
latch_registers(dsp);
}
-static void build_dvsecs(CXLComponentState *cxl)
+static void build_dvsecs(PCIDevice *d, CXLComponentState *cxl)
{
+ PCIESlot *s = PCIE_SLOT(d);
CXLDVSECRegisterLocator *regloc_dvsec;
uint8_t *dvsec;
int i;
@@ -129,9 +130,10 @@ static void build_dvsecs(CXLComponentState *cxl)
EXTENSIONS_PORT_DVSEC_REVID, dvsec);
dvsec = (uint8_t *)&(CXLDVSECPortFlexBus){
- .cap = 0x27, /* Cache, IO, Mem, non-MLD */
+ /* 68B (maybe), IO, Mem, non-MLD */
+ .cap = s->flitmode ? 0x7 : 0x27,
.ctrl = 0x02, /* IO always enabled */
- .status = 0x26, /* same */
+ .status = s->flitmode ? 0x6 : 0x26, /* same */
.rcvd_mod_ts_data_phase1 = 0xef, /* WTF? */
};
cxl_component_create_dvsec(cxl, CXL2_DOWNSTREAM_PORT,
@@ -212,7 +214,7 @@ static void cxl_dsp_realize(PCIDevice *d, Error **errp)
cxl_cstate->dvsec_offset = CXL_DOWNSTREAM_PORT_DVSEC_OFFSET;
cxl_cstate->pdev = d;
- build_dvsecs(cxl_cstate);
+ build_dvsecs(d, cxl_cstate);
cxl_component_register_block_init(OBJECT(d), cxl_cstate, TYPE_CXL_DSP);
memory_region_init(&dsp->bar, OBJECT(d), "registers", (2 << 16));
memory_region_add_subregion(&dsp->bar, 0, component_bar);
@@ -257,6 +259,7 @@ static const Property cxl_dsp_props[] = {
speed, PCIE_LINK_SPEED_64),
DEFINE_PROP_PCIE_LINK_WIDTH("x-width", PCIESlot,
width, PCIE_LINK_WIDTH_16),
+ DEFINE_PROP_BOOL("x-256b-flit", PCIESlot, flitmode, true),
};
static void cxl_dsp_class_init(ObjectClass *oc, const void *data)
diff --git a/hw/pci-bridge/cxl_root_port.c b/hw/pci-bridge/cxl_root_port.c
index f035987b6f1f..f3472f081707 100644
--- a/hw/pci-bridge/cxl_root_port.c
+++ b/hw/pci-bridge/cxl_root_port.c
@@ -109,8 +109,9 @@ static void latch_registers(CXLRootPort *crp)
cxl_component_register_init_common(reg_state, write_msk, CXL2_ROOT_PORT);
}
-static void build_dvsecs(CXLComponentState *cxl)
+static void build_dvsecs(PCIDevice *d, CXLComponentState *cxl)
{
+ PCIESlot *s = PCIE_SLOT(d);
CXLDVSECRegisterLocator *regloc_dvsec;
uint8_t *dvsec;
int i;
@@ -131,9 +132,10 @@ static void build_dvsecs(CXLComponentState *cxl)
GPF_PORT_DVSEC_REVID, dvsec);
dvsec = (uint8_t *)&(CXLDVSECPortFlexBus){
- .cap = 0x26, /* IO, Mem, non-MLD */
+ /* 68B (maybe), IO, Mem, non-MLD */
+ .cap = s->flitmode ? 0x6 : 0x26,
.ctrl = 0x2,
- .status = 0x26, /* same */
+ .status = s->flitmode ? 0x6 : 0x26, /* same */
.rcvd_mod_ts_data_phase1 = 0xef,
};
cxl_component_create_dvsec(cxl, CXL2_ROOT_PORT,
@@ -189,7 +191,7 @@ static void cxl_rp_realize(DeviceState *dev, Error **errp)
cxl_cstate->dvsec_offset = CXL_ROOT_PORT_DVSEC_OFFSET;
cxl_cstate->pdev = pci_dev;
- build_dvsecs(cxl_cstate);
+ build_dvsecs(pci_dev, cxl_cstate);
cxl_component_register_block_init(OBJECT(pci_dev), cxl_cstate,
TYPE_CXL_ROOT_PORT);
@@ -235,6 +237,7 @@ static const Property gen_rp_props[] = {
speed, PCIE_LINK_SPEED_64),
DEFINE_PROP_PCIE_LINK_WIDTH("x-width", PCIESlot,
width, PCIE_LINK_WIDTH_32),
+ DEFINE_PROP_BOOL("x-256b-flit", PCIESlot, flitmode, true),
};
static void cxl_rp_dvsec_write_config(PCIDevice *dev, uint32_t addr,
diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
index c2150afff39b..e5a0d1fb308c 100644
--- a/hw/pci-bridge/cxl_upstream.c
+++ b/hw/pci-bridge/cxl_upstream.c
@@ -147,12 +147,13 @@ static void cxl_usp_reset(DeviceState *qdev)
pci_bridge_reset(qdev);
pcie_cap_deverr_reset(d);
- pcie_cap_fill_link_ep_usp(d, usp->width, usp->speed);
+ pcie_cap_fill_link_ep_usp(d, usp->width, usp->speed, usp->flitmode);
latch_registers(usp);
}
-static void build_dvsecs(CXLComponentState *cxl)
+static void build_dvsecs(CXLUpstreamPort *usp)
{
+ CXLComponentState *cxl_cstate = &usp->cxl_cstate;
CXLDVSECRegisterLocator *regloc_dvsec;
uint8_t *dvsec;
int i;
@@ -160,17 +161,18 @@ static void build_dvsecs(CXLComponentState *cxl)
dvsec = (uint8_t *)&(CXLDVSECPortExt){
.status = 0x1, /* Port Power Management Init Complete */
};
- cxl_component_create_dvsec(cxl, CXL2_UPSTREAM_PORT,
+ cxl_component_create_dvsec(cxl_cstate, CXL2_UPSTREAM_PORT,
EXTENSIONS_PORT_DVSEC_LENGTH,
EXTENSIONS_PORT_DVSEC,
EXTENSIONS_PORT_DVSEC_REVID, dvsec);
dvsec = (uint8_t *)&(CXLDVSECPortFlexBus){
- .cap = 0x27, /* Cache, IO, Mem, non-MLD */
+ /* 68B (maybe), Cache, IO, Mem, non-MLD */
+ .cap = usp->flitmode ? 0x7 : 0x27,
.ctrl = 0x27, /* Cache, IO, Mem */
- .status = 0x26, /* same */
+ .status = usp->flitmode ? 0x6 : 0x26, /* same */
.rcvd_mod_ts_data_phase1 = 0xef, /* WTF? */
};
- cxl_component_create_dvsec(cxl, CXL2_UPSTREAM_PORT,
+ cxl_component_create_dvsec(cxl_cstate, CXL2_UPSTREAM_PORT,
PCIE_CXL3_FLEXBUS_PORT_DVSEC_LENGTH,
PCIE_FLEXBUS_PORT_DVSEC,
PCIE_CXL3_FLEXBUS_PORT_DVSEC_REVID, dvsec);
@@ -186,7 +188,7 @@ static void build_dvsecs(CXLComponentState *cxl)
RBI_CXL_CPMU_REG | 0; /* Port so only one 64 bit bar */
regloc_dvsec->reg_base[1 + i].hi = 0;
}
- cxl_component_create_dvsec(cxl, CXL2_UPSTREAM_PORT,
+ cxl_component_create_dvsec(cxl_cstate, CXL2_UPSTREAM_PORT,
REG_LOC_DVSEC_LENGTH, REG_LOC_DVSEC,
REG_LOC_DVSEC_REVID, (uint8_t *)regloc_dvsec);
}
@@ -381,7 +383,7 @@ static void cxl_usp_realize(PCIDevice *d, Error **errp)
}
cxl_cstate->dvsec_offset = CXL_UPSTREAM_PORT_DVSEC_OFFSET;
cxl_cstate->pdev = d;
- build_dvsecs(cxl_cstate);
+ build_dvsecs(usp);
cxl_component_register_block_init(OBJECT(d), cxl_cstate, TYPE_CXL_USP);
memory_region_init(&usp->bar, OBJECT(d), "registers", (2 << 16));
memory_region_add_subregion(&usp->bar, 0, component_bar);
@@ -433,6 +435,7 @@ static const Property cxl_upstream_props[] = {
speed, PCIE_LINK_SPEED_32),
DEFINE_PROP_PCIE_LINK_WIDTH("x-width", CXLUpstreamPort,
width, PCIE_LINK_WIDTH_16),
+ DEFINE_PROP_BOOL("x-256b-flit", CXLUpstreamPort, flitmode, false),
};
static void cxl_upstream_class_init(ObjectClass *oc, const void *data)
diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
index d9078e783bf0..6b97da0b4cbb 100644
--- a/hw/pci-bridge/gen_pcie_root_port.c
+++ b/hw/pci-bridge/gen_pcie_root_port.c
@@ -145,6 +145,7 @@ static const Property gen_rp_props[] = {
speed, PCIE_LINK_SPEED_16),
DEFINE_PROP_PCIE_LINK_WIDTH("x-width", PCIESlot,
width, PCIE_LINK_WIDTH_32),
+ DEFINE_PROP_BOOL("x-256b-flit", PCIESlot, flitmode, false),
};
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
@@ -113,7 +113,7 @@ pcie_cap_v1_fill(PCIDevice *dev, uint8_t port, uint8_t type, uint8_t version)
/* Includes setting the target speed default */
static void pcie_cap_fill_lnk(uint8_t *exp_cap, PCIExpLinkWidth width,
- PCIExpLinkSpeed speed)
+ PCIExpLinkSpeed speed, bool flitmode)
{
/* Clear and fill LNKCAP from what was configured above */
pci_long_test_and_clear_mask(exp_cap + PCI_EXP_LNKCAP,
@@ -158,10 +158,15 @@ static void pcie_cap_fill_lnk(uint8_t *exp_cap, PCIExpLinkWidth width,
PCI_EXP_LNKCAP2_SLS_64_0GB);
}
}
+
+ if (flitmode) {
+ pci_long_test_and_set_mask(exp_cap + PCI_EXP_LNKSTA2,
+ PCI_EXP_LNKSTA2_FLIT);
+ }
}
void pcie_cap_fill_link_ep_usp(PCIDevice *dev, PCIExpLinkWidth width,
- PCIExpLinkSpeed speed)
+ PCIExpLinkSpeed speed, bool flitmode)
{
uint8_t *exp_cap = dev->config + dev->exp.exp_cap;
@@ -175,7 +180,7 @@ void pcie_cap_fill_link_ep_usp(PCIDevice *dev, PCIExpLinkWidth width,
QEMU_PCI_EXP_LNKSTA_NLW(width) |
QEMU_PCI_EXP_LNKSTA_CLS(speed));
- pcie_cap_fill_lnk(exp_cap, width, speed);
+ pcie_cap_fill_lnk(exp_cap, width, speed, flitmode);
}
static void pcie_cap_fill_slot_lnk(PCIDevice *dev)
@@ -212,7 +217,7 @@ static void pcie_cap_fill_slot_lnk(PCIDevice *dev)
/* the PCI_EXP_LNKSTA_DLLLA will be set in the hotplug function */
}
- pcie_cap_fill_lnk(exp_cap, s->width, s->speed);
+ pcie_cap_fill_lnk(exp_cap, s->width, s->speed, s->flitmode);
}
int pcie_cap_init(PCIDevice *dev, uint8_t offset,
@@ -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);
}
if (!(lnksta & PCI_EXP_LNKSTA_NLW)) {
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index 068c20d61ebc..0abfd678b875 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -796,6 +796,7 @@ struct CXLType3Dev {
/* PCIe link characteristics */
PCIExpLinkSpeed speed;
PCIExpLinkWidth width;
+ bool flitmode;
/* DOE */
DOECap doe_cdat;
diff --git a/include/hw/pci-bridge/cxl_upstream_port.h b/include/hw/pci-bridge/cxl_upstream_port.h
index db1dfb6afd98..4862601b2a60 100644
--- a/include/hw/pci-bridge/cxl_upstream_port.h
+++ b/include/hw/pci-bridge/cxl_upstream_port.h
@@ -20,6 +20,7 @@ typedef struct CXLUpstreamPort {
PCIExpLinkSpeed speed;
PCIExpLinkWidth width;
+ bool flitmode;
DOECap doe_cdat;
uint64_t sn;
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index ff6ce08e135a..358374b1df81 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -142,7 +142,7 @@ void pcie_ari_init(PCIDevice *dev, uint16_t offset);
void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);
void pcie_ats_init(PCIDevice *dev, uint16_t offset, bool aligned);
void pcie_cap_fill_link_ep_usp(PCIDevice *dev, PCIExpLinkWidth width,
- PCIExpLinkSpeed speed);
+ PCIExpLinkSpeed speed, bool flitmode);
void pcie_cap_slot_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp);
diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
index 7cd7af8cfa4b..53cd64c5edff 100644
--- a/include/hw/pci/pcie_port.h
+++ b/include/hw/pci/pcie_port.h
@@ -58,6 +58,7 @@ struct PCIESlot {
PCIExpLinkSpeed speed;
PCIExpLinkWidth width;
+ bool flitmode;
/* Disable ACS (really for a pcie_root_port) */
bool disable_acs;
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/5] hw/pcie: Support enabling flit mode
2025-09-30 3:21 ` [PATCH 1/5] hw/pcie: Support enabling flit mode Davidlohr Bueso
@ 2025-09-30 15:19 ` Jonathan Cameron via
2025-10-08 19:20 ` Davidlohr Bueso
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron via @ 2025-09-30 15:19 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: ira.weiny, lucerop, a.manzanares, mst, marcel.apfelbaum, armbru,
linux-cxl, qemu-devel
On Mon, 29 Sep 2025 20:21:49 -0700
Davidlohr Bueso <dave@stgolabs.net> 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 <Jonathan.Cameron@huawei.com>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
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.
> .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.
> .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)) {
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/5] hw/pcie: Support enabling flit mode
2025-09-30 15:19 ` Jonathan Cameron via
@ 2025-10-08 19:20 ` Davidlohr Bueso
0 siblings, 0 replies; 10+ messages in thread
From: Davidlohr Bueso @ 2025-10-08 19:20 UTC (permalink / raw)
To: Jonathan Cameron
Cc: ira.weiny, lucerop, a.manzanares, mst, marcel.apfelbaum, armbru,
linux-cxl, qemu-devel, ravis.opensrc
On Tue, 30 Sep 2025, Jonathan Cameron wrote:
>On Mon, 29 Sep 2025 20:21:49 -0700
>Davidlohr Bueso <dave@stgolabs.net> 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 <Jonathan.Cameron@huawei.com>
>> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
>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)) {
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/5] hw/cxl: Refactor component register initialization
2025-09-30 3:21 [PATCH v3 -qemu 0/5] hw/cxl: Support Back-Invalidate Davidlohr Bueso
2025-09-30 3:21 ` [PATCH 1/5] hw/pcie: Support enabling flit mode Davidlohr Bueso
@ 2025-09-30 3:21 ` Davidlohr Bueso
2025-09-30 3:21 ` [PATCH 3/5] hw/cxl: Allow BI by default in Window restrictions Davidlohr Bueso
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Davidlohr Bueso @ 2025-09-30 3:21 UTC (permalink / raw)
To: jonathan.cameron
Cc: ira.weiny, lucerop, a.manzanares, mst, marcel.apfelbaum, armbru,
linux-cxl, qemu-devel, dave
From: Ira Weiny <ira.weiny@intel.com>
CXL 3.2 8.2.4 Table 8-22 defines which capabilities are mandatory, not
permitted, or optional for each type of device.
cxl_component_register_init_common() uses a rather odd 'fall through'
mechanism to define each component register set. This assumes that any
device or capability being added builds on the previous devices
capabilities. This is not true as there are mutually exclusive
capabilities defined. For example, downstream ports can not have snoop
but it can have Back Invalidate capable decoders.
Refactor this code to make it easier to add individual capabilities as
defined by a device type. Any capability which is not specified by the
type is left NULL'ed out which complies with the packed nature of the
register array.
Update all spec references to 3.2.
No functional changes should be seen with this patch.
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
[rebased, no RAS for HBs, r3.2 references]
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
hw/cxl/cxl-component-utils.c | 75 +++++++++++-----------------------
include/hw/cxl/cxl_component.h | 33 ++++++++++-----
2 files changed, 46 insertions(+), 62 deletions(-)
diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
index ce42fa4a2946..a43d227336ca 100644
--- a/hw/cxl/cxl-component-utils.c
+++ b/hw/cxl/cxl-component-utils.c
@@ -289,32 +289,6 @@ void cxl_component_register_init_common(uint32_t *reg_state,
{
int caps = 0;
- /*
- * In CXL 2.0 the capabilities required for each CXL component are such
- * that, with the ordering chosen here, a single number can be used to
- * define which capabilities should be provided.
- */
- switch (type) {
- case CXL2_DOWNSTREAM_PORT:
- case CXL2_DEVICE:
- /* RAS, Link */
- caps = 2;
- break;
- case CXL2_UPSTREAM_PORT:
- case CXL2_TYPE3_DEVICE:
- case CXL2_LOGICAL_DEVICE:
- /* + HDM */
- caps = 3;
- break;
- case CXL2_ROOT_PORT:
- case CXL2_RC:
- /* + Extended Security, + Snoop */
- caps = 5;
- break;
- default:
- abort();
- }
-
memset(reg_state, 0, CXL2_COMPONENT_CM_REGION_SIZE);
/* CXL Capability Header Register */
@@ -322,11 +296,12 @@ void cxl_component_register_init_common(uint32_t *reg_state,
ARRAY_FIELD_DP32(reg_state, CXL_CAPABILITY_HEADER, VERSION,
CXL_CAPABILITY_VERSION);
ARRAY_FIELD_DP32(reg_state, CXL_CAPABILITY_HEADER, CACHE_MEM_VERSION, 1);
- ARRAY_FIELD_DP32(reg_state, CXL_CAPABILITY_HEADER, ARRAY_SIZE, caps);
#define init_cap_reg(reg, id, version) \
do { \
- int which = R_CXL_##reg##_CAPABILITY_HEADER; \
+ int which = CXL_##reg##_CAP_HDR_IDX; \
+ if (CXL_##reg##_CAP_HDR_IDX > caps) \
+ caps = CXL_##reg##_CAP_HDR_IDX; \
reg_state[which] = FIELD_DP32(reg_state[which], \
CXL_##reg##_CAPABILITY_HEADER, ID, id); \
reg_state[which] = \
@@ -337,37 +312,35 @@ void cxl_component_register_init_common(uint32_t *reg_state,
CXL_##reg##_REGISTERS_OFFSET); \
} while (0)
+ /* CXL r3.2 8.2.4 Table 8-22 */
switch (type) {
- case CXL2_DEVICE:
- case CXL2_TYPE3_DEVICE:
- case CXL2_LOGICAL_DEVICE:
case CXL2_ROOT_PORT:
+ case CXL2_RC:
+ /* + Extended Security, + Snoop */
+ init_cap_reg(EXTSEC, 6, 1);
+ init_cap_reg(SNOOP, 8, 1);
+ /* fallthrough */
case CXL2_UPSTREAM_PORT:
+ case CXL2_TYPE3_DEVICE:
+ case CXL2_LOGICAL_DEVICE:
+ /* + HDM */
+ init_cap_reg(HDM, 5, 1);
+ hdm_init_common(reg_state, write_msk, type);
+ /* fallthrough */
case CXL2_DOWNSTREAM_PORT:
- init_cap_reg(RAS, 2, CXL_RAS_CAPABILITY_VERSION);
- ras_init_common(reg_state, write_msk);
+ case CXL2_DEVICE:
+ /* RAS, Link */
+ if (type != CXL2_RC) {
+ init_cap_reg(RAS, 2, 2);
+ ras_init_common(reg_state, write_msk);
+ }
+ init_cap_reg(LINK, 4, 2);
break;
default:
- break;
- }
-
- init_cap_reg(LINK, 4, CXL_LINK_CAPABILITY_VERSION);
-
- if (caps < 3) {
- return;
- }
-
- if (type != CXL2_ROOT_PORT) {
- init_cap_reg(HDM, 5, CXL_HDM_CAPABILITY_VERSION);
- hdm_init_common(reg_state, write_msk, type);
- }
- if (caps < 5) {
- return;
+ abort();
}
- init_cap_reg(EXTSEC, 6, CXL_EXTSEC_CAP_VERSION);
- init_cap_reg(SNOOP, 8, CXL_SNOOP_CAP_VERSION);
-
+ ARRAY_FIELD_DP32(reg_state, CXL_CAPABILITY_HEADER, ARRAY_SIZE, caps);
#undef init_cap_reg
}
diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
index b721333cb7aa..cd92cb02532a 100644
--- a/include/hw/cxl/cxl_component.h
+++ b/include/hw/cxl/cxl_component.h
@@ -32,10 +32,20 @@ enum reg_type {
};
/*
- * Capability registers are defined at the top of the CXL.cache/mem region and
- * are packed. For our purposes we will always define the caps in the same
- * order.
- * CXL r3.1 Table 8-22: CXL_CAPABILITY_ID Assignment for details.
+ * CXL r3.2 - 8.2.4 Table 8-22 and 8-23
+ *
+ * Capability registers are defined at the top of the CXL.cache/mem region.
+ * They are defined to be packed and at variable offsets. However, NULL
+ * capabilities can be added to the packed array. To facilitate easier access
+ * within the QEMU code, define these at specified offsets. Then NULL out any
+ * capabilities for devices which don't (or can't) have a particular capability
+ * (see cxl_component_register_init_common). NULL capabilities are to be
+ * ignored by software.
+ *
+ * 'offsets' are based on index's which can then be used to report the array
+ * size in CXL Capability Header Register (index/offset 0).
+ *
+ * See CXL r3.2 Table 8-25 for an example of allowing a 'NULL' header.
*/
/* CXL r3.1 Section 8.2.4.1: CXL Capability Header Register */
@@ -46,16 +56,17 @@ REG32(CXL_CAPABILITY_HEADER, 0)
FIELD(CXL_CAPABILITY_HEADER, CACHE_MEM_VERSION, 20, 4)
FIELD(CXL_CAPABILITY_HEADER, ARRAY_SIZE, 24, 8)
-#define CXLx_CAPABILITY_HEADER(type, offset) \
- REG32(CXL_##type##_CAPABILITY_HEADER, offset) \
+#define CXLx_CAPABILITY_HEADER(type, idx) \
+ enum { CXL_##type##_CAP_HDR_IDX = idx }; \
+ REG32(CXL_##type##_CAPABILITY_HEADER, (idx * 0x4)) \
FIELD(CXL_##type##_CAPABILITY_HEADER, ID, 0, 16) \
FIELD(CXL_##type##_CAPABILITY_HEADER, VERSION, 16, 4) \
FIELD(CXL_##type##_CAPABILITY_HEADER, PTR, 20, 12)
-CXLx_CAPABILITY_HEADER(RAS, 0x4)
-CXLx_CAPABILITY_HEADER(LINK, 0x8)
-CXLx_CAPABILITY_HEADER(HDM, 0xc)
-CXLx_CAPABILITY_HEADER(EXTSEC, 0x10)
-CXLx_CAPABILITY_HEADER(SNOOP, 0x14)
+CXLx_CAPABILITY_HEADER(RAS, 1)
+CXLx_CAPABILITY_HEADER(LINK, 2)
+CXLx_CAPABILITY_HEADER(HDM, 3)
+CXLx_CAPABILITY_HEADER(EXTSEC, 4)
+CXLx_CAPABILITY_HEADER(SNOOP, 5)
/*
* Capability structures contain the actual registers that the CXL component
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/5] hw/cxl: Allow BI by default in Window restrictions
2025-09-30 3:21 [PATCH v3 -qemu 0/5] hw/cxl: Support Back-Invalidate Davidlohr Bueso
2025-09-30 3:21 ` [PATCH 1/5] hw/pcie: Support enabling flit mode Davidlohr Bueso
2025-09-30 3:21 ` [PATCH 2/5] hw/cxl: Refactor component register initialization Davidlohr Bueso
@ 2025-09-30 3:21 ` Davidlohr Bueso
2025-09-30 3:21 ` [PATCH 4/5] hw/cxl: Support type3 HDM-DB Davidlohr Bueso
2025-09-30 3:21 ` [PATCH 5/5] hw/cxl: Remove register special_ops->read() Davidlohr Bueso
4 siblings, 0 replies; 10+ messages in thread
From: Davidlohr Bueso @ 2025-09-30 3:21 UTC (permalink / raw)
To: jonathan.cameron
Cc: ira.weiny, lucerop, a.manzanares, mst, marcel.apfelbaum, armbru,
linux-cxl, qemu-devel, dave
Update the CFMW restrictions to also permit Back-Invalidate
flows by default, which is aligned with the no-restrictions
policy.
While at it, document the 'restrictions=' option.
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
hw/cxl/cxl-host.c | 2 +-
qapi/machine.json | 3 ++-
qemu-options.hx | 4 +++-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index def2cf75be61..0d17ea3e4c26 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -64,7 +64,7 @@ static void cxl_fixed_memory_window_config(CXLFixedMemoryWindowOptions *object,
if (object->has_restrictions) {
fw->restrictions = object->restrictions;
} else {
- fw->restrictions = 0xf; /* No restrictions */
+ fw->restrictions = 0x2f; /* No restrictions */
}
fw->targets = g_malloc0_n(fw->num_targets, sizeof(*fw->targets));
diff --git a/qapi/machine.json b/qapi/machine.json
index ac258578e4ab..ea8ba71305b0 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -589,7 +589,8 @@
# BIT(2) - Volatile
# BIT(3) - Persistent
# BIT(4) - Fixed Device Config
-# Default is 0xF
+# BIT(5) - BI
+# Default is 0x2F
#
# @targets: Target root bridge IDs from -device ...,id=<ID> for each
# root bridge.
diff --git a/qemu-options.hx b/qemu-options.hx
index 1f862b19a676..2da65fad3c9f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -142,7 +142,7 @@ SRST
-machine memory-backend=pc.ram
-m 512M
- ``cxl-fmw.0.targets.0=firsttarget,cxl-fmw.0.targets.1=secondtarget,cxl-fmw.0.size=size[,cxl-fmw.0.interleave-granularity=granularity]``
+ ``cxl-fmw.0.targets.0=firsttarget,cxl-fmw.0.targets.1=secondtarget,cxl-fmw.0.size=size[,cxl-fmw.0.interleave-granularity=granularity,restrictions=restrictions]``
Define a CXL Fixed Memory Window (CFMW).
Described in the CXL 2.0 ECN: CEDT CFMWS & QTG _DSM.
@@ -168,6 +168,8 @@ SRST
interleave. Default 256 (bytes). Only 256, 512, 1k, 2k,
4k, 8k and 16k granularities supported.
+ ``restrictions=restrictions`` bitmask with the restrictions of the CFMW.
+
Example:
::
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/5] hw/cxl: Support type3 HDM-DB
2025-09-30 3:21 [PATCH v3 -qemu 0/5] hw/cxl: Support Back-Invalidate Davidlohr Bueso
` (2 preceding siblings ...)
2025-09-30 3:21 ` [PATCH 3/5] hw/cxl: Allow BI by default in Window restrictions Davidlohr Bueso
@ 2025-09-30 3:21 ` Davidlohr Bueso
2025-09-30 15:36 ` Jonathan Cameron via
2025-09-30 3:21 ` [PATCH 5/5] hw/cxl: Remove register special_ops->read() Davidlohr Bueso
4 siblings, 1 reply; 10+ messages in thread
From: Davidlohr Bueso @ 2025-09-30 3:21 UTC (permalink / raw)
To: jonathan.cameron
Cc: ira.weiny, lucerop, a.manzanares, mst, marcel.apfelbaum, armbru,
linux-cxl, qemu-devel, dave
Add basic plumbing for memory expander devices that support Back
Invalidation. This introduces a 'hdm-db=on|off' parameter and
exposes the relevant BI RT/Decoder component cachemem registers.
Some noteworthy properties:
- Devices require enabling Flit mode.
- Explicit BI-ID commit is required.
- HDM decoder support both host and dev coherency models.
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
docs/system/devices/cxl.rst | 23 +++++
hw/cxl/cxl-component-utils.c | 147 ++++++++++++++++++++++++++--
hw/mem/cxl_type3.c | 9 +-
hw/pci-bridge/cxl_downstream.c | 2 +-
hw/pci-bridge/cxl_root_port.c | 3 +-
hw/pci-bridge/cxl_upstream.c | 2 +-
hw/pci-bridge/pci_expander_bridge.c | 2 +-
include/hw/cxl/cxl_component.h | 54 +++++++++-
include/hw/cxl/cxl_device.h | 3 +
9 files changed, 228 insertions(+), 17 deletions(-)
diff --git a/docs/system/devices/cxl.rst b/docs/system/devices/cxl.rst
index bf7908429af8..9cdf097fc644 100644
--- a/docs/system/devices/cxl.rst
+++ b/docs/system/devices/cxl.rst
@@ -384,6 +384,29 @@ An example of 4 devices below a switch suitable for 1, 2 or 4 way interleave::
-device cxl-type3,bus=swport3,persistent-memdev=cxl-mem3,lsa=cxl-lsa3,id=cxl-pmem3,sn=0x4 \
-M cxl-fmw.0.targets.0=cxl.1,cxl-fmw.0.size=4G,cxl-fmw.0.interleave-granularity=4k
+An example of 4 type3 devices with volatile memory below a switch. Two of the devices
+use HDM-DB for coherence, which requires operating in Flit mode::
+
+ qemu-system-x86_64 -M q35,cxl=on -m 4G,maxmem=8G,slots=8 -smp 4 \
+ ...
+ -object memory-backend-ram,id=cxl-mem0,share=on,mem-path=/tmp/cxltest.raw,size=256M \
+ -object memory-backend-ram,id=cxl-mem1,share=on,mem-path=/tmp/cxltest1.raw,size=256M \
+ -object memory-backend-ram,id=cxl-mem2,share=on,mem-path=/tmp/cxltest2.raw,size=256M \
+ -object memory-backend-ram,id=cxl-mem3,share=on,mem-path=/tmp/cxltest3.raw,size=256M \
+ -device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1 \
+ -device cxl-rp,port=0,bus=cxl.1,id=root_port0,chassis=0,slot=0 \
+ -device cxl-rp,port=1,bus=cxl.1,id=root_port1,chassis=0,slot=1 \
+ -device cxl-upstream,bus=root_port0,id=us0,x-256b-flit=on \
+ -device cxl-downstream,port=0,bus=us0,id=swport0,chassis=0,slot=4 \
+ -device cxl-type3,bus=swport0,volatile-memdev=cxl-mem0,id=cxl-mem0,sn=0x1,x-256b-flit=on,hdm-db=on \
+ -device cxl-downstream,port=1,bus=us0,id=swport1,chassis=0,slot=5 \
+ -device cxl-type3,bus=swport1,volatile-memdev=cxl-mem1,id=cxl-mem1,sn=0x2,x-256b-flit=on,hdm-db=on \
+ -device cxl-downstream,port=2,bus=us0,id=swport2,chassis=0,slot=6 \
+ -device cxl-type3,bus=swport2,volatile-memdev=cxl-mem2,id=cxl-mem2,sn=0x3 \
+ -device cxl-downstream,port=3,bus=us0,id=swport3,chassis=0,slot=7 \
+ -device cxl-type3,bus=swport3,volatile-memdev=cxl-mem3,id=cxl-mem3,sn=0x4 \
+ -M cxl-fmw.0.targets.0=cxl.1,cxl-fmw.0.size=4G,cxl-fmw.0.interleave-granularity=4k
+
A simple arm/virt example featuring a single direct connected CXL Type 3
Volatile Memory device::
diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
index a43d227336ca..2098e9999a88 100644
--- a/hw/cxl/cxl-component-utils.c
+++ b/hw/cxl/cxl-component-utils.c
@@ -71,10 +71,40 @@ static uint64_t cxl_cache_mem_read_reg(void *opaque, hwaddr offset,
case 4:
if (cregs->special_ops && cregs->special_ops->read) {
return cregs->special_ops->read(cxl_cstate, offset, 4);
- } else {
- QEMU_BUILD_BUG_ON(sizeof(*cregs->cache_mem_registers) != 4);
- return cregs->cache_mem_registers[offset / 4];
}
+
+ QEMU_BUILD_BUG_ON(sizeof(*cregs->cache_mem_registers) != 4);
+
+ if (offset == A_CXL_BI_RT_STATUS ||
+ offset == A_CXL_BI_DECODER_STATUS) {
+ int type;
+ uint64_t started;
+
+ type = (offset == A_CXL_BI_RT_STATUS) ?
+ CXL_BISTATE_RT : CXL_BISTATE_DECODER;
+ started = cxl_cstate->bi_state[type].last_commit;
+
+ if (started) {
+ uint32_t *cache_mem = cregs->cache_mem_registers;
+ uint32_t val = cache_mem[offset / 4];
+ uint64_t now;
+ int set;
+
+ now = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
+ /* arbitrary 100 ms to do the commit */
+ set = !!(now >= started + 100);
+
+ if (offset == A_CXL_BI_RT_STATUS) {
+ val = FIELD_DP32(val, CXL_BI_RT_STATUS, COMMITTED, set);
+ } else {
+ val = FIELD_DP32(val, CXL_BI_DECODER_STATUS, COMMITTED,
+ set);
+ }
+ stl_le_p((uint8_t *)cache_mem + offset, val);
+ }
+ }
+
+ return cregs->cache_mem_registers[offset / 4];
case 8:
qemu_log_mask(LOG_UNIMP,
"CXL 8 byte cache mem registers not implemented\n");
@@ -123,6 +153,47 @@ static void dumb_hdm_handler(CXLComponentState *cxl_cstate, hwaddr offset,
}
}
+static void bi_handler(CXLComponentState *cxl_cstate, hwaddr offset,
+ uint32_t value)
+{
+ ComponentRegisters *cregs = &cxl_cstate->crb;
+ uint32_t sts, *cache_mem = cregs->cache_mem_registers;
+ bool to_commit = false;
+ int type;
+
+ switch (offset) {
+ case A_CXL_BI_RT_CTRL:
+ to_commit = FIELD_EX32(value, CXL_BI_RT_CTRL, COMMIT);
+ if (to_commit) {
+ sts = cxl_cache_mem_read_reg(cxl_cstate,
+ R_CXL_BI_RT_STATUS, 4);
+ sts = FIELD_DP32(sts, CXL_BI_RT_STATUS, COMMITTED, 0);
+ stl_le_p((uint8_t *)cache_mem + R_CXL_BI_RT_STATUS, sts);
+ type = CXL_BISTATE_RT;
+ }
+ break;
+ case A_CXL_BI_DECODER_CTRL:
+ to_commit = FIELD_EX32(value, CXL_BI_DECODER_CTRL, COMMIT);
+ if (to_commit) {
+ sts = cxl_cache_mem_read_reg(cxl_cstate,
+ R_CXL_BI_DECODER_STATUS, 4);
+ sts = FIELD_DP32(sts, CXL_BI_DECODER_STATUS, COMMITTED, 0);
+ stl_le_p((uint8_t *)cache_mem + R_CXL_BI_DECODER_STATUS, sts);
+ type = CXL_BISTATE_DECODER;
+ }
+ break;
+ default:
+ break;
+ }
+
+ if (to_commit) {
+ cxl_cstate->bi_state[type].last_commit =
+ qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
+ }
+
+ stl_le_p((uint8_t *)cache_mem + offset, value);
+}
+
static void cxl_cache_mem_write_reg(void *opaque, hwaddr offset, uint64_t value,
unsigned size)
{
@@ -146,6 +217,9 @@ static void cxl_cache_mem_write_reg(void *opaque, hwaddr offset, uint64_t value,
if (offset >= A_CXL_HDM_DECODER_CAPABILITY &&
offset <= A_CXL_HDM_DECODER3_TARGET_LIST_HI) {
dumb_hdm_handler(cxl_cstate, offset, value);
+ } else if (offset == A_CXL_BI_RT_CTRL ||
+ offset == A_CXL_BI_DECODER_CTRL) {
+ bi_handler(cxl_cstate, offset, value);
} else {
cregs->cache_mem_registers[offset / 4] = value;
}
@@ -235,7 +309,7 @@ static void ras_init_common(uint32_t *reg_state, uint32_t *write_msk)
}
static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
- enum reg_type type)
+ enum reg_type type, bool bi)
{
int decoder_count = CXL_HDM_DECODER_COUNT;
int hdm_inc = R_CXL_HDM_DECODER1_BASE_LO - R_CXL_HDM_DECODER0_BASE_LO;
@@ -260,7 +334,9 @@ static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
UIO_DECODER_COUNT, 0);
ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, MEMDATA_NXM_CAP, 0);
ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY,
- SUPPORTED_COHERENCY_MODEL, 0); /* Unknown */
+ SUPPORTED_COHERENCY_MODEL,
+ /* host+dev or Unknown */
+ type == CXL2_TYPE3_DEVICE && bi ? 3 : 0);
ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_GLOBAL_CONTROL,
HDM_DECODER_ENABLE, 0);
write_msk[R_CXL_HDM_DECODER_GLOBAL_CONTROL] = 0x3;
@@ -271,8 +347,7 @@ static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
write_msk[R_CXL_HDM_DECODER0_SIZE_HI + i * hdm_inc] = 0xffffffff;
write_msk[R_CXL_HDM_DECODER0_CTRL + i * hdm_inc] = 0x13ff;
if (type == CXL2_DEVICE ||
- type == CXL2_TYPE3_DEVICE ||
- type == CXL2_LOGICAL_DEVICE) {
+ type == CXL2_TYPE3_DEVICE || type == CXL2_LOGICAL_DEVICE) {
write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * hdm_inc] =
0xf0000000;
} else {
@@ -283,9 +358,43 @@ static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
}
}
+static void bi_rt_init_common(uint32_t *reg_state, uint32_t *write_msk)
+{
+ /* switch usp must commit the new BI-ID, timeout of 2secs */
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_RT_CAPABILITY, EXPLICIT_COMMIT, 1);
+
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_RT_CTRL, COMMIT, 0);
+ write_msk[R_CXL_BI_RT_CTRL] = 0x1;
+
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_RT_STATUS, COMMITTED, 0);
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_RT_STATUS, ERR_NOT_COMMITTED, 0);
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_RT_STATUS, COMMIT_TMO_SCALE, 0x6);
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_RT_STATUS, COMMIT_TMO_BASE, 0x2);
+}
+
+static void bi_decoder_init_common(uint32_t *reg_state, uint32_t *write_msk,
+ enum reg_type type)
+{
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_DECODER_CAPABILITY, HDM_D, 0);
+ /* switch dsp must commit the new BI-ID, timeout of 2secs */
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_DECODER_CAPABILITY, EXPLICIT_COMMIT,
+ (type != CXL2_ROOT_PORT && type != CXL2_TYPE3_DEVICE));
+
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_DECODER_CTRL, BI_FW, 0);
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_DECODER_CTRL, BI_ENABLE, 0);
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_DECODER_CTRL, COMMIT, 0);
+ write_msk[R_CXL_BI_DECODER_CTRL] = 0x7;
+
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_DECODER_STATUS, COMMITTED, 0);
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_DECODER_STATUS, ERR_NOT_COMMITTED, 0);
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_DECODER_STATUS, COMMIT_TMO_SCALE, 0x6);
+ ARRAY_FIELD_DP32(reg_state, CXL_BI_DECODER_STATUS, COMMIT_TMO_BASE, 0x2);
+}
+
void cxl_component_register_init_common(uint32_t *reg_state,
uint32_t *write_msk,
- enum reg_type type)
+ enum reg_type type,
+ bool bi)
{
int caps = 0;
@@ -325,7 +434,7 @@ void cxl_component_register_init_common(uint32_t *reg_state,
case CXL2_LOGICAL_DEVICE:
/* + HDM */
init_cap_reg(HDM, 5, 1);
- hdm_init_common(reg_state, write_msk, type);
+ hdm_init_common(reg_state, write_msk, type, bi);
/* fallthrough */
case CXL2_DOWNSTREAM_PORT:
case CXL2_DEVICE:
@@ -340,6 +449,26 @@ void cxl_component_register_init_common(uint32_t *reg_state,
abort();
}
+ /* back invalidate */
+ switch (type) {
+ case CXL2_UPSTREAM_PORT:
+ init_cap_reg(BI_RT, 11, CXL_BI_RT_CAP_VERSION);
+ bi_rt_init_common(reg_state, write_msk);
+ break;
+ case CXL2_ROOT_PORT:
+ case CXL2_DOWNSTREAM_PORT:
+ case CXL2_TYPE3_DEVICE:
+ if (type == CXL2_TYPE3_DEVICE && !bi) {
+ break;
+ }
+
+ init_cap_reg(BI_DECODER, 12, CXL_BI_DECODER_CAP_VERSION);
+ bi_decoder_init_common(reg_state, write_msk, type);
+ break;
+ default:
+ break;
+ }
+
ARRAY_FIELD_DP32(reg_state, CXL_CAPABILITY_HEADER, ARRAY_SIZE, caps);
#undef init_cap_reg
}
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 891b75618892..cac69adf6b76 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -969,6 +969,11 @@ static bool cxl_setup_memory(CXLType3Dev *ct3d, Error **errp)
return false;
}
+ if (!ct3d->flitmode && ct3d->hdmdb) {
+ error_setg(errp, "hdm-db requires operating in 256b flit");
+ return false;
+ }
+
if (ct3d->hostvmem) {
MemoryRegion *vmr;
char *v_name;
@@ -1504,7 +1509,8 @@ void ct3d_reset(DeviceState *dev)
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_component_register_init_common(reg_state, write_msk,
+ CXL2_TYPE3_DEVICE, ct3d->hdmdb);
cxl_device_register_init_t3(ct3d, CXL_T3_MSIX_MBOX);
/*
@@ -1544,6 +1550,7 @@ static const Property ct3_props[] = {
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),
+ DEFINE_PROP_BOOL("hdm-db", CXLType3Dev, hdmdb, false),
};
static uint64_t get_lsa_size(CXLType3Dev *ct3d)
diff --git a/hw/pci-bridge/cxl_downstream.c b/hw/pci-bridge/cxl_downstream.c
index f8d64263ac08..e0593e783803 100644
--- a/hw/pci-bridge/cxl_downstream.c
+++ b/hw/pci-bridge/cxl_downstream.c
@@ -42,7 +42,7 @@ static void latch_registers(CXLDownstreamPort *dsp)
uint32_t *write_msk = dsp->cxl_cstate.crb.cache_mem_regs_write_mask;
cxl_component_register_init_common(reg_state, write_msk,
- CXL2_DOWNSTREAM_PORT);
+ CXL2_DOWNSTREAM_PORT, false);
}
/* TODO: Look at sharing this code across all CXL port types */
diff --git a/hw/pci-bridge/cxl_root_port.c b/hw/pci-bridge/cxl_root_port.c
index f3472f081707..1c0087d3f111 100644
--- a/hw/pci-bridge/cxl_root_port.c
+++ b/hw/pci-bridge/cxl_root_port.c
@@ -106,7 +106,8 @@ static void latch_registers(CXLRootPort *crp)
uint32_t *reg_state = crp->cxl_cstate.crb.cache_mem_registers;
uint32_t *write_msk = crp->cxl_cstate.crb.cache_mem_regs_write_mask;
- cxl_component_register_init_common(reg_state, write_msk, CXL2_ROOT_PORT);
+ cxl_component_register_init_common(reg_state, write_msk,
+ CXL2_ROOT_PORT, false);
}
static void build_dvsecs(PCIDevice *d, CXLComponentState *cxl)
diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
index e5a0d1fb308c..4bc185df8c87 100644
--- a/hw/pci-bridge/cxl_upstream.c
+++ b/hw/pci-bridge/cxl_upstream.c
@@ -136,7 +136,7 @@ static void latch_registers(CXLUpstreamPort *usp)
uint32_t *write_msk = usp->cxl_cstate.crb.cache_mem_regs_write_mask;
cxl_component_register_init_common(reg_state, write_msk,
- CXL2_UPSTREAM_PORT);
+ CXL2_UPSTREAM_PORT, false);
ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, TARGET_COUNT, 8);
}
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 3a29dfefc2c3..a8f7bf68f1d2 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -302,7 +302,7 @@ static void pxb_cxl_dev_reset(DeviceState *dev)
uint32_t *write_msk = cxl_cstate->crb.cache_mem_regs_write_mask;
int dsp_count = 0;
- cxl_component_register_init_common(reg_state, write_msk, CXL2_RC);
+ cxl_component_register_init_common(reg_state, write_msk, CXL2_RC, false);
/*
* The CXL specification allows for host bridges with no HDM decoders
* if they only have a single root port.
diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
index cd92cb02532a..0ff9f5b0fddf 100644
--- a/include/hw/cxl/cxl_component.h
+++ b/include/hw/cxl/cxl_component.h
@@ -67,6 +67,8 @@ CXLx_CAPABILITY_HEADER(LINK, 2)
CXLx_CAPABILITY_HEADER(HDM, 3)
CXLx_CAPABILITY_HEADER(EXTSEC, 4)
CXLx_CAPABILITY_HEADER(SNOOP, 5)
+CXLx_CAPABILITY_HEADER(BI_RT, 6)
+CXLx_CAPABILITY_HEADER(BI_DECODER, 7)
/*
* Capability structures contain the actual registers that the CXL component
@@ -211,10 +213,55 @@ HDM_DECODER_INIT(3);
(CXL_IDE_REGISTERS_OFFSET + CXL_IDE_REGISTERS_SIZE)
#define CXL_SNOOP_REGISTERS_SIZE 0x8
-QEMU_BUILD_BUG_MSG((CXL_SNOOP_REGISTERS_OFFSET +
- CXL_SNOOP_REGISTERS_SIZE) >= 0x1000,
+#define CXL_BI_RT_CAP_VERSION 1
+#define CXL_BI_RT_REGISTERS_OFFSET \
+ (CXL_SNOOP_REGISTERS_OFFSET + CXL_SNOOP_REGISTERS_SIZE)
+#define CXL_BI_RT_REGISTERS_SIZE 0xC
+
+REG32(CXL_BI_RT_CAPABILITY, CXL_BI_RT_REGISTERS_OFFSET)
+ FIELD(CXL_BI_RT_CAPABILITY, EXPLICIT_COMMIT, 0, 1)
+REG32(CXL_BI_RT_CTRL, CXL_BI_RT_REGISTERS_OFFSET + 0x4)
+ FIELD(CXL_BI_RT_CTRL, COMMIT, 0, 1)
+REG32(CXL_BI_RT_STATUS, CXL_BI_RT_REGISTERS_OFFSET + 0x8)
+ FIELD(CXL_BI_RT_STATUS, COMMITTED, 0, 1)
+ FIELD(CXL_BI_RT_STATUS, ERR_NOT_COMMITTED, 1, 1)
+ FIELD(CXL_BI_RT_STATUS, COMMIT_TMO_SCALE, 8, 4)
+ FIELD(CXL_BI_RT_STATUS, COMMIT_TMO_BASE, 12, 4)
+
+/* CXL r3.2 8.2.4.27 - CXL BI Decoder Capability Structure */
+#define CXL_BI_DECODER_CAP_VERSION 1
+#define CXL_BI_DECODER_REGISTERS_OFFSET \
+ (CXL_BI_RT_REGISTERS_OFFSET + CXL_BI_RT_REGISTERS_SIZE)
+#define CXL_BI_DECODER_REGISTERS_SIZE 0xC
+
+REG32(CXL_BI_DECODER_CAPABILITY, CXL_BI_DECODER_REGISTERS_OFFSET)
+ FIELD(CXL_BI_DECODER_CAPABILITY, HDM_D, 0, 1)
+ FIELD(CXL_BI_DECODER_CAPABILITY, EXPLICIT_COMMIT, 1, 1)
+REG32(CXL_BI_DECODER_CTRL, CXL_BI_DECODER_REGISTERS_OFFSET + 0x4)
+ FIELD(CXL_BI_DECODER_CTRL, BI_FW, 0, 1)
+ FIELD(CXL_BI_DECODER_CTRL, BI_ENABLE, 1, 1)
+ FIELD(CXL_BI_DECODER_CTRL, COMMIT, 2, 1)
+REG32(CXL_BI_DECODER_STATUS, CXL_BI_DECODER_REGISTERS_OFFSET + 0x8)
+ FIELD(CXL_BI_DECODER_STATUS, COMMITTED, 0, 1)
+ FIELD(CXL_BI_DECODER_STATUS, ERR_NOT_COMMITTED, 1, 1)
+ FIELD(CXL_BI_DECODER_STATUS, COMMIT_TMO_SCALE, 8, 4)
+ FIELD(CXL_BI_DECODER_STATUS, COMMIT_TMO_BASE, 12, 4)
+
+QEMU_BUILD_BUG_MSG((CXL_BI_DECODER_REGISTERS_OFFSET +
+ CXL_BI_DECODER_REGISTERS_SIZE) >= 0x1000,
"No space for registers");
+/* track BI explicit commit handling for route table and decoder */
+enum {
+ CXL_BISTATE_RT = 0,
+ CXL_BISTATE_DECODER,
+ CXL_BISTATE_MAX
+};
+
+typedef struct bi_state {
+ uint64_t last_commit; /* last 0->1 transition */
+} BIState;
+
typedef struct component_registers {
/*
* Main memory region to be registered with QEMU core.
@@ -260,6 +307,7 @@ typedef struct cxl_component {
CDATObject cdat;
CXLCompObject compliance;
+ BIState bi_state[CXL_BISTATE_MAX];
} CXLComponentState;
void cxl_component_register_block_init(Object *obj,
@@ -267,7 +315,7 @@ void cxl_component_register_block_init(Object *obj,
const char *type);
void cxl_component_register_init_common(uint32_t *reg_state,
uint32_t *write_msk,
- enum reg_type type);
+ enum reg_type type, bool bi);
void cxl_component_create_dvsec(CXLComponentState *cxl_cstate,
enum reg_type cxl_dev_type, uint16_t length,
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index 0abfd678b875..75603b8180b5 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -841,6 +841,9 @@ struct CXLType3Dev {
CXLMemSparingReadAttrs rank_sparing_attrs;
CXLMemSparingWriteAttrs rank_sparing_wr_attrs;
+ /* BI flows */
+ bool hdmdb;
+
struct dynamic_capacity {
HostMemoryBackend *host_dc;
AddressSpace host_dc_as;
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 4/5] hw/cxl: Support type3 HDM-DB
2025-09-30 3:21 ` [PATCH 4/5] hw/cxl: Support type3 HDM-DB Davidlohr Bueso
@ 2025-09-30 15:36 ` Jonathan Cameron via
2025-10-08 21:18 ` Davidlohr Bueso
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron via @ 2025-09-30 15:36 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: ira.weiny, lucerop, a.manzanares, mst, marcel.apfelbaum, armbru,
linux-cxl, qemu-devel
On Mon, 29 Sep 2025 20:21:52 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:
> Add basic plumbing for memory expander devices that support Back
> Invalidation. This introduces a 'hdm-db=on|off' parameter and
> exposes the relevant BI RT/Decoder component cachemem registers.
>
> Some noteworthy properties:
> - Devices require enabling Flit mode.
> - Explicit BI-ID commit is required.
> - HDM decoder support both host and dev coherency models.
>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
Hi Davidlohr,
Comments inline mostly focus on the bi parameter. I think flipping
it to true for components where we are hard coding it as true will
move that logic decision up a layer and make the code easier to follow.
Thanks,
Jonathan
> diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> index a43d227336ca..2098e9999a88 100644
> --- a/hw/cxl/cxl-component-utils.c
> +++ b/hw/cxl/cxl-component-utils.c
> @@ -235,7 +309,7 @@ static void ras_init_common(uint32_t *reg_state, uint32_t *write_msk)
> }
>
> static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
> - enum reg_type type)
> + enum reg_type type, bool bi)
> {
> int decoder_count = CXL_HDM_DECODER_COUNT;
> int hdm_inc = R_CXL_HDM_DECODER1_BASE_LO - R_CXL_HDM_DECODER0_BASE_LO;
> @@ -260,7 +334,9 @@ static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
> UIO_DECODER_COUNT, 0);
> ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, MEMDATA_NXM_CAP, 0);
> ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY,
> - SUPPORTED_COHERENCY_MODEL, 0); /* Unknown */
> + SUPPORTED_COHERENCY_MODEL,
> + /* host+dev or Unknown */
> + type == CXL2_TYPE3_DEVICE && bi ? 3 : 0);
> ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_GLOBAL_CONTROL,
> HDM_DECODER_ENABLE, 0);
> write_msk[R_CXL_HDM_DECODER_GLOBAL_CONTROL] = 0x3;
> @@ -271,8 +347,7 @@ static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
> write_msk[R_CXL_HDM_DECODER0_SIZE_HI + i * hdm_inc] = 0xffffffff;
> write_msk[R_CXL_HDM_DECODER0_CTRL + i * hdm_inc] = 0x13ff;
> if (type == CXL2_DEVICE ||
> - type == CXL2_TYPE3_DEVICE ||
> - type == CXL2_LOGICAL_DEVICE) {
> + type == CXL2_TYPE3_DEVICE || type == CXL2_LOGICAL_DEVICE) {
Unrelated change? Or am I missing something real here?
> write_msk[R_CXL_HDM_DECODER0_TARGET_LIST_LO + i * hdm_inc] =
> 0xf0000000;
> } else {
> @@ -283,9 +358,43 @@ static void hdm_init_common(uint32_t *reg_state, uint32_t *write_msk,
> void cxl_component_register_init_common(uint32_t *reg_state,
> uint32_t *write_msk,
> - enum reg_type type)
> + enum reg_type type,
> + bool bi)
I wonder if we shouldn't set bi for all 3 component types that actually
have the BI related capability? For Type3 we keep it controllable
and for DSP and RP hard code the parameter to true.
> {
> int caps = 0;
>
> @@ -325,7 +434,7 @@ void cxl_component_register_init_common(uint32_t *reg_state,
> case CXL2_LOGICAL_DEVICE:
> /* + HDM */
> init_cap_reg(HDM, 5, 1);
> - hdm_init_common(reg_state, write_msk, type);
> + hdm_init_common(reg_state, write_msk, type, bi);
> /* fallthrough */
> case CXL2_DOWNSTREAM_PORT:
> case CXL2_DEVICE:
> @@ -340,6 +449,26 @@ void cxl_component_register_init_common(uint32_t *reg_state,
> abort();
> }
>
> + /* back invalidate */
With bi set true in cases where there is anything to do here, could wrap
this in an if (bi)
> + switch (type) {
> + case CXL2_UPSTREAM_PORT:
> + init_cap_reg(BI_RT, 11, CXL_BI_RT_CAP_VERSION);
> + bi_rt_init_common(reg_state, write_msk);
> + break;
> + case CXL2_ROOT_PORT:
> + case CXL2_DOWNSTREAM_PORT:
> + case CXL2_TYPE3_DEVICE:
> + if (type == CXL2_TYPE3_DEVICE && !bi) {
With the values for the other types tweaked this check becomes unnecessary
> + break;
> + }
> +
> + init_cap_reg(BI_DECODER, 12, CXL_BI_DECODER_CAP_VERSION);
> + bi_decoder_init_common(reg_state, write_msk, type);
> + break;
> + default:
> + break;
> + }
> +
> ARRAY_FIELD_DP32(reg_state, CXL_CAPABILITY_HEADER, ARRAY_SIZE, caps);
> #undef init_cap_reg
> }
>
> static uint64_t get_lsa_size(CXLType3Dev *ct3d)
> diff --git a/hw/pci-bridge/cxl_downstream.c b/hw/pci-bridge/cxl_downstream.c
> index f8d64263ac08..e0593e783803 100644
> --- a/hw/pci-bridge/cxl_downstream.c
> +++ b/hw/pci-bridge/cxl_downstream.c
> @@ -42,7 +42,7 @@ static void latch_registers(CXLDownstreamPort *dsp)
> uint32_t *write_msk = dsp->cxl_cstate.crb.cache_mem_regs_write_mask;
>
> cxl_component_register_init_common(reg_state, write_msk,
> - CXL2_DOWNSTREAM_PORT);
> + CXL2_DOWNSTREAM_PORT, false);
This false briefly confused me and is the reason for comment above.
DSPs and RPs have BI support and it's odd to set a parameter here called
bi to false, only to enable it always.
> }
>
> /* TODO: Look at sharing this code across all CXL port types */
> diff --git a/hw/pci-bridge/cxl_root_port.c b/hw/pci-bridge/cxl_root_port.c
> index f3472f081707..1c0087d3f111 100644
> --- a/hw/pci-bridge/cxl_root_port.c
> +++ b/hw/pci-bridge/cxl_root_port.c
> @@ -106,7 +106,8 @@ static void latch_registers(CXLRootPort *crp)
> uint32_t *reg_state = crp->cxl_cstate.crb.cache_mem_registers;
> uint32_t *write_msk = crp->cxl_cstate.crb.cache_mem_regs_write_mask;
>
> - cxl_component_register_init_common(reg_state, write_msk, CXL2_ROOT_PORT);
> + cxl_component_register_init_common(reg_state, write_msk,
> + CXL2_ROOT_PORT, false);
Same here. Also I think under 80 chars with CXL2_ROOT_PORT, on first line
> }
>
> static void build_dvsecs(PCIDevice *d, CXLComponentState *cxl)
> diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
> index e5a0d1fb308c..4bc185df8c87 100644
> --- a/hw/pci-bridge/cxl_upstream.c
> +++ b/hw/pci-bridge/cxl_upstream.c
> @@ -136,7 +136,7 @@ static void latch_registers(CXLUpstreamPort *usp)
> uint32_t *write_msk = usp->cxl_cstate.crb.cache_mem_regs_write_mask;
>
> cxl_component_register_init_common(reg_state, write_msk,
> - CXL2_UPSTREAM_PORT);
> + CXL2_UPSTREAM_PORT, false);
Obviously different structure but still seems odd that bi is false yet we create
BI_RT structure in this call.
> ARRAY_FIELD_DP32(reg_state, CXL_HDM_DECODER_CAPABILITY, TARGET_COUNT, 8);
> }
> diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
> index cd92cb02532a..0ff9f5b0fddf 100644
> --- a/include/hw/cxl/cxl_component.h
> +++ b/include/hw/cxl/cxl_component.h
> @@ -67,6 +67,8 @@ CXLx_CAPABILITY_HEADER(LINK, 2)
> CXLx_CAPABILITY_HEADER(HDM, 3)
> CXLx_CAPABILITY_HEADER(EXTSEC, 4)
> CXLx_CAPABILITY_HEADER(SNOOP, 5)
> +CXLx_CAPABILITY_HEADER(BI_RT, 6)
> +CXLx_CAPABILITY_HEADER(BI_DECODER, 7)
>
> /*
> * Capability structures contain the actual registers that the CXL component
> @@ -211,10 +213,55 @@ HDM_DECODER_INIT(3);
> (CXL_IDE_REGISTERS_OFFSET + CXL_IDE_REGISTERS_SIZE)
> #define CXL_SNOOP_REGISTERS_SIZE 0x8
>
> -QEMU_BUILD_BUG_MSG((CXL_SNOOP_REGISTERS_OFFSET +
> - CXL_SNOOP_REGISTERS_SIZE) >= 0x1000,
> +#define CXL_BI_RT_CAP_VERSION 1
> +#define CXL_BI_RT_REGISTERS_OFFSET \
> + (CXL_SNOOP_REGISTERS_OFFSET + CXL_SNOOP_REGISTERS_SIZE)
> +#define CXL_BI_RT_REGISTERS_SIZE 0xC
> +
> +REG32(CXL_BI_RT_CAPABILITY, CXL_BI_RT_REGISTERS_OFFSET)
> + FIELD(CXL_BI_RT_CAPABILITY, EXPLICIT_COMMIT, 0, 1)
> +REG32(CXL_BI_RT_CTRL, CXL_BI_RT_REGISTERS_OFFSET + 0x4)
> + FIELD(CXL_BI_RT_CTRL, COMMIT, 0, 1)
> +REG32(CXL_BI_RT_STATUS, CXL_BI_RT_REGISTERS_OFFSET + 0x8)
> + FIELD(CXL_BI_RT_STATUS, COMMITTED, 0, 1)
> + FIELD(CXL_BI_RT_STATUS, ERR_NOT_COMMITTED, 1, 1)
> + FIELD(CXL_BI_RT_STATUS, COMMIT_TMO_SCALE, 8, 4)
> + FIELD(CXL_BI_RT_STATUS, COMMIT_TMO_BASE, 12, 4)
> +
> +/* CXL r3.2 8.2.4.27 - CXL BI Decoder Capability Structure */
> +#define CXL_BI_DECODER_CAP_VERSION 1
> +#define CXL_BI_DECODER_REGISTERS_OFFSET \
> + (CXL_BI_RT_REGISTERS_OFFSET + CXL_BI_RT_REGISTERS_SIZE)
> +#define CXL_BI_DECODER_REGISTERS_SIZE 0xC
> +
> +REG32(CXL_BI_DECODER_CAPABILITY, CXL_BI_DECODER_REGISTERS_OFFSET)
> + FIELD(CXL_BI_DECODER_CAPABILITY, HDM_D, 0, 1)
> + FIELD(CXL_BI_DECODER_CAPABILITY, EXPLICIT_COMMIT, 1, 1)
> +REG32(CXL_BI_DECODER_CTRL, CXL_BI_DECODER_REGISTERS_OFFSET + 0x4)
> + FIELD(CXL_BI_DECODER_CTRL, BI_FW, 0, 1)
> + FIELD(CXL_BI_DECODER_CTRL, BI_ENABLE, 1, 1)
> + FIELD(CXL_BI_DECODER_CTRL, COMMIT, 2, 1)
> +REG32(CXL_BI_DECODER_STATUS, CXL_BI_DECODER_REGISTERS_OFFSET + 0x8)
> + FIELD(CXL_BI_DECODER_STATUS, COMMITTED, 0, 1)
> + FIELD(CXL_BI_DECODER_STATUS, ERR_NOT_COMMITTED, 1, 1)
> + FIELD(CXL_BI_DECODER_STATUS, COMMIT_TMO_SCALE, 8, 4)
> + FIELD(CXL_BI_DECODER_STATUS, COMMIT_TMO_BASE, 12, 4)
> +
> +QEMU_BUILD_BUG_MSG((CXL_BI_DECODER_REGISTERS_OFFSET +
> + CXL_BI_DECODER_REGISTERS_SIZE) >= 0x1000,
> "No space for registers");
>
> +/* track BI explicit commit handling for route table and decoder */
> +enum {
> + CXL_BISTATE_RT = 0,
> + CXL_BISTATE_DECODER,
> + CXL_BISTATE_MAX
> +};
> +
> +typedef struct bi_state {
> + uint64_t last_commit; /* last 0->1 transition */
> +} BIState;
> +
> typedef struct component_registers {
> /*
> * Main memory region to be registered with QEMU core.
> @@ -260,6 +307,7 @@ typedef struct cxl_component {
>
> CDATObject cdat;
> CXLCompObject compliance;
> + BIState bi_state[CXL_BISTATE_MAX];
> } CXLComponentState;
>
> void cxl_component_register_block_init(Object *obj,
> @@ -267,7 +315,7 @@ void cxl_component_register_block_init(Object *obj,
> const char *type);
> void cxl_component_register_init_common(uint32_t *reg_state,
> uint32_t *write_msk,
> - enum reg_type type);
> + enum reg_type type, bool bi);
>
> void cxl_component_create_dvsec(CXLComponentState *cxl_cstate,
> enum reg_type cxl_dev_type, uint16_t length,
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index 0abfd678b875..75603b8180b5 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -841,6 +841,9 @@ struct CXLType3Dev {
> CXLMemSparingReadAttrs rank_sparing_attrs;
> CXLMemSparingWriteAttrs rank_sparing_wr_attrs;
>
> + /* BI flows */
> + bool hdmdb;
> +
> struct dynamic_capacity {
> HostMemoryBackend *host_dc;
> AddressSpace host_dc_as;
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 4/5] hw/cxl: Support type3 HDM-DB
2025-09-30 15:36 ` Jonathan Cameron via
@ 2025-10-08 21:18 ` Davidlohr Bueso
0 siblings, 0 replies; 10+ messages in thread
From: Davidlohr Bueso @ 2025-10-08 21:18 UTC (permalink / raw)
To: Jonathan Cameron
Cc: ira.weiny, lucerop, a.manzanares, mst, marcel.apfelbaum, armbru,
linux-cxl, qemu-devel
On Tue, 30 Sep 2025, Jonathan Cameron wrote:
>On Mon, 29 Sep 2025 20:21:52 -0700
>Davidlohr Bueso <dave@stgolabs.net> wrote:
>
>> Add basic plumbing for memory expander devices that support Back
>> Invalidation. This introduces a 'hdm-db=on|off' parameter and
>> exposes the relevant BI RT/Decoder component cachemem registers.
>>
>> Some noteworthy properties:
>> - Devices require enabling Flit mode.
>> - Explicit BI-ID commit is required.
>> - HDM decoder support both host and dev coherency models.
>>
>> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
>Hi Davidlohr,
>
>Comments inline mostly focus on the bi parameter. I think flipping
>it to true for components where we are hard coding it as true will
>move that logic decision up a layer and make the code easier to follow.
Agreed.
...
>> diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
>> index e5a0d1fb308c..4bc185df8c87 100644
>> --- a/hw/pci-bridge/cxl_upstream.c
>> +++ b/hw/pci-bridge/cxl_upstream.c
>> @@ -136,7 +136,7 @@ static void latch_registers(CXLUpstreamPort *usp)
>> uint32_t *write_msk = usp->cxl_cstate.crb.cache_mem_regs_write_mask;
>>
>> cxl_component_register_init_common(reg_state, write_msk,
>> - CXL2_UPSTREAM_PORT);
>> + CXL2_UPSTREAM_PORT, false);
>
>Obviously different structure but still seems odd that bi is false yet we create
>BI_RT structure in this call.
I'm thinking maybe use usp->flitmode instead. Yeah it's not ideal to associate
them like that, but it makes less sense to always export the registers if 256B
is not even enabled. Otherwise now this would have to be hard coded to true as well.
Thanks,
Davidlohr
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 5/5] hw/cxl: Remove register special_ops->read()
2025-09-30 3:21 [PATCH v3 -qemu 0/5] hw/cxl: Support Back-Invalidate Davidlohr Bueso
` (3 preceding siblings ...)
2025-09-30 3:21 ` [PATCH 4/5] hw/cxl: Support type3 HDM-DB Davidlohr Bueso
@ 2025-09-30 3:21 ` Davidlohr Bueso
4 siblings, 0 replies; 10+ messages in thread
From: Davidlohr Bueso @ 2025-09-30 3:21 UTC (permalink / raw)
To: jonathan.cameron
Cc: ira.weiny, lucerop, a.manzanares, mst, marcel.apfelbaum, armbru,
linux-cxl, qemu-devel, dave, Jonathan.Cameron
... this is unused, unlike its write counterpart.
Suggested-by: <Jonathan.Cameron@huawei.com>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
hw/cxl/cxl-component-utils.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
index 2098e9999a88..47a8d905f5b7 100644
--- a/hw/cxl/cxl-component-utils.c
+++ b/hw/cxl/cxl-component-utils.c
@@ -69,10 +69,6 @@ static uint64_t cxl_cache_mem_read_reg(void *opaque, hwaddr offset,
switch (size) {
case 4:
- if (cregs->special_ops && cregs->special_ops->read) {
- return cregs->special_ops->read(cxl_cstate, offset, 4);
- }
-
QEMU_BUILD_BUG_ON(sizeof(*cregs->cache_mem_registers) != 4);
if (offset == A_CXL_BI_RT_STATUS ||
--
2.39.5
^ permalink raw reply related [flat|nested] 10+ messages in thread