Skip to content

Commit 303d328

Browse files
committed
Merge branch 'for-7.1/dax-hmem' into cxl-for-next
The series addresses conflicts between HMEM and CXL when handling Soft Reserved memory ranges. CXL will try best effort in claiming the Soft Reserved memory region that are CXL regions. If fails, it will punt back to HMEM. tools/testing/cxl: Test dax_hmem takeover of CXL regions tools/testing/cxl: Simulate auto-assembly failure dax/hmem: Parent dax_hmem devices dax/hmem: Fix singleton confusion between dax_hmem_work and hmem devices dax/hmem: Reduce visibility of dax_cxl coordination symbols cxl/region: Constify cxl_region_resource_contains() cxl/region: Limit visibility of cxl_region_contains_resource() dax/cxl: Fix HMEM dependencies cxl/region: Fix use-after-free from auto assembly failure dax/hmem, cxl: Defer and resolve Soft Reserved ownership cxl/region: Add helper to check Soft Reserved containment by CXL regions dax: Track all dax_region allocations under a global resource tree dax/cxl, hmem: Initialize hmem early and defer dax_cxl binding dax/hmem: Gate Soft Reserved deferral on DEV_DAX_CXL dax/hmem: Request cxl_acpi and cxl_pci before walking Soft Reserved ranges dax/hmem: Factor HMEM registration into __hmem_register_device() dax/bus: Use dax_region_put() in alloc_dax_region() error path
2 parents 7aacc62 + 549b5c1 commit 303d328

16 files changed

Lines changed: 462 additions & 35 deletions

File tree

