Skip to content

Commit

Permalink
Merge tag 'cxl-fixes-for-6.1-rc4' of git://git.kernel.org/pub/scm/lin…
Browse files Browse the repository at this point in the history
…ux/kernel/git/cxl/cxl

Pull cxl fixes from Dan Williams:
 "Several fixes for CXL region creation crashes, leaks and failures.

  This is mainly fallout from the original implementation of dynamic CXL
  region creation (instantiate new physical memory pools) that arrived
  in v6.0-rc1.

  Given the theme of "failures in the presence of pass-through decoders"
  this also includes new regression test infrastructure for that case.

  Summary:

   - Fix region creation crash with pass-through decoders

   - Fix region creation crash when no decoder allocation fails

   - Fix region creation crash when scanning regions to enforce the
     increasing physical address order constraint that CXL mandates

   - Fix a memory leak for cxl_pmem_region objects, track 1:N instead of
     1:1 memory-device-to-region associations.

   - Fix a memory leak for cxl_region objects when regions with active
     targets are deleted

   - Fix assignment of NUMA nodes to CXL regions by CFMWS (CXL Window)
     emulated proximity domains.

   - Fix region creation failure for switch attached devices downstream
     of a single-port host-bridge

   - Fix false positive memory leak of cxl_region objects by recycling
     recently used region ids rather than freeing them

   - Add regression test infrastructure for a pass-through decoder
     configuration

   - Fix some mailbox payload handling corner cases"

* tag 'cxl-fixes-for-6.1-rc4' of git://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl:
  cxl/region: Recycle region ids
  cxl/region: Fix 'distance' calculation with passthrough ports
  tools/testing/cxl: Add a single-port host-bridge regression config
  tools/testing/cxl: Fix some error exits
  cxl/pmem: Fix cxl_pmem_region and cxl_memdev leak
  cxl/region: Fix cxl_region leak, cleanup targets at region delete
  cxl/region: Fix region HPA ordering validation
  cxl/pmem: Use size_add() against integer overflow
  cxl/region: Fix decoder allocation crash
  ACPI: NUMA: Add CXL CFMWS 'nodes' to the possible nodes set
  cxl/pmem: Fix failure to account for 8 byte header for writes to the device LSA.
  cxl/region: Fix null pointer dereference due to pass through decoder commit
  cxl/mbox: Add a check on input payload size
  • Loading branch information
torvalds committed Nov 6, 2022
2 parents aa52994 + 8f401ec commit 16c7a36
Show file tree
Hide file tree
Showing 8 changed files with 448 additions and 91 deletions.
1 change: 1 addition & 0 deletions drivers/acpi/numa/srat.c
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ static int __init acpi_parse_cfmws(union acpi_subtable_headers *header,
pr_warn("ACPI NUMA: Failed to add memblk for CFMWS node %d [mem %#llx-%#llx]\n",
node, start, end);
}
node_set(node, numa_nodes_parsed);

/* Set the next available fake_pxm value */
(*fake_pxm)++;
Expand Down
2 changes: 1 addition & 1 deletion drivers/cxl/core/mbox.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ int cxl_mbox_send_cmd(struct cxl_dev_state *cxlds, u16 opcode, void *in,
};
int rc;

if (out_size > cxlds->payload_size)
if (in_size > cxlds->payload_size || out_size > cxlds->payload_size)
return -E2BIG;

rc = cxlds->mbox_send(cxlds, &mbox_cmd);
Expand Down
2 changes: 2 additions & 0 deletions drivers/cxl/core/pmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ static void cxl_nvdimm_release(struct device *dev)
{
struct cxl_nvdimm *cxl_nvd = to_cxl_nvdimm(dev);

xa_destroy(&cxl_nvd->pmem_regions);
kfree(cxl_nvd);
}

Expand Down Expand Up @@ -230,6 +231,7 @@ static struct cxl_nvdimm *cxl_nvdimm_alloc(struct cxl_memdev *cxlmd)

dev = &cxl_nvd->dev;
cxl_nvd->cxlmd = cxlmd;
xa_init(&cxl_nvd->pmem_regions);
device_initialize(dev);
lockdep_set_class(&dev->mutex, &cxl_nvdimm_key);
device_set_pm_not_required(dev);
Expand Down
11 changes: 9 additions & 2 deletions drivers/cxl/core/port.c
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,7 @@ static struct cxl_dport *find_dport(struct cxl_port *port, int id)
static int add_dport(struct cxl_port *port, struct cxl_dport *new)
{
struct cxl_dport *dup;
int rc;

device_lock_assert(&port->dev);
dup = find_dport(port, new->port_id);
Expand All @@ -821,8 +822,14 @@ static int add_dport(struct cxl_port *port, struct cxl_dport *new)
dev_name(dup->dport));
return -EBUSY;
}
return xa_insert(&port->dports, (unsigned long)new->dport, new,
GFP_KERNEL);

