* [PATCH v5 0/5] cxl: Support Back-Invalidate
@ 2026-06-15 14:55 Davidlohr Bueso
2026-06-15 14:55 ` [PATCH v5 1/5] cxl: Add BI register probing and port initialization Davidlohr Bueso
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Davidlohr Bueso @ 2026-06-15 14:55 UTC (permalink / raw)
To: dave.jiang, djbw
Cc: jic23, benjamin.cheatham, icheng, alucerop, alison.schofield,
gourry, dongjoo.seo1, dave, linux-cxl
Hello,
Changes from v4 (https://lore.kernel.org/all/20260605003329.2584012-1-dave@stgolabs.net):
- Patch 1 moved cxl_port_map_bi() into cxl_port_add_dport() (Ben, Richard, Sashiko).
- Patch 2 fixed readx_poll_timeout() for base = 0 (Richard, Sashiko).
- Patch 2 changed some dev_dgb to dev_err and dev_warn during probing and configuration (Richard, Ben).
- Patch 4 fixed corner case for type2 in cxl_region_attach() (Richard).
- Patch 4 added the type mismatch WARN back in cxl_rr_assign_decoder() (Ben).
- Small cleanups + picked up review tags.
This series is the initial plumbing to enable HDM-DB in Linux. The
model allows Type 2 and Type 3 devices to expose their local memory
to the host CPU in a coherent manner. In alignment with what was
discussed at 2024 LPC type2 support session, this series takes the
Type 3 memory expander approach, which is more direct. Further,
afaik there is no Type 2 + BI hardware out there.
A flagship use case of Type 3 + BI is coherent shared memory, and
there is currently a big gap in this regard (ie: GFAM). Another
is P2P via PCIe UIO, which is also lacking today. Media Operation
(4402h) for ranged sanitize/zero also trigger snoops and there are
some patches out there[0]. As such this series focuses on BI enablement
in terms of discovery and configuration.
The idea is for Type 3 memdevs and future Type 2 devices to make use
of cxlds->bi when committing HDM decoders, such that different device
coherence models can be differentiated as:
type2 hdm-d: cxlds->type == CXL_DEVTYPE_DEVMEM && cxlds->bi == false
type2 hdm-db: cxlds->type == CXL_DEVTYPE_DEVMEM && cxlds->bi == true
type3 hdm-h: cxlds->type == CXL_DEVTYPE_CLASSMEM && cxlds->bi == false
type3 hdm-db: cxlds->type == CXL_DEVTYPE_CLASSMEM && cxlds->bi == true
Because ->bi becoming true does not depend on auto-committing upon HDM
decoder enumeration (port driver), for now an already-committed BI
decoder during enumeration is unsupported and the driver errors out.
o Patch 1 probes BI capabilities during register discovery, and maps BI
Decoder registers.
o Patch 2 implements the BI-ID allocation (and deallocation).
o Patch 3 adds coherency support for endpoint decoders based on what the
root decoder restricts.
o Patch 4 adds support for HDM-DB region creation.
o Patch 5 trivially renames some coherency flags.
Testing has been done with the qemu HDM-DB Type 3 counterpart + this series:
https://lore.kernel.org/all/20260602231806.2469011-1-dave@stgolabs.net/
1. HDM Decoder with BI through ad-hoc region creation.
------------------------------------------------------
# cxl list -D
[
{
"decoder":"decoder0.0",
"resource":4563402752,
"size":10737418240,
"interleave_ways":1,
"accelmem_capable":true,
"nr_targets":1
}
]
# ROOT=/sys/bus/cxl/devices/decoder0.0
# EP=/sys/bus/cxl/devices/decoder2.0
# echo region0 > $ROOT/create_ram_region
# REG=/sys/bus/cxl/devices/region0
# echo ram > $EP/mode
# echo 0x40000000 > $EP/dpa_size
# echo 1 > $REG/interleave_ways
# echo 4096 > $REG/interleave_granularity
# echo 0x40000000 > $REG/size
# cat $EP/bi
0
# echo decoder2.0 > $REG/target0
[ 53.495224] cxl_core:cxl_port_attach_region:1290: cxl region0: mem0:endpoint2 decoder2.0 add: mem0:decoder2.0 @ 0 next: none nr_eps:1
[ 53.497758] cxl_core:cxl_port_attach_region:1290: cxl region0: pci0000:0c:port1 decoder1.0 add: mem0:decoder2.0 @ 0 next: mem0 nr_ep1
[ 53.499892] cxl_core:cxl_port_setup_targets:1608: cxl region0: pci0000:0c:port1 iw: 1 ig: 4096
[ 53.501233] cxl_core:cxl_port_setup_targets:1633: cxl region0: pci0000:0c:port1 target[0] = 0000:0c:00.0 for mem0:decoder2.0 @ 0
[ 53.502985] cxl_core:cxl_calc_interleave_pos:2003: cxl_mem mem0: decoder:decoder2.0 parent:0000:0d:00.0 port:endpoint2 range:0x110000
[ 53.504895] cxl_core:cxl_region_attach:2226: cxl decoder2.0: Test cxl_calc_interleave_pos(): success test_pos:0 cxled->pos:0
# cat $EP/bi
1
# echo 1 > $REG/commit
# cxl list -D
[
{
"root decoders":[
{
"decoder":"decoder0.0",
"resource":4563402752,
"size":10737418240,
"interleave_ways":1,
"accelmem_capable":true,
"nr_targets":1
}
]
},
{
"port decoders":[
{
"decoder":"decoder1.0",
"resource":4563402752,
"size":1073741824,
"interleave_ways":1,
"region":"region0",
"nr_targets":1
}
]
},
{
"endpoint decoders":[
{
"decoder":"decoder2.0",
"resource":4563402752,
"size":1073741824,
"interleave_ways":1,
"region":"region0",
"dpa_resource":0,
"dpa_size":1073741824,
"mode":"ram"
}
]
}
]
2. Type3 device does not support BI, but CFMW has BI restriction
----------------------------------------------------------------
# echo decoder2.0 > $REG/target0
[ 104.557797] cxl region0: mem0:decoder2.0 BI not enabled on device
[ 104.561014] cxl_port endpoint2: failed to attach decoder2.0 to region0: -6
3. Type3 device supports BI but CFMW does not, create HDM-H region
------------------------------------------------------------------
# cxl list -D
[
{
"decoder":"decoder0.0",
"resource":4563402752,
"size":10737418240,
"interleave_ways":1,
"max_available_extent":10737418240,
"pmem_capable":true,
"volatile_capable":true,
"nr_targets":1
}
]
# echo decoder2.0 > $REG/target0
[ 124.712027] cxl_core:cxl_port_attach_region:1290: cxl region0: mem0:endpoint2 decoder2.0 add: mem0:decoder2.0 @ 0 next: none nr_eps:1
[ 124.714329] cxl_core:cxl_port_attach_region:1290: cxl region0: pci0000:0c:port1 decoder1.0 add: mem0:decoder2.0 @ 0 next: mem0 nr_ep1
[ 124.716407] cxl_core:cxl_port_setup_targets:1608: cxl region0: pci0000:0c:port1 iw: 1 ig: 4096
[ 124.718269] cxl_core:cxl_port_setup_targets:1633: cxl region0: pci0000:0c:port1 target[0] = 0000:0c:00.0 for mem0:decoder2.0 @ 0
[ 124.722076] cxl_core:cxl_calc_interleave_pos:2003: cxl_mem mem0: decoder:decoder2.0 parent:0000:0d:00.0 port:endpoint2 range:0x110000
[ 124.724454] cxl_core:cxl_region_attach:2226: cxl decoder2.0: Test cxl_calc_interleave_pos(): success test_pos:0 cxled->pos:0
# cxl list -D
[
{
"root decoders":[
{
"decoder":"decoder0.0",
"resource":4563402752,
"size":10737418240,
"interleave_ways":1,
"max_available_extent":9663676416,
"pmem_capable":true,
"volatile_capable":true,
"nr_targets":1
}
]
},
{
"port decoders":[
{
"decoder":"decoder1.0",
"resource":4563402752,
"size":1073741824,
"interleave_ways":1,
"region":"region0",
"nr_targets":1
}
]
},
{
"endpoint decoders":[
{
"decoder":"decoder2.0",
"resource":4563402752,
"size":1073741824,
"interleave_ways":1,
"region":"region0",
"dpa_resource":0,
"dpa_size":1073741824,
"mode":"ram"
}
]
}
]
3. Unbind + Bind
----------------
# echo mem0 > /sys/bus/cxl/drivers/cxl_mem/unbind
[ 64.553865] cxl_core:__cxl_bi_ctrl_endpoint:1163: cxl_pci 0000:0d:00.0: BI requests disabled
[ 64.557464] cxl_core:cxl_port_commit_reap:898: cxl_port port1: reap: decoder1.0 commit_end: -1
[ 64.559529] cxl_core:cxl_port_commit_reap:898: cxl_port endpoint2: reap: decoder2.0 commit_end: -1
[ 64.563576] cxl_core:cxl_detach_ep:1541: cxl_mem mem0: disconnect mem0 from port1
echo mem0 > /sys/bus/cxl/drivers/cxl_mem/bind
[ 5073.975585] cxl_core:devm_cxl_enumerate_ports:1856: cxl_mem mem0: scan: iter: mem0 dport_dev: 0000:0c:00.0 parent: pci0000:0c
[ 5073.978125] cxl_core:devm_cxl_enumerate_ports:1862: cxl_mem mem0: found already registered port port1:pci0000:0c
[ 5073.980013] cxl_core:cxl_port_alloc:752: cxl_mem mem0: host-bridge: pci0000:0c
[ 5073.982601] cxl_core:cxl_cdat_get_length:488: cxl_port endpoint2: CDAT length 160
[ 5073.988634] cxl_core:cxl_port_perf_data_calculate:207: cxl_port endpoint2: Failed to retrieve ep perf coordinates.
[ 5073.991824] cxl_core:cxl_endpoint_parse_cdat:423: cxl_port endpoint2: Failed to do perf coord calculations.
[ 5074.016377] cxl_core:init_hdm_decoder:1112: cxl_port endpoint2: decoder2.0: range: 0x0-0xffffffffffffffff iw: 1 ig: 4096
[ 5074.021643] cxl_core:add_hdm_decoder:39: cxl_mem mem0: decoder2.0 added to endpoint2
[ 5074.023072] cxl_core:init_hdm_decoder:1112: cxl_port endpoint2: decoder2.1: range: 0x0-0xffffffffffffffff iw: 1 ig: 256
[ 5074.025527] cxl_core:add_hdm_decoder:39: cxl_mem mem0: decoder2.1 added to endpoint2
[ 5074.027751] cxl_core:init_hdm_decoder:1112: cxl_port endpoint2: decoder2.2: range: 0x0-0xffffffffffffffff iw: 1 ig: 256
[ 5074.032182] cxl_core:add_hdm_decoder:39: cxl_mem mem0: decoder2.2 added to endpoint2
[ 5074.033535] cxl_core:init_hdm_decoder:1112: cxl_port endpoint2: decoder2.3: range: 0x0-0xffffffffffffffff iw: 1 ig: 256
[ 5074.035690] cxl_core:add_hdm_decoder:39: cxl_mem mem0: decoder2.3 added to endpoint2
[ 5074.036904] cxl_core:cxl_bus_probe:2281: cxl_port endpoint2: probe: 0
[ 5074.038017] cxl_core:devm_cxl_add_port:960: cxl_mem mem0: endpoint2 added to port1
[ 5074.039153] cxl_core:__cxl_bi_ctrl_endpoint:1163: cxl_pci 0000:0d:00.0: BI requests enabled
[ 5074.040168] cxl_core:cxl_bus_probe:2281: cxl_mem mem0: probe: 0
4. Discovery behind a Switch
----------------------------
# dmesg | grep BI
[ 0.760688] cxl_core:cxl_probe_component_regs:102: cxl_pci 0000:0f:00.0: found BI Decoder capability (0xab4)
[ 0.872325] cxl_core:cxl_probe_component_regs:102: pcieport 0000:0c:00.0: found BI Decoder capability (0xab4)
[ 0.888387] cxl_core:cxl_probe_component_regs:96: cxl_port port2: found BI RT capability (0xaa8)
[ 0.924125] cxl_core:cxl_probe_component_regs:102: pcieport 0000:0e:00.0: found BI Decoder capability (0xab4)
[ 1.087716] cxl_core:__cxl_bi_wait_commit:995: pcieport 0000:0e:00.0: BI-ID commit wait took 114533us
[ 1.288190] cxl_core:__cxl_bi_wait_commit:995: cxl_port port2: BI-ID commit wait took 200377us
[ 1.292282] cxl_core:__cxl_bi_ctrl_endpoint:1164: cxl_pci 0000:0f:00.0: BI requests enabled
5. Corrupted hw (commit timeout handling)
-----------------------------------------
[ 0.617234] cxl_core:cxl_probe_component_regs:102: cxl_pci 0000:0f:00.0: found BI Decoder capability (0xab4)
[ 0.706959] cxl_core:cxl_probe_component_regs:102: pcieport 0000:0c:00.0: found BI Decoder capability (0xab4)
[ 0.743932] cxl_core:cxl_probe_component_regs:96: cxl_port port2: found BI RT capability (0xaa8)
[ 0.783715] cxl_core:cxl_probe_component_regs:102: pcieport 0000:0e:00.0: found BI Decoder capability (0xab4)
[ 2.994821] cxl_core:__cxl_bi_wait_commit:985: pcieport 0000:0e:00.0: BI-ID commit timed out (2000000us)
[ 5.010606] cxl_core:__cxl_bi_wait_commit:985: pcieport 0000:0e:00.0: BI-ID commit timed out (2000000us)
[ 5.013423] cxl_mem:cxl_mem_probe:155: cxl_mem mem0: BI setup failed rc=-110
6. Mixed Configurations (BI-capable Type 3 but DSP 68b)
-------------------------------------------------------
[ 0.837902] cxl_mem:cxl_mem_probe:155: cxl_mem mem0: BI setup failed rc=-22
Applies against 'next' branch from cxl.git.
[0] https://lore.kernel.org/linux-cxl/20260428200410.705675-1-dave@stgolabs.net/
Thanks!
Davidlohr Bueso (5):
cxl: Add BI register probing and port initialization
cxl/pci: Add BI topology enable/disable
cxl/hdm: Add BI coherency support for endpoint decoders
cxl: Add HDM-DB region creation
cxl/hdm: Rename decoder coherency flags
Documentation/ABI/testing/sysfs-bus-cxl | 18 +-
drivers/cxl/acpi.c | 15 +-
drivers/cxl/core/hdm.c | 40 ++-
drivers/cxl/core/pci.c | 366 ++++++++++++++++++++++++
drivers/cxl/core/port.c | 36 ++-
drivers/cxl/core/region.c | 46 ++-
drivers/cxl/core/regs.c | 14 +
drivers/cxl/cxl.h | 49 +++-
drivers/cxl/cxlmem.h | 2 +
drivers/cxl/mem.c | 4 +
drivers/cxl/port.c | 71 +++++
include/cxl/cxl.h | 8 +
12 files changed, 645 insertions(+), 24 deletions(-)
--
2.39.5
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 1/5] cxl: Add BI register probing and port initialization
2026-06-15 14:55 [PATCH v5 0/5] cxl: Support Back-Invalidate Davidlohr Bueso
@ 2026-06-15 14:55 ` Davidlohr Bueso
2026-06-15 17:35 ` sashiko-bot
2026-07-01 16:57 ` Dave Jiang
2026-06-15 14:55 ` [PATCH v5 2/5] cxl/pci: Add BI topology enable/disable Davidlohr Bueso
` (3 subsequent siblings)
4 siblings, 2 replies; 20+ messages in thread
From: Davidlohr Bueso @ 2026-06-15 14:55 UTC (permalink / raw)
To: dave.jiang, djbw
Cc: jic23, benjamin.cheatham, icheng, alucerop, alison.schofield,
gourry, dongjoo.seo1, dave, linux-cxl
Add register probing for BI Route Table and BI Decoder capability
structures in cxl_probe_component_regs(), and helpers to map them.
cxl_dport_map_bi() maps the BI Decoder of a downstream port (root
port or switch DSP) at dport-creation time via cxl_port_add_dport();
cxl_port_map_bi() maps a port's own BI capability during port probe
when the upstream link is in 256B Flit operation -- BI Decoder for
an endpoint, BI RT for a switch USP.
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
drivers/cxl/core/regs.c | 14 ++++++++
drivers/cxl/cxl.h | 6 ++++
drivers/cxl/port.c | 71 +++++++++++++++++++++++++++++++++++++++++
include/cxl/cxl.h | 6 ++++
4 files changed, 97 insertions(+)
diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
index 93710cf4f0a6..a6caa793e7a4 100644
--- a/drivers/cxl/core/regs.c
+++ b/drivers/cxl/core/regs.c
@@ -92,6 +92,18 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base,
length = CXL_RAS_CAPABILITY_LENGTH;
rmap = &map->ras;
break;
+ case CXL_CM_CAP_CAP_ID_BI_RT:
+ dev_dbg(dev, "found BI RT capability (0x%x)\n",
+ offset);
+ length = CXL_BI_RT_CAPABILITY_LENGTH;
+ rmap = &map->bi_rt;
+ break;
+ case CXL_CM_CAP_CAP_ID_BI_DECODER:
+ dev_dbg(dev, "found BI Decoder capability (0x%x)\n",
+ offset);
+ length = CXL_BI_DECODER_CAPABILITY_LENGTH;
+ rmap = &map->bi_decoder;
+ break;
default:
dev_dbg(dev, "Unknown CM cap ID: %d (0x%x)\n", cap_id,
offset);
@@ -211,6 +223,8 @@ int cxl_map_component_regs(const struct cxl_register_map *map,
} mapinfo[] = {
{ &map->component_map.hdm_decoder, ®s->hdm_decoder },
{ &map->component_map.ras, ®s->ras },
+ { &map->component_map.bi_rt, ®s->bi_rt },
+ { &map->component_map.bi_decoder, ®s->bi_decoder },
};
int i;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 1297594beaec..760f51b43891 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -42,6 +42,12 @@ extern const struct nvdimm_security_ops *cxl_security_ops;
#define CXL_CM_CAP_CAP_ID_RAS 0x2
#define CXL_CM_CAP_CAP_ID_HDM 0x5
#define CXL_CM_CAP_CAP_HDM_VERSION 1
+#define CXL_CM_CAP_CAP_ID_BI_RT 0xB
+#define CXL_CM_CAP_CAP_ID_BI_DECODER 0xC
+
+/* CXL 4.0 8.2.4.26 / 8.2.4.27 BI Capability Structures */
+#define CXL_BI_RT_CAPABILITY_LENGTH 0xC
+#define CXL_BI_DECODER_CAPABILITY_LENGTH 0xC
/* HDM decoders CXL 2.0 8.2.5.12 CXL HDM Decoder Capability Structure */
#define CXL_HDM_DECODER_CAP_OFFSET 0x0
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index ada51948d52f..ca8d8c14b787 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -58,6 +58,62 @@ static int discover_region(struct device *dev, void *unused)
return 0;
}
+static void cxl_dport_map_bi(struct cxl_dport *dport)
+{
+ struct cxl_register_map *map = &dport->reg_map;
+ struct device *dev = dport->dport_dev;
+
+ if (!map->component_map.bi_decoder.valid) {
+ dev_dbg(dev, "BI Decoder registers not found\n");
+ return;
+ }
+
+ if (cxl_map_component_regs(map, &dport->regs.component,
+ BIT(CXL_CM_CAP_CAP_ID_BI_DECODER)))
+ dev_dbg(dev, "Failed to map BI Decoder capability\n");
+}
+
+static void cxl_port_map_bi(struct cxl_port *port)
+{
+ struct cxl_register_map *map = &port->reg_map;
+ struct cxl_dport *parent_dport = port->parent_dport;
+ struct device *udev;
+ int cap_id;
+
+ /* no upstream BI registers above host bridges or the cxl_root */
+ if (!parent_dport || is_cxl_root(parent_dport->port))
+ return;
+
+ udev = is_cxl_endpoint(port) ?
+ port->uport_dev->parent : port->uport_dev;
+ if (!dev_is_pci(udev))
+ return;
+
+ /* BI requires 256B Flit on the upstream link */
+ if (!cxl_pci_flit_256(to_pci_dev(udev)))
+ return;
+
+ /* map this port's own BI capability */
+ if (is_cxl_endpoint(port)) {
+ if (!map->component_map.bi_decoder.valid) {
+ dev_dbg(&port->dev, "BI Decoder registers not found\n");
+ return;
+ }
+ cap_id = CXL_CM_CAP_CAP_ID_BI_DECODER;
+ } else {
+ if (!map->component_map.bi_rt.valid) {
+ dev_dbg(&port->dev, "BI RT registers not found\n");
+ return;
+ }
+ cap_id = CXL_CM_CAP_CAP_ID_BI_RT;
+ }
+
+ map->host = &port->dev;
+ if (cxl_map_component_regs(map, &port->regs, BIT(cap_id)))
+ dev_dbg(&port->dev, "Failed to map BI capability 0x%x\n",
+ cap_id);
+}
+
static int cxl_switch_port_probe(struct cxl_port *port)
{
/* Reset nr_dports for rebind of driver */
@@ -128,6 +184,8 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
read_cdat_data(port);
cxl_endpoint_parse_cdat(port);
+ cxl_port_map_bi(port);
+
get_device(&cxlmd->dev);
rc = devm_add_action_or_reset(&port->dev, schedule_detach, cxlmd);
if (rc)
@@ -252,6 +310,8 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
* on failure, or the device does not implement RAS registers.
*/
devm_cxl_port_ras_setup(port);
+
+ cxl_port_map_bi(port);
}
dport = devm_cxl_add_dport_by_dev(port, dport_dev);
@@ -261,6 +321,17 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
/* This group was only needed for early exit above */
devres_remove_group(&port->dev, no_free_ptr(port_dr_group));
+ if (dev_is_pci(dport_dev)) {
+ switch (pci_pcie_type(to_pci_dev(dport_dev))) {
+ case PCI_EXP_TYPE_ROOT_PORT:
+ case PCI_EXP_TYPE_DOWNSTREAM:
+ cxl_dport_map_bi(dport);
+ break;
+ default:
+ break;
+ }
+ }
+
cxl_switch_parse_cdat(dport);
/* New dport added, update the decoder targets */
diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
index fa7269154620..8ce9b4e9ca73 100644
--- a/include/cxl/cxl.h
+++ b/include/cxl/cxl.h
@@ -34,10 +34,14 @@ struct cxl_regs {
* Common set of CXL Component register block base pointers
* @hdm_decoder: CXL 2.0 8.2.5.12 CXL HDM Decoder Capability Structure
* @ras: CXL 2.0 8.2.5.9 CXL RAS Capability Structure
+ * @bi_rt: CXL 4.0 8.2.4.26 CXL BI Route Table Capability Structure
+ * @bi_decoder: CXL 4.0 8.2.4.27 CXL BI Decoder Capability Structure
*/
struct_group_tagged(cxl_component_regs, component,
void __iomem *hdm_decoder;
void __iomem *ras;
+ void __iomem *bi_rt;
+ void __iomem *bi_decoder;
);
/*
* Common set of CXL Device register block base pointers
@@ -80,6 +84,8 @@ struct cxl_reg_map {
struct cxl_component_reg_map {
struct cxl_reg_map hdm_decoder;
struct cxl_reg_map ras;
+ struct cxl_reg_map bi_rt;
+ struct cxl_reg_map bi_decoder;
};
struct cxl_device_reg_map {
--
2.39.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 2/5] cxl/pci: Add BI topology enable/disable
2026-06-15 14:55 [PATCH v5 0/5] cxl: Support Back-Invalidate Davidlohr Bueso
2026-06-15 14:55 ` [PATCH v5 1/5] cxl: Add BI register probing and port initialization Davidlohr Bueso
@ 2026-06-15 14:55 ` Davidlohr Bueso
2026-06-15 17:34 ` sashiko-bot
2026-06-23 8:24 ` Richard Cheng
2026-06-15 14:55 ` [PATCH v5 3/5] cxl/hdm: Add BI coherency support for endpoint decoders Davidlohr Bueso
` (2 subsequent siblings)
4 siblings, 2 replies; 20+ messages in thread
From: Davidlohr Bueso @ 2026-06-15 14:55 UTC (permalink / raw)
To: dave.jiang, djbw
Cc: jic23, benjamin.cheatham, icheng, alucerop, alison.schofield,
gourry, dongjoo.seo1, dave, linux-cxl
Implement cxl_bi_setup(), which walks the CXL port topology to enable
BI flows on the device and every component in the path between the
device and the root port. The walk is two-pass: first to verify each
component advertises BI capability, then to write BI Enable on each
dport's BI Decoder Control register and on the device. On failure
during the enable pass, partially-enabled dports are rolled back.
The teardown counterpart, cxl_bi_dealloc(), runs automatically on
cxl_mem unbind.
cxl_bi_setup() does not touch HDM decoder configuration -- the BI bit
in each decoder is programmed separately at commit time. After a
successful setup the device is BI-capable but not yet using BI for
decode coherence.
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
drivers/cxl/core/pci.c | 366 +++++++++++++++++++++++++++++++++++++++++
drivers/cxl/cxl.h | 28 ++++
drivers/cxl/mem.c | 4 +
include/cxl/cxl.h | 2 +
4 files changed, 400 insertions(+)
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index e4338fd7e01b..cdb741d09b9e 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -2,11 +2,13 @@
/* Copyright(c) 2021 Intel Corporation. All rights reserved. */
#include <linux/units.h>
#include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/iopoll.h>
#include <linux/device.h>
#include <linux/delay.h>
#include <linux/pci.h>
#include <linux/pci-doe.h>
#include <linux/aer.h>
+#include <linux/string_choices.h>
#include <cxlpci.h>
#include <cxlmem.h>
#include <cxl.h>
@@ -926,3 +928,367 @@ int cxl_port_get_possible_dports(struct cxl_port *port)
return ctx.count;
}
+
+/*
+ * BI requires 256B Flit operation on the link. RP/DSP/endpoint must
+ * also have the BI Decoder cap mapped (@bi); for USPs the BI RT cap
+ * is optional per CXL 4.0 8.2.4.26, so absent @bi is allowed.
+ */
+static bool cxl_is_bi_capable(struct pci_dev *pdev, void __iomem *bi)
+{
+ if (!cxl_pci_flit_256(pdev))
+ return false;
+ if (pci_pcie_type(pdev) != PCI_EXP_TYPE_UPSTREAM && !bi) {
+ dev_dbg(&pdev->dev, "No BI Decoder registers.\n");
+ return false;
+ }
+ return true;
+}
+
+/* limit any insane timeouts from hw */
+#define CXL_BI_COMMIT_MAXTMO_US (5 * USEC_PER_SEC)
+
+static unsigned long __cxl_bi_get_timeout_us(struct device *dev,
+ unsigned int scale,
+ unsigned int base)
+{
+ static const unsigned long scale_tbl[] = {
+ 1, 10, 100, 1000, 10000, 100000, 1000000, 10000000,
+ };
+
+ if (scale >= ARRAY_SIZE(scale_tbl) || !base) {
+ dev_dbg(dev, "Invalid BI commit timeout: scale=%u base=%u\n",
+ scale, base);
+ return CXL_BI_COMMIT_MAXTMO_US;
+ }
+
+ return scale_tbl[scale] * base;
+}
+
+static int __cxl_bi_wait_commit(struct device *dev, void __iomem *status_reg,
+ u32 committed_bit, u32 err_bit,
+ unsigned int scale, unsigned int base)
+{
+ unsigned long tmo_us, poll_us;
+ ktime_t start;
+ u32 status;
+ int rc;
+
+ tmo_us = min_t(unsigned long, CXL_BI_COMMIT_MAXTMO_US,
+ __cxl_bi_get_timeout_us(dev, scale, base));
+ poll_us = max_t(unsigned long, tmo_us / 10, 1); /* ~10% */
+ start = ktime_get();
+
+ rc = readx_poll_timeout(readl, status_reg, status,
+ status & (committed_bit | err_bit),
+ poll_us, tmo_us);
+ if (rc) {
+ dev_err(dev, "BI-ID commit timed out (%luus)\n", tmo_us);
+ return rc; /* -ETIMEDOUT */
+ }
+
+ if (status & err_bit) {
+ dev_err(dev, "BI-ID commit rejected by hardware\n");
+ return -EIO;
+ }
+
+ dev_dbg(dev, "BI-ID commit wait took %lluus\n",
+ ktime_to_us(ktime_sub(ktime_get(), start)));
+ return 0;
+}
+
+/* BI RT only exists on switch upstream ports. */
+static int __cxl_bi_commit_rt(struct device *dev, void __iomem *bi)
+{
+ u32 status, ctrl;
+ unsigned int scale, base;
+
+ if (!FIELD_GET(CXL_BI_RT_CAPS_EXPLICIT_COMMIT_REQ,
+ readl(bi + CXL_BI_RT_CAPS_OFFSET)))
+ return 0;
+
+ ctrl = readl(bi + CXL_BI_RT_CTRL_OFFSET);
+ writel(ctrl & ~CXL_BI_RT_CTRL_BI_COMMIT, bi + CXL_BI_RT_CTRL_OFFSET);
+ writel(ctrl | CXL_BI_RT_CTRL_BI_COMMIT, bi + CXL_BI_RT_CTRL_OFFSET);
+
+ status = readl(bi + CXL_BI_RT_STATUS_OFFSET);
+ scale = FIELD_GET(CXL_BI_RT_STATUS_BI_COMMIT_TM_SCALE, status);
+ base = FIELD_GET(CXL_BI_RT_STATUS_BI_COMMIT_TM_BASE, status);
+
+ return __cxl_bi_wait_commit(dev, bi + CXL_BI_RT_STATUS_OFFSET,
+ CXL_BI_RT_STATUS_BI_COMMITTED,
+ CXL_BI_RT_STATUS_BI_ERR_NOT_COMMITTED,
+ scale, base);
+}
+
+static int __cxl_bi_commit_decoder(struct device *dev, void __iomem *bi)
+{
+ u32 status, ctrl;
+ unsigned int scale, base;
+
+ if (!FIELD_GET(CXL_BI_DECODER_CAPS_EXPLICIT_COMMIT_REQ,
+ readl(bi + CXL_BI_DECODER_CAPS_OFFSET)))
+ return 0;
+
+ ctrl = readl(bi + CXL_BI_DECODER_CTRL_OFFSET);
+ writel(ctrl & ~CXL_BI_DECODER_CTRL_BI_COMMIT,
+ bi + CXL_BI_DECODER_CTRL_OFFSET);
+ writel(ctrl | CXL_BI_DECODER_CTRL_BI_COMMIT,
+ bi + CXL_BI_DECODER_CTRL_OFFSET);
+
+ status = readl(bi + CXL_BI_DECODER_STATUS_OFFSET);
+ scale = FIELD_GET(CXL_BI_DECODER_STATUS_BI_COMMIT_TM_SCALE, status);
+ base = FIELD_GET(CXL_BI_DECODER_STATUS_BI_COMMIT_TM_BASE, status);
+
+ return __cxl_bi_wait_commit(dev, bi + CXL_BI_DECODER_STATUS_OFFSET,
+ CXL_BI_DECODER_STATUS_BI_COMMITTED,
+ CXL_BI_DECODER_STATUS_BI_ERR_NOT_COMMITTED,
+ scale, base);
+}
+
+/* Enable or dealloc BI-ID changes in the given level of the topology. */
+static int __cxl_bi_ctrl_dport(struct cxl_dport *dport, bool enable)
+{
+ struct pci_dev *pdev = to_pci_dev(dport->dport_dev);
+ void __iomem *bi = dport->regs.bi_decoder;
+ struct cxl_port *port = dport->port;
+ u32 ctrl, value;
+ int rc;
+
+ guard(device)(&port->dev);
+ if (!bi)
+ return -EINVAL;
+
+ ctrl = readl(bi + CXL_BI_DECODER_CTRL_OFFSET);
+
+ switch (pci_pcie_type(pdev)) {
+ case PCI_EXP_TYPE_ROOT_PORT:
+ if (enable) {
+ /*
+ * There is no point of failure from here on,
+ * BI will be enabled on the endpoint device.
+ */
+ dport->nr_bi++;
+
+ if (FIELD_GET(CXL_BI_DECODER_CTRL_BI_FW, ctrl) &&
+ !FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl))
+ return 0;
+
+ value = ctrl | CXL_BI_DECODER_CTRL_BI_FW;
+ value &= ~CXL_BI_DECODER_CTRL_BI_ENABLE;
+ } else {
+ if (WARN_ON_ONCE(dport->nr_bi == 0))
+ return -EINVAL;
+ if (--dport->nr_bi > 0)
+ return 0;
+
+ value = ctrl & ~(CXL_BI_DECODER_CTRL_BI_FW |
+ CXL_BI_DECODER_CTRL_BI_ENABLE);
+ }
+
+ writel(value, bi + CXL_BI_DECODER_CTRL_OFFSET);
+ return 0;
+ case PCI_EXP_TYPE_DOWNSTREAM:
+ if (enable) {
+ value = ctrl & ~CXL_BI_DECODER_CTRL_BI_FW;
+ value |= CXL_BI_DECODER_CTRL_BI_ENABLE;
+ } else {
+ if (!FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl))
+ return 0;
+ value = ctrl & ~(CXL_BI_DECODER_CTRL_BI_FW |
+ CXL_BI_DECODER_CTRL_BI_ENABLE);
+ }
+
+ writel(value, bi + CXL_BI_DECODER_CTRL_OFFSET);
+
+ rc = __cxl_bi_commit_decoder(dport->dport_dev, bi);
+ if (rc)
+ return rc;
+
+ if (port->regs.bi_rt)
+ return __cxl_bi_commit_rt(&port->dev, port->regs.bi_rt);
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int cxl_bi_ctrl_dport_enable(struct cxl_dport *dport)
+{
+ return __cxl_bi_ctrl_dport(dport, true);
+}
+
+static int cxl_bi_ctrl_dport_disable(struct cxl_dport *dport)
+{
+ return __cxl_bi_ctrl_dport(dport, false);
+}
+
+static int __cxl_bi_ctrl_endpoint(struct cxl_dev_state *cxlds, bool enable)
+{
+ struct cxl_port *endpoint = cxlds->cxlmd->endpoint;
+ void __iomem *bi = endpoint->regs.bi_decoder;
+ u32 ctrl, val;
+
+ if (!bi)
+ return -EINVAL;
+
+ ctrl = readl(bi + CXL_BI_DECODER_CTRL_OFFSET);
+
+ if (enable) {
+ if (FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl)) {
+ if (cxlds->bi)
+ return 0;
+ dev_err(cxlds->dev,
+ "BI already enabled in hardware\n");
+ return -EBUSY;
+ }
+ val = ctrl | CXL_BI_DECODER_CTRL_BI_ENABLE;
+ } else {
+ if (!FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl)) {
+ if (!cxlds->bi)
+ return 0;
+ dev_err(cxlds->dev,
+ "BI already disabled in hardware\n");
+ return -EBUSY;
+ }
+ val = ctrl & ~CXL_BI_DECODER_CTRL_BI_ENABLE;
+ }
+
+ writel(val, bi + CXL_BI_DECODER_CTRL_OFFSET);
+ cxlds->bi = enable;
+
+ dev_dbg(cxlds->dev, "BI requests %s\n",
+ str_enabled_disabled(enable));
+
+ return 0;
+}
+
+static int cxl_bi_ctrl_endpoint_enable(struct cxl_dev_state *cxlds)
+{
+ return __cxl_bi_ctrl_endpoint(cxlds, true);
+}
+
+static int cxl_bi_ctrl_endpoint_disable(struct cxl_dev_state *cxlds)
+{
+ return __cxl_bi_ctrl_endpoint(cxlds, false);
+}
+
+/*
+ * devm teardown on cxl_mem unbind. Endpoint decoders may still be
+ * committed here (cxl_workqueue tears them down asynchronously), but
+ * memory access has been quiesced.
+ */
+static void cxl_bi_dealloc(void *data)
+{
+ struct cxl_dev_state *cxlds = data;
+ struct cxl_dport *dport_iter, *dport;
+ struct cxl_port *port_iter;
+
+ if (!dev_is_pci(cxlds->dev))
+ return;
+
+ struct cxl_port *port __free(put_cxl_port) =
+ cxl_pci_find_port(to_pci_dev(cxlds->dev), &dport);
+ if (!port || !cxlds->bi)
+ return;
+
+ scoped_guard(rwsem_read, &cxl_rwsem.region)
+ cxl_bi_ctrl_endpoint_disable(cxlds);
+
+ port_iter = port;
+ dport_iter = dport;
+ while (!is_cxl_root(port_iter)) {
+ int rc = cxl_bi_ctrl_dport_disable(dport_iter);
+
+ /* best effort */
+ if (rc)
+ dev_dbg(&port_iter->dev,
+ "BI dport disable failed: %d\n", rc);
+
+ dport_iter = port_iter->parent_dport;
+ port_iter = dport_iter->port;
+ }
+}
+
+int cxl_bi_setup(struct cxl_dev_state *cxlds)
+{
+ struct cxl_port *endpoint = cxlds->cxlmd->endpoint;
+ struct cxl_dport *dport_iter, *dport, *failed;
+ struct cxl_port *port_iter;
+ struct pci_dev *pdev;
+ int rc;
+
+ if (!dev_is_pci(cxlds->dev))
+ return 0;
+
+ pdev = to_pci_dev(cxlds->dev);
+ struct cxl_port *port __free(put_cxl_port) =
+ cxl_pci_find_port(pdev, &dport);
+
+ if (!port)
+ return -EINVAL;
+
+ if (!cxl_is_bi_capable(pdev, endpoint->regs.bi_decoder))
+ return 0;
+
+ /* walkup the topology twice, first to check, then to enable */
+ port_iter = port;
+ dport_iter = dport;
+ while (!is_cxl_root(port_iter)) {
+ /* check rp, dsp */
+ if (!cxl_is_bi_capable(to_pci_dev(dport_iter->dport_dev),
+ dport_iter->regs.bi_decoder)) {
+ dev_dbg(cxlds->dev, "BI not supported by topology\n");
+ return 0;
+ }
+
+ /* check usp */
+ if (dev_is_pci(port_iter->uport_dev) &&
+ pci_pcie_type(to_pci_dev(port_iter->uport_dev)) ==
+ PCI_EXP_TYPE_UPSTREAM &&
+ !cxl_is_bi_capable(to_pci_dev(port_iter->uport_dev),
+ port_iter->regs.bi_rt)) {
+ dev_dbg(cxlds->dev, "BI not supported by USP\n");
+ return 0;
+ }
+
+ dport_iter = port_iter->parent_dport;
+ port_iter = dport_iter->port;
+ }
+
+ port_iter = port;
+ dport_iter = dport;
+ while (!is_cxl_root(port_iter)) {
+ rc = cxl_bi_ctrl_dport_enable(dport_iter);
+ if (rc)
+ goto err_rollback;
+
+ dport_iter = port_iter->parent_dport;
+ port_iter = dport_iter->port;
+ }
+
+ /* finally, enable BI on the device */
+ rc = cxl_bi_ctrl_endpoint_enable(cxlds);
+ if (rc)
+ goto err_rollback;
+
+ return devm_add_action_or_reset(&cxlds->cxlmd->dev,
+ cxl_bi_dealloc, cxlds);
+
+err_rollback:
+ /*
+ * Undo all dports enabled so far, including the failed one.
+ */
+ failed = dport_iter;
+ dport_iter = dport;
+ port_iter = port;
+ while (!is_cxl_root(port_iter)) {
+ cxl_bi_ctrl_dport_disable(dport_iter);
+ if (dport_iter == failed)
+ break;
+ dport_iter = port_iter->parent_dport;
+ port_iter = dport_iter->port;
+ }
+ return rc;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_bi_setup, "CXL");
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 760f51b43891..89c0c6ce24a3 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -167,6 +167,31 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw)
#define CXL_HEADERLOG_SIZE SZ_512
#define CXL_HEADERLOG_SIZE_U32 SZ_512 / sizeof(u32)
+/* CXL 4.0 8.2.4.26 CXL BI Route Table Capability Structure */
+#define CXL_BI_RT_CAPS_OFFSET 0x0
+#define CXL_BI_RT_CAPS_EXPLICIT_COMMIT_REQ BIT(0)
+#define CXL_BI_RT_CTRL_OFFSET 0x4
+#define CXL_BI_RT_CTRL_BI_COMMIT BIT(0)
+#define CXL_BI_RT_STATUS_OFFSET 0x8
+#define CXL_BI_RT_STATUS_BI_COMMITTED BIT(0)
+#define CXL_BI_RT_STATUS_BI_ERR_NOT_COMMITTED BIT(1)
+#define CXL_BI_RT_STATUS_BI_COMMIT_TM_SCALE GENMASK(11, 8)
+#define CXL_BI_RT_STATUS_BI_COMMIT_TM_BASE GENMASK(15, 12)
+
+/* CXL 4.0 8.2.4.27 CXL BI Decoder Capability Structure */
+#define CXL_BI_DECODER_CAPS_OFFSET 0x0
+#define CXL_BI_DECODER_CAPS_HDMD_CAP BIT(0)
+#define CXL_BI_DECODER_CAPS_EXPLICIT_COMMIT_REQ BIT(1)
+#define CXL_BI_DECODER_CTRL_OFFSET 0x4
+#define CXL_BI_DECODER_CTRL_BI_FW BIT(0)
+#define CXL_BI_DECODER_CTRL_BI_ENABLE BIT(1)
+#define CXL_BI_DECODER_CTRL_BI_COMMIT BIT(2)
+#define CXL_BI_DECODER_STATUS_OFFSET 0x8
+#define CXL_BI_DECODER_STATUS_BI_COMMITTED BIT(0)
+#define CXL_BI_DECODER_STATUS_BI_ERR_NOT_COMMITTED BIT(1)
+#define CXL_BI_DECODER_STATUS_BI_COMMIT_TM_SCALE GENMASK(11, 8)
+#define CXL_BI_DECODER_STATUS_BI_COMMIT_TM_BASE GENMASK(15, 12)
+
/* CXL 2.0 8.2.8.1 Device Capabilities Array Register */
#define CXLDEV_CAP_ARRAY_OFFSET 0x0
#define CXLDEV_CAP_ARRAY_CAP_ID 0
@@ -630,6 +655,7 @@ struct cxl_rcrb_info {
* @coord: access coordinates (bandwidth and latency performance attributes)
* @link_latency: calculated PCIe downstream latency
* @gpf_dvsec: Cached GPF port DVSEC
+ * @nr_bi: number of BI-enabled endpoints below this dport
*/
struct cxl_dport {
struct device *dport_dev;
@@ -642,6 +668,7 @@ struct cxl_dport {
struct access_coordinate coord[ACCESS_COORDINATE_MAX];
long link_latency;
int gpf_dvsec;
+ int nr_bi;
};
/**
@@ -906,6 +933,7 @@ void cxl_coordinates_combine(struct access_coordinate *out,
struct access_coordinate *c2);
bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port);
+int cxl_bi_setup(struct cxl_dev_state *cxlds);
struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
struct device *dport_dev);
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index ab88eaa31d1d..c40f482b7c14 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -160,6 +160,10 @@ static int cxl_mem_probe(struct device *dev)
if (rc)
dev_dbg(dev, "CXL memdev EDAC registration failed rc=%d\n", rc);
+ rc = cxl_bi_setup(cxlds);
+ if (rc)
+ dev_dbg(dev, "BI setup failed rc=%d\n", rc);
+
/*
* The kernel may be operating out of CXL memory on this device,
* there is no spec defined way to determine whether this device
diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
index 8ce9b4e9ca73..4938e93edb63 100644
--- a/include/cxl/cxl.h
+++ b/include/cxl/cxl.h
@@ -168,6 +168,7 @@ struct cxl_dpa_partition {
* @regs: Parsed register blocks
* @cxl_dvsec: Offset to the PCIe device DVSEC
* @rcd: operating in RCD mode (CXL 3.0 9.11.8 CXL Devices Attached to an RCH)
+ * @bi: device is BI (Back-Invalidate) enabled
* @media_ready: Indicate whether the device media is usable
* @dpa_res: Overall DPA resource tree for the device
* @part: DPA partition array
@@ -187,6 +188,7 @@ struct cxl_dev_state {
struct cxl_device_regs regs;
int cxl_dvsec;
bool rcd;
+ bool bi;
bool media_ready;
struct resource dpa_res;
struct cxl_dpa_partition part[CXL_NR_PARTITIONS_MAX];
--
2.39.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 3/5] cxl/hdm: Add BI coherency support for endpoint decoders
2026-06-15 14:55 [PATCH v5 0/5] cxl: Support Back-Invalidate Davidlohr Bueso
2026-06-15 14:55 ` [PATCH v5 1/5] cxl: Add BI register probing and port initialization Davidlohr Bueso
2026-06-15 14:55 ` [PATCH v5 2/5] cxl/pci: Add BI topology enable/disable Davidlohr Bueso
@ 2026-06-15 14:55 ` Davidlohr Bueso
2026-06-15 17:32 ` sashiko-bot
2026-07-01 18:10 ` Dave Jiang
2026-06-15 14:55 ` [PATCH v5 4/5] cxl: Add HDM-DB region creation Davidlohr Bueso
2026-06-15 14:55 ` [PATCH v5 5/5] cxl/hdm: Rename decoder coherency flags Davidlohr Bueso
4 siblings, 2 replies; 20+ messages in thread
From: Davidlohr Bueso @ 2026-06-15 14:55 UTC (permalink / raw)
To: dave.jiang, djbw
Cc: jic23, benjamin.cheatham, icheng, alucerop, alison.schofield,
gourry, dongjoo.seo1, dave, linux-cxl
Cache the HDM decoder's Supported Coherency Models on struct cxl_hdm.
Region attach consults this to verify the HDM supports the region's
coherency type.
For uncommitted endpoint decoders, init_hdm_decoder() defaults
target_type from supported_coherency: Type 3 devices default to
HDM-DB when the HDM is device-coherent-only, HDM-H otherwise.
Pre-committed decoders with the BI bit set are not supported because
endpoint and port enumerations are independent -- at decoder
enumeration cxlds->bi is not yet known, so the topology cannot be
verified.
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
drivers/cxl/core/hdm.c | 24 +++++++++++++++++++++---
drivers/cxl/cxl.h | 6 ++++++
drivers/cxl/cxlmem.h | 2 ++
3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 0c80b76a5f9b..4a41cde9fdba 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -87,6 +87,8 @@ static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
cxlhdm->iw_cap_mask |= BIT(3) | BIT(6) | BIT(12);
if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_16_WAY, hdm_cap))
cxlhdm->iw_cap_mask |= BIT(16);
+ cxlhdm->supported_coherency =
+ FIELD_GET(CXL_HDM_DECODER_SUPPORTED_COHERENCY_MASK, hdm_cap);
}
static bool should_emulate_decoders(struct cxl_endpoint_dvsec_info *info)
@@ -1023,6 +1025,14 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
else
cxld->target_type = CXL_DECODER_DEVMEM;
+ /*
+ * Autocommit BI-enabled decoders is not supported.
+ * At this point cxlds->bi is not yet setup, so there
+ * are no guarantees that the platform supports BI.
+ */
+ if (FIELD_GET(CXL_HDM_DECODER0_CTRL_BI, ctrl))
+ return -ENXIO;
+
guard(rwsem_write)(&cxl_rwsem.region);
if (cxld->id != cxl_num_decoders_committed(port)) {
dev_warn(&port->dev,
@@ -1042,12 +1052,20 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
if (cxled) {
struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
struct cxl_dev_state *cxlds = cxlmd->cxlds;
+ struct cxl_hdm *cxlhdm = dev_get_drvdata(&port->dev);
/*
- * Default by devtype until a device arrives that needs
- * more precision.
+ * HDMs that advertise support for both coherency
+ * modes (CXL_HDM_DECODER_COHERENCY_BOTH) default to
+ * host-only here; the region attach path will switch
+ * target_type to device-coherent if the region's
+ * root decoder has the CFMWS BI bit set. Only HDMs
+ * that strictly support device-coherent mode default
+ * to HDM-DB.
*/
- if (cxlds->type == CXL_DEVTYPE_CLASSMEM)
+ if (cxlds->type == CXL_DEVTYPE_CLASSMEM &&
+ cxlhdm->supported_coherency !=
+ CXL_HDM_DECODER_COHERENCY_DEV)
cxld->target_type = CXL_DECODER_HOSTONLYMEM;
else
cxld->target_type = CXL_DECODER_DEVMEM;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 89c0c6ce24a3..917a6aee5936 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -57,6 +57,11 @@ extern const struct nvdimm_security_ops *cxl_security_ops;
#define CXL_HDM_DECODER_INTERLEAVE_14_12 BIT(9)
#define CXL_HDM_DECODER_INTERLEAVE_3_6_12_WAY BIT(11)
#define CXL_HDM_DECODER_INTERLEAVE_16_WAY BIT(12)
+#define CXL_HDM_DECODER_SUPPORTED_COHERENCY_MASK GENMASK(22, 21)
+#define CXL_HDM_DECODER_COHERENCY_UNKNOWN 0x0
+#define CXL_HDM_DECODER_COHERENCY_DEV 0x1
+#define CXL_HDM_DECODER_COHERENCY_HOST 0x2
+#define CXL_HDM_DECODER_COHERENCY_BOTH 0x3
#define CXL_HDM_DECODER_CTRL_OFFSET 0x4
#define CXL_HDM_DECODER_ENABLE BIT(1)
#define CXL_HDM_DECODER0_BASE_LOW_OFFSET(i) (0x20 * (i) + 0x10)
@@ -71,6 +76,7 @@ extern const struct nvdimm_security_ops *cxl_security_ops;
#define CXL_HDM_DECODER0_CTRL_COMMITTED BIT(10)
#define CXL_HDM_DECODER0_CTRL_COMMIT_ERROR BIT(11)
#define CXL_HDM_DECODER0_CTRL_HOSTONLY BIT(12)
+#define CXL_HDM_DECODER0_CTRL_BI BIT(13)
#define CXL_HDM_DECODER0_TL_LOW(i) (0x20 * (i) + 0x24)
#define CXL_HDM_DECODER0_TL_HIGH(i) (0x20 * (i) + 0x28)
#define CXL_HDM_DECODER0_SKIP_LOW(i) CXL_HDM_DECODER0_TL_LOW(i)
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 776c50d1db51..f41d39cc3795 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -837,6 +837,7 @@ int cxl_mem_sanitize(struct cxl_memdev *cxlmd, u16 cmd);
* @target_count: for switch decoders, max downstream port targets
* @interleave_mask: interleave granularity capability, see check_interleave_cap()
* @iw_cap_mask: bitmask of supported interleave ways, see check_interleave_cap()
+ * @supported_coherency: HDM Decoder Capability supported coherency mask
* @port: mapped cxl_port, see devm_cxl_setup_hdm()
*/
struct cxl_hdm {
@@ -845,6 +846,7 @@ struct cxl_hdm {
unsigned int target_count;
unsigned int interleave_mask;
unsigned long iw_cap_mask;
+ unsigned int supported_coherency;
struct cxl_port *port;
};
--
2.39.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 4/5] cxl: Add HDM-DB region creation
2026-06-15 14:55 [PATCH v5 0/5] cxl: Support Back-Invalidate Davidlohr Bueso
` (2 preceding siblings ...)
2026-06-15 14:55 ` [PATCH v5 3/5] cxl/hdm: Add BI coherency support for endpoint decoders Davidlohr Bueso
@ 2026-06-15 14:55 ` Davidlohr Bueso
2026-06-15 17:41 ` sashiko-bot
` (2 more replies)
2026-06-15 14:55 ` [PATCH v5 5/5] cxl/hdm: Rename decoder coherency flags Davidlohr Bueso
4 siblings, 3 replies; 20+ messages in thread
From: Davidlohr Bueso @ 2026-06-15 14:55 UTC (permalink / raw)
To: dave.jiang, djbw
Cc: jic23, benjamin.cheatham, icheng, alucerop, alison.schofield,
gourry, dongjoo.seo1, dave, linux-cxl
A region inherits its coherency from the chosen root decoder: HDM-DB
if the root has CXL_DECODER_F_BI, otherwise HDM-H.
Surface the topology through read-only sysfs:
- decoderX.Y/cap_bi (root): CFMWS BI restriction.
- decoderX.Y/bi (endpoint): '1' when configured for HDM-DB.
cxl_region_attach() rejects endpoints whose device or HDM cannot
serve the region's type; target_type is inherited from cxlr->type
in cxl_rr_assign_decoder().
The HDM Decoder Control BI bit is set at commit time only when the
target_type is DEVMEM and the BI capability is advertised
(cxlds->bi for endpoints, root F_BI for switches).
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
Documentation/ABI/testing/sysfs-bus-cxl | 18 ++++++++--
drivers/cxl/acpi.c | 11 +++++-
drivers/cxl/core/hdm.c | 16 +++++++++
drivers/cxl/core/port.c | 32 ++++++++++++++---
drivers/cxl/core/region.c | 46 +++++++++++++++++++++----
drivers/cxl/cxl.h | 5 +++
6 files changed, 113 insertions(+), 15 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 16a9b3d2e2c0..3db6ac1a700a 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -297,7 +297,7 @@ Description:
Each entry in the list is a dport id.
-What: /sys/bus/cxl/devices/decoderX.Y/cap_{pmem,ram,type2,type3}
+What: /sys/bus/cxl/devices/decoderX.Y/cap_{pmem,ram,type2,type3,bi}
Date: June, 2021
KernelVersion: v5.14
Contact: linux-cxl@vger.kernel.org
@@ -306,8 +306,9 @@ Description:
represents a fixed memory window identified by platform
firmware. A fixed window may only support a subset of memory
types. The 'cap_*' attributes indicate whether persistent
- memory, volatile memory, accelerator memory, and / or expander
- memory may be mapped behind this decoder's memory window.
+ memory, volatile memory, accelerator memory, expander memory,
+ and / or back-invalidate (HDM-DB) memory may be mapped behind
+ this decoder's memory window.
What: /sys/bus/cxl/devices/decoderX.Y/target_type
@@ -426,6 +427,17 @@ Description:
current cached value.
+What: /sys/bus/cxl/devices/decoderX.Y/bi
+Date: June, 2026
+KernelVersion: v7.3
+Contact: linux-cxl@vger.kernel.org
+Description:
+ (RO) Shows '1' if this endpoint decoder is currently configured
+ for HDM-DB (device-managed coherency with back-invalidate).
+ The HDM-DB state is inherited from the region the decoder is
+ attached to, which is in turn set from the chosen root
+ decoder's CFMWS BI restriction (see cap_bi).
+
What: /sys/bus/cxl/devices/decoderX.Y/delete_region
Date: May, 2022
KernelVersion: v6.0
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 127537628817..6a9ba82719e9 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -152,6 +152,8 @@ static unsigned long cfmws_to_decoder_flags(int restrictions)
flags |= CXL_DECODER_F_PMEM;
if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_FIXED)
flags |= CXL_DECODER_F_LOCK;
+ if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_BI)
+ flags |= CXL_DECODER_F_BI;
return flags;
}
@@ -198,6 +200,12 @@ static int cxl_acpi_cfmws_verify(struct device *dev,
dev_dbg(dev, "CFMWS length %d greater than expected %d\n",
cfmws->header.length, expected_len);
+ if ((cfmws->restrictions & ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM) &&
+ (cfmws->restrictions & ACPI_CEDT_CFMWS_RESTRICT_BI)) {
+ dev_err(dev, "CFMWS cannot have both HDM-H and HDM-DB\n");
+ return -EINVAL;
+ }
+
return 0;
}
@@ -437,7 +445,8 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
cxld = &cxlrd->cxlsd.cxld;
cxld->flags = cfmws_to_decoder_flags(cfmws->restrictions);
- cxld->target_type = CXL_DECODER_HOSTONLYMEM;
+ cxld->target_type = (cxld->flags & CXL_DECODER_F_TYPE2) ?
+ CXL_DECODER_DEVMEM : CXL_DECODER_HOSTONLYMEM;
cxld->hpa_range = (struct range) {
.start = cfmws->base_hpa,
.end = cfmws->base_hpa + cfmws->window_size - 1,
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 4a41cde9fdba..a6844b4577be 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -705,9 +705,25 @@ static void cxld_set_interleave(struct cxl_decoder *cxld, u32 *ctrl)
static void cxld_set_type(struct cxl_decoder *cxld, u32 *ctrl)
{
+ bool bi = cxld->target_type == CXL_DECODER_DEVMEM;
+
+ if (bi) {
+ if (is_endpoint_decoder(&cxld->dev)) {
+ struct cxl_endpoint_decoder *cxled =
+ to_cxl_endpoint_decoder(&cxld->dev);
+ struct cxl_dev_state *cxlds =
+ cxled_to_memdev(cxled)->cxlds;
+
+ bi = cxlds->bi;
+ } else if (cxld->region) {
+ bi = cxl_root_decoder_is_bi(cxld->region->cxlrd);
+ }
+ }
+
u32p_replace_bits(ctrl,
!!(cxld->target_type == CXL_DECODER_HOSTONLYMEM),
CXL_HDM_DECODER0_CTRL_HOSTONLY);
+ u32p_replace_bits(ctrl, bi, CXL_HDM_DECODER0_CTRL_BI);
}
static void cxlsd_set_targets(struct cxl_switch_decoder *cxlsd, u64 *tgt)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index c5aacd7054f1..71801dfd614b 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -131,6 +131,7 @@ CXL_DECODER_FLAG_ATTR(cap_ram, CXL_DECODER_F_RAM);
CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_TYPE2);
CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_TYPE3);
CXL_DECODER_FLAG_ATTR(locked, CXL_DECODER_F_LOCK);
+CXL_DECODER_FLAG_ATTR(cap_bi, CXL_DECODER_F_BI);
static ssize_t target_type_show(struct device *dev,
struct device_attribute *attr, char *buf)
@@ -233,6 +234,19 @@ static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RW(mode);
+static ssize_t bi_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(dev);
+ struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
+ struct cxl_dev_state *cxlds = cxlmd->cxlds;
+
+ guard(rwsem_read)(&cxl_rwsem.region);
+ return sysfs_emit(buf, "%d\n", cxlds->bi && cxled->cxld.region &&
+ cxled->cxld.target_type == CXL_DECODER_DEVMEM);
+}
+static DEVICE_ATTR_RO(bi);
+
static ssize_t dpa_resource_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -329,6 +343,7 @@ static struct attribute *cxl_decoder_root_attrs[] = {
&dev_attr_cap_ram.attr,
&dev_attr_cap_type2.attr,
&dev_attr_cap_type3.attr,
+ &dev_attr_cap_bi.attr,
&dev_attr_target_list.attr,
&dev_attr_qos_class.attr,
SET_CXL_REGION_ATTR(create_pmem_region)
@@ -339,16 +354,24 @@ static struct attribute *cxl_decoder_root_attrs[] = {
static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
{
- unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM;
+ unsigned long flags = cxlrd->cxlsd.cxld.flags;
+ unsigned long hdm_h, hdm_db;
- return (cxlrd->cxlsd.cxld.flags & flags) == flags;
+ hdm_h = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM;
+ hdm_db = CXL_DECODER_F_TYPE2 | CXL_DECODER_F_BI | CXL_DECODER_F_PMEM;
+
+ return (flags & hdm_h) == hdm_h || (flags & hdm_db) == hdm_db;
}
static bool can_create_ram(struct cxl_root_decoder *cxlrd)
{
- unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM;
+ unsigned long flags = cxlrd->cxlsd.cxld.flags;
+ unsigned long hdm_h, hdm_db;
+
+ hdm_h = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM;
+ hdm_db = CXL_DECODER_F_TYPE2 | CXL_DECODER_F_BI | CXL_DECODER_F_RAM;
- return (cxlrd->cxlsd.cxld.flags & flags) == flags;
+ return (flags & hdm_h) == hdm_h || (flags & hdm_db) == hdm_db;
}
static umode_t cxl_root_decoder_visible(struct kobject *kobj, struct attribute *a, int n)
@@ -402,6 +425,7 @@ static const struct attribute_group *cxl_decoder_switch_attribute_groups[] = {
static struct attribute *cxl_decoder_endpoint_attrs[] = {
&dev_attr_target_type.attr,
&dev_attr_mode.attr,
+ &dev_attr_bi.attr,
&dev_attr_dpa_size.attr,
&dev_attr_dpa_resource.attr,
SET_CXL_REGION_ATTR(region)
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index cc41c08c0c0c..79cfb31f09c7 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1132,11 +1132,13 @@ static int cxl_rr_assign_decoder(struct cxl_port *port, struct cxl_region *cxlr,
/*
* Endpoints should already match the region type, but backstop that
* assumption with an assertion. Switch-decoders change mapping-type
- * based on what is mapped when they are assigned to a region.
+ * based on what is mapped when they are assigned to a region. HDM-DB
+ * endpoint decoders inherit their type from cxlr->type below.
*/
dev_WARN_ONCE(&cxlr->dev,
port == cxled_to_port(cxled) &&
- cxld->target_type != cxlr->type,
+ cxld->target_type != cxlr->type &&
+ !cxl_root_decoder_is_bi(cxlr->cxlrd),
"%s:%s mismatch decoder type %d -> %d\n",
dev_name(&cxled_to_memdev(cxled)->dev),
dev_name(&cxld->dev), cxld->target_type, cxlr->type);
@@ -2034,6 +2036,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
struct cxl_region_params *p = &cxlr->params;
struct cxl_port *ep_port, *root_port;
struct cxl_dport *dport;
+ struct cxl_hdm *cxlhdm;
int rc = -ENXIO;
rc = check_interleave_cap(&cxled->cxld, p->interleave_ways,
@@ -2083,10 +2086,31 @@ static int cxl_region_attach(struct cxl_region *cxlr,
return -ENXIO;
}
- if (cxled->cxld.target_type != cxlr->type) {
- dev_dbg(&cxlr->dev, "%s:%s type mismatch: %d vs %d\n",
- dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
- cxled->cxld.target_type, cxlr->type);
+ /*
+ * Verify the device and HDM are capable of the region's flavor before
+ * proceeding. The endpoint decoder's target_type is then inherited
+ * from cxlr->type later in cxl_rr_assign_decoder().
+ */
+ if (cxlr->type == CXL_DECODER_DEVMEM &&
+ cxl_root_decoder_is_bi(cxlrd) && !cxlds->bi) {
+ dev_err(&cxlr->dev, "%s:%s BI not enabled on device\n",
+ dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev));
+ return -ENXIO;
+ }
+
+ cxlhdm = dev_get_drvdata(&ep_port->dev);
+ if (!cxlhdm)
+ return -ENXIO;
+ if (cxlr->type == CXL_DECODER_HOSTONLYMEM &&
+ cxlhdm->supported_coherency == CXL_HDM_DECODER_COHERENCY_DEV) {
+ dev_warn(&cxlr->dev, "%s:%s HDM is device-coherent only\n",
+ dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev));
+ return -ENXIO;
+ }
+ if (cxlr->type == CXL_DECODER_DEVMEM &&
+ cxlhdm->supported_coherency == CXL_HDM_DECODER_COHERENCY_HOST) {
+ dev_warn(&cxlr->dev, "%s:%s HDM is host-only coherent\n",
+ dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev));
return -ENXIO;
}
@@ -2772,6 +2796,7 @@ static ssize_t create_region_store(struct device *dev, const char *buf,
size_t len, enum cxl_partition_mode mode)
{
struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
+ enum cxl_decoder_type target_type;
struct cxl_region *cxlr;
int rc, id;
@@ -2779,7 +2804,14 @@ static ssize_t create_region_store(struct device *dev, const char *buf,
if (rc != 1)
return -EINVAL;
- cxlr = __create_region(cxlrd, mode, id, CXL_DECODER_HOSTONLYMEM);
+ /*
+ * The CFMWS dictates endpoint coherency: a BI-restricted Window
+ * produces an HDM-DB region; otherwise HDM-H.
+ */
+ target_type = cxl_root_decoder_is_bi(cxlrd) ?
+ CXL_DECODER_DEVMEM : CXL_DECODER_HOSTONLYMEM;
+
+ cxlr = __create_region(cxlrd, mode, id, target_type);
if (IS_ERR(cxlr))
return PTR_ERR(cxlr);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 917a6aee5936..29231d0db470 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -287,6 +287,7 @@ int cxl_dport_map_rcd_linkcap(struct pci_dev *pdev, struct cxl_dport *dport);
#define CXL_DECODER_F_LOCK BIT(4)
#define CXL_DECODER_F_ENABLE BIT(5)
#define CXL_DECODER_F_NORMALIZED_ADDRESSING BIT(6)
+#define CXL_DECODER_F_BI BIT(7)
#define CXL_DECODER_F_RESET_MASK (CXL_DECODER_F_ENABLE | CXL_DECODER_F_LOCK)
enum cxl_decoder_type {
@@ -807,6 +808,10 @@ static inline int cxl_root_decoder_autoremove(struct device *host,
{
return cxl_decoder_autoremove(host, &cxlrd->cxlsd.cxld);
}
+static inline bool cxl_root_decoder_is_bi(struct cxl_root_decoder *cxlrd)
+{
+ return cxlrd->cxlsd.cxld.flags & CXL_DECODER_F_BI;
+}
int cxl_endpoint_autoremove(struct cxl_memdev *cxlmd, struct cxl_port *endpoint);
/**
--
2.39.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 5/5] cxl/hdm: Rename decoder coherency flags
2026-06-15 14:55 [PATCH v5 0/5] cxl: Support Back-Invalidate Davidlohr Bueso
` (3 preceding siblings ...)
2026-06-15 14:55 ` [PATCH v5 4/5] cxl: Add HDM-DB region creation Davidlohr Bueso
@ 2026-06-15 14:55 ` Davidlohr Bueso
2026-07-01 18:12 ` Dave Jiang
4 siblings, 1 reply; 20+ messages in thread
From: Davidlohr Bueso @ 2026-06-15 14:55 UTC (permalink / raw)
To: dave.jiang, djbw
Cc: jic23, benjamin.cheatham, icheng, alucerop, alison.schofield,
gourry, dongjoo.seo1, dave, linux-cxl
Align with the ACPI CXL Window restriction naming and convert
CXL_DECODER_F_TYPE2/F_TYPE3 to F_DEVMEM/F_HOSTONLY. Type2 and
Type3 coherency models were named prior to Back-Invalidate.
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
drivers/cxl/acpi.c | 6 +++---
drivers/cxl/core/port.c | 12 ++++++------
drivers/cxl/cxl.h | 4 ++--
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 6a9ba82719e9..087dd1e86d40 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -143,9 +143,9 @@ static unsigned long cfmws_to_decoder_flags(int restrictions)
unsigned long flags = CXL_DECODER_F_ENABLE;
if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_DEVMEM)
- flags |= CXL_DECODER_F_TYPE2;
+ flags |= CXL_DECODER_F_DEVMEM;
if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM)
- flags |= CXL_DECODER_F_TYPE3;
+ flags |= CXL_DECODER_F_HOSTONLY;
if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_VOLATILE)
flags |= CXL_DECODER_F_RAM;
if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_PMEM)
@@ -445,7 +445,7 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
cxld = &cxlrd->cxlsd.cxld;
cxld->flags = cfmws_to_decoder_flags(cfmws->restrictions);
- cxld->target_type = (cxld->flags & CXL_DECODER_F_TYPE2) ?
+ cxld->target_type = (cxld->flags & CXL_DECODER_F_DEVMEM) ?
CXL_DECODER_DEVMEM : CXL_DECODER_HOSTONLYMEM;
cxld->hpa_range = (struct range) {
.start = cfmws->base_hpa,
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 71801dfd614b..63005cf0fa91 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -128,8 +128,8 @@ static DEVICE_ATTR_RO(name)
CXL_DECODER_FLAG_ATTR(cap_pmem, CXL_DECODER_F_PMEM);
CXL_DECODER_FLAG_ATTR(cap_ram, CXL_DECODER_F_RAM);
-CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_TYPE2);
-CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_TYPE3);
+CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_DEVMEM);
+CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_HOSTONLY);
CXL_DECODER_FLAG_ATTR(locked, CXL_DECODER_F_LOCK);
CXL_DECODER_FLAG_ATTR(cap_bi, CXL_DECODER_F_BI);
@@ -357,8 +357,8 @@ static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
unsigned long flags = cxlrd->cxlsd.cxld.flags;
unsigned long hdm_h, hdm_db;
- hdm_h = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM;
- hdm_db = CXL_DECODER_F_TYPE2 | CXL_DECODER_F_BI | CXL_DECODER_F_PMEM;
+ hdm_h = CXL_DECODER_F_HOSTONLY | CXL_DECODER_F_PMEM;
+ hdm_db = CXL_DECODER_F_DEVMEM | CXL_DECODER_F_BI | CXL_DECODER_F_PMEM;
return (flags & hdm_h) == hdm_h || (flags & hdm_db) == hdm_db;
}
@@ -368,8 +368,8 @@ static bool can_create_ram(struct cxl_root_decoder *cxlrd)
unsigned long flags = cxlrd->cxlsd.cxld.flags;
unsigned long hdm_h, hdm_db;
- hdm_h = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM;
- hdm_db = CXL_DECODER_F_TYPE2 | CXL_DECODER_F_BI | CXL_DECODER_F_RAM;
+ hdm_h = CXL_DECODER_F_HOSTONLY | CXL_DECODER_F_RAM;
+ hdm_db = CXL_DECODER_F_DEVMEM | CXL_DECODER_F_BI | CXL_DECODER_F_RAM;
return (flags & hdm_h) == hdm_h || (flags & hdm_db) == hdm_db;
}
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 29231d0db470..b0b4ea4ff8a5 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -282,8 +282,8 @@ int cxl_dport_map_rcd_linkcap(struct pci_dev *pdev, struct cxl_dport *dport);
*/
#define CXL_DECODER_F_RAM BIT(0)
#define CXL_DECODER_F_PMEM BIT(1)
-#define CXL_DECODER_F_TYPE2 BIT(2)
-#define CXL_DECODER_F_TYPE3 BIT(3)
+#define CXL_DECODER_F_DEVMEM BIT(2)
+#define CXL_DECODER_F_HOSTONLY BIT(3)
#define CXL_DECODER_F_LOCK BIT(4)
#define CXL_DECODER_F_ENABLE BIT(5)
#define CXL_DECODER_F_NORMALIZED_ADDRESSING BIT(6)
--
2.39.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v5 3/5] cxl/hdm: Add BI coherency support for endpoint decoders
2026-06-15 14:55 ` [PATCH v5 3/5] cxl/hdm: Add BI coherency support for endpoint decoders Davidlohr Bueso
@ 2026-06-15 17:32 ` sashiko-bot
2026-07-01 18:10 ` Dave Jiang
1 sibling, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-06-15 17:32 UTC (permalink / raw)
To: Davidlohr Bueso; +Cc: linux-cxl
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Out-of-bounds stack read causing information leak when interleave ways > 8
--
commit 9868ecd56176bc1d5132787a81a79aa48ca7bada
Author: Davidlohr Bueso <dave@stgolabs.net>
cxl/hdm: Add BI coherency support for endpoint decoders
This commit caches the HDM decoder's Supported Coherency Models on struct
cxl_hdm. It consults this during region attach and defaults target_type
appropriately for uncommitted endpoint decoders.
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 0c80b76a5f9b4..4a41cde9fdba7 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
[ ... ]
> @@ -1042,12 +1052,20 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
> if (cxled) {
> struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> struct cxl_dev_state *cxlds = cxlmd->cxlds;
> + struct cxl_hdm *cxlhdm = dev_get_drvdata(&port->dev);
>
> /*
> - * Default by devtype until a device arrives that needs
> - * more precision.
> + * HDMs that advertise support for both coherency
> + * modes (CXL_HDM_DECODER_COHERENCY_BOTH) default to
> + * host-only here; the region attach path will switch
> + * target_type to device-coherent if the region's
> + * root decoder has the CFMWS BI bit set. Only HDMs
> + * that strictly support device-coherent mode default
> + * to HDM-DB.
> */
> - if (cxlds->type == CXL_DEVTYPE_CLASSMEM)
> + if (cxlds->type == CXL_DEVTYPE_CLASSMEM &&
> + cxlhdm->supported_coherency !=
> + CXL_HDM_DECODER_COHERENCY_DEV)
> cxld->target_type = CXL_DECODER_HOSTONLYMEM;
> else
> cxld->target_type = CXL_DECODER_DEVMEM;
[Severity: High]
This is a pre-existing issue, but does the later switch decoder initialization
in init_hdm_decoder() read out-of-bounds stack memory when interleave_ways
is greater than 8?
If a device advertises up to 16-way interleave, the target mapping loop
reads past the 8-byte target_list union:
drivers/cxl/core/hdm.c:init_hdm_decoder() {
union {
u64 value;
unsigned char target_id[8];
} target_list;
...
if (!cxled) {
lo = readl(hdm + CXL_HDM_DECODER0_TL_LOW(which));
hi = readl(hdm + CXL_HDM_DECODER0_TL_HIGH(which));
target_list.value = (hi << 32) + lo;
for (i = 0; i < cxld->interleave_ways; i++)
cxld->target_map[i] = target_list.target_id[i];
return 0;
}
}
Can this loop copy up to 8 bytes of uninitialized stack memory into
cxld->target_map, which might then be exposed to unprivileged userspace
via the world-readable target_list sysfs attribute?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615145529.13848-1-dave@stgolabs.net?part=3
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 2/5] cxl/pci: Add BI topology enable/disable
2026-06-15 14:55 ` [PATCH v5 2/5] cxl/pci: Add BI topology enable/disable Davidlohr Bueso
@ 2026-06-15 17:34 ` sashiko-bot
2026-06-23 8:24 ` Richard Cheng
1 sibling, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-06-15 17:34 UTC (permalink / raw)
To: Davidlohr Bueso; +Cc: linux-cxl
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Mixing `goto` error handling with `__free()` scope-based cleanup declarations.
- [High] Missing explicit commit for the Endpoint BI Decoder Control Register.
- [High] Root port BI enablement logic is unreachable and contradicts the commit message.
- [High] Missing reference counting (`nr_bi`) for shared downstream ports in cascaded switch topologies.
--
commit 31994c2bf9d14374bacdb07d505a19b18a761e35
Author: Davidlohr Bueso <dave@stgolabs.net>
cxl/pci: Add BI topology enable/disable
This commit implements cxl_bi_setup() to walk the CXL port topology and enable
BI flows on the device and every component in the path. It also implements a
teardown counterpart, cxl_bi_dealloc().
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index d1f487b3d809a..b1b0f3eab90e8 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
[ ... ]
> +static int __cxl_bi_ctrl_dport(struct cxl_dport *dport, bool enable)
> +{
> + struct pci_dev *pdev = to_pci_dev(dport->dport_dev);
> + void __iomem *bi = dport->regs.bi_decoder;
> + struct cxl_port *port = dport->port;
> + u32 ctrl, value;
> + int rc;
[ ... ]
> + case PCI_EXP_TYPE_DOWNSTREAM:
> + if (enable) {
> + value = ctrl & ~CXL_BI_DECODER_CTRL_BI_FW;
> + value |= CXL_BI_DECODER_CTRL_BI_ENABLE;
> + } else {
> + if (!FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl))
> + return 0;
> + value = ctrl & ~(CXL_BI_DECODER_CTRL_BI_FW |
> + CXL_BI_DECODER_CTRL_BI_ENABLE);
> + }
[Severity: High]
In cascaded switch topologies, can this unconditionally clear the
CXL_BI_DECODER_CTRL_BI_ENABLE bit while other endpoints below the switch
are still actively using it?
Unlike the root port case which uses dport->nr_bi for reference counting,
downstream ports appear to clear this bit as soon as the first endpoint
unbinds. This might break BI routing for other active endpoints on a shared
downstream port.
Does this need reference counting similar to the root port?
[ ... ]
> +static int __cxl_bi_ctrl_endpoint(struct cxl_dev_state *cxlds, bool enable)
> +{
[ ... ]
> + writel(val, bi + CXL_BI_DECODER_CTRL_OFFSET);
> + cxlds->bi = enable;
> +
> + dev_dbg(cxlds->dev, "BI requests %s\n",
> + str_enabled_disabled(enable));
> +
> + return 0;
> +}
[Severity: High]
Is this missing an explicit commit for the Endpoint BI Decoder Control
Register?
The CXL 4.0 specification requires an explicit commit after changing
BI_ENABLE for any BI Decoder via CXL_BI_DECODER_CAPS_EXPLICIT_COMMIT_REQ.
While __cxl_bi_ctrl_dport() correctly calls __cxl_bi_commit_decoder() after
updating the downstream ports, __cxl_bi_ctrl_endpoint() omits this step.
Could this cause the endpoint device to ignore the BI enablement state and
silently fail to enable BI flows?
[ ... ]
> +int cxl_bi_setup(struct cxl_dev_state *cxlds)
> +{
> + struct cxl_port *endpoint = cxlds->cxlmd->endpoint;
> + struct cxl_dport *dport_iter, *dport, *failed;
> + struct cxl_port *port_iter;
> + struct pci_dev *pdev;
> + int rc;
> +
> + if (!dev_is_pci(cxlds->dev))
> + return 0;
> +
> + pdev = to_pci_dev(cxlds->dev);
> + struct cxl_port *port __free(put_cxl_port) =
> + cxl_pci_find_port(pdev, &dport);
> +
> + if (!port)
> + return -EINVAL;
[ ... ]
> + port_iter = port;
> + dport_iter = dport;
> + while (!is_cxl_root(port_iter)) {
> + rc = cxl_bi_ctrl_dport_enable(dport_iter);
> + if (rc)
> + goto err_rollback;
[Severity: High]
The commit message states this will write BI Enable on each dport's BI
Decoder Control register. However, doesn't this loop terminate before
processing the root port?
Since the loop condition is !is_cxl_root(port_iter), when port_iter reaches
the CXL root, the loop exits and skips configuring the root port entirely.
Additionally, if the loop did reach the PCI_EXP_TYPE_ROOT_PORT code inside
__cxl_bi_ctrl_dport(), that code actually clears the enable bit:
__cxl_bi_ctrl_dport() {
...
case PCI_EXP_TYPE_ROOT_PORT:
...
value &= ~CXL_BI_DECODER_CTRL_BI_ENABLE;
}
Should this loop include the root port, and should the root port logic actually
set the enable bit rather than clearing it?
[ ... ]
> + /* finally, enable BI on the device */
> + rc = cxl_bi_ctrl_endpoint_enable(cxlds);
> + if (rc)
> + goto err_rollback;
> +
> + return devm_add_action_or_reset(&cxlds->cxlmd->dev,
> + cxl_bi_dealloc, cxlds);
> +
> +err_rollback:
> + /*
> + * Undo all dports enabled so far, including the failed one.
> + */
> + failed = dport_iter;
> + dport_iter = dport;
> + port_iter = port;
> + while (!is_cxl_root(port_iter)) {
> + cxl_bi_ctrl_dport_disable(dport_iter);
> + if (dport_iter == failed)
> + break;
> + dport_iter = port_iter->parent_dport;
> + port_iter = dport_iter->port;
> + }
> + return rc;
> +}
[Severity: High]
Is it safe to mix goto-based error handling with scope-based cleanup in
this function?
This function declares struct cxl_port *port __free(put_cxl_port) at the
top, but also uses a goto err_rollback pattern for failure paths.
Mixing goto labels with __free() declarations in the same function creates
confusing ownership semantics and violates subsystem cleanup guidelines.
Should this be fully converted to scope-based cleanup, or avoid using
__free() here?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615145529.13848-1-dave@stgolabs.net?part=2
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 1/5] cxl: Add BI register probing and port initialization
2026-06-15 14:55 ` [PATCH v5 1/5] cxl: Add BI register probing and port initialization Davidlohr Bueso
@ 2026-06-15 17:35 ` sashiko-bot
2026-06-23 8:02 ` Richard Cheng
2026-07-01 16:57 ` Dave Jiang
1 sibling, 1 reply; 20+ messages in thread
From: sashiko-bot @ 2026-06-15 17:35 UTC (permalink / raw)
To: Davidlohr Bueso; +Cc: linux-cxl
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] devm mappings for dport BI registers leak on hotplug due to being called outside the dport devres group in cxl_port_add_dport.
--
commit a836080fdf13fa334055f7f13a95491b66ef793f
Author: Davidlohr Bueso <dave@stgolabs.net>
cxl: Add BI register probing and port initialization
This commit adds register probing for BI Route Table and BI Decoder capability
structures. It introduces helpers to map the BI Decoder of a downstream port
at dport-creation time and a port's own BI capability during port probe.
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index ada51948d52fa..ca8d8c14b7876 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
[ ... ]
> @@ -261,6 +321,17 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
> /* This group was only needed for early exit above */
> devres_remove_group(&port->dev, no_free_ptr(port_dr_group));
>
> + if (dev_is_pci(dport_dev)) {
> + switch (pci_pcie_type(to_pci_dev(dport_dev))) {
> + case PCI_EXP_TYPE_ROOT_PORT:
> + case PCI_EXP_TYPE_DOWNSTREAM:
> + cxl_dport_map_bi(dport);
[Severity: High]
Does this leak devm mappings if the dport is hot-removed?
Looking at cxl_port_add_dport(), devm_cxl_add_dport_by_dev() creates a
devres group for the dport on the parent port's device. However,
cxl_dport_map_bi() is called after devm_cxl_add_dport_by_dev() returns,
meaning the devres group has already been closed.
Because cxl_dport_map_bi() calls devm_cxl_iomap_block() against the parent
port device, these devm allocations bypass the dport group and attach
directly to the parent port device.
If the dport is hot-removed, the dport devres group is released, but the BI
capability mappings appear to be left behind. If the dport is subsequently
re-added, won't devm_request_mem_region() fail with -EBUSY due to the
physical address region conflict, permanently breaking BI capabilities?
> + break;
> + default:
> + break;
> + }
> + }
> +
> cxl_switch_parse_cdat(dport);
>
> /* New dport added, update the decoder targets */
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615145529.13848-1-dave@stgolabs.net?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/5] cxl: Add HDM-DB region creation
2026-06-15 14:55 ` [PATCH v5 4/5] cxl: Add HDM-DB region creation Davidlohr Bueso
@ 2026-06-15 17:41 ` sashiko-bot
2026-06-23 8:39 ` Richard Cheng
2026-06-23 8:47 ` Richard Cheng
2 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-06-15 17:41 UTC (permalink / raw)
To: Davidlohr Bueso; +Cc: linux-cxl
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Auto-discovered regions fail to inherit their coherency type from the root decoder, violating the design rule and potentially creating invalid region configurations.
- [Medium] Spurious `dev_WARN_ONCE` triggers in `cxl_rr_assign_decoder()` when attaching an endpoint decoder with a stale `DEVMEM` target type to an HDM-H (non-BI) region.
- [Medium] `bi_show` incorrectly reports that an auto-discovered endpoint decoder is configured for HDM-DB (BI=1) when it is operating with BI=0 in hardware.
--
commit 913c2a59fed5b8381543ceb7345cd15ac2ea9d17
Author: Davidlohr Bueso <dave@stgolabs.net>
cxl: Add HDM-DB region creation
A region inherits its coherency from the chosen root decoder: HDM-DB
if the root has CXL_DECODER_F_BI, otherwise HDM-H.
[ ... ]
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index c5aacd7054f1d..71801dfd614b1 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
[ ... ]
> @@ -233,6 +234,19 @@ static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR_RW(mode);
>
> +static ssize_t bi_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(dev);
> + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> + struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> + guard(rwsem_read)(&cxl_rwsem.region);
> + return sysfs_emit(buf, "%d\n", cxlds->bi && cxled->cxld.region &&
> + cxled->cxld.target_type == CXL_DECODER_DEVMEM);
> +}
[Severity: Medium]
Will this attribute show '1' for an auto-discovered DEVMEM endpoint decoder
even if it operates with BI=0 in hardware?
If Linux auto-discovers a DEVMEM decoder that intrinsically supports BI
(cxlds->bi is true), init_hdm_decoder() currently rejects it if the BI
control bit is set, so the hardware must be operating with BI=0. However,
this attribute seems to evaluate the state dynamically based on the device
capability rather than the hardware's actual BI configuration, contradicting
the sysfs documentation that says it shows '1' when "currently configured".
> +static DEVICE_ATTR_RO(bi);
[ ... ]
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e50dc716d4e82..6eb852b514d3a 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1132,11 +1132,13 @@ static int cxl_rr_assign_decoder(struct cxl_port *port, struct cxl_region *cxlr,
> /*
> * Endpoints should already match the region type, but backstop that
> * assumption with an assertion. Switch-decoders change mapping-type
> - * based on what is mapped when they are assigned to a region.
> + * based on what is mapped when they are assigned to a region. HDM-DB
> + * endpoint decoders inherit their type from cxlr->type below.
> */
> dev_WARN_ONCE(&cxlr->dev,
> port == cxled_to_port(cxled) &&
> - cxld->target_type != cxlr->type,
> + cxld->target_type != cxlr->type &&
> + !cxl_root_decoder_is_bi(cxlr->cxlrd),
> "%s:%s mismatch decoder type %d -> %d\n",
> dev_name(&cxled_to_memdev(cxled)->dev),
> dev_name(&cxld->dev), cxld->target_type, cxlr->type);
[Severity: Medium]
Does this logic trigger a spurious dev_WARN_ONCE() when attaching an
endpoint decoder with a stale DEVMEM target type to an HDM-H region?
When a region is dismantled, __cxl_decoder_detach() clears the decoder's
region assignment but doesn't reset its target_type, leaving it with a
stale DEVMEM target type. If it is later attached to an HDM-H (non-BI)
region, the check for cxld->target_type != cxlr->type evaluates to true,
and the !cxl_root_decoder_is_bi(cxlr->cxlrd) check also evaluates to true.
Could this generate a spurious warning stack trace in dmesg during valid
user reconfigurations, potentially causing a panic if panic_on_warn is set?
[ ... ]
> @@ -2772,6 +2796,7 @@ static ssize_t create_region_store(struct device *dev, const char *buf,
> size_t len, enum cxl_partition_mode mode)
> {
> struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
> + enum cxl_decoder_type target_type;
> struct cxl_region *cxlr;
> int rc, id;
>
> @@ -2779,7 +2804,14 @@ static ssize_t create_region_store(struct device *dev, const char *buf,
> if (rc != 1)
> return -EINVAL;
>
> - cxlr = __create_region(cxlrd, mode, id, CXL_DECODER_HOSTONLYMEM);
> + /*
> + * The CFMWS dictates endpoint coherency: a BI-restricted Window
> + * produces an HDM-DB region; otherwise HDM-H.
> + */
> + target_type = cxl_root_decoder_is_bi(cxlrd) ?
> + CXL_DECODER_DEVMEM : CXL_DECODER_HOSTONLYMEM;
> +
> + cxlr = __create_region(cxlrd, mode, id, target_type);
[Severity: High]
Does the auto-discovery path in construct_region() still blindly create
regions using the endpoint decoder's type?
The commit message dictates a strict design rule that a region inherits its
coherency from the chosen root decoder. The sysfs creation path correctly
enforces this here, but construct_region() appears to miss this update:
drivers/cxl/core/region.c:construct_region() {
...
cxlr = __create_region(cxlrd, cxlds->part[part].mode,
atomic_read(&cxlrd->region_id),
cxled->cxld.target_type);
...
}
If firmware incorrectly programs an endpoint decoder as DEVMEM under an HDM-H
root decoder, could construct_region() create a DEVMEM region? Because both
the region and the endpoint have target_type == DEVMEM, the mismatch warning
in cxl_rr_assign_decoder() would be bypassed, resulting in an invalid HDM-DB
region under a root decoder that cannot generate Back-Invalidates.
> if (IS_ERR(cxlr))
> return PTR_ERR(cxlr);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615145529.13848-1-dave@stgolabs.net?part=4
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 1/5] cxl: Add BI register probing and port initialization
2026-06-15 17:35 ` sashiko-bot
@ 2026-06-23 8:02 ` Richard Cheng
2026-06-29 14:44 ` Davidlohr Bueso
0 siblings, 1 reply; 20+ messages in thread
From: Richard Cheng @ 2026-06-23 8:02 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Davidlohr Bueso, linux-cxl
On Mon, Jun 15, 2026 at 05:35:41PM +0800, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] devm mappings for dport BI registers leak on hotplug due to being called outside the dport devres group in cxl_port_add_dport.
> --
>
> commit a836080fdf13fa334055f7f13a95491b66ef793f
> Author: Davidlohr Bueso <dave@stgolabs.net>
>
> cxl: Add BI register probing and port initialization
>
> This commit adds register probing for BI Route Table and BI Decoder capability
> structures. It introduces helpers to map the BI Decoder of a downstream port
> at dport-creation time and a port's own BI capability during port probe.
>
> > diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> > index ada51948d52fa..ca8d8c14b7876 100644
> > --- a/drivers/cxl/port.c
> > +++ b/drivers/cxl/port.c
>
> [ ... ]
>
> > @@ -261,6 +321,17 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
> > /* This group was only needed for early exit above */
> > devres_remove_group(&port->dev, no_free_ptr(port_dr_group));
> >
> > + if (dev_is_pci(dport_dev)) {
> > + switch (pci_pcie_type(to_pci_dev(dport_dev))) {
> > + case PCI_EXP_TYPE_ROOT_PORT:
> > + case PCI_EXP_TYPE_DOWNSTREAM:
> > + cxl_dport_map_bi(dport);
>
> [Severity: High]
> Does this leak devm mappings if the dport is hot-removed?
>
> Looking at cxl_port_add_dport(), devm_cxl_add_dport_by_dev() creates a
> devres group for the dport on the parent port's device. However,
> cxl_dport_map_bi() is called after devm_cxl_add_dport_by_dev() returns,
> meaning the devres group has already been closed.
>
> Because cxl_dport_map_bi() calls devm_cxl_iomap_block() against the parent
> port device, these devm allocations bypass the dport group and attach
> directly to the parent port device.
>
> If the dport is hot-removed, the dport devres group is released, but the BI
> capability mappings appear to be left behind. If the dport is subsequently
> re-added, won't devm_request_mem_region() fail with -EBUSY due to the
> physical address region conflict, permanently breaking BI capabilities?
>
Hi Davidlohr,
I think sashiko-bot's is right on this part, this part runs after devm_cxl_add_dport_by_dev()
has already closed the dport's devres group, so the BI request_mem_region()/ioremap() land on
the port's devres list outside that group. On runtime dport removal the group release won't free
them, and re-adding the dport then fails the BI region request with -EBUSY.
The dport RAS mapping is done inside __devm_cxl_add_dport() while the group is still open for
this reason.
I would suggest mapping BI the same way which is to move cxl_dport_map_bi() into __devm_cxl_add_dport()
next to the RAS setup, and set map->host like RAS does.
What do you think ?
Best regards,
Richard Cheng.
> > + break;
> > + default:
> > + break;
> > + }
> > + }
> > +
> > cxl_switch_parse_cdat(dport);
> >
> > /* New dport added, update the decoder targets */
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260615145529.13848-1-dave@stgolabs.net?part=1
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 2/5] cxl/pci: Add BI topology enable/disable
2026-06-15 14:55 ` [PATCH v5 2/5] cxl/pci: Add BI topology enable/disable Davidlohr Bueso
2026-06-15 17:34 ` sashiko-bot
@ 2026-06-23 8:24 ` Richard Cheng
2026-06-29 15:08 ` Davidlohr Bueso
1 sibling, 1 reply; 20+ messages in thread
From: Richard Cheng @ 2026-06-23 8:24 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: dave.jiang, djbw, jic23, benjamin.cheatham, alucerop,
alison.schofield, gourry, dongjoo.seo1, linux-cxl
On Mon, Jun 15, 2026 at 07:55:26AM +0800, Davidlohr Bueso wrote:
> Implement cxl_bi_setup(), which walks the CXL port topology to enable
> BI flows on the device and every component in the path between the
> device and the root port. The walk is two-pass: first to verify each
> component advertises BI capability, then to write BI Enable on each
> dport's BI Decoder Control register and on the device. On failure
> during the enable pass, partially-enabled dports are rolled back.
>
> The teardown counterpart, cxl_bi_dealloc(), runs automatically on
> cxl_mem unbind.
>
> cxl_bi_setup() does not touch HDM decoder configuration -- the BI bit
> in each decoder is programmed separately at commit time. After a
> successful setup the device is BI-capable but not yet using BI for
> decode coherence.
>
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
> drivers/cxl/core/pci.c | 366 +++++++++++++++++++++++++++++++++++++++++
> drivers/cxl/cxl.h | 28 ++++
> drivers/cxl/mem.c | 4 +
> include/cxl/cxl.h | 2 +
> 4 files changed, 400 insertions(+)
>
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index e4338fd7e01b..cdb741d09b9e 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -2,11 +2,13 @@
> /* Copyright(c) 2021 Intel Corporation. All rights reserved. */
> #include <linux/units.h>
> #include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/iopoll.h>
> #include <linux/device.h>
> #include <linux/delay.h>
> #include <linux/pci.h>
> #include <linux/pci-doe.h>
> #include <linux/aer.h>
> +#include <linux/string_choices.h>
> #include <cxlpci.h>
> #include <cxlmem.h>
> #include <cxl.h>
> @@ -926,3 +928,367 @@ int cxl_port_get_possible_dports(struct cxl_port *port)
>
> return ctx.count;
> }
> +
> +/*
> + * BI requires 256B Flit operation on the link. RP/DSP/endpoint must
> + * also have the BI Decoder cap mapped (@bi); for USPs the BI RT cap
> + * is optional per CXL 4.0 8.2.4.26, so absent @bi is allowed.
> + */
> +static bool cxl_is_bi_capable(struct pci_dev *pdev, void __iomem *bi)
> +{
> + if (!cxl_pci_flit_256(pdev))
> + return false;
> + if (pci_pcie_type(pdev) != PCI_EXP_TYPE_UPSTREAM && !bi) {
> + dev_dbg(&pdev->dev, "No BI Decoder registers.\n");
> + return false;
> + }
> + return true;
> +}
> +
> +/* limit any insane timeouts from hw */
> +#define CXL_BI_COMMIT_MAXTMO_US (5 * USEC_PER_SEC)
> +
> +static unsigned long __cxl_bi_get_timeout_us(struct device *dev,
> + unsigned int scale,
> + unsigned int base)
> +{
> + static const unsigned long scale_tbl[] = {
> + 1, 10, 100, 1000, 10000, 100000, 1000000, 10000000,
> + };
> +
> + if (scale >= ARRAY_SIZE(scale_tbl) || !base) {
> + dev_dbg(dev, "Invalid BI commit timeout: scale=%u base=%u\n",
> + scale, base);
> + return CXL_BI_COMMIT_MAXTMO_US;
> + }
> +
> + return scale_tbl[scale] * base;
> +}
> +
> +static int __cxl_bi_wait_commit(struct device *dev, void __iomem *status_reg,
> + u32 committed_bit, u32 err_bit,
> + unsigned int scale, unsigned int base)
> +{
> + unsigned long tmo_us, poll_us;
> + ktime_t start;
> + u32 status;
> + int rc;
> +
> + tmo_us = min_t(unsigned long, CXL_BI_COMMIT_MAXTMO_US,
> + __cxl_bi_get_timeout_us(dev, scale, base));
> + poll_us = max_t(unsigned long, tmo_us / 10, 1); /* ~10% */
> + start = ktime_get();
> +
> + rc = readx_poll_timeout(readl, status_reg, status,
> + status & (committed_bit | err_bit),
> + poll_us, tmo_us);
> + if (rc) {
> + dev_err(dev, "BI-ID commit timed out (%luus)\n", tmo_us);
> + return rc; /* -ETIMEDOUT */
> + }
> +
> + if (status & err_bit) {
> + dev_err(dev, "BI-ID commit rejected by hardware\n");
> + return -EIO;
> + }
> +
> + dev_dbg(dev, "BI-ID commit wait took %lluus\n",
> + ktime_to_us(ktime_sub(ktime_get(), start)));
> + return 0;
> +}
> +
> +/* BI RT only exists on switch upstream ports. */
> +static int __cxl_bi_commit_rt(struct device *dev, void __iomem *bi)
> +{
> + u32 status, ctrl;
> + unsigned int scale, base;
> +
> + if (!FIELD_GET(CXL_BI_RT_CAPS_EXPLICIT_COMMIT_REQ,
> + readl(bi + CXL_BI_RT_CAPS_OFFSET)))
> + return 0;
> +
> + ctrl = readl(bi + CXL_BI_RT_CTRL_OFFSET);
> + writel(ctrl & ~CXL_BI_RT_CTRL_BI_COMMIT, bi + CXL_BI_RT_CTRL_OFFSET);
> + writel(ctrl | CXL_BI_RT_CTRL_BI_COMMIT, bi + CXL_BI_RT_CTRL_OFFSET);
> +
> + status = readl(bi + CXL_BI_RT_STATUS_OFFSET);
> + scale = FIELD_GET(CXL_BI_RT_STATUS_BI_COMMIT_TM_SCALE, status);
> + base = FIELD_GET(CXL_BI_RT_STATUS_BI_COMMIT_TM_BASE, status);
> +
> + return __cxl_bi_wait_commit(dev, bi + CXL_BI_RT_STATUS_OFFSET,
> + CXL_BI_RT_STATUS_BI_COMMITTED,
> + CXL_BI_RT_STATUS_BI_ERR_NOT_COMMITTED,
> + scale, base);
> +}
> +
> +static int __cxl_bi_commit_decoder(struct device *dev, void __iomem *bi)
> +{
> + u32 status, ctrl;
> + unsigned int scale, base;
> +
> + if (!FIELD_GET(CXL_BI_DECODER_CAPS_EXPLICIT_COMMIT_REQ,
> + readl(bi + CXL_BI_DECODER_CAPS_OFFSET)))
> + return 0;
> +
> + ctrl = readl(bi + CXL_BI_DECODER_CTRL_OFFSET);
> + writel(ctrl & ~CXL_BI_DECODER_CTRL_BI_COMMIT,
> + bi + CXL_BI_DECODER_CTRL_OFFSET);
> + writel(ctrl | CXL_BI_DECODER_CTRL_BI_COMMIT,
> + bi + CXL_BI_DECODER_CTRL_OFFSET);
> +
> + status = readl(bi + CXL_BI_DECODER_STATUS_OFFSET);
> + scale = FIELD_GET(CXL_BI_DECODER_STATUS_BI_COMMIT_TM_SCALE, status);
> + base = FIELD_GET(CXL_BI_DECODER_STATUS_BI_COMMIT_TM_BASE, status);
> +
> + return __cxl_bi_wait_commit(dev, bi + CXL_BI_DECODER_STATUS_OFFSET,
> + CXL_BI_DECODER_STATUS_BI_COMMITTED,
> + CXL_BI_DECODER_STATUS_BI_ERR_NOT_COMMITTED,
> + scale, base);
> +}
> +
> +/* Enable or dealloc BI-ID changes in the given level of the topology. */
> +static int __cxl_bi_ctrl_dport(struct cxl_dport *dport, bool enable)
> +{
> + struct pci_dev *pdev = to_pci_dev(dport->dport_dev);
> + void __iomem *bi = dport->regs.bi_decoder;
> + struct cxl_port *port = dport->port;
> + u32 ctrl, value;
> + int rc;
> +
> + guard(device)(&port->dev);
> + if (!bi)
> + return -EINVAL;
> +
> + ctrl = readl(bi + CXL_BI_DECODER_CTRL_OFFSET);
> +
> + switch (pci_pcie_type(pdev)) {
> + case PCI_EXP_TYPE_ROOT_PORT:
> + if (enable) {
> + /*
> + * There is no point of failure from here on,
> + * BI will be enabled on the endpoint device.
> + */
> + dport->nr_bi++;
> +
> + if (FIELD_GET(CXL_BI_DECODER_CTRL_BI_FW, ctrl) &&
> + !FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl))
> + return 0;
> +
> + value = ctrl | CXL_BI_DECODER_CTRL_BI_FW;
> + value &= ~CXL_BI_DECODER_CTRL_BI_ENABLE;
> + } else {
> + if (WARN_ON_ONCE(dport->nr_bi == 0))
> + return -EINVAL;
> + if (--dport->nr_bi > 0)
> + return 0;
> +
> + value = ctrl & ~(CXL_BI_DECODER_CTRL_BI_FW |
> + CXL_BI_DECODER_CTRL_BI_ENABLE);
> + }
> +
> + writel(value, bi + CXL_BI_DECODER_CTRL_OFFSET);
> + return 0;
> + case PCI_EXP_TYPE_DOWNSTREAM:
> + if (enable) {
> + value = ctrl & ~CXL_BI_DECODER_CTRL_BI_FW;
> + value |= CXL_BI_DECODER_CTRL_BI_ENABLE;
> + } else {
> + if (!FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl))
> + return 0;
> + value = ctrl & ~(CXL_BI_DECODER_CTRL_BI_FW |
> + CXL_BI_DECODER_CTRL_BI_ENABLE);
> + }
> +
> + writel(value, bi + CXL_BI_DECODER_CTRL_OFFSET);
> +
> + rc = __cxl_bi_commit_decoder(dport->dport_dev, bi);
> + if (rc)
> + return rc;
> +
> + if (port->regs.bi_rt)
> + return __cxl_bi_commit_rt(&port->dev, port->regs.bi_rt);
> + return 0;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int cxl_bi_ctrl_dport_enable(struct cxl_dport *dport)
> +{
> + return __cxl_bi_ctrl_dport(dport, true);
> +}
> +
> +static int cxl_bi_ctrl_dport_disable(struct cxl_dport *dport)
> +{
> + return __cxl_bi_ctrl_dport(dport, false);
> +}
> +
> +static int __cxl_bi_ctrl_endpoint(struct cxl_dev_state *cxlds, bool enable)
> +{
> + struct cxl_port *endpoint = cxlds->cxlmd->endpoint;
> + void __iomem *bi = endpoint->regs.bi_decoder;
> + u32 ctrl, val;
> +
> + if (!bi)
> + return -EINVAL;
> +
> + ctrl = readl(bi + CXL_BI_DECODER_CTRL_OFFSET);
> +
> + if (enable) {
> + if (FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl)) {
> + if (cxlds->bi)
> + return 0;
> + dev_err(cxlds->dev,
> + "BI already enabled in hardware\n");
> + return -EBUSY;
> + }
> + val = ctrl | CXL_BI_DECODER_CTRL_BI_ENABLE;
> + } else {
> + if (!FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl)) {
> + if (!cxlds->bi)
> + return 0;
> + dev_err(cxlds->dev,
> + "BI already disabled in hardware\n");
> + return -EBUSY;
> + }
> + val = ctrl & ~CXL_BI_DECODER_CTRL_BI_ENABLE;
> + }
> +
> + writel(val, bi + CXL_BI_DECODER_CTRL_OFFSET);
> + cxlds->bi = enable;
> +
> + dev_dbg(cxlds->dev, "BI requests %s\n",
> + str_enabled_disabled(enable));
> +
> + return 0;
> +}
> +
> +static int cxl_bi_ctrl_endpoint_enable(struct cxl_dev_state *cxlds)
> +{
> + return __cxl_bi_ctrl_endpoint(cxlds, true);
> +}
> +
> +static int cxl_bi_ctrl_endpoint_disable(struct cxl_dev_state *cxlds)
> +{
> + return __cxl_bi_ctrl_endpoint(cxlds, false);
> +}
> +
> +/*
> + * devm teardown on cxl_mem unbind. Endpoint decoders may still be
> + * committed here (cxl_workqueue tears them down asynchronously), but
> + * memory access has been quiesced.
> + */
> +static void cxl_bi_dealloc(void *data)
> +{
> + struct cxl_dev_state *cxlds = data;
> + struct cxl_dport *dport_iter, *dport;
> + struct cxl_port *port_iter;
> +
> + if (!dev_is_pci(cxlds->dev))
> + return;
> +
> + struct cxl_port *port __free(put_cxl_port) =
> + cxl_pci_find_port(to_pci_dev(cxlds->dev), &dport);
> + if (!port || !cxlds->bi)
> + return;
> +
> + scoped_guard(rwsem_read, &cxl_rwsem.region)
> + cxl_bi_ctrl_endpoint_disable(cxlds);
> +
> + port_iter = port;
> + dport_iter = dport;
> + while (!is_cxl_root(port_iter)) {
> + int rc = cxl_bi_ctrl_dport_disable(dport_iter);
> +
> + /* best effort */
> + if (rc)
> + dev_dbg(&port_iter->dev,
> + "BI dport disable failed: %d\n", rc);
> +
> + dport_iter = port_iter->parent_dport;
> + port_iter = dport_iter->port;
> + }
> +}
Hi Davidlohr,
I think the failing dport enablement will get disabled here, too.
Because you put the "== failed" check after the disable. For a DSP that
re-runs __cxl_bi_commit_decoder() on HW that just failed to commit during
enable, I think a second commit timeout up to 5s will occur on every failed
bring-up.
Rollback should only unwind the dports that fully enabled, stop before the failed one.
Correct me if I'm wrong.
Best regards,
Richard Cheng.
> +
> +int cxl_bi_setup(struct cxl_dev_state *cxlds)
> +{
> + struct cxl_port *endpoint = cxlds->cxlmd->endpoint;
> + struct cxl_dport *dport_iter, *dport, *failed;
> + struct cxl_port *port_iter;
> + struct pci_dev *pdev;
> + int rc;
> +
> + if (!dev_is_pci(cxlds->dev))
> + return 0;
> +
> + pdev = to_pci_dev(cxlds->dev);
> + struct cxl_port *port __free(put_cxl_port) =
> + cxl_pci_find_port(pdev, &dport);
> +
> + if (!port)
> + return -EINVAL;
> +
> + if (!cxl_is_bi_capable(pdev, endpoint->regs.bi_decoder))
> + return 0;
> +
> + /* walkup the topology twice, first to check, then to enable */
> + port_iter = port;
> + dport_iter = dport;
> + while (!is_cxl_root(port_iter)) {
> + /* check rp, dsp */
> + if (!cxl_is_bi_capable(to_pci_dev(dport_iter->dport_dev),
> + dport_iter->regs.bi_decoder)) {
> + dev_dbg(cxlds->dev, "BI not supported by topology\n");
> + return 0;
> + }
> +
> + /* check usp */
> + if (dev_is_pci(port_iter->uport_dev) &&
> + pci_pcie_type(to_pci_dev(port_iter->uport_dev)) ==
> + PCI_EXP_TYPE_UPSTREAM &&
> + !cxl_is_bi_capable(to_pci_dev(port_iter->uport_dev),
> + port_iter->regs.bi_rt)) {
> + dev_dbg(cxlds->dev, "BI not supported by USP\n");
> + return 0;
> + }
> +
> + dport_iter = port_iter->parent_dport;
> + port_iter = dport_iter->port;
> + }
> +
> + port_iter = port;
> + dport_iter = dport;
> + while (!is_cxl_root(port_iter)) {
> + rc = cxl_bi_ctrl_dport_enable(dport_iter);
> + if (rc)
> + goto err_rollback;
> +
> + dport_iter = port_iter->parent_dport;
> + port_iter = dport_iter->port;
> + }
> +
> + /* finally, enable BI on the device */
> + rc = cxl_bi_ctrl_endpoint_enable(cxlds);
> + if (rc)
> + goto err_rollback;
> +
> + return devm_add_action_or_reset(&cxlds->cxlmd->dev,
> + cxl_bi_dealloc, cxlds);
> +
> +err_rollback:
> + /*
> + * Undo all dports enabled so far, including the failed one.
> + */
> + failed = dport_iter;
> + dport_iter = dport;
> + port_iter = port;
> + while (!is_cxl_root(port_iter)) {
> + cxl_bi_ctrl_dport_disable(dport_iter);
> + if (dport_iter == failed)
> + break;
> + dport_iter = port_iter->parent_dport;
> + port_iter = dport_iter->port;
> + }
> + return rc;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_bi_setup, "CXL");
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 760f51b43891..89c0c6ce24a3 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -167,6 +167,31 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw)
> #define CXL_HEADERLOG_SIZE SZ_512
> #define CXL_HEADERLOG_SIZE_U32 SZ_512 / sizeof(u32)
>
> +/* CXL 4.0 8.2.4.26 CXL BI Route Table Capability Structure */
> +#define CXL_BI_RT_CAPS_OFFSET 0x0
> +#define CXL_BI_RT_CAPS_EXPLICIT_COMMIT_REQ BIT(0)
> +#define CXL_BI_RT_CTRL_OFFSET 0x4
> +#define CXL_BI_RT_CTRL_BI_COMMIT BIT(0)
> +#define CXL_BI_RT_STATUS_OFFSET 0x8
> +#define CXL_BI_RT_STATUS_BI_COMMITTED BIT(0)
> +#define CXL_BI_RT_STATUS_BI_ERR_NOT_COMMITTED BIT(1)
> +#define CXL_BI_RT_STATUS_BI_COMMIT_TM_SCALE GENMASK(11, 8)
> +#define CXL_BI_RT_STATUS_BI_COMMIT_TM_BASE GENMASK(15, 12)
> +
> +/* CXL 4.0 8.2.4.27 CXL BI Decoder Capability Structure */
> +#define CXL_BI_DECODER_CAPS_OFFSET 0x0
> +#define CXL_BI_DECODER_CAPS_HDMD_CAP BIT(0)
> +#define CXL_BI_DECODER_CAPS_EXPLICIT_COMMIT_REQ BIT(1)
> +#define CXL_BI_DECODER_CTRL_OFFSET 0x4
> +#define CXL_BI_DECODER_CTRL_BI_FW BIT(0)
> +#define CXL_BI_DECODER_CTRL_BI_ENABLE BIT(1)
> +#define CXL_BI_DECODER_CTRL_BI_COMMIT BIT(2)
> +#define CXL_BI_DECODER_STATUS_OFFSET 0x8
> +#define CXL_BI_DECODER_STATUS_BI_COMMITTED BIT(0)
> +#define CXL_BI_DECODER_STATUS_BI_ERR_NOT_COMMITTED BIT(1)
> +#define CXL_BI_DECODER_STATUS_BI_COMMIT_TM_SCALE GENMASK(11, 8)
> +#define CXL_BI_DECODER_STATUS_BI_COMMIT_TM_BASE GENMASK(15, 12)
> +
> /* CXL 2.0 8.2.8.1 Device Capabilities Array Register */
> #define CXLDEV_CAP_ARRAY_OFFSET 0x0
> #define CXLDEV_CAP_ARRAY_CAP_ID 0
> @@ -630,6 +655,7 @@ struct cxl_rcrb_info {
> * @coord: access coordinates (bandwidth and latency performance attributes)
> * @link_latency: calculated PCIe downstream latency
> * @gpf_dvsec: Cached GPF port DVSEC
> + * @nr_bi: number of BI-enabled endpoints below this dport
> */
> struct cxl_dport {
> struct device *dport_dev;
> @@ -642,6 +668,7 @@ struct cxl_dport {
> struct access_coordinate coord[ACCESS_COORDINATE_MAX];
> long link_latency;
> int gpf_dvsec;
> + int nr_bi;
> };
>
> /**
> @@ -906,6 +933,7 @@ void cxl_coordinates_combine(struct access_coordinate *out,
> struct access_coordinate *c2);
>
> bool cxl_endpoint_decoder_reset_detected(struct cxl_port *port);
> +int cxl_bi_setup(struct cxl_dev_state *cxlds);
> struct cxl_dport *devm_cxl_add_dport_by_dev(struct cxl_port *port,
> struct device *dport_dev);
>
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index ab88eaa31d1d..c40f482b7c14 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -160,6 +160,10 @@ static int cxl_mem_probe(struct device *dev)
> if (rc)
> dev_dbg(dev, "CXL memdev EDAC registration failed rc=%d\n", rc);
>
> + rc = cxl_bi_setup(cxlds);
> + if (rc)
> + dev_dbg(dev, "BI setup failed rc=%d\n", rc);
> +
> /*
> * The kernel may be operating out of CXL memory on this device,
> * there is no spec defined way to determine whether this device
> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> index 8ce9b4e9ca73..4938e93edb63 100644
> --- a/include/cxl/cxl.h
> +++ b/include/cxl/cxl.h
> @@ -168,6 +168,7 @@ struct cxl_dpa_partition {
> * @regs: Parsed register blocks
> * @cxl_dvsec: Offset to the PCIe device DVSEC
> * @rcd: operating in RCD mode (CXL 3.0 9.11.8 CXL Devices Attached to an RCH)
> + * @bi: device is BI (Back-Invalidate) enabled
> * @media_ready: Indicate whether the device media is usable
> * @dpa_res: Overall DPA resource tree for the device
> * @part: DPA partition array
> @@ -187,6 +188,7 @@ struct cxl_dev_state {
> struct cxl_device_regs regs;
> int cxl_dvsec;
> bool rcd;
> + bool bi;
> bool media_ready;
> struct resource dpa_res;
> struct cxl_dpa_partition part[CXL_NR_PARTITIONS_MAX];
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/5] cxl: Add HDM-DB region creation
2026-06-15 14:55 ` [PATCH v5 4/5] cxl: Add HDM-DB region creation Davidlohr Bueso
2026-06-15 17:41 ` sashiko-bot
@ 2026-06-23 8:39 ` Richard Cheng
2026-06-23 8:47 ` Richard Cheng
2 siblings, 0 replies; 20+ messages in thread
From: Richard Cheng @ 2026-06-23 8:39 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: dave.jiang, djbw, jic23, benjamin.cheatham, alucerop,
alison.schofield, gourry, dongjoo.seo1, linux-cxl
On Mon, Jun 15, 2026 at 07:55:28AM +0800, Davidlohr Bueso wrote:
> A region inherits its coherency from the chosen root decoder: HDM-DB
> if the root has CXL_DECODER_F_BI, otherwise HDM-H.
>
> Surface the topology through read-only sysfs:
>
> - decoderX.Y/cap_bi (root): CFMWS BI restriction.
> - decoderX.Y/bi (endpoint): '1' when configured for HDM-DB.
>
> cxl_region_attach() rejects endpoints whose device or HDM cannot
> serve the region's type; target_type is inherited from cxlr->type
> in cxl_rr_assign_decoder().
>
> The HDM Decoder Control BI bit is set at commit time only when the
> target_type is DEVMEM and the BI capability is advertised
> (cxlds->bi for endpoints, root F_BI for switches).
>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
> Documentation/ABI/testing/sysfs-bus-cxl | 18 ++++++++--
> drivers/cxl/acpi.c | 11 +++++-
> drivers/cxl/core/hdm.c | 16 +++++++++
> drivers/cxl/core/port.c | 32 ++++++++++++++---
> drivers/cxl/core/region.c | 46 +++++++++++++++++++++----
> drivers/cxl/cxl.h | 5 +++
> 6 files changed, 113 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 16a9b3d2e2c0..3db6ac1a700a 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -297,7 +297,7 @@ Description:
> Each entry in the list is a dport id.
>
>
> -What: /sys/bus/cxl/devices/decoderX.Y/cap_{pmem,ram,type2,type3}
> +What: /sys/bus/cxl/devices/decoderX.Y/cap_{pmem,ram,type2,type3,bi}
> Date: June, 2021
> KernelVersion: v5.14
> Contact: linux-cxl@vger.kernel.org
> @@ -306,8 +306,9 @@ Description:
> represents a fixed memory window identified by platform
> firmware. A fixed window may only support a subset of memory
> types. The 'cap_*' attributes indicate whether persistent
> - memory, volatile memory, accelerator memory, and / or expander
> - memory may be mapped behind this decoder's memory window.
> + memory, volatile memory, accelerator memory, expander memory,
> + and / or back-invalidate (HDM-DB) memory may be mapped behind
> + this decoder's memory window.
>
>
> What: /sys/bus/cxl/devices/decoderX.Y/target_type
> @@ -426,6 +427,17 @@ Description:
> current cached value.
>
>
> +What: /sys/bus/cxl/devices/decoderX.Y/bi
> +Date: June, 2026
> +KernelVersion: v7.3
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RO) Shows '1' if this endpoint decoder is currently configured
> + for HDM-DB (device-managed coherency with back-invalidate).
> + The HDM-DB state is inherited from the region the decoder is
> + attached to, which is in turn set from the chosen root
> + decoder's CFMWS BI restriction (see cap_bi).
> +
> What: /sys/bus/cxl/devices/decoderX.Y/delete_region
> Date: May, 2022
> KernelVersion: v6.0
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 127537628817..6a9ba82719e9 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -152,6 +152,8 @@ static unsigned long cfmws_to_decoder_flags(int restrictions)
> flags |= CXL_DECODER_F_PMEM;
> if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_FIXED)
> flags |= CXL_DECODER_F_LOCK;
> + if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_BI)
> + flags |= CXL_DECODER_F_BI;
>
> return flags;
> }
> @@ -198,6 +200,12 @@ static int cxl_acpi_cfmws_verify(struct device *dev,
> dev_dbg(dev, "CFMWS length %d greater than expected %d\n",
> cfmws->header.length, expected_len);
>
> + if ((cfmws->restrictions & ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM) &&
> + (cfmws->restrictions & ACPI_CEDT_CFMWS_RESTRICT_BI)) {
> + dev_err(dev, "CFMWS cannot have both HDM-H and HDM-DB\n");
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> @@ -437,7 +445,8 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
>
> cxld = &cxlrd->cxlsd.cxld;
> cxld->flags = cfmws_to_decoder_flags(cfmws->restrictions);
> - cxld->target_type = CXL_DECODER_HOSTONLYMEM;
> + cxld->target_type = (cxld->flags & CXL_DECODER_F_TYPE2) ?
> + CXL_DECODER_DEVMEM : CXL_DECODER_HOSTONLYMEM;
> cxld->hpa_range = (struct range) {
> .start = cfmws->base_hpa,
> .end = cfmws->base_hpa + cfmws->window_size - 1,
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 4a41cde9fdba..a6844b4577be 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -705,9 +705,25 @@ static void cxld_set_interleave(struct cxl_decoder *cxld, u32 *ctrl)
>
> static void cxld_set_type(struct cxl_decoder *cxld, u32 *ctrl)
> {
> + bool bi = cxld->target_type == CXL_DECODER_DEVMEM;
> +
> + if (bi) {
> + if (is_endpoint_decoder(&cxld->dev)) {
> + struct cxl_endpoint_decoder *cxled =
> + to_cxl_endpoint_decoder(&cxld->dev);
> + struct cxl_dev_state *cxlds =
> + cxled_to_memdev(cxled)->cxlds;
> +
> + bi = cxlds->bi;
> + } else if (cxld->region) {
> + bi = cxl_root_decoder_is_bi(cxld->region->cxlrd);
> + }
> + }
> +
> u32p_replace_bits(ctrl,
> !!(cxld->target_type == CXL_DECODER_HOSTONLYMEM),
> CXL_HDM_DECODER0_CTRL_HOSTONLY);
> + u32p_replace_bits(ctrl, bi, CXL_HDM_DECODER0_CTRL_BI);
> }
>
> static void cxlsd_set_targets(struct cxl_switch_decoder *cxlsd, u64 *tgt)
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index c5aacd7054f1..71801dfd614b 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -131,6 +131,7 @@ CXL_DECODER_FLAG_ATTR(cap_ram, CXL_DECODER_F_RAM);
> CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_TYPE2);
> CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_TYPE3);
> CXL_DECODER_FLAG_ATTR(locked, CXL_DECODER_F_LOCK);
> +CXL_DECODER_FLAG_ATTR(cap_bi, CXL_DECODER_F_BI);
>
> static ssize_t target_type_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> @@ -233,6 +234,19 @@ static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR_RW(mode);
>
> +static ssize_t bi_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(dev);
> + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> + struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> + guard(rwsem_read)(&cxl_rwsem.region);
> + return sysfs_emit(buf, "%d\n", cxlds->bi && cxled->cxld.region &&
> + cxled->cxld.target_type == CXL_DECODER_DEVMEM);
> +}
> +static DEVICE_ATTR_RO(bi);
> +
> static ssize_t dpa_resource_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> @@ -329,6 +343,7 @@ static struct attribute *cxl_decoder_root_attrs[] = {
> &dev_attr_cap_ram.attr,
> &dev_attr_cap_type2.attr,
> &dev_attr_cap_type3.attr,
> + &dev_attr_cap_bi.attr,
> &dev_attr_target_list.attr,
> &dev_attr_qos_class.attr,
> SET_CXL_REGION_ATTR(create_pmem_region)
> @@ -339,16 +354,24 @@ static struct attribute *cxl_decoder_root_attrs[] = {
>
> static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
> {
> - unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM;
> + unsigned long flags = cxlrd->cxlsd.cxld.flags;
> + unsigned long hdm_h, hdm_db;
>
> - return (cxlrd->cxlsd.cxld.flags & flags) == flags;
> + hdm_h = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM;
> + hdm_db = CXL_DECODER_F_TYPE2 | CXL_DECODER_F_BI | CXL_DECODER_F_PMEM;
> +
> + return (flags & hdm_h) == hdm_h || (flags & hdm_db) == hdm_db;
> }
>
> static bool can_create_ram(struct cxl_root_decoder *cxlrd)
> {
> - unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM;
> + unsigned long flags = cxlrd->cxlsd.cxld.flags;
> + unsigned long hdm_h, hdm_db;
> +
> + hdm_h = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM;
> + hdm_db = CXL_DECODER_F_TYPE2 | CXL_DECODER_F_BI | CXL_DECODER_F_RAM;
>
> - return (cxlrd->cxlsd.cxld.flags & flags) == flags;
> + return (flags & hdm_h) == hdm_h || (flags & hdm_db) == hdm_db;
> }
>
> static umode_t cxl_root_decoder_visible(struct kobject *kobj, struct attribute *a, int n)
> @@ -402,6 +425,7 @@ static const struct attribute_group *cxl_decoder_switch_attribute_groups[] = {
> static struct attribute *cxl_decoder_endpoint_attrs[] = {
> &dev_attr_target_type.attr,
> &dev_attr_mode.attr,
> + &dev_attr_bi.attr,
> &dev_attr_dpa_size.attr,
> &dev_attr_dpa_resource.attr,
> SET_CXL_REGION_ATTR(region)
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index cc41c08c0c0c..79cfb31f09c7 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1132,11 +1132,13 @@ static int cxl_rr_assign_decoder(struct cxl_port *port, struct cxl_region *cxlr,
> /*
> * Endpoints should already match the region type, but backstop that
> * assumption with an assertion. Switch-decoders change mapping-type
> - * based on what is mapped when they are assigned to a region.
> + * based on what is mapped when they are assigned to a region. HDM-DB
> + * endpoint decoders inherit their type from cxlr->type below.
> */
> dev_WARN_ONCE(&cxlr->dev,
> port == cxled_to_port(cxled) &&
> - cxld->target_type != cxlr->type,
> + cxld->target_type != cxlr->type &&
> + !cxl_root_decoder_is_bi(cxlr->cxlrd),
> "%s:%s mismatch decoder type %d -> %d\n",
> dev_name(&cxled_to_memdev(cxled)->dev),
> dev_name(&cxld->dev), cxld->target_type, cxlr->type);
> @@ -2034,6 +2036,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
> struct cxl_region_params *p = &cxlr->params;
> struct cxl_port *ep_port, *root_port;
> struct cxl_dport *dport;
> + struct cxl_hdm *cxlhdm;
> int rc = -ENXIO;
>
> rc = check_interleave_cap(&cxled->cxld, p->interleave_ways,
> @@ -2083,10 +2086,31 @@ static int cxl_region_attach(struct cxl_region *cxlr,
> return -ENXIO;
> }
>
> - if (cxled->cxld.target_type != cxlr->type) {
> - dev_dbg(&cxlr->dev, "%s:%s type mismatch: %d vs %d\n",
> - dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
> - cxled->cxld.target_type, cxlr->type);
> + /*
> + * Verify the device and HDM are capable of the region's flavor before
> + * proceeding. The endpoint decoder's target_type is then inherited
> + * from cxlr->type later in cxl_rr_assign_decoder().
> + */
> + if (cxlr->type == CXL_DECODER_DEVMEM &&
> + cxl_root_decoder_is_bi(cxlrd) && !cxlds->bi) {
> + dev_err(&cxlr->dev, "%s:%s BI not enabled on device\n",
> + dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev));
> + return -ENXIO;
> + }
> +
> + cxlhdm = dev_get_drvdata(&ep_port->dev);
> + if (!cxlhdm)
> + return -ENXIO;
> + if (cxlr->type == CXL_DECODER_HOSTONLYMEM &&
> + cxlhdm->supported_coherency == CXL_HDM_DECODER_COHERENCY_DEV) {
> + dev_warn(&cxlr->dev, "%s:%s HDM is device-coherent only\n",
> + dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev));
> + return -ENXIO;
> + }
> + if (cxlr->type == CXL_DECODER_DEVMEM &&
> + cxlhdm->supported_coherency == CXL_HDM_DECODER_COHERENCY_HOST) {
> + dev_warn(&cxlr->dev, "%s:%s HDM is host-only coherent\n",
> + dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev));
> return -ENXIO;
> }
>
> @@ -2772,6 +2796,7 @@ static ssize_t create_region_store(struct device *dev, const char *buf,
> size_t len, enum cxl_partition_mode mode)
> {
> struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
> + enum cxl_decoder_type target_type;
> struct cxl_region *cxlr;
> int rc, id;
>
> @@ -2779,7 +2804,14 @@ static ssize_t create_region_store(struct device *dev, const char *buf,
> if (rc != 1)
> return -EINVAL;
>
> - cxlr = __create_region(cxlrd, mode, id, CXL_DECODER_HOSTONLYMEM);
> + /*
> + * The CFMWS dictates endpoint coherency: a BI-restricted Window
> + * produces an HDM-DB region; otherwise HDM-H.
> + */
> + target_type = cxl_root_decoder_is_bi(cxlrd) ?
> + CXL_DECODER_DEVMEM : CXL_DECODER_HOSTONLYMEM;
> +
> + cxlr = __create_region(cxlrd, mode, id, target_type);
Hi Davidlohr,
I have a question about this part, I do not understand FW so please correct me
if I'm raising some stupid questions.
It derives the region type from the root decoder's BI flag, but on auto-discovery
path, it still and it needs to take target_type from the endpoint decoder's HW-read
type, 2 different sources that can disagree.
An auto-discovered DEVMEM region could hit the "DEVMEM && root bit && !cxlds->bi" check
and reject in cxl_region_attach() before cxl_bi_setup() has executed.
But path 3's init_hdm_decoder() -ENXIO on a committed BI bit means such a decoder will
never reach construct_region(), so this scenario might never happen.
If so, maybe a changelog or comment noting that HDM-DB auto-discovery is out of scope would
help?
Best regards,
Richard Cheng.
> if (IS_ERR(cxlr))
> return PTR_ERR(cxlr);
>
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 917a6aee5936..29231d0db470 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -287,6 +287,7 @@ int cxl_dport_map_rcd_linkcap(struct pci_dev *pdev, struct cxl_dport *dport);
> #define CXL_DECODER_F_LOCK BIT(4)
> #define CXL_DECODER_F_ENABLE BIT(5)
> #define CXL_DECODER_F_NORMALIZED_ADDRESSING BIT(6)
> +#define CXL_DECODER_F_BI BIT(7)
> #define CXL_DECODER_F_RESET_MASK (CXL_DECODER_F_ENABLE | CXL_DECODER_F_LOCK)
>
> enum cxl_decoder_type {
> @@ -807,6 +808,10 @@ static inline int cxl_root_decoder_autoremove(struct device *host,
> {
> return cxl_decoder_autoremove(host, &cxlrd->cxlsd.cxld);
> }
> +static inline bool cxl_root_decoder_is_bi(struct cxl_root_decoder *cxlrd)
> +{
> + return cxlrd->cxlsd.cxld.flags & CXL_DECODER_F_BI;
> +}
> int cxl_endpoint_autoremove(struct cxl_memdev *cxlmd, struct cxl_port *endpoint);
>
> /**
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/5] cxl: Add HDM-DB region creation
2026-06-15 14:55 ` [PATCH v5 4/5] cxl: Add HDM-DB region creation Davidlohr Bueso
2026-06-15 17:41 ` sashiko-bot
2026-06-23 8:39 ` Richard Cheng
@ 2026-06-23 8:47 ` Richard Cheng
2026-06-29 15:26 ` Davidlohr Bueso
2 siblings, 1 reply; 20+ messages in thread
From: Richard Cheng @ 2026-06-23 8:47 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: dave.jiang, djbw, jic23, benjamin.cheatham, alucerop,
alison.schofield, gourry, dongjoo.seo1, linux-cxl
On Mon, Jun 15, 2026 at 07:55:28AM +0800, Davidlohr Bueso wrote:
> A region inherits its coherency from the chosen root decoder: HDM-DB
> if the root has CXL_DECODER_F_BI, otherwise HDM-H.
>
> Surface the topology through read-only sysfs:
>
> - decoderX.Y/cap_bi (root): CFMWS BI restriction.
> - decoderX.Y/bi (endpoint): '1' when configured for HDM-DB.
>
> cxl_region_attach() rejects endpoints whose device or HDM cannot
> serve the region's type; target_type is inherited from cxlr->type
> in cxl_rr_assign_decoder().
>
> The HDM Decoder Control BI bit is set at commit time only when the
> target_type is DEVMEM and the BI capability is advertised
> (cxlds->bi for endpoints, root F_BI for switches).
>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
> Documentation/ABI/testing/sysfs-bus-cxl | 18 ++++++++--
> drivers/cxl/acpi.c | 11 +++++-
> drivers/cxl/core/hdm.c | 16 +++++++++
> drivers/cxl/core/port.c | 32 ++++++++++++++---
> drivers/cxl/core/region.c | 46 +++++++++++++++++++++----
> drivers/cxl/cxl.h | 5 +++
> 6 files changed, 113 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 16a9b3d2e2c0..3db6ac1a700a 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -297,7 +297,7 @@ Description:
> Each entry in the list is a dport id.
>
>
> -What: /sys/bus/cxl/devices/decoderX.Y/cap_{pmem,ram,type2,type3}
> +What: /sys/bus/cxl/devices/decoderX.Y/cap_{pmem,ram,type2,type3,bi}
> Date: June, 2021
> KernelVersion: v5.14
> Contact: linux-cxl@vger.kernel.org
> @@ -306,8 +306,9 @@ Description:
> represents a fixed memory window identified by platform
> firmware. A fixed window may only support a subset of memory
> types. The 'cap_*' attributes indicate whether persistent
> - memory, volatile memory, accelerator memory, and / or expander
> - memory may be mapped behind this decoder's memory window.
> + memory, volatile memory, accelerator memory, expander memory,
> + and / or back-invalidate (HDM-DB) memory may be mapped behind
> + this decoder's memory window.
>
>
> What: /sys/bus/cxl/devices/decoderX.Y/target_type
> @@ -426,6 +427,17 @@ Description:
> current cached value.
>
>
> +What: /sys/bus/cxl/devices/decoderX.Y/bi
> +Date: June, 2026
> +KernelVersion: v7.3
> +Contact: linux-cxl@vger.kernel.org
> +Description:
> + (RO) Shows '1' if this endpoint decoder is currently configured
> + for HDM-DB (device-managed coherency with back-invalidate).
> + The HDM-DB state is inherited from the region the decoder is
> + attached to, which is in turn set from the chosen root
> + decoder's CFMWS BI restriction (see cap_bi).
> +
> What: /sys/bus/cxl/devices/decoderX.Y/delete_region
> Date: May, 2022
> KernelVersion: v6.0
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 127537628817..6a9ba82719e9 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -152,6 +152,8 @@ static unsigned long cfmws_to_decoder_flags(int restrictions)
> flags |= CXL_DECODER_F_PMEM;
> if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_FIXED)
> flags |= CXL_DECODER_F_LOCK;
> + if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_BI)
> + flags |= CXL_DECODER_F_BI;
>
> return flags;
> }
> @@ -198,6 +200,12 @@ static int cxl_acpi_cfmws_verify(struct device *dev,
> dev_dbg(dev, "CFMWS length %d greater than expected %d\n",
> cfmws->header.length, expected_len);
>
> + if ((cfmws->restrictions & ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM) &&
> + (cfmws->restrictions & ACPI_CEDT_CFMWS_RESTRICT_BI)) {
> + dev_err(dev, "CFMWS cannot have both HDM-H and HDM-DB\n");
> + return -EINVAL;
> + }
> +
Plus for this part, it rejects host-only + BI, but device-coherent + host-only isn't caught.
For that combo __cxl_parse_cfmws() now picks DEVMEM, whereas before this series it always defaulted
to HOSTONLYMEM, so it silently changes from host-only to device-coherent.
Is it meant to be legal ? why DEVMEM wins the precedence in __cxl_parse_cfmws() ?
Those 2 coherency models are mutually exclusive, I thought both should be rejected, symmetric with the BI check.
Best regards,
Richard Cheng.
> return 0;
> }
>
> @@ -437,7 +445,8 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
>
> cxld = &cxlrd->cxlsd.cxld;
> cxld->flags = cfmws_to_decoder_flags(cfmws->restrictions);
> - cxld->target_type = CXL_DECODER_HOSTONLYMEM;
> + cxld->target_type = (cxld->flags & CXL_DECODER_F_TYPE2) ?
> + CXL_DECODER_DEVMEM : CXL_DECODER_HOSTONLYMEM;
> cxld->hpa_range = (struct range) {
> .start = cfmws->base_hpa,
> .end = cfmws->base_hpa + cfmws->window_size - 1,
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 4a41cde9fdba..a6844b4577be 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -705,9 +705,25 @@ static void cxld_set_interleave(struct cxl_decoder *cxld, u32 *ctrl)
>
> static void cxld_set_type(struct cxl_decoder *cxld, u32 *ctrl)
> {
> + bool bi = cxld->target_type == CXL_DECODER_DEVMEM;
> +
> + if (bi) {
> + if (is_endpoint_decoder(&cxld->dev)) {
> + struct cxl_endpoint_decoder *cxled =
> + to_cxl_endpoint_decoder(&cxld->dev);
> + struct cxl_dev_state *cxlds =
> + cxled_to_memdev(cxled)->cxlds;
> +
> + bi = cxlds->bi;
> + } else if (cxld->region) {
> + bi = cxl_root_decoder_is_bi(cxld->region->cxlrd);
> + }
> + }
> +
> u32p_replace_bits(ctrl,
> !!(cxld->target_type == CXL_DECODER_HOSTONLYMEM),
> CXL_HDM_DECODER0_CTRL_HOSTONLY);
> + u32p_replace_bits(ctrl, bi, CXL_HDM_DECODER0_CTRL_BI);
> }
>
> static void cxlsd_set_targets(struct cxl_switch_decoder *cxlsd, u64 *tgt)
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index c5aacd7054f1..71801dfd614b 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -131,6 +131,7 @@ CXL_DECODER_FLAG_ATTR(cap_ram, CXL_DECODER_F_RAM);
> CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_TYPE2);
> CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_TYPE3);
> CXL_DECODER_FLAG_ATTR(locked, CXL_DECODER_F_LOCK);
> +CXL_DECODER_FLAG_ATTR(cap_bi, CXL_DECODER_F_BI);
>
> static ssize_t target_type_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> @@ -233,6 +234,19 @@ static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR_RW(mode);
>
> +static ssize_t bi_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(dev);
> + struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> + struct cxl_dev_state *cxlds = cxlmd->cxlds;
> +
> + guard(rwsem_read)(&cxl_rwsem.region);
> + return sysfs_emit(buf, "%d\n", cxlds->bi && cxled->cxld.region &&
> + cxled->cxld.target_type == CXL_DECODER_DEVMEM);
> +}
> +static DEVICE_ATTR_RO(bi);
> +
> static ssize_t dpa_resource_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> @@ -329,6 +343,7 @@ static struct attribute *cxl_decoder_root_attrs[] = {
> &dev_attr_cap_ram.attr,
> &dev_attr_cap_type2.attr,
> &dev_attr_cap_type3.attr,
> + &dev_attr_cap_bi.attr,
> &dev_attr_target_list.attr,
> &dev_attr_qos_class.attr,
> SET_CXL_REGION_ATTR(create_pmem_region)
> @@ -339,16 +354,24 @@ static struct attribute *cxl_decoder_root_attrs[] = {
>
> static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
> {
> - unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM;
> + unsigned long flags = cxlrd->cxlsd.cxld.flags;
> + unsigned long hdm_h, hdm_db;
>
> - return (cxlrd->cxlsd.cxld.flags & flags) == flags;
> + hdm_h = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM;
> + hdm_db = CXL_DECODER_F_TYPE2 | CXL_DECODER_F_BI | CXL_DECODER_F_PMEM;
> +
> + return (flags & hdm_h) == hdm_h || (flags & hdm_db) == hdm_db;
> }
>
> static bool can_create_ram(struct cxl_root_decoder *cxlrd)
> {
> - unsigned long flags = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM;
> + unsigned long flags = cxlrd->cxlsd.cxld.flags;
> + unsigned long hdm_h, hdm_db;
> +
> + hdm_h = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM;
> + hdm_db = CXL_DECODER_F_TYPE2 | CXL_DECODER_F_BI | CXL_DECODER_F_RAM;
>
> - return (cxlrd->cxlsd.cxld.flags & flags) == flags;
> + return (flags & hdm_h) == hdm_h || (flags & hdm_db) == hdm_db;
> }
>
> static umode_t cxl_root_decoder_visible(struct kobject *kobj, struct attribute *a, int n)
> @@ -402,6 +425,7 @@ static const struct attribute_group *cxl_decoder_switch_attribute_groups[] = {
> static struct attribute *cxl_decoder_endpoint_attrs[] = {
> &dev_attr_target_type.attr,
> &dev_attr_mode.attr,
> + &dev_attr_bi.attr,
> &dev_attr_dpa_size.attr,
> &dev_attr_dpa_resource.attr,
> SET_CXL_REGION_ATTR(region)
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index cc41c08c0c0c..79cfb31f09c7 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1132,11 +1132,13 @@ static int cxl_rr_assign_decoder(struct cxl_port *port, struct cxl_region *cxlr,
> /*
> * Endpoints should already match the region type, but backstop that
> * assumption with an assertion. Switch-decoders change mapping-type
> - * based on what is mapped when they are assigned to a region.
> + * based on what is mapped when they are assigned to a region. HDM-DB
> + * endpoint decoders inherit their type from cxlr->type below.
> */
> dev_WARN_ONCE(&cxlr->dev,
> port == cxled_to_port(cxled) &&
> - cxld->target_type != cxlr->type,
> + cxld->target_type != cxlr->type &&
> + !cxl_root_decoder_is_bi(cxlr->cxlrd),
> "%s:%s mismatch decoder type %d -> %d\n",
> dev_name(&cxled_to_memdev(cxled)->dev),
> dev_name(&cxld->dev), cxld->target_type, cxlr->type);
> @@ -2034,6 +2036,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
> struct cxl_region_params *p = &cxlr->params;
> struct cxl_port *ep_port, *root_port;
> struct cxl_dport *dport;
> + struct cxl_hdm *cxlhdm;
> int rc = -ENXIO;
>
> rc = check_interleave_cap(&cxled->cxld, p->interleave_ways,
> @@ -2083,10 +2086,31 @@ static int cxl_region_attach(struct cxl_region *cxlr,
> return -ENXIO;
> }
>
> - if (cxled->cxld.target_type != cxlr->type) {
> - dev_dbg(&cxlr->dev, "%s:%s type mismatch: %d vs %d\n",
> - dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
> - cxled->cxld.target_type, cxlr->type);
> + /*
> + * Verify the device and HDM are capable of the region's flavor before
> + * proceeding. The endpoint decoder's target_type is then inherited
> + * from cxlr->type later in cxl_rr_assign_decoder().
> + */
> + if (cxlr->type == CXL_DECODER_DEVMEM &&
> + cxl_root_decoder_is_bi(cxlrd) && !cxlds->bi) {
> + dev_err(&cxlr->dev, "%s:%s BI not enabled on device\n",
> + dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev));
> + return -ENXIO;
> + }
> +
> + cxlhdm = dev_get_drvdata(&ep_port->dev);
> + if (!cxlhdm)
> + return -ENXIO;
> + if (cxlr->type == CXL_DECODER_HOSTONLYMEM &&
> + cxlhdm->supported_coherency == CXL_HDM_DECODER_COHERENCY_DEV) {
> + dev_warn(&cxlr->dev, "%s:%s HDM is device-coherent only\n",
> + dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev));
> + return -ENXIO;
> + }
> + if (cxlr->type == CXL_DECODER_DEVMEM &&
> + cxlhdm->supported_coherency == CXL_HDM_DECODER_COHERENCY_HOST) {
> + dev_warn(&cxlr->dev, "%s:%s HDM is host-only coherent\n",
> + dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev));
> return -ENXIO;
> }
>
> @@ -2772,6 +2796,7 @@ static ssize_t create_region_store(struct device *dev, const char *buf,
> size_t len, enum cxl_partition_mode mode)
> {
> struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev);
> + enum cxl_decoder_type target_type;
> struct cxl_region *cxlr;
> int rc, id;
>
> @@ -2779,7 +2804,14 @@ static ssize_t create_region_store(struct device *dev, const char *buf,
> if (rc != 1)
> return -EINVAL;
>
> - cxlr = __create_region(cxlrd, mode, id, CXL_DECODER_HOSTONLYMEM);
> + /*
> + * The CFMWS dictates endpoint coherency: a BI-restricted Window
> + * produces an HDM-DB region; otherwise HDM-H.
> + */
> + target_type = cxl_root_decoder_is_bi(cxlrd) ?
> + CXL_DECODER_DEVMEM : CXL_DECODER_HOSTONLYMEM;
> +
> + cxlr = __create_region(cxlrd, mode, id, target_type);
> if (IS_ERR(cxlr))
> return PTR_ERR(cxlr);
>
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 917a6aee5936..29231d0db470 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -287,6 +287,7 @@ int cxl_dport_map_rcd_linkcap(struct pci_dev *pdev, struct cxl_dport *dport);
> #define CXL_DECODER_F_LOCK BIT(4)
> #define CXL_DECODER_F_ENABLE BIT(5)
> #define CXL_DECODER_F_NORMALIZED_ADDRESSING BIT(6)
> +#define CXL_DECODER_F_BI BIT(7)
> #define CXL_DECODER_F_RESET_MASK (CXL_DECODER_F_ENABLE | CXL_DECODER_F_LOCK)
>
> enum cxl_decoder_type {
> @@ -807,6 +808,10 @@ static inline int cxl_root_decoder_autoremove(struct device *host,
> {
> return cxl_decoder_autoremove(host, &cxlrd->cxlsd.cxld);
> }
> +static inline bool cxl_root_decoder_is_bi(struct cxl_root_decoder *cxlrd)
> +{
> + return cxlrd->cxlsd.cxld.flags & CXL_DECODER_F_BI;
> +}
> int cxl_endpoint_autoremove(struct cxl_memdev *cxlmd, struct cxl_port *endpoint);
>
> /**
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 1/5] cxl: Add BI register probing and port initialization
2026-06-23 8:02 ` Richard Cheng
@ 2026-06-29 14:44 ` Davidlohr Bueso
0 siblings, 0 replies; 20+ messages in thread
From: Davidlohr Bueso @ 2026-06-29 14:44 UTC (permalink / raw)
To: Richard Cheng; +Cc: sashiko-reviews, linux-cxl
On Tue, 23 Jun 2026, Richard Cheng wrote:
>On Mon, Jun 15, 2026 at 05:35:41PM +0800, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [High] devm mappings for dport BI registers leak on hotplug due to being called outside the dport devres group in cxl_port_add_dport.
>> --
>>
>> commit a836080fdf13fa334055f7f13a95491b66ef793f
>> Author: Davidlohr Bueso <dave@stgolabs.net>
>>
>> cxl: Add BI register probing and port initialization
>>
>> This commit adds register probing for BI Route Table and BI Decoder capability
>> structures. It introduces helpers to map the BI Decoder of a downstream port
>> at dport-creation time and a port's own BI capability during port probe.
>>
>> > diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
>> > index ada51948d52fa..ca8d8c14b7876 100644
>> > --- a/drivers/cxl/port.c
>> > +++ b/drivers/cxl/port.c
>>
>> [ ... ]
>>
>> > @@ -261,6 +321,17 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
>> > /* This group was only needed for early exit above */
>> > devres_remove_group(&port->dev, no_free_ptr(port_dr_group));
>> >
>> > + if (dev_is_pci(dport_dev)) {
>> > + switch (pci_pcie_type(to_pci_dev(dport_dev))) {
>> > + case PCI_EXP_TYPE_ROOT_PORT:
>> > + case PCI_EXP_TYPE_DOWNSTREAM:
>> > + cxl_dport_map_bi(dport);
>>
>> [Severity: High]
>> Does this leak devm mappings if the dport is hot-removed?
>>
>> Looking at cxl_port_add_dport(), devm_cxl_add_dport_by_dev() creates a
>> devres group for the dport on the parent port's device. However,
>> cxl_dport_map_bi() is called after devm_cxl_add_dport_by_dev() returns,
>> meaning the devres group has already been closed.
>>
>> Because cxl_dport_map_bi() calls devm_cxl_iomap_block() against the parent
>> port device, these devm allocations bypass the dport group and attach
>> directly to the parent port device.
>>
>> If the dport is hot-removed, the dport devres group is released, but the BI
>> capability mappings appear to be left behind. If the dport is subsequently
>> re-added, won't devm_request_mem_region() fail with -EBUSY due to the
>> physical address region conflict, permanently breaking BI capabilities?
>>
>
>Hi Davidlohr,
>
>I think sashiko-bot's is right on this part, this part runs after devm_cxl_add_dport_by_dev()
>has already closed the dport's devres group, so the BI request_mem_region()/ioremap() land on
>the port's devres list outside that group. On runtime dport removal the group release won't free
>them, and re-adding the dport then fails the BI region request with -EBUSY.
>
>The dport RAS mapping is done inside __devm_cxl_add_dport() while the group is still open for
>this reason.
>I would suggest mapping BI the same way which is to move cxl_dport_map_bi() into __devm_cxl_add_dport()
>next to the RAS setup, and set map->host like RAS does.
I went with this adding a new devm_cxl_dport_bi_setup() helper and calling it from
__devm_cxl_add_dport():
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index cdb741d09b9e..5511a8b33062 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -929,6 +929,44 @@ int cxl_port_get_possible_dports(struct cxl_port *port)
return ctx.count;
}
+static void cxl_dport_map_bi(struct cxl_dport *dport)
+{
+ struct cxl_register_map *map = &dport->reg_map;
+ struct device *dev = dport->dport_dev;
+
+ if (!map->component_map.bi_decoder.valid) {
+ dev_dbg(dev, "BI Decoder registers not found\n");
+ return;
+ }
+
+ if (cxl_map_component_regs(map, &dport->regs.component,
+ BIT(CXL_CM_CAP_CAP_ID_BI_DECODER)))
+ dev_dbg(dev, "Failed to map BI Decoder capability\n");
+}
+
+/**
+ * devm_cxl_dport_bi_setup - Map BI Decoder registers on a CXL dport
+ * @dport: the cxl_dport that needs to be initialized
+ *
+ * Must be called while the dport's devres group is open so iomap
+ * allocations are released on dport removal.
+ */
+void devm_cxl_dport_bi_setup(struct cxl_dport *dport)
+{
+ if (!dev_is_pci(dport->dport_dev))
+ return;
+
+ switch (pci_pcie_type(to_pci_dev(dport->dport_dev))) {
+ case PCI_EXP_TYPE_ROOT_PORT:
+ case PCI_EXP_TYPE_DOWNSTREAM:
+ dport->reg_map.host = dport_to_host(dport);
+ cxl_dport_map_bi(dport);
+ break;
+ default:
+ break;
+ }
+}
+
/*
* BI requires 256B Flit operation on the link. RP/DSP/endpoint must
* also have the BI Decoder cap mapped (@bi); for USPs the BI RT cap
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 63005cf0fa91..74bbc195d624 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1264,8 +1264,10 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev,
cxl_debugfs_create_dport_dir(dport);
- if (!dport->rch)
+ if (!dport->rch) {
devm_cxl_dport_ras_setup(dport);
+ devm_cxl_dport_bi_setup(dport);
+ }
/* keep the group, and mark the end of devm actions */
cxl_dport_close_dr_group(dport, no_free_ptr(dport_dr_group));
Thanks,
Davidlohr
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v5 2/5] cxl/pci: Add BI topology enable/disable
2026-06-23 8:24 ` Richard Cheng
@ 2026-06-29 15:08 ` Davidlohr Bueso
0 siblings, 0 replies; 20+ messages in thread
From: Davidlohr Bueso @ 2026-06-29 15:08 UTC (permalink / raw)
To: Richard Cheng
Cc: dave.jiang, djbw, jic23, benjamin.cheatham, alucerop,
alison.schofield, gourry, dongjoo.seo1, linux-cxl
On Tue, 23 Jun 2026, Richard Cheng wrote:
>On Mon, Jun 15, 2026 at 07:55:26AM +0800, Davidlohr Bueso wrote:
>> Implement cxl_bi_setup(), which walks the CXL port topology to enable
>> BI flows on the device and every component in the path between the
>> device and the root port. The walk is two-pass: first to verify each
>> component advertises BI capability, then to write BI Enable on each
>> dport's BI Decoder Control register and on the device. On failure
>> during the enable pass, partially-enabled dports are rolled back.
>>
>> The teardown counterpart, cxl_bi_dealloc(), runs automatically on
>> cxl_mem unbind.
>>
>> cxl_bi_setup() does not touch HDM decoder configuration -- the BI bit
>> in each decoder is programmed separately at commit time. After a
>> successful setup the device is BI-capable but not yet using BI for
>> decode coherence.
>>
>> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
>> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
>> ---
>> drivers/cxl/core/pci.c | 366 +++++++++++++++++++++++++++++++++++++++++
>> drivers/cxl/cxl.h | 28 ++++
>> drivers/cxl/mem.c | 4 +
>> include/cxl/cxl.h | 2 +
>> 4 files changed, 400 insertions(+)
>>
>> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
>> index e4338fd7e01b..cdb741d09b9e 100644
>> --- a/drivers/cxl/core/pci.c
>> +++ b/drivers/cxl/core/pci.c
>> @@ -2,11 +2,13 @@
>> /* Copyright(c) 2021 Intel Corporation. All rights reserved. */
>> #include <linux/units.h>
>> #include <linux/io-64-nonatomic-lo-hi.h>
>> +#include <linux/iopoll.h>
>> #include <linux/device.h>
>> #include <linux/delay.h>
>> #include <linux/pci.h>
>> #include <linux/pci-doe.h>
>> #include <linux/aer.h>
>> +#include <linux/string_choices.h>
>> #include <cxlpci.h>
>> #include <cxlmem.h>
>> #include <cxl.h>
>> @@ -926,3 +928,367 @@ int cxl_port_get_possible_dports(struct cxl_port *port)
>>
>> return ctx.count;
>> }
>> +
>> +/*
>> + * BI requires 256B Flit operation on the link. RP/DSP/endpoint must
>> + * also have the BI Decoder cap mapped (@bi); for USPs the BI RT cap
>> + * is optional per CXL 4.0 8.2.4.26, so absent @bi is allowed.
>> + */
>> +static bool cxl_is_bi_capable(struct pci_dev *pdev, void __iomem *bi)
>> +{
>> + if (!cxl_pci_flit_256(pdev))
>> + return false;
>> + if (pci_pcie_type(pdev) != PCI_EXP_TYPE_UPSTREAM && !bi) {
>> + dev_dbg(&pdev->dev, "No BI Decoder registers.\n");
>> + return false;
>> + }
>> + return true;
>> +}
>> +
>> +/* limit any insane timeouts from hw */
>> +#define CXL_BI_COMMIT_MAXTMO_US (5 * USEC_PER_SEC)
>> +
>> +static unsigned long __cxl_bi_get_timeout_us(struct device *dev,
>> + unsigned int scale,
>> + unsigned int base)
>> +{
>> + static const unsigned long scale_tbl[] = {
>> + 1, 10, 100, 1000, 10000, 100000, 1000000, 10000000,
>> + };
>> +
>> + if (scale >= ARRAY_SIZE(scale_tbl) || !base) {
>> + dev_dbg(dev, "Invalid BI commit timeout: scale=%u base=%u\n",
>> + scale, base);
>> + return CXL_BI_COMMIT_MAXTMO_US;
>> + }
>> +
>> + return scale_tbl[scale] * base;
>> +}
>> +
>> +static int __cxl_bi_wait_commit(struct device *dev, void __iomem *status_reg,
>> + u32 committed_bit, u32 err_bit,
>> + unsigned int scale, unsigned int base)
>> +{
>> + unsigned long tmo_us, poll_us;
>> + ktime_t start;
>> + u32 status;
>> + int rc;
>> +
>> + tmo_us = min_t(unsigned long, CXL_BI_COMMIT_MAXTMO_US,
>> + __cxl_bi_get_timeout_us(dev, scale, base));
>> + poll_us = max_t(unsigned long, tmo_us / 10, 1); /* ~10% */
>> + start = ktime_get();
>> +
>> + rc = readx_poll_timeout(readl, status_reg, status,
>> + status & (committed_bit | err_bit),
>> + poll_us, tmo_us);
>> + if (rc) {
>> + dev_err(dev, "BI-ID commit timed out (%luus)\n", tmo_us);
>> + return rc; /* -ETIMEDOUT */
>> + }
>> +
>> + if (status & err_bit) {
>> + dev_err(dev, "BI-ID commit rejected by hardware\n");
>> + return -EIO;
>> + }
>> +
>> + dev_dbg(dev, "BI-ID commit wait took %lluus\n",
>> + ktime_to_us(ktime_sub(ktime_get(), start)));
>> + return 0;
>> +}
>> +
>> +/* BI RT only exists on switch upstream ports. */
>> +static int __cxl_bi_commit_rt(struct device *dev, void __iomem *bi)
>> +{
>> + u32 status, ctrl;
>> + unsigned int scale, base;
>> +
>> + if (!FIELD_GET(CXL_BI_RT_CAPS_EXPLICIT_COMMIT_REQ,
>> + readl(bi + CXL_BI_RT_CAPS_OFFSET)))
>> + return 0;
>> +
>> + ctrl = readl(bi + CXL_BI_RT_CTRL_OFFSET);
>> + writel(ctrl & ~CXL_BI_RT_CTRL_BI_COMMIT, bi + CXL_BI_RT_CTRL_OFFSET);
>> + writel(ctrl | CXL_BI_RT_CTRL_BI_COMMIT, bi + CXL_BI_RT_CTRL_OFFSET);
>> +
>> + status = readl(bi + CXL_BI_RT_STATUS_OFFSET);
>> + scale = FIELD_GET(CXL_BI_RT_STATUS_BI_COMMIT_TM_SCALE, status);
>> + base = FIELD_GET(CXL_BI_RT_STATUS_BI_COMMIT_TM_BASE, status);
>> +
>> + return __cxl_bi_wait_commit(dev, bi + CXL_BI_RT_STATUS_OFFSET,
>> + CXL_BI_RT_STATUS_BI_COMMITTED,
>> + CXL_BI_RT_STATUS_BI_ERR_NOT_COMMITTED,
>> + scale, base);
>> +}
>> +
>> +static int __cxl_bi_commit_decoder(struct device *dev, void __iomem *bi)
>> +{
>> + u32 status, ctrl;
>> + unsigned int scale, base;
>> +
>> + if (!FIELD_GET(CXL_BI_DECODER_CAPS_EXPLICIT_COMMIT_REQ,
>> + readl(bi + CXL_BI_DECODER_CAPS_OFFSET)))
>> + return 0;
>> +
>> + ctrl = readl(bi + CXL_BI_DECODER_CTRL_OFFSET);
>> + writel(ctrl & ~CXL_BI_DECODER_CTRL_BI_COMMIT,
>> + bi + CXL_BI_DECODER_CTRL_OFFSET);
>> + writel(ctrl | CXL_BI_DECODER_CTRL_BI_COMMIT,
>> + bi + CXL_BI_DECODER_CTRL_OFFSET);
>> +
>> + status = readl(bi + CXL_BI_DECODER_STATUS_OFFSET);
>> + scale = FIELD_GET(CXL_BI_DECODER_STATUS_BI_COMMIT_TM_SCALE, status);
>> + base = FIELD_GET(CXL_BI_DECODER_STATUS_BI_COMMIT_TM_BASE, status);
>> +
>> + return __cxl_bi_wait_commit(dev, bi + CXL_BI_DECODER_STATUS_OFFSET,
>> + CXL_BI_DECODER_STATUS_BI_COMMITTED,
>> + CXL_BI_DECODER_STATUS_BI_ERR_NOT_COMMITTED,
>> + scale, base);
>> +}
>> +
>> +/* Enable or dealloc BI-ID changes in the given level of the topology. */
>> +static int __cxl_bi_ctrl_dport(struct cxl_dport *dport, bool enable)
>> +{
>> + struct pci_dev *pdev = to_pci_dev(dport->dport_dev);
>> + void __iomem *bi = dport->regs.bi_decoder;
>> + struct cxl_port *port = dport->port;
>> + u32 ctrl, value;
>> + int rc;
>> +
>> + guard(device)(&port->dev);
>> + if (!bi)
>> + return -EINVAL;
>> +
>> + ctrl = readl(bi + CXL_BI_DECODER_CTRL_OFFSET);
>> +
>> + switch (pci_pcie_type(pdev)) {
>> + case PCI_EXP_TYPE_ROOT_PORT:
>> + if (enable) {
>> + /*
>> + * There is no point of failure from here on,
>> + * BI will be enabled on the endpoint device.
>> + */
>> + dport->nr_bi++;
>> +
>> + if (FIELD_GET(CXL_BI_DECODER_CTRL_BI_FW, ctrl) &&
>> + !FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl))
>> + return 0;
>> +
>> + value = ctrl | CXL_BI_DECODER_CTRL_BI_FW;
>> + value &= ~CXL_BI_DECODER_CTRL_BI_ENABLE;
>> + } else {
>> + if (WARN_ON_ONCE(dport->nr_bi == 0))
>> + return -EINVAL;
>> + if (--dport->nr_bi > 0)
>> + return 0;
>> +
>> + value = ctrl & ~(CXL_BI_DECODER_CTRL_BI_FW |
>> + CXL_BI_DECODER_CTRL_BI_ENABLE);
>> + }
>> +
>> + writel(value, bi + CXL_BI_DECODER_CTRL_OFFSET);
>> + return 0;
>> + case PCI_EXP_TYPE_DOWNSTREAM:
>> + if (enable) {
>> + value = ctrl & ~CXL_BI_DECODER_CTRL_BI_FW;
>> + value |= CXL_BI_DECODER_CTRL_BI_ENABLE;
>> + } else {
>> + if (!FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl))
>> + return 0;
>> + value = ctrl & ~(CXL_BI_DECODER_CTRL_BI_FW |
>> + CXL_BI_DECODER_CTRL_BI_ENABLE);
>> + }
>> +
>> + writel(value, bi + CXL_BI_DECODER_CTRL_OFFSET);
>> +
>> + rc = __cxl_bi_commit_decoder(dport->dport_dev, bi);
>> + if (rc)
>> + return rc;
>> +
>> + if (port->regs.bi_rt)
>> + return __cxl_bi_commit_rt(&port->dev, port->regs.bi_rt);
>> + return 0;
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static int cxl_bi_ctrl_dport_enable(struct cxl_dport *dport)
>> +{
>> + return __cxl_bi_ctrl_dport(dport, true);
>> +}
>> +
>> +static int cxl_bi_ctrl_dport_disable(struct cxl_dport *dport)
>> +{
>> + return __cxl_bi_ctrl_dport(dport, false);
>> +}
>> +
>> +static int __cxl_bi_ctrl_endpoint(struct cxl_dev_state *cxlds, bool enable)
>> +{
>> + struct cxl_port *endpoint = cxlds->cxlmd->endpoint;
>> + void __iomem *bi = endpoint->regs.bi_decoder;
>> + u32 ctrl, val;
>> +
>> + if (!bi)
>> + return -EINVAL;
>> +
>> + ctrl = readl(bi + CXL_BI_DECODER_CTRL_OFFSET);
>> +
>> + if (enable) {
>> + if (FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl)) {
>> + if (cxlds->bi)
>> + return 0;
>> + dev_err(cxlds->dev,
>> + "BI already enabled in hardware\n");
>> + return -EBUSY;
>> + }
>> + val = ctrl | CXL_BI_DECODER_CTRL_BI_ENABLE;
>> + } else {
>> + if (!FIELD_GET(CXL_BI_DECODER_CTRL_BI_ENABLE, ctrl)) {
>> + if (!cxlds->bi)
>> + return 0;
>> + dev_err(cxlds->dev,
>> + "BI already disabled in hardware\n");
>> + return -EBUSY;
>> + }
>> + val = ctrl & ~CXL_BI_DECODER_CTRL_BI_ENABLE;
>> + }
>> +
>> + writel(val, bi + CXL_BI_DECODER_CTRL_OFFSET);
>> + cxlds->bi = enable;
>> +
>> + dev_dbg(cxlds->dev, "BI requests %s\n",
>> + str_enabled_disabled(enable));
>> +
>> + return 0;
>> +}
>> +
>> +static int cxl_bi_ctrl_endpoint_enable(struct cxl_dev_state *cxlds)
>> +{
>> + return __cxl_bi_ctrl_endpoint(cxlds, true);
>> +}
>> +
>> +static int cxl_bi_ctrl_endpoint_disable(struct cxl_dev_state *cxlds)
>> +{
>> + return __cxl_bi_ctrl_endpoint(cxlds, false);
>> +}
>> +
>> +/*
>> + * devm teardown on cxl_mem unbind. Endpoint decoders may still be
>> + * committed here (cxl_workqueue tears them down asynchronously), but
>> + * memory access has been quiesced.
>> + */
>> +static void cxl_bi_dealloc(void *data)
>> +{
>> + struct cxl_dev_state *cxlds = data;
>> + struct cxl_dport *dport_iter, *dport;
>> + struct cxl_port *port_iter;
>> +
>> + if (!dev_is_pci(cxlds->dev))
>> + return;
>> +
>> + struct cxl_port *port __free(put_cxl_port) =
>> + cxl_pci_find_port(to_pci_dev(cxlds->dev), &dport);
>> + if (!port || !cxlds->bi)
>> + return;
>> +
>> + scoped_guard(rwsem_read, &cxl_rwsem.region)
>> + cxl_bi_ctrl_endpoint_disable(cxlds);
>> +
>> + port_iter = port;
>> + dport_iter = dport;
>> + while (!is_cxl_root(port_iter)) {
>> + int rc = cxl_bi_ctrl_dport_disable(dport_iter);
>> +
>> + /* best effort */
>> + if (rc)
>> + dev_dbg(&port_iter->dev,
>> + "BI dport disable failed: %d\n", rc);
>> +
>> + dport_iter = port_iter->parent_dport;
>> + port_iter = dport_iter->port;
>> + }
>> +}
>
>Hi Davidlohr,
>
>I think the failing dport enablement will get disabled here, too.
>Because you put the "== failed" check after the disable. For a DSP that
>re-runs __cxl_bi_commit_decoder() on HW that just failed to commit during
>enable, I think a second commit timeout up to 5s will occur on every failed
>bring-up.
>
>Rollback should only unwind the dports that fully enabled, stop before the failed one.
Agreed, I had this at some point but then changed it. I never liked the double
timeout, as you point out. See the cover letter:
[ 2.994821] cxl_core:__cxl_bi_wait_commit:985: pcieport 0000:0e:00.0: BI-ID commit timed out (2000000us)
[ 5.010606] cxl_core:__cxl_bi_wait_commit:985: pcieport 0000:0e:00.0: BI-ID commit timed out (2000000us)
I have gone back to only undoing the successfully enabled dports before the error.
Thanks,
Davidlohr
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 4/5] cxl: Add HDM-DB region creation
2026-06-23 8:47 ` Richard Cheng
@ 2026-06-29 15:26 ` Davidlohr Bueso
0 siblings, 0 replies; 20+ messages in thread
From: Davidlohr Bueso @ 2026-06-29 15:26 UTC (permalink / raw)
To: Richard Cheng
Cc: dave.jiang, djbw, jic23, benjamin.cheatham, alucerop,
alison.schofield, gourry, dongjoo.seo1, linux-cxl
On Tue, 23 Jun 2026, Richard Cheng wrote:
>On Mon, Jun 15, 2026 at 07:55:28AM +0800, Davidlohr Bueso wrote:
>> @@ -198,6 +200,12 @@ static int cxl_acpi_cfmws_verify(struct device *dev,
>> dev_dbg(dev, "CFMWS length %d greater than expected %d\n",
>> cfmws->header.length, expected_len);
>>
>> + if ((cfmws->restrictions & ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM) &&
>> + (cfmws->restrictions & ACPI_CEDT_CFMWS_RESTRICT_BI)) {
>> + dev_err(dev, "CFMWS cannot have both HDM-H and HDM-DB\n");
>> + return -EINVAL;
>> + }
>> +
>
>Plus for this part, it rejects host-only + BI, but device-coherent + host-only isn't caught.
>For that combo __cxl_parse_cfmws() now picks DEVMEM, whereas before this series it always defaulted
>to HOSTONLYMEM, so it silently changes from host-only to device-coherent.
I will default back to HOSTONLY.
>
>Is it meant to be legal ? why DEVMEM wins the precedence in __cxl_parse_cfmws() ?
>Those 2 coherency models are mutually exclusive, I thought both should be rejected, symmetric with the BI check.
The text in the spec is not 100% clear, so I just went with the BI+HOSTONLY check (and on the
last cxl upstream call nobody really objected to this). Further, please see the discussion
with Jonathan when doing the qemu window restriction flags:
https://lore.kernel.org/all/20260616141834.012ad646@jic23-huawei/
Thanks,
Davidlohr
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 1/5] cxl: Add BI register probing and port initialization
2026-06-15 14:55 ` [PATCH v5 1/5] cxl: Add BI register probing and port initialization Davidlohr Bueso
2026-06-15 17:35 ` sashiko-bot
@ 2026-07-01 16:57 ` Dave Jiang
1 sibling, 0 replies; 20+ messages in thread
From: Dave Jiang @ 2026-07-01 16:57 UTC (permalink / raw)
To: Davidlohr Bueso, djbw
Cc: jic23, benjamin.cheatham, icheng, alucerop, alison.schofield,
gourry, dongjoo.seo1, linux-cxl
On 6/15/26 7:55 AM, Davidlohr Bueso wrote:
> Add register probing for BI Route Table and BI Decoder capability
> structures in cxl_probe_component_regs(), and helpers to map them.
>
> cxl_dport_map_bi() maps the BI Decoder of a downstream port (root
> port or switch DSP) at dport-creation time via cxl_port_add_dport();
> cxl_port_map_bi() maps a port's own BI capability during port probe
> when the upstream link is in 256B Flit operation -- BI Decoder for
> an endpoint, BI RT for a switch USP.
>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
> drivers/cxl/core/regs.c | 14 ++++++++
> drivers/cxl/cxl.h | 6 ++++
> drivers/cxl/port.c | 71 +++++++++++++++++++++++++++++++++++++++++
> include/cxl/cxl.h | 6 ++++
> 4 files changed, 97 insertions(+)
>
> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> index 93710cf4f0a6..a6caa793e7a4 100644
> --- a/drivers/cxl/core/regs.c
> +++ b/drivers/cxl/core/regs.c
> @@ -92,6 +92,18 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base,
> length = CXL_RAS_CAPABILITY_LENGTH;
> rmap = &map->ras;
> break;
> + case CXL_CM_CAP_CAP_ID_BI_RT:
> + dev_dbg(dev, "found BI RT capability (0x%x)\n",
> + offset);
> + length = CXL_BI_RT_CAPABILITY_LENGTH;
> + rmap = &map->bi_rt;
> + break;
> + case CXL_CM_CAP_CAP_ID_BI_DECODER:
> + dev_dbg(dev, "found BI Decoder capability (0x%x)\n",
> + offset);
> + length = CXL_BI_DECODER_CAPABILITY_LENGTH;
> + rmap = &map->bi_decoder;
> + break;
> default:
> dev_dbg(dev, "Unknown CM cap ID: %d (0x%x)\n", cap_id,
> offset);
> @@ -211,6 +223,8 @@ int cxl_map_component_regs(const struct cxl_register_map *map,
> } mapinfo[] = {
> { &map->component_map.hdm_decoder, ®s->hdm_decoder },
> { &map->component_map.ras, ®s->ras },
> + { &map->component_map.bi_rt, ®s->bi_rt },
> + { &map->component_map.bi_decoder, ®s->bi_decoder },
> };
> int i;
>
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 1297594beaec..760f51b43891 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -42,6 +42,12 @@ extern const struct nvdimm_security_ops *cxl_security_ops;
> #define CXL_CM_CAP_CAP_ID_RAS 0x2
> #define CXL_CM_CAP_CAP_ID_HDM 0x5
> #define CXL_CM_CAP_CAP_HDM_VERSION 1
> +#define CXL_CM_CAP_CAP_ID_BI_RT 0xB
> +#define CXL_CM_CAP_CAP_ID_BI_DECODER 0xC
While we are here, do you mind adding a comment above for the defines and point to 8.2.4 Table 8-74 in r4.0?
DJ
> +
> +/* CXL 4.0 8.2.4.26 / 8.2.4.27 BI Capability Structures */
> +#define CXL_BI_RT_CAPABILITY_LENGTH 0xC
> +#define CXL_BI_DECODER_CAPABILITY_LENGTH 0xC
>
> /* HDM decoders CXL 2.0 8.2.5.12 CXL HDM Decoder Capability Structure */
> #define CXL_HDM_DECODER_CAP_OFFSET 0x0
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index ada51948d52f..ca8d8c14b787 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -58,6 +58,62 @@ static int discover_region(struct device *dev, void *unused)
> return 0;
> }
>
> +static void cxl_dport_map_bi(struct cxl_dport *dport)
> +{
> + struct cxl_register_map *map = &dport->reg_map;
> + struct device *dev = dport->dport_dev;
> +
> + if (!map->component_map.bi_decoder.valid) {
> + dev_dbg(dev, "BI Decoder registers not found\n");
> + return;
> + }
> +
> + if (cxl_map_component_regs(map, &dport->regs.component,
> + BIT(CXL_CM_CAP_CAP_ID_BI_DECODER)))
> + dev_dbg(dev, "Failed to map BI Decoder capability\n");
> +}
> +
> +static void cxl_port_map_bi(struct cxl_port *port)
> +{
> + struct cxl_register_map *map = &port->reg_map;
> + struct cxl_dport *parent_dport = port->parent_dport;
> + struct device *udev;
> + int cap_id;
> +
> + /* no upstream BI registers above host bridges or the cxl_root */
> + if (!parent_dport || is_cxl_root(parent_dport->port))
> + return;
> +
> + udev = is_cxl_endpoint(port) ?
> + port->uport_dev->parent : port->uport_dev;
> + if (!dev_is_pci(udev))
> + return;
> +
> + /* BI requires 256B Flit on the upstream link */
> + if (!cxl_pci_flit_256(to_pci_dev(udev)))
> + return;
> +
> + /* map this port's own BI capability */
> + if (is_cxl_endpoint(port)) {
> + if (!map->component_map.bi_decoder.valid) {
> + dev_dbg(&port->dev, "BI Decoder registers not found\n");
> + return;
> + }
> + cap_id = CXL_CM_CAP_CAP_ID_BI_DECODER;
> + } else {
> + if (!map->component_map.bi_rt.valid) {
> + dev_dbg(&port->dev, "BI RT registers not found\n");
> + return;
> + }
> + cap_id = CXL_CM_CAP_CAP_ID_BI_RT;
> + }
> +
> + map->host = &port->dev;
> + if (cxl_map_component_regs(map, &port->regs, BIT(cap_id)))
> + dev_dbg(&port->dev, "Failed to map BI capability 0x%x\n",
> + cap_id);
> +}
> +
> static int cxl_switch_port_probe(struct cxl_port *port)
> {
> /* Reset nr_dports for rebind of driver */
> @@ -128,6 +184,8 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
> read_cdat_data(port);
> cxl_endpoint_parse_cdat(port);
>
> + cxl_port_map_bi(port);
> +
> get_device(&cxlmd->dev);
> rc = devm_add_action_or_reset(&port->dev, schedule_detach, cxlmd);
> if (rc)
> @@ -252,6 +310,8 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
> * on failure, or the device does not implement RAS registers.
> */
> devm_cxl_port_ras_setup(port);
> +
> + cxl_port_map_bi(port);
> }
>
> dport = devm_cxl_add_dport_by_dev(port, dport_dev);
> @@ -261,6 +321,17 @@ static struct cxl_dport *cxl_port_add_dport(struct cxl_port *port,
> /* This group was only needed for early exit above */
> devres_remove_group(&port->dev, no_free_ptr(port_dr_group));
>
> + if (dev_is_pci(dport_dev)) {
> + switch (pci_pcie_type(to_pci_dev(dport_dev))) {
> + case PCI_EXP_TYPE_ROOT_PORT:
> + case PCI_EXP_TYPE_DOWNSTREAM:
> + cxl_dport_map_bi(dport);
> + break;
> + default:
> + break;
> + }
> + }
> +
> cxl_switch_parse_cdat(dport);
>
> /* New dport added, update the decoder targets */
> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> index fa7269154620..8ce9b4e9ca73 100644
> --- a/include/cxl/cxl.h
> +++ b/include/cxl/cxl.h
> @@ -34,10 +34,14 @@ struct cxl_regs {
> * Common set of CXL Component register block base pointers
> * @hdm_decoder: CXL 2.0 8.2.5.12 CXL HDM Decoder Capability Structure
> * @ras: CXL 2.0 8.2.5.9 CXL RAS Capability Structure
> + * @bi_rt: CXL 4.0 8.2.4.26 CXL BI Route Table Capability Structure
> + * @bi_decoder: CXL 4.0 8.2.4.27 CXL BI Decoder Capability Structure
> */
> struct_group_tagged(cxl_component_regs, component,
> void __iomem *hdm_decoder;
> void __iomem *ras;
> + void __iomem *bi_rt;
> + void __iomem *bi_decoder;
> );
> /*
> * Common set of CXL Device register block base pointers
> @@ -80,6 +84,8 @@ struct cxl_reg_map {
> struct cxl_component_reg_map {
> struct cxl_reg_map hdm_decoder;
> struct cxl_reg_map ras;
> + struct cxl_reg_map bi_rt;
> + struct cxl_reg_map bi_decoder;
> };
>
> struct cxl_device_reg_map {
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 3/5] cxl/hdm: Add BI coherency support for endpoint decoders
2026-06-15 14:55 ` [PATCH v5 3/5] cxl/hdm: Add BI coherency support for endpoint decoders Davidlohr Bueso
2026-06-15 17:32 ` sashiko-bot
@ 2026-07-01 18:10 ` Dave Jiang
1 sibling, 0 replies; 20+ messages in thread
From: Dave Jiang @ 2026-07-01 18:10 UTC (permalink / raw)
To: Davidlohr Bueso, djbw
Cc: jic23, benjamin.cheatham, icheng, alucerop, alison.schofield,
gourry, dongjoo.seo1, linux-cxl
On 6/15/26 7:55 AM, Davidlohr Bueso wrote:
> Cache the HDM decoder's Supported Coherency Models on struct cxl_hdm.
> Region attach consults this to verify the HDM supports the region's
> coherency type.
>
> For uncommitted endpoint decoders, init_hdm_decoder() defaults
> target_type from supported_coherency: Type 3 devices default to
> HDM-DB when the HDM is device-coherent-only, HDM-H otherwise.
>
> Pre-committed decoders with the BI bit set are not supported because
> endpoint and port enumerations are independent -- at decoder
> enumeration cxlds->bi is not yet known, so the topology cannot be
> verified.
>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Just a small request below
> ---
> drivers/cxl/core/hdm.c | 24 +++++++++++++++++++++---
> drivers/cxl/cxl.h | 6 ++++++
> drivers/cxl/cxlmem.h | 2 ++
> 3 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 0c80b76a5f9b..4a41cde9fdba 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -87,6 +87,8 @@ static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
> cxlhdm->iw_cap_mask |= BIT(3) | BIT(6) | BIT(12);
> if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_16_WAY, hdm_cap))
> cxlhdm->iw_cap_mask |= BIT(16);
> + cxlhdm->supported_coherency =
> + FIELD_GET(CXL_HDM_DECODER_SUPPORTED_COHERENCY_MASK, hdm_cap);
> }
>
> static bool should_emulate_decoders(struct cxl_endpoint_dvsec_info *info)
> @@ -1023,6 +1025,14 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
> else
> cxld->target_type = CXL_DECODER_DEVMEM;
>
> + /*
> + * Autocommit BI-enabled decoders is not supported.
> + * At this point cxlds->bi is not yet setup, so there
> + * are no guarantees that the platform supports BI.
> + */
> + if (FIELD_GET(CXL_HDM_DECODER0_CTRL_BI, ctrl))
> + return -ENXIO;
> +
> guard(rwsem_write)(&cxl_rwsem.region);
> if (cxld->id != cxl_num_decoders_committed(port)) {
> dev_warn(&port->dev,
> @@ -1042,12 +1052,20 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
> if (cxled) {
> struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> struct cxl_dev_state *cxlds = cxlmd->cxlds;
> + struct cxl_hdm *cxlhdm = dev_get_drvdata(&port->dev);
>
> /*
> - * Default by devtype until a device arrives that needs
> - * more precision.
> + * HDMs that advertise support for both coherency
> + * modes (CXL_HDM_DECODER_COHERENCY_BOTH) default to
> + * host-only here; the region attach path will switch
> + * target_type to device-coherent if the region's
> + * root decoder has the CFMWS BI bit set. Only HDMs
> + * that strictly support device-coherent mode default
> + * to HDM-DB.
> */
> - if (cxlds->type == CXL_DEVTYPE_CLASSMEM)
> + if (cxlds->type == CXL_DEVTYPE_CLASSMEM &&
> + cxlhdm->supported_coherency !=
> + CXL_HDM_DECODER_COHERENCY_DEV)
> cxld->target_type = CXL_DECODER_HOSTONLYMEM;
> else
> cxld->target_type = CXL_DECODER_DEVMEM;
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 89c0c6ce24a3..917a6aee5936 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -57,6 +57,11 @@ extern const struct nvdimm_security_ops *cxl_security_ops;
> #define CXL_HDM_DECODER_INTERLEAVE_14_12 BIT(9)
> #define CXL_HDM_DECODER_INTERLEAVE_3_6_12_WAY BIT(11)
> #define CXL_HDM_DECODER_INTERLEAVE_16_WAY BIT(12)
> +#define CXL_HDM_DECODER_SUPPORTED_COHERENCY_MASK GENMASK(22, 21)
> +#define CXL_HDM_DECODER_COHERENCY_UNKNOWN 0x0
> +#define CXL_HDM_DECODER_COHERENCY_DEV 0x1
> +#define CXL_HDM_DECODER_COHERENCY_HOST 0x2
> +#define CXL_HDM_DECODER_COHERENCY_BOTH 0x3
Can you update the CXL spec reference at the top of this block to r4.0?
DJ
> #define CXL_HDM_DECODER_CTRL_OFFSET 0x4
> #define CXL_HDM_DECODER_ENABLE BIT(1)
> #define CXL_HDM_DECODER0_BASE_LOW_OFFSET(i) (0x20 * (i) + 0x10)
> @@ -71,6 +76,7 @@ extern const struct nvdimm_security_ops *cxl_security_ops;
> #define CXL_HDM_DECODER0_CTRL_COMMITTED BIT(10)
> #define CXL_HDM_DECODER0_CTRL_COMMIT_ERROR BIT(11)
> #define CXL_HDM_DECODER0_CTRL_HOSTONLY BIT(12)
> +#define CXL_HDM_DECODER0_CTRL_BI BIT(13)
> #define CXL_HDM_DECODER0_TL_LOW(i) (0x20 * (i) + 0x24)
> #define CXL_HDM_DECODER0_TL_HIGH(i) (0x20 * (i) + 0x28)
> #define CXL_HDM_DECODER0_SKIP_LOW(i) CXL_HDM_DECODER0_TL_LOW(i)
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 776c50d1db51..f41d39cc3795 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -837,6 +837,7 @@ int cxl_mem_sanitize(struct cxl_memdev *cxlmd, u16 cmd);
> * @target_count: for switch decoders, max downstream port targets
> * @interleave_mask: interleave granularity capability, see check_interleave_cap()
> * @iw_cap_mask: bitmask of supported interleave ways, see check_interleave_cap()
> + * @supported_coherency: HDM Decoder Capability supported coherency mask
> * @port: mapped cxl_port, see devm_cxl_setup_hdm()
> */
> struct cxl_hdm {
> @@ -845,6 +846,7 @@ struct cxl_hdm {
> unsigned int target_count;
> unsigned int interleave_mask;
> unsigned long iw_cap_mask;
> + unsigned int supported_coherency;
> struct cxl_port *port;
> };
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 5/5] cxl/hdm: Rename decoder coherency flags
2026-06-15 14:55 ` [PATCH v5 5/5] cxl/hdm: Rename decoder coherency flags Davidlohr Bueso
@ 2026-07-01 18:12 ` Dave Jiang
0 siblings, 0 replies; 20+ messages in thread
From: Dave Jiang @ 2026-07-01 18:12 UTC (permalink / raw)
To: Davidlohr Bueso, djbw
Cc: jic23, benjamin.cheatham, icheng, alucerop, alison.schofield,
gourry, dongjoo.seo1, linux-cxl
On 6/15/26 7:55 AM, Davidlohr Bueso wrote:
> Align with the ACPI CXL Window restriction naming and convert
> CXL_DECODER_F_TYPE2/F_TYPE3 to F_DEVMEM/F_HOSTONLY. Type2 and
> Type3 coherency models were named prior to Back-Invalidate.
>
> Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/cxl/acpi.c | 6 +++---
> drivers/cxl/core/port.c | 12 ++++++------
> drivers/cxl/cxl.h | 4 ++--
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 6a9ba82719e9..087dd1e86d40 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -143,9 +143,9 @@ static unsigned long cfmws_to_decoder_flags(int restrictions)
> unsigned long flags = CXL_DECODER_F_ENABLE;
>
> if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_DEVMEM)
> - flags |= CXL_DECODER_F_TYPE2;
> + flags |= CXL_DECODER_F_DEVMEM;
> if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM)
> - flags |= CXL_DECODER_F_TYPE3;
> + flags |= CXL_DECODER_F_HOSTONLY;
> if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_VOLATILE)
> flags |= CXL_DECODER_F_RAM;
> if (restrictions & ACPI_CEDT_CFMWS_RESTRICT_PMEM)
> @@ -445,7 +445,7 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
>
> cxld = &cxlrd->cxlsd.cxld;
> cxld->flags = cfmws_to_decoder_flags(cfmws->restrictions);
> - cxld->target_type = (cxld->flags & CXL_DECODER_F_TYPE2) ?
> + cxld->target_type = (cxld->flags & CXL_DECODER_F_DEVMEM) ?
> CXL_DECODER_DEVMEM : CXL_DECODER_HOSTONLYMEM;
> cxld->hpa_range = (struct range) {
> .start = cfmws->base_hpa,
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 71801dfd614b..63005cf0fa91 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -128,8 +128,8 @@ static DEVICE_ATTR_RO(name)
>
> CXL_DECODER_FLAG_ATTR(cap_pmem, CXL_DECODER_F_PMEM);
> CXL_DECODER_FLAG_ATTR(cap_ram, CXL_DECODER_F_RAM);
> -CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_TYPE2);
> -CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_TYPE3);
> +CXL_DECODER_FLAG_ATTR(cap_type2, CXL_DECODER_F_DEVMEM);
> +CXL_DECODER_FLAG_ATTR(cap_type3, CXL_DECODER_F_HOSTONLY);
> CXL_DECODER_FLAG_ATTR(locked, CXL_DECODER_F_LOCK);
> CXL_DECODER_FLAG_ATTR(cap_bi, CXL_DECODER_F_BI);
>
> @@ -357,8 +357,8 @@ static bool can_create_pmem(struct cxl_root_decoder *cxlrd)
> unsigned long flags = cxlrd->cxlsd.cxld.flags;
> unsigned long hdm_h, hdm_db;
>
> - hdm_h = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_PMEM;
> - hdm_db = CXL_DECODER_F_TYPE2 | CXL_DECODER_F_BI | CXL_DECODER_F_PMEM;
> + hdm_h = CXL_DECODER_F_HOSTONLY | CXL_DECODER_F_PMEM;
> + hdm_db = CXL_DECODER_F_DEVMEM | CXL_DECODER_F_BI | CXL_DECODER_F_PMEM;
>
> return (flags & hdm_h) == hdm_h || (flags & hdm_db) == hdm_db;
> }
> @@ -368,8 +368,8 @@ static bool can_create_ram(struct cxl_root_decoder *cxlrd)
> unsigned long flags = cxlrd->cxlsd.cxld.flags;
> unsigned long hdm_h, hdm_db;
>
> - hdm_h = CXL_DECODER_F_TYPE3 | CXL_DECODER_F_RAM;
> - hdm_db = CXL_DECODER_F_TYPE2 | CXL_DECODER_F_BI | CXL_DECODER_F_RAM;
> + hdm_h = CXL_DECODER_F_HOSTONLY | CXL_DECODER_F_RAM;
> + hdm_db = CXL_DECODER_F_DEVMEM | CXL_DECODER_F_BI | CXL_DECODER_F_RAM;
>
> return (flags & hdm_h) == hdm_h || (flags & hdm_db) == hdm_db;
> }
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 29231d0db470..b0b4ea4ff8a5 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -282,8 +282,8 @@ int cxl_dport_map_rcd_linkcap(struct pci_dev *pdev, struct cxl_dport *dport);
> */
> #define CXL_DECODER_F_RAM BIT(0)
> #define CXL_DECODER_F_PMEM BIT(1)
> -#define CXL_DECODER_F_TYPE2 BIT(2)
> -#define CXL_DECODER_F_TYPE3 BIT(3)
> +#define CXL_DECODER_F_DEVMEM BIT(2)
> +#define CXL_DECODER_F_HOSTONLY BIT(3)
> #define CXL_DECODER_F_LOCK BIT(4)
> #define CXL_DECODER_F_ENABLE BIT(5)
> #define CXL_DECODER_F_NORMALIZED_ADDRESSING BIT(6)
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2026-07-01 18:12 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-15 14:55 [PATCH v5 0/5] cxl: Support Back-Invalidate Davidlohr Bueso
2026-06-15 14:55 ` [PATCH v5 1/5] cxl: Add BI register probing and port initialization Davidlohr Bueso
2026-06-15 17:35 ` sashiko-bot
2026-06-23 8:02 ` Richard Cheng
2026-06-29 14:44 ` Davidlohr Bueso
2026-07-01 16:57 ` Dave Jiang
2026-06-15 14:55 ` [PATCH v5 2/5] cxl/pci: Add BI topology enable/disable Davidlohr Bueso
2026-06-15 17:34 ` sashiko-bot
2026-06-23 8:24 ` Richard Cheng
2026-06-29 15:08 ` Davidlohr Bueso
2026-06-15 14:55 ` [PATCH v5 3/5] cxl/hdm: Add BI coherency support for endpoint decoders Davidlohr Bueso
2026-06-15 17:32 ` sashiko-bot
2026-07-01 18:10 ` Dave Jiang
2026-06-15 14:55 ` [PATCH v5 4/5] cxl: Add HDM-DB region creation Davidlohr Bueso
2026-06-15 17:41 ` sashiko-bot
2026-06-23 8:39 ` Richard Cheng
2026-06-23 8:47 ` Richard Cheng
2026-06-29 15:26 ` Davidlohr Bueso
2026-06-15 14:55 ` [PATCH v5 5/5] cxl/hdm: Rename decoder coherency flags Davidlohr Bueso
2026-07-01 18:12 ` Dave Jiang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.