| Message ID | 20250122235159.2716036-11-dave.jiang@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | cxl: Add CXL feature commands support via fwctl | expand |
On Wed, 22 Jan 2025 16:50:41 -0700 Dave Jiang <dave.jiang@intel.com> wrote: > Add fwctl support code to allow sending of CXL feature commands from > userspace through as ioctls via FWCTL. Provide initial setup bits. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h > index f9b27fb5c161..2c3817592308 100644 > --- a/include/uapi/fwctl/fwctl.h > +++ b/include/uapi/fwctl/fwctl.h > @@ -43,6 +43,7 @@ enum { > enum fwctl_device_type { > FWCTL_DEVICE_TYPE_ERROR = 0, > FWCTL_DEVICE_TYPE_MLX5 = 1, > + FWCTL_DEVICE_TYPE_CXL, Seems we are giving explicit numbers but not for CXL? I don't greatly care either way but we should be consistent. Jason? > }; > > /**
On Thu, Jan 23, 2025 at 06:04:30PM +0000, Jonathan Cameron wrote: > > diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h > > index f9b27fb5c161..2c3817592308 100644 > > --- a/include/uapi/fwctl/fwctl.h > > +++ b/include/uapi/fwctl/fwctl.h > > @@ -43,6 +43,7 @@ enum { > > enum fwctl_device_type { > > FWCTL_DEVICE_TYPE_ERROR = 0, > > FWCTL_DEVICE_TYPE_MLX5 = 1, > > + FWCTL_DEVICE_TYPE_CXL, > Seems we are giving explicit numbers but not for CXL? > I don't greatly care either way but we should be consistent. Jason? Please write out the numbers in the enum, it aids backporters in the long run. Jason
Dave Jiang wrote: > Add fwctl support code to allow sending of CXL feature commands from > userspace through as ioctls via FWCTL. Provide initial setup bits. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > --- > v1: > - Add missing dev_set_name() (Jason) > --- > drivers/cxl/Kconfig | 1 + > drivers/cxl/features.c | 44 ++++++++++++++++++++++++++++++++++++-- > include/cxl/features.h | 2 ++ > include/uapi/fwctl/fwctl.h | 1 + > 4 files changed, 46 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig > index 9a6ffd81ac0e..5de06c3806ce 100644 > --- a/drivers/cxl/Kconfig > +++ b/drivers/cxl/Kconfig > @@ -148,6 +148,7 @@ config CXL_REGION_INVALIDATION_TEST > > config CXL_FEATURES > tristate "CXL: Features support" > + depends on FWCTL > default CXL_BUS > help > Enable CXL features support that are tied to a CXL mailbox. > diff --git a/drivers/cxl/features.c b/drivers/cxl/features.c > index 1c4bcd7465cb..5a2b771586d3 100644 > --- a/drivers/cxl/features.c > +++ b/drivers/cxl/features.c > @@ -9,6 +9,37 @@ > #include "cxl.h" > #include "cxlmem.h" > > +static int cxlctl_open_uctx(struct fwctl_uctx *uctx) > +{ > + return 0; > +} > + > +static void cxlctl_close_uctx(struct fwctl_uctx *uctx) > +{ > +} > + > +static void *cxlctl_info(struct fwctl_uctx *uctx, size_t *length) > +{ > + /* Place holder */ > + return ERR_PTR(-EOPNOTSUPP); > +} > + > +static void *cxlctl_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope, > + void *rpc_in, size_t in_len, size_t *out_len) > +{ > + /* Place holder */ > + return ERR_PTR(-EOPNOTSUPP); > +} > + > +static const struct fwctl_ops cxlctl_ops = { > + .device_type = FWCTL_DEVICE_TYPE_CXL, > + .uctx_size = sizeof(struct cxl_features), > + .open_uctx = cxlctl_open_uctx, > + .close_uctx = cxlctl_close_uctx, > + .info = cxlctl_info, > + .fw_rpc = cxlctl_fw_rpc, > +}; > + > static void cxl_free_feature_entries(void *entries) > { > kvfree(entries); > @@ -146,13 +177,16 @@ static int cxl_get_supported_features(struct cxl_features_state *cfs) > cxl_free_feature_entries, cfs->entries); > } > > +DEFINE_FREE(cfs, struct cxl_features_state *, if (_T) fwctl_put(&_T->fwctl)) > + > static int cxl_features_probe(struct device *dev) > { > struct cxl_features *features = to_cxl_features(dev); > int rc; > > - struct cxl_features_state *cfs __free(kfree) = > - kzalloc(sizeof(*cfs), GFP_KERNEL); > + struct cxl_features_state *cfs __free(cfs) = > + fwctl_alloc_device(dev, &cxlctl_ops, > + struct cxl_features_state, fwctl); > if (!cfs) > return -ENOMEM; > > @@ -161,6 +195,10 @@ static int cxl_features_probe(struct device *dev) > if (rc) > return rc; > > + rc = fwctl_register(&cfs->fwctl); > + if (rc) > + return rc; Repeating our offline discussion... After looking at what cxl_features_driver does and the fact that the EDAC scrub support is enabled by registration calls from cxl_mem and cxl_region. Let's just have cxl_mem or cxl_pci register fwctl relative to a cxl_memdev. So, similar to how a cxl_memdev hosts an ioctl interface and a fw_upload interface, add a fwctl interface with no new device+driver objects. When / if other components like CXL switches want to support fwctl that can also be had as just a fwctl registration relative to a cxl_port device which would have been awkward to handle with a sub-device+driver. > diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h > index f9b27fb5c161..2c3817592308 100644 > --- a/include/uapi/fwctl/fwctl.h > +++ b/include/uapi/fwctl/fwctl.h > @@ -43,6 +43,7 @@ enum { > enum fwctl_device_type { > FWCTL_DEVICE_TYPE_ERROR = 0, > FWCTL_DEVICE_TYPE_MLX5 = 1, > + FWCTL_DEVICE_TYPE_CXL, Jonathan and Jason already handled the comment here.
diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig index 9a6ffd81ac0e..5de06c3806ce 100644 --- a/drivers/cxl/Kconfig +++ b/drivers/cxl/Kconfig @@ -148,6 +148,7 @@ config CXL_REGION_INVALIDATION_TEST config CXL_FEATURES tristate "CXL: Features support" + depends on FWCTL default CXL_BUS help Enable CXL features support that are tied to a CXL mailbox. diff --git a/drivers/cxl/features.c b/drivers/cxl/features.c index 1c4bcd7465cb..5a2b771586d3 100644 --- a/drivers/cxl/features.c +++ b/drivers/cxl/features.c @@ -9,6 +9,37 @@ #include "cxl.h" #include "cxlmem.h" +static int cxlctl_open_uctx(struct fwctl_uctx *uctx) +{ + return 0; +} + +static void cxlctl_close_uctx(struct fwctl_uctx *uctx) +{ +} + +static void *cxlctl_info(struct fwctl_uctx *uctx, size_t *length) +{ + /* Place holder */ + return ERR_PTR(-EOPNOTSUPP); +} + +static void *cxlctl_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope, + void *rpc_in, size_t in_len, size_t *out_len) +{ + /* Place holder */ + return ERR_PTR(-EOPNOTSUPP); +} + +static const struct fwctl_ops cxlctl_ops = { + .device_type = FWCTL_DEVICE_TYPE_CXL, + .uctx_size = sizeof(struct cxl_features), + .open_uctx = cxlctl_open_uctx, + .close_uctx = cxlctl_close_uctx, + .info = cxlctl_info, + .fw_rpc = cxlctl_fw_rpc, +}; + static void cxl_free_feature_entries(void *entries) { kvfree(entries); @@ -146,13 +177,16 @@ static int cxl_get_supported_features(struct cxl_features_state *cfs) cxl_free_feature_entries, cfs->entries); } +DEFINE_FREE(cfs, struct cxl_features_state *, if (_T) fwctl_put(&_T->fwctl)) + static int cxl_features_probe(struct device *dev) { struct cxl_features *features = to_cxl_features(dev); int rc; - struct cxl_features_state *cfs __free(kfree) = - kzalloc(sizeof(*cfs), GFP_KERNEL); + struct cxl_features_state *cfs __free(cfs) = + fwctl_alloc_device(dev, &cxlctl_ops, + struct cxl_features_state, fwctl); if (!cfs) return -ENOMEM; @@ -161,6 +195,10 @@ static int cxl_features_probe(struct device *dev) if (rc) return rc; + rc = fwctl_register(&cfs->fwctl); + if (rc) + return rc; + dev_set_drvdata(dev, no_free_ptr(cfs)); return 0; @@ -170,6 +208,7 @@ static void cxl_features_remove(struct device *dev) { struct cxl_features_state *cfs = dev_get_drvdata(dev); + fwctl_unregister(&cfs->fwctl); kfree(cfs); } @@ -212,4 +251,5 @@ module_cxl_driver(cxl_features_driver); MODULE_DESCRIPTION("CXL: Features"); MODULE_LICENSE("GPL"); MODULE_IMPORT_NS("CXL"); +MODULE_IMPORT_NS("FWCTL"); MODULE_ALIAS_CXL(CXL_DEVICE_FEATURES); diff --git a/include/cxl/features.h b/include/cxl/features.h index f746358ba5f9..69697c069e63 100644 --- a/include/cxl/features.h +++ b/include/cxl/features.h @@ -4,6 +4,7 @@ #define __CXL_FEATURES_H__ #include <linux/uuid.h> +#include <linux/fwctl.h> #define CXL_FEAT_PATROL_SCRUB_UUID \ UUID_INIT(0x96dad7d6, 0xfde8, 0x482b, 0xa7, 0x33, 0x75, 0x77, 0x4e, \ @@ -103,6 +104,7 @@ struct cxl_mbox_get_sup_feats_out { } __packed; struct cxl_features_state { + struct fwctl_device fwctl; struct cxl_features *features; int num_features; int num_user_features; diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h index f9b27fb5c161..2c3817592308 100644 --- a/include/uapi/fwctl/fwctl.h +++ b/include/uapi/fwctl/fwctl.h @@ -43,6 +43,7 @@ enum { enum fwctl_device_type { FWCTL_DEVICE_TYPE_ERROR = 0, FWCTL_DEVICE_TYPE_MLX5 = 1, + FWCTL_DEVICE_TYPE_CXL, }; /**
Add fwctl support code to allow sending of CXL feature commands from userspace through as ioctls via FWCTL. Provide initial setup bits. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- v1: - Add missing dev_set_name() (Jason) --- drivers/cxl/Kconfig | 1 + drivers/cxl/features.c | 44 ++++++++++++++++++++++++++++++++++++-- include/cxl/features.h | 2 ++ include/uapi/fwctl/fwctl.h | 1 + 4 files changed, 46 insertions(+), 2 deletions(-)