Skip to content

Commit 7c22677

Browse files
committed
Merge tag 'cxl-fixes-for-5.12-rc8' of git://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl
Pull CXL memory class fixes from Dan Williams: "A collection of fixes for the CXL memory class driver introduced in this release cycle. The driver was primarily developed on a work-in-progress QEMU emulation of the interface and we have since found a couple places where it hid spec compliance bugs in the driver, or had a spec implementation bug itself. The biggest change here is replacing a percpu_ref with an rwsem to cleanup a couple bugs in the error unwind path during ioctl device init. Lastly there were some minor cleanups to not export the power-management sysfs-ABI for the ioctl device, use the proper sysfs helper for emitting values, and prevent subtle bugs as new administration commands are added to the supported list. The bulk of it has appeared in -next save for the top commit which was found today and validated on a fixed-up QEMU model. Summary: - Fix support for CXL memory devices with registers offset from the BAR base. - Fix the reporting of device capacity. - Fix the driver commands list definition to be disconnected from the UAPI command list. - Replace percpu_ref with rwsem to fix initialization error path. - Fix leaks in the driver initialization error path. - Drop the power/ directory from CXL device sysfs. - Use the recommended sysfs helper for attribute 'show' implementations" * tag 'cxl-fixes-for-5.12-rc8' of git://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl: cxl/mem: Fix memory device capacity probing cxl/mem: Fix register block offset calculation cxl/mem: Force array size of mem_commands[] to CXL_MEM_COMMAND_ID_MAX cxl/mem: Disable cxl device power management cxl/mem: Do not rely on device_add() side effects for dev_set_name() failures cxl/mem: Fix synchronization mechanism for device removal vs ioctl operations cxl/mem: Use sysfs_emit() for attribute show routines
2 parents fdb5d6c + fae8817 commit 7c22677

File tree

1 file changed

+89
-63
lines changed

1 file changed

+89
-63
lines changed

drivers/cxl/mem.c