drivers/cxl/core/region.c

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1103,6 +1103,14 @@ static int cxl_rr_ep_add(struct cxl_region_ref *cxl_rr,
11031103

11041104
if (!cxld->region) {
11051105
cxld->region = cxlr;
1106+
1107+
/*
1108+
* Now that cxld->region is set the intermediate staging state
1109+
* can be cleared.
1110+
*/
1111+
if (cxld == &cxled->cxld &&
1112+
cxled->state == CXL_DECODER_STATE_AUTO_STAGED)
1113+
cxled->state = CXL_DECODER_STATE_AUTO;
11061114
get_device(&cxlr->dev);
11071115
}
11081116

@@ -1844,6 +1852,7 @@ static int cxl_region_attach_auto(struct cxl_region *cxlr,
18441852
pos = p->nr_targets;
18451853
p->targets[pos] = cxled;
18461854
cxled->pos = pos;
1855+
cxled->state = CXL_DECODER_STATE_AUTO_STAGED;
18471856
p->nr_targets++;
18481857

18491858
return 0;
@@ -2193,6 +2202,47 @@ static int cxl_region_attach(struct cxl_region *cxlr,
21932202
return 0;
21942203
}
21952204

2205+
static int cxl_region_by_target(struct device *dev, const void *data)
2206+
{
2207+
const struct cxl_endpoint_decoder *cxled = data;
2208+
struct cxl_region_params *p;
2209+
struct cxl_region *cxlr;
2210+
2211+
if (!is_cxl_region(dev))
2212+
return 0;
2213+
2214+
cxlr = to_cxl_region(dev);
2215+
p = &cxlr->params;
2216+
return p->targets[cxled->pos] == cxled;
2217+
}
2218+
2219+
/*
2220+
* When an auto-region fails to assemble the decoder may be listed as a target,
2221+
* but not fully attached.
2222+
*/
2223+
static void cxl_cancel_auto_attach(struct cxl_endpoint_decoder *cxled)
2224+
{
2225+
struct cxl_region_params *p;
2226+
struct cxl_region *cxlr;
2227+
int pos = cxled->pos;
2228+
2229+
if (cxled->state != CXL_DECODER_STATE_AUTO_STAGED)
2230+
return;
2231+
2232+
struct device *dev __free(put_device) =
2233+
bus_find_device(&cxl_bus_type, NULL, cxled, cxl_region_by_target);
2234+
if (!dev)
2235+
return;
2236+
2237+
cxlr = to_cxl_region(dev);
2238+
p = &cxlr->params;
2239+
2240+
p->nr_targets--;
2241+
cxled->state = CXL_DECODER_STATE_AUTO;
2242+
cxled->pos = -1;
2243+
p->targets[pos] = NULL;
2244+
}
2245+
21962246
static struct cxl_region *
21972247
__cxl_decoder_detach(struct cxl_region *cxlr,
21982248
struct cxl_endpoint_decoder *cxled, int pos,
@@ -2216,8 +2266,10 @@ __cxl_decoder_detach(struct cxl_region *cxlr,
22162266
cxled = p->targets[pos];
22172267
} else {
22182268
cxlr = cxled->cxld.region;
2219-
if (!cxlr)
2269+
if (!cxlr) {
2270+
cxl_cancel_auto_attach(cxled);
22202271
return NULL;
2272+
}
22212273
p = &cxlr->params;
22222274
}
22232275

@@ -4217,6 +4269,36 @@ static int cxl_region_setup_poison(struct cxl_region *cxlr)
42174269
return devm_add_action_or_reset(dev, remove_debugfs, dentry);
42184270
}
42194271

4272+
static int region_contains_resource(struct device *dev, const void *data)
4273+
{
4274+
const struct resource *res = data;
4275+
struct cxl_region *cxlr;
4276+
struct cxl_region_params *p;
4277+
4278+
if (!is_cxl_region(dev))
4279+
return 0;
4280+
4281+
cxlr = to_cxl_region(dev);
4282+
p = &cxlr->params;
4283+
4284+
if (p->state != CXL_CONFIG_COMMIT)
4285+
return 0;
4286+
4287+
if (!p->res)
4288+
return 0;
4289+
4290+
return resource_contains(p->res, res) ? 1 : 0;
4291+
}
4292+
4293+
bool cxl_region_contains_resource(const struct resource *res)
4294+
{
4295+
guard(rwsem_read)(&cxl_rwsem.region);
4296+
struct device *dev __free(put_device) = bus_find_device(
4297+
&cxl_bus_type, NULL, res, region_contains_resource);
4298+
return !!dev;
4299+
}
4300+
EXPORT_SYMBOL_FOR_MODULES(cxl_region_contains_resource, "dax_hmem");
4301+
42204302
static int cxl_region_can_probe(struct cxl_region *cxlr)
42214303
{
42224304
struct cxl_region_params *p = &cxlr->params;

drivers/cxl/cxl.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,12 +287,14 @@ struct cxl_decoder {
287287
};
288288

289289
/*
290-
* Track whether this decoder is reserved for region autodiscovery, or
291-
* free for userspace provisioning.
290+
* Track whether this decoder is free for userspace provisioning, reserved for
291+
* region autodiscovery, whether it is started connecting (awaiting other
292+
* peers), or has completed auto assembly.
292293
*/
293294
enum cxl_decoder_state {
294295
CXL_DECODER_STATE_MANUAL,
295296
CXL_DECODER_STATE_AUTO,
297+
CXL_DECODER_STATE_AUTO_STAGED,
296298
};
297299

298300
/**
@@ -843,6 +845,7 @@ struct cxl_pmem_region *to_cxl_pmem_region(struct device *dev);
843845
int cxl_add_to_region(struct cxl_endpoint_decoder *cxled);
844846
struct cxl_dax_region *to_cxl_dax_region(struct device *dev);
845847
u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa);
848+
bool cxl_region_contains_resource(const struct resource *res);
846849
#else
847850
static inline bool is_cxl_pmem_region(struct device *dev)
848851
{
@@ -865,6 +868,10 @@ static inline u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint,
865868
{
866869
return 0;
867870
}
871+
static inline bool cxl_region_contains_resource(const struct resource *res)
872+
{
873+
return false;
874+
}
868875
#endif
869876

870877
void cxl_endpoint_parse_cdat(struct cxl_port *port);

drivers/dax/Kconfig

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ config DEV_DAX_HMEM
3232
depends on EFI_SOFT_RESERVE
3333
select NUMA_KEEP_MEMINFO if NUMA_MEMBLKS
3434
default DEV_DAX
35+
depends on CXL_ACPI || !CXL_ACPI
36+
depends on CXL_PCI || !CXL_PCI
37+
depends on CXL_BUS || !CXL_BUS
3538
help
3639
EFI 2.8 platforms, and others, may advertise 'specific purpose'
3740
memory. For example, a high bandwidth memory pool. The
@@ -48,6 +51,7 @@ config DEV_DAX_CXL
4851
tristate "CXL DAX: direct access to CXL RAM regions"
4952
depends on CXL_BUS && CXL_REGION && DEV_DAX
5053
default CXL_REGION && DEV_DAX
54+
depends on DEV_DAX_HMEM || !DEV_DAX_HMEM
5155
help
5256
CXL RAM regions are either mapped by platform-firmware
5357
and published in the initial system-memory map as "System RAM", mapped

drivers/dax/Makefile

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
# SPDX-License-Identifier: GPL-2.0
2+
obj-y += hmem/
23
obj-$(CONFIG_DAX) += dax.o
34
obj-$(CONFIG_DEV_DAX) += device_dax.o
45
obj-$(CONFIG_DEV_DAX_KMEM) += kmem.o
@@ -10,5 +11,3 @@ dax-y += bus.o
1011
device_dax-y := device.o
1112
dax_pmem-y := pmem.o
1213
dax_cxl-y := cxl.o
13-
14-
obj-y += hmem/

drivers/dax/bus.c

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "dax-private.h"
1111
#include "bus.h"
1212

13+
static struct resource dax_regions = DEFINE_RES_MEM_NAMED(0, -1, "DAX Regions");
1314
static DEFINE_MUTEX(dax_bus_lock);
1415

1516
/*
@@ -627,6 +628,7 @@ static void dax_region_unregister(void *region)
627628

628629
sysfs_remove_groups(&dax_region->dev->kobj,
629630
dax_region_attribute_groups);
631+
release_resource(&dax_region->res);
630632
dax_region_put(dax_region);
631633
}
632634

@@ -635,6 +637,7 @@ struct dax_region *alloc_dax_region(struct device *parent, int region_id,
635637
unsigned long flags)
636638
{
637639
struct dax_region *dax_region;
640+
int rc;
638641

639642
/*
640643
* The DAX core assumes that it can store its private data in
@@ -667,14 +670,25 @@ struct dax_region *alloc_dax_region(struct device *parent, int region_id,
667670
.flags = IORESOURCE_MEM | flags,
668671
};
669672

670-
if (sysfs_create_groups(&parent->kobj, dax_region_attribute_groups)) {
671-
kfree(dax_region);
672-
return NULL;
673+
rc = request_resource(&dax_regions, &dax_region->res);
674+
if (rc) {
675+
dev_dbg(parent, "dax_region resource conflict for %pR\n",
676+
&dax_region->res);
677+
goto err_res;
673678
}
674679

680+
if (sysfs_create_groups(&parent->kobj, dax_region_attribute_groups))
681+
goto err_sysfs;
682+
675683
if (devm_add_action_or_reset(parent, dax_region_unregister, dax_region))
676684
return NULL;
677685
return dax_region;
686+
687+
err_sysfs:
688+
release_resource(&dax_region->res);
689+
err_res:
690+
dax_region_put(dax_region);
691+
return NULL;
678692
}
679693
EXPORT_SYMBOL_GPL(alloc_dax_region);
680694

drivers/dax/bus.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
#ifndef __DAX_BUS_H__
44
#define __DAX_BUS_H__
55
#include <linux/device.h>
6+
#include <linux/platform_device.h>
67
#include <linux/range.h>
8+
#include <linux/workqueue.h>
79

810
struct dev_dax;
911
struct resource;
@@ -49,6 +51,24 @@ void dax_driver_unregister(struct dax_device_driver *dax_drv);
4951
void kill_dev_dax(struct dev_dax *dev_dax);
5052
bool static_dev_dax(struct dev_dax *dev_dax);
5153

54+
struct hmem_platform_device {
55+
struct platform_device pdev;
56+
struct work_struct work;
57+
bool did_probe;
58+
};
59+
60+
static inline struct hmem_platform_device *
61+
to_hmem_platform_device(struct platform_device *pdev)
62+
{
63+
return container_of(pdev, struct hmem_platform_device, pdev);
64+
}
65+
66+
#if IS_ENABLED(CONFIG_DEV_DAX_HMEM)
67+
void dax_hmem_flush_work(void);
68+
#else
69+
static inline void dax_hmem_flush_work(void) { }
70+
#endif
71+
5272
#define MODULE_ALIAS_DAX_DEVICE(type) \
5373
MODULE_ALIAS("dax:t" __stringify(type) "*")
5474
#define DAX_DEVICE_MODALIAS_FMT "dax:t%d"

drivers/dax/cxl.c

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,36 @@ static struct cxl_driver cxl_dax_region_driver = {
3838
.id = CXL_DEVICE_DAX_REGION,
3939
.drv = {
4040
.suppress_bind_attrs = true,
41+
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
4142
},
4243
};
4344

44-
module_cxl_driver(cxl_dax_region_driver);
45+
static void cxl_dax_region_driver_register(struct work_struct *work)
46+
{
47+
dax_hmem_flush_work();
48+
cxl_driver_register(&cxl_dax_region_driver);
49+
}
50+
51+
static DECLARE_WORK(cxl_dax_region_driver_work, cxl_dax_region_driver_register);
52+
53+
static int __init cxl_dax_region_init(void)
54+
{
55+
/*
56+
* Need to resolve a race with dax_hmem wanting to drive regions
57+
* instead of CXL
58+
*/
59+
queue_work(system_long_wq, &cxl_dax_region_driver_work);
60+
return 0;
61+
}
62+
module_init(cxl_dax_region_init);
63+
64+
static void __exit cxl_dax_region_exit(void)
65+
{
66+
flush_work(&cxl_dax_region_driver_work);
67+
cxl_driver_unregister(&cxl_dax_region_driver);
68+
}
69+
module_exit(cxl_dax_region_exit);
70+
4571
MODULE_ALIAS_CXL(CXL_DEVICE_DAX_REGION);
4672
MODULE_DESCRIPTION("CXL DAX: direct access to CXL regions");
4773
MODULE_LICENSE("GPL");

drivers/dax/hmem/device.c

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <linux/module.h>
55
#include <linux/dax.h>
66
#include <linux/mm.h>
7+
#include "../bus.h"
78

89
static bool nohmem;
910
module_param_named(disable, nohmem, bool, 0444);
@@ -33,9 +34,21 @@ int walk_hmem_resources(struct device *host, walk_hmem_fn fn)
3334
}
3435
EXPORT_SYMBOL_GPL(walk_hmem_resources);
3536

37+
static void hmem_work(struct work_struct *work)
38+
{
39+
/* place holder until dax_hmem driver attaches */
40+
}
41+
42+
static struct hmem_platform_device hmem_platform = {
43+
.pdev = {
44+
.name = "hmem_platform",
45+
.id = 0,
46+
},
47+
.work = __WORK_INITIALIZER(hmem_platform.work, hmem_work),
48+
};
49+
3650
static void __hmem_register_resource(int target_nid, struct resource *res)
3751
{
38-
struct platform_device *pdev;
3952
struct resource *new;
4053
int rc;
4154

@@ -51,17 +64,13 @@ static void __hmem_register_resource(int target_nid, struct resource *res)
5164
if (platform_initialized)
5265
return;
5366

54-
pdev = platform_device_alloc("hmem_platform", 0);
55-
if (!pdev) {
67+
rc = platform_device_register(&hmem_platform.pdev);
68+
if (rc) {
5669
pr_err_once("failed to register device-dax hmem_platform device\n");
5770
return;
5871
}
5972

60-
rc = platform_device_add(pdev);
61-
if (rc)
62-
platform_device_put(pdev);
63-
else
64-
platform_initialized = true;
73+
platform_initialized = true;
6574
}
6675

6776
void hmem_register_resource(int target_nid, struct resource *res)

0 commit comments

Comments
 (0)