rc = xa_insert(&port->dports, (unsigned long)new->dport, new,
GFP_KERNEL);
if (rc)
return rc;

port->nr_dports++;
return 0;
}

/*
Expand Down
113 changes: 85 additions & 28 deletions drivers/cxl/core/region.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ static int cxl_region_decode_commit(struct cxl_region *cxlr)
iter = to_cxl_port(iter->dev.parent)) {
cxl_rr = cxl_rr_load(iter, cxlr);
cxld = cxl_rr->decoder;
rc = cxld->commit(cxld);
if (cxld->commit)
rc = cxld->commit(cxld);
if (rc)
break;
}
Expand Down Expand Up @@ -657,6 +658,9 @@ static struct cxl_region_ref *alloc_region_ref(struct cxl_port *port,
xa_for_each(&port->regions, index, iter) {
struct cxl_region_params *ip = &iter->region->params;

if (!ip->res)
continue;

if (ip->res->start > p->res->start) {
dev_dbg(&cxlr->dev,
"%s: HPA order violation %s:%pr vs %pr\n",
Expand Down Expand Up @@ -686,18 +690,27 @@ static struct cxl_region_ref *alloc_region_ref(struct cxl_port *port,
return cxl_rr;
}

static void free_region_ref(struct cxl_region_ref *cxl_rr)
static void cxl_rr_free_decoder(struct cxl_region_ref *cxl_rr)
{
struct cxl_port *port = cxl_rr->port;
struct cxl_region *cxlr = cxl_rr->region;
struct cxl_decoder *cxld = cxl_rr->decoder;

if (!cxld)
return;

dev_WARN_ONCE(&cxlr->dev, cxld->region != cxlr, "region mismatch\n");
if (cxld->region == cxlr) {
cxld->region = NULL;
put_device(&cxlr->dev);
}
}

static void free_region_ref(struct cxl_region_ref *cxl_rr)
{
struct cxl_port *port = cxl_rr->port;
struct cxl_region *cxlr = cxl_rr->region;

cxl_rr_free_decoder(cxl_rr);
xa_erase(&port->regions, (unsigned long)cxlr);
xa_destroy(&cxl_rr->endpoints);
kfree(cxl_rr);
Expand Down Expand Up @@ -728,6 +741,33 @@ static int cxl_rr_ep_add(struct cxl_region_ref *cxl_rr,
return 0;
}

static int cxl_rr_alloc_decoder(struct cxl_port *port, struct cxl_region *cxlr,
struct cxl_endpoint_decoder *cxled,
struct cxl_region_ref *cxl_rr)
{
struct cxl_decoder *cxld;

if (port == cxled_to_port(cxled))
cxld = &cxled->cxld;
else
cxld = cxl_region_find_decoder(port, cxlr);
if (!cxld) {
dev_dbg(&cxlr->dev, "%s: no decoder available\n",
dev_name(&port->dev));
return -EBUSY;
}

if (cxld->region) {
dev_dbg(&cxlr->dev, "%s: %s already attached to %s\n",
dev_name(&port->dev), dev_name(&cxld->dev),
dev_name(&cxld->region->dev));
return -EBUSY;
}

cxl_rr->decoder = cxld;
return 0;
}

/**
* cxl_port_attach_region() - track a region's interest in a port by endpoint
* @port: port to add a new region reference 'struct cxl_region_ref'
Expand Down Expand Up @@ -794,12 +834,6 @@ static int cxl_port_attach_region(struct cxl_port *port,
cxl_rr->nr_targets++;
nr_targets_inc = true;
}

/*
* The decoder for @cxlr was allocated when the region was first
* attached to @port.
*/
cxld = cxl_rr->decoder;
} else {
cxl_rr = alloc_region_ref(port, cxlr);
if (IS_ERR(cxl_rr)) {
Expand All @@ -810,26 +844,11 @@ static int cxl_port_attach_region(struct cxl_port *port,
}
nr_targets_inc = true;

if (port == cxled_to_port(cxled))
cxld = &cxled->cxld;
else
cxld = cxl_region_find_decoder(port, cxlr);
if (!cxld) {
dev_dbg(&cxlr->dev, "%s: no decoder available\n",
dev_name(&port->dev));
goto out_erase;
}

if (cxld->region) {
dev_dbg(&cxlr->dev, "%s: %s already attached to %s\n",
dev_name(&port->dev), dev_name(&cxld->dev),
dev_name(&cxld->region->dev));
rc = -EBUSY;
rc = cxl_rr_alloc_decoder(port, cxlr, cxled, cxl_rr);
if (rc)
goto out_erase;
}

cxl_rr->decoder = cxld;
}
cxld = cxl_rr->decoder;

rc = cxl_rr_ep_add(cxl_rr, cxled);
if (rc) {
Expand Down Expand Up @@ -971,7 +990,14 @@ static int cxl_port_setup_targets(struct cxl_port *port,
if (cxl_rr->nr_targets_set) {
int i, distance;

distance = p->nr_targets / cxl_rr->nr_targets;
/*
* Passthrough ports impose no distance requirements between
* peers
*/
if (port->nr_dports == 1)
distance = 0;
else
distance = p->nr_targets / cxl_rr->nr_targets;
for (i = 0; i < cxl_rr->nr_targets_set; i++)
if (ep->dport == cxlsd->target[i]) {
rc = check_last_peer(cxled, ep, cxl_rr,
Expand Down Expand Up @@ -1508,9 +1534,24 @@ static const struct attribute_group *region_groups[] = {

static void cxl_region_release(struct device *dev)
{
struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev->parent);
struct cxl_region *cxlr = to_cxl_region(dev);
int id = atomic_read(&cxlrd->region_id);

/*
* Try to reuse the recently idled id rather than the cached
* next id to prevent the region id space from increasing
* unnecessarily.
*/
if (cxlr->id < id)
if (atomic_try_cmpxchg(&cxlrd->region_id, &id, cxlr->id)) {
memregion_free(id);
goto out;
}

memregion_free(cxlr->id);
out:
put_device(dev->parent);
kfree(cxlr);
}

Expand Down Expand Up @@ -1538,8 +1579,19 @@ static struct cxl_region *to_cxl_region(struct device *dev)
static void unregister_region(void *dev)
{
struct cxl_region *cxlr = to_cxl_region(dev);
struct cxl_region_params *p = &cxlr->params;
int i;

device_del(dev);

/*
* Now that region sysfs is shutdown, the parameter block is now
* read-only, so no need to hold the region rwsem to access the
* region parameters.
*/
for (i = 0; i < p->interleave_ways; i++)
detach_target(cxlr, i);

cxl_region_iomem_release(cxlr);
put_device(dev);
}
Expand All @@ -1561,6 +1613,11 @@ static struct cxl_region *cxl_region_alloc(struct cxl_root_decoder *cxlrd, int i
device_initialize(dev);
lockdep_set_class(&dev->mutex, &cxl_region_key);
dev->parent = &cxlrd->cxlsd.cxld.dev;
/*
* Keep root decoder pinned through cxl_region_release to fixup
* region id allocations
*/
get_device(dev->parent);
device_set_pm_not_required(dev);
dev->bus = &cxl_bus_type;
dev->type = &cxl_region_type;
Expand Down
4 changes: 3 additions & 1 deletion drivers/cxl/cxl.h
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ struct cxl_nvdimm {
struct device dev;
struct cxl_memdev *cxlmd;
struct cxl_nvdimm_bridge *bridge;
struct cxl_pmem_region *region;
struct xarray pmem_regions;
};

struct cxl_pmem_region_mapping {
Expand Down Expand Up @@ -457,6 +457,7 @@ struct cxl_pmem_region {
* @regions: cxl_region_ref instances, regions mapped by this port
* @parent_dport: dport that points to this port in the parent
* @decoder_ida: allocator for decoder ids
* @nr_dports: number of entries in @dports
* @hdm_end: track last allocated HDM decoder instance for allocation ordering
* @commit_end: cursor to track highest committed decoder for commit ordering
* @component_reg_phys: component register capability base address (optional)
Expand All @@ -475,6 +476,7 @@ struct cxl_port {
struct xarray regions;
struct cxl_dport *parent_dport;
struct ida decoder_ida;
int nr_dports;
int hdm_end;
int commit_end;
resource_size_t component_reg_phys;
Expand Down
Loading

0 comments on commit 16c7a36

Please sign in to comment.