Lines changed: 89 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <linux/security.h>
55
#include <linux/debugfs.h>
66
#include <linux/module.h>
7+
#include <linux/sizes.h>
78
#include <linux/mutex.h>
89
#include <linux/cdev.h>
910
#include <linux/idr.h>
@@ -96,21 +97,18 @@ struct mbox_cmd {
9697
* @dev: driver core device object
9798
* @cdev: char dev core object for ioctl operations
9899
* @cxlm: pointer to the parent device driver data
99-
* @ops_active: active user of @cxlm in ops handlers
100-
* @ops_dead: completion when all @cxlm ops users have exited
101100
* @id: id number of this memdev instance.
102101
*/
103102
struct cxl_memdev {
104103
struct device dev;
105104
struct cdev cdev;
106105
struct cxl_mem *cxlm;
107-
struct percpu_ref ops_active;
108-
struct completion ops_dead;
109106
int id;
110107
};
111108

112109
static int cxl_mem_major;
113110
static DEFINE_IDA(cxl_memdev_ida);
111+
static DECLARE_RWSEM(cxl_memdev_rwsem);
114112
static struct dentry *cxl_debugfs;
115113
static bool cxl_raw_allow_all;
116114

@@ -169,7 +167,7 @@ struct cxl_mem_command {
169167
* table will be validated against the user's input. For example, if size_in is
170168
* 0, and the user passed in 1, it is an error.
171169
*/
172-
static struct cxl_mem_command mem_commands[] = {
170+
static struct cxl_mem_command mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
173171
CXL_CMD(IDENTIFY, 0, 0x43, CXL_CMD_FLAG_FORCE_ENABLE),
174172
#ifdef CONFIG_CXL_MEM_RAW_COMMANDS
175173
CXL_CMD(RAW, ~0, ~0, 0),
@@ -776,26 +774,43 @@ static long __cxl_memdev_ioctl(struct cxl_memdev *cxlmd, unsigned int cmd,
776774
static long cxl_memdev_ioctl(struct file *file, unsigned int cmd,
777775
unsigned long arg)
778776
{
779-
struct cxl_memdev *cxlmd;
780-
struct inode *inode;
781-
int rc = -ENOTTY;
777+
struct cxl_memdev *cxlmd = file->private_data;
778+
int rc = -ENXIO;
782779

783-
inode = file_inode(file);
784-
cxlmd = container_of(inode->i_cdev, typeof(*cxlmd), cdev);
780+
down_read(&cxl_memdev_rwsem);
781+
if (cxlmd->cxlm)
782+
rc = __cxl_memdev_ioctl(cxlmd, cmd, arg);
783+
up_read(&cxl_memdev_rwsem);
785784

786-
if (!percpu_ref_tryget_live(&cxlmd->ops_active))
787-
return -ENXIO;
785+
return rc;
786+
}
788787

789-
rc = __cxl_memdev_ioctl(cxlmd, cmd, arg);
788+
static int cxl_memdev_open(struct inode *inode, struct file *file)
789+
{
790+
struct cxl_memdev *cxlmd =
791+
container_of(inode->i_cdev, typeof(*cxlmd), cdev);
790792

791-
percpu_ref_put(&cxlmd->ops_active);
793+
get_device(&cxlmd->dev);
794+
file->private_data = cxlmd;
792795

793-
return rc;
796+
return 0;
797+
}
798+
799+
static int cxl_memdev_release_file(struct inode *inode, struct file *file)
800+
{
801+
struct cxl_memdev *cxlmd =
802+
container_of(inode->i_cdev, typeof(*cxlmd), cdev);
803+
804+
put_device(&cxlmd->dev);
805+
806+
return 0;
794807
}
795808

796809
static const struct file_operations cxl_memdev_fops = {
797810
.owner = THIS_MODULE,
798811
.unlocked_ioctl = cxl_memdev_ioctl,
812+
.open = cxl_memdev_open,
813+
.release = cxl_memdev_release_file,
799814
.compat_ioctl = compat_ptr_ioctl,
800815
.llseek = noop_llseek,
801816
};
@@ -984,7 +999,7 @@ static struct cxl_mem *cxl_mem_create(struct pci_dev *pdev, u32 reg_lo,
984999
return NULL;
9851000
}
9861001

987-
offset = ((u64)reg_hi << 32) | FIELD_GET(CXL_REGLOC_ADDR_MASK, reg_lo);
1002+
offset = ((u64)reg_hi << 32) | (reg_lo & CXL_REGLOC_ADDR_MASK);
9881003
bar = FIELD_GET(CXL_REGLOC_BIR_MASK, reg_lo);
9891004

9901005
/* Basic sanity check that BAR is big enough */
@@ -1049,7 +1064,6 @@ static void cxl_memdev_release(struct device *dev)
10491064
{
10501065
struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
10511066

1052-
percpu_ref_exit(&cxlmd->ops_active);
10531067
ida_free(&cxl_memdev_ida, cxlmd->id);
10541068
kfree(cxlmd);
10551069
}
@@ -1066,7 +1080,7 @@ static ssize_t firmware_version_show(struct device *dev,
10661080
struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
10671081
struct cxl_mem *cxlm = cxlmd->cxlm;
10681082

1069-
return sprintf(buf, "%.16s\n", cxlm->firmware_version);
1083+
return sysfs_emit(buf, "%.16s\n", cxlm->firmware_version);
10701084
}
10711085
static DEVICE_ATTR_RO(firmware_version);
10721086

@@ -1076,7 +1090,7 @@ static ssize_t payload_max_show(struct device *dev,
10761090
struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
10771091
struct cxl_mem *cxlm = cxlmd->cxlm;
10781092

1079-
return sprintf(buf, "%zu\n", cxlm->payload_size);
1093+
return sysfs_emit(buf, "%zu\n", cxlm->payload_size);
10801094
}
10811095
static DEVICE_ATTR_RO(payload_max);
10821096

@@ -1087,7 +1101,7 @@ static ssize_t ram_size_show(struct device *dev, struct device_attribute *attr,
10871101
struct cxl_mem *cxlm = cxlmd->cxlm;
10881102
unsigned long long len = range_len(&cxlm->ram_range);
10891103

1090-
return sprintf(buf, "%#llx\n", len);
1104+
return sysfs_emit(buf, "%#llx\n", len);
10911105
}
10921106

10931107
static struct device_attribute dev_attr_ram_size =
@@ -1100,7 +1114,7 @@ static ssize_t pmem_size_show(struct device *dev, struct device_attribute *attr,
11001114
struct cxl_mem *cxlm = cxlmd->cxlm;
11011115
unsigned long long len = range_len(&cxlm->pmem_range);
11021116

1103-
return sprintf(buf, "%#llx\n", len);
1117+
return sysfs_emit(buf, "%#llx\n", len);
11041118
}
11051119

11061120
static struct device_attribute dev_attr_pmem_size =
@@ -1150,27 +1164,24 @@ static const struct device_type cxl_memdev_type = {
11501164
.groups = cxl_memdev_attribute_groups,
11511165
};
11521166

1153-
static void cxlmdev_unregister(void *_cxlmd)
1167+
static void cxl_memdev_shutdown(struct cxl_memdev *cxlmd)
11541168
{
1155-
struct cxl_memdev *cxlmd = _cxlmd;
1156-
struct device *dev = &cxlmd->dev;
1157-
1158-
percpu_ref_kill(&cxlmd->ops_active);
1159-
cdev_device_del(&cxlmd->cdev, dev);
1160-
wait_for_completion(&cxlmd->ops_dead);
1169+
down_write(&cxl_memdev_rwsem);
11611170
cxlmd->cxlm = NULL;
1162-
put_device(dev);
1171+
up_write(&cxl_memdev_rwsem);
11631172
}
11641173

1165-
static void cxlmdev_ops_active_release(struct percpu_ref *ref)
1174+
static void cxl_memdev_unregister(void *_cxlmd)
11661175
{
1167-
struct cxl_memdev *cxlmd =
1168-
container_of(ref, typeof(*cxlmd), ops_active);
1176+
struct cxl_memdev *cxlmd = _cxlmd;
1177+
struct device *dev = &cxlmd->dev;
11691178

1170-
complete(&cxlmd->ops_dead);
1179+
cdev_device_del(&cxlmd->cdev, dev);
1180+
cxl_memdev_shutdown(cxlmd);
1181+
put_device(dev);
11711182
}
11721183

1173-
static int cxl_mem_add_memdev(struct cxl_mem *cxlm)
1184+
static struct cxl_memdev *cxl_memdev_alloc(struct cxl_mem *cxlm)
11741185
{
11751186
struct pci_dev *pdev = cxlm->pdev;
11761187
struct cxl_memdev *cxlmd;
@@ -1180,22 +1191,11 @@ static int cxl_mem_add_memdev(struct cxl_mem *cxlm)
11801191

11811192
cxlmd = kzalloc(sizeof(*cxlmd), GFP_KERNEL);
11821193
if (!cxlmd)
1183-
return -ENOMEM;
1184-
init_completion(&cxlmd->ops_dead);
1185-
1186-
/*
1187-
* @cxlm is deallocated when the driver unbinds so operations
1188-
* that are using it need to hold a live reference.
1189-
*/
1190-
cxlmd->cxlm = cxlm;
1191-
rc = percpu_ref_init(&cxlmd->ops_active, cxlmdev_ops_active_release, 0,
1192-
GFP_KERNEL);
1193-
if (rc)
1194-
goto err_ref;
1194+
return ERR_PTR(-ENOMEM);
11951195

11961196
rc = ida_alloc_range(&cxl_memdev_ida, 0, CXL_MEM_MAX_DEVS, GFP_KERNEL);
11971197
if (rc < 0)
1198-
goto err_id;
1198+
goto err;
11991199
cxlmd->id = rc;
12001200

12011201
dev = &cxlmd->dev;
@@ -1204,30 +1204,54 @@ static int cxl_mem_add_memdev(struct cxl_mem *cxlm)
12041204
dev->bus = &cxl_bus_type;
12051205
dev->devt = MKDEV(cxl_mem_major, cxlmd->id);
12061206
dev->type = &cxl_memdev_type;
1207-
dev_set_name(dev, "mem%d", cxlmd->id);
1207+
device_set_pm_not_required(dev);
12081208

12091209
cdev = &cxlmd->cdev;
12101210
cdev_init(cdev, &cxl_memdev_fops);
1211+
return cxlmd;
1212+
1213+
err:
1214+
kfree(cxlmd);
1215+
return ERR_PTR(rc);
1216+
}
1217+
1218+
static int cxl_mem_add_memdev(struct cxl_mem *cxlm)
1219+
{
1220+
struct cxl_memdev *cxlmd;
1221+
struct device *dev;
1222+
struct cdev *cdev;
1223+
int rc;
1224+
1225+
cxlmd = cxl_memdev_alloc(cxlm);
1226+
if (IS_ERR(cxlmd))
1227+
return PTR_ERR(cxlmd);
1228+
1229+
dev = &cxlmd->dev;
1230+
rc = dev_set_name(dev, "mem%d", cxlmd->id);
1231+
if (rc)
1232+
goto err;
1233+
1234+
/*
1235+
* Activate ioctl operations, no cxl_memdev_rwsem manipulation
1236+
* needed as this is ordered with cdev_add() publishing the device.
1237+
*/
1238+
cxlmd->cxlm = cxlm;
12111239

1240+
cdev = &cxlmd->cdev;
12121241
rc = cdev_device_add(cdev, dev);
12131242
if (rc)
1214-
goto err_add;
1243+
goto err;
12151244

1216-
return devm_add_action_or_reset(dev->parent, cxlmdev_unregister, cxlmd);
1245+
return devm_add_action_or_reset(dev->parent, cxl_memdev_unregister,
1246+
cxlmd);
12171247

1218-
err_add:
1219-
ida_free(&cxl_memdev_ida, cxlmd->id);
1220-
err_id:
1248+
err:
12211249
/*
1222-
* Theoretically userspace could have already entered the fops,
1223-
* so flush ops_active.
1250+
* The cdev was briefly live, shutdown any ioctl operations that
1251+
* saw that state.
12241252
*/
1225-
percpu_ref_kill(&cxlmd->ops_active);
1226-
wait_for_completion(&cxlmd->ops_dead);
1227-
percpu_ref_exit(&cxlmd->ops_active);
1228-
err_ref:
1229-
kfree(cxlmd);
1230-
1253+
cxl_memdev_shutdown(cxlmd);
1254+
put_device(dev);
12311255
return rc;
12321256
}
12331257

@@ -1396,6 +1420,7 @@ static int cxl_mem_enumerate_cmds(struct cxl_mem *cxlm)
13961420
*/
13971421
static int cxl_mem_identify(struct cxl_mem *cxlm)
13981422
{
1423+
/* See CXL 2.0 Table 175 Identify Memory Device Output Payload */
13991424
struct cxl_mbox_identify {
14001425
char fw_revision[0x10];
14011426
__le64 total_capacity;
@@ -1424,10 +1449,11 @@ static int cxl_mem_identify(struct cxl_mem *cxlm)
14241449
* For now, only the capacity is exported in sysfs
14251450
*/
14261451
cxlm->ram_range.start = 0;
1427-
cxlm->ram_range.end = le64_to_cpu(id.volatile_capacity) - 1;
1452+
cxlm->ram_range.end = le64_to_cpu(id.volatile_capacity) * SZ_256M - 1;
14281453

14291454
cxlm->pmem_range.start = 0;
1430-
cxlm->pmem_range.end = le64_to_cpu(id.persistent_capacity) - 1;
1455+
cxlm->pmem_range.end =
1456+
le64_to_cpu(id.persistent_capacity) * SZ_256M - 1;
14311457

14321458
memcpy(cxlm->firmware_version, id.fw_revision, sizeof(id.fw_revision));
14331459

0 commit comments

Comments
 (0)