diff options
| author | Dan Williams <dan.j.williams@intel.com> | 2023-12-21 21:05:01 -0800 |
|---|---|---|
| committer | Dan Williams <dan.j.williams@intel.com> | 2024-01-04 13:59:50 -0800 |
| commit | 5459e186a5c9f412334321cff58d70dcb0e48a04 (patch) | |
| tree | cac3af985a300d1ee43ffe0bb6f3a0c1fe37685e | |
| parent | d6488fee66472b468ed88d265b14aa3f04dc3bdf (diff) | |
| download | lwn-5459e186a5c9f412334321cff58d70dcb0e48a04.tar.gz lwn-5459e186a5c9f412334321cff58d70dcb0e48a04.zip | |
cxl/port: Fix missing target list lock
cxl_port_setup_targets() modifies the ->targets[] array of a switch
decoder. target_list_show() expects to be able to emit a coherent
snapshot of that array by "holding" ->target_lock for read. The
target_lock is held for write during initialization of the ->targets[]
array, but it is not held for write during cxl_port_setup_targets().
The ->target_lock() predates the introduction of @cxl_region_rwsem. That
semaphore protects changes to host-physical-address (HPA) decode which
is precisely what writes to a switch decoder's target list affects.
Replace ->target_lock with @cxl_region_rwsem.
Now the side-effect of snapshotting a unstable view of a decoder's
target list is likely benign so the Fixes: tag is presumptive.
Fixes: 27b3f8d13830 ("cxl/region: Program target lists")
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
| -rw-r--r-- | drivers/cxl/core/port.c | 22 | ||||
| -rw-r--r-- | drivers/cxl/cxl.h | 2 |
2 files changed, 7 insertions, 17 deletions
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 57495cdc181f..6b386771cc25 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -172,14 +172,10 @@ static ssize_t target_list_show(struct device *dev, { struct cxl_switch_decoder *cxlsd = to_cxl_switch_decoder(dev); ssize_t offset; - unsigned int seq; int rc; - do { - seq = read_seqbegin(&cxlsd->target_lock); - rc = emit_target_list(cxlsd, buf); - } while (read_seqretry(&cxlsd->target_lock, seq)); - + guard(rwsem_read)(&cxl_region_rwsem); + rc = emit_target_list(cxlsd, buf); if (rc < 0) return rc; offset = rc; @@ -1633,7 +1629,7 @@ EXPORT_SYMBOL_NS_GPL(cxl_mem_find_port, CXL); static int decoder_populate_targets(struct cxl_switch_decoder *cxlsd, struct cxl_port *port, int *target_map) { - int i, rc = 0; + int i; if (!target_map) return 0; @@ -1643,19 +1639,16 @@ static int decoder_populate_targets(struct cxl_switch_decoder *cxlsd, if (xa_empty(&port->dports)) return -EINVAL; - write_seqlock(&cxlsd->target_lock); + guard(rwsem_write)(&cxl_region_rwsem); for (i = 0; i < cxlsd->cxld.interleave_ways; i++) { struct cxl_dport *dport = find_dport(port, target_map[i]); - if (!dport) { - rc = -ENXIO; - break; - } + if (!dport) + return -ENXIO; cxlsd->target[i] = dport; } - write_sequnlock(&cxlsd->target_lock); - return rc; + return 0; } struct cxl_dport *cxl_hb_modulo(struct cxl_root_decoder *cxlrd, int pos) @@ -1725,7 +1718,6 @@ static int cxl_switch_decoder_init(struct cxl_port *port, return -EINVAL; cxlsd->nr_targets = nr_targets; - seqlock_init(&cxlsd->target_lock); return cxl_decoder_init(port, &cxlsd->cxld); } diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 687043ece101..62fa96f8567e 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -412,7 +412,6 @@ struct cxl_endpoint_decoder { /** * struct cxl_switch_decoder - Switch specific CXL HDM Decoder * @cxld: base cxl_decoder object - * @target_lock: coordinate coherent reads of the target list * @nr_targets: number of elements in @target * @target: active ordered target list in current decoder configuration * @@ -424,7 +423,6 @@ struct cxl_endpoint_decoder { */ struct cxl_switch_decoder { struct cxl_decoder cxld; - seqlock_t target_lock; int nr_targets; struct cxl_dport *target[]; }; |