Re: [PATCH v5 17/17] crypto/nx: Add sysfs interface to export NX capabilities

Nicholas Piggin
2021-06-14T03:46:41Z

Excerpts from Haren Myneni's message of June 13, 2021 9:05 pm:
>
> Changes to export the following NXGZIP capabilities through sysfs:
>
> /sys/devices/vio/ibm,compression-v1/nx_gzip_caps:
> min_compress_len /*Recommended minimum compress length in bytes*/
> min_decompress_len /*Recommended minimum decompress length in bytes*/
> req_max_processed_len /* Maximum number of bytes processed in one
> request */
>
> NX will return RMA_Reject if the request buffer size is greater
> than req_max_processed_len.

Similar for the previous patch, can userspace sanely use the API without
these capabilities? If not, reorder so the final enable comes last.

I would put those comments in the code, and make the changelog a little
higher level, including a reference or example of how userspace uses it,
which should come with any new userspace ABI.

"Export NX-GZIP capabilities to usrespace in sysfs /sys/devices/vio/...
directory. These are queried by userspace accelerator libraries to set
minimum length heuristics and maximum limits on request sizes."

Something like that. Otherwise looks okay to me.

Thanks,
Nick

>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
> ---
> drivers/crypto/nx/nx-common-pseries.c | 43 +++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/drivers/crypto/nx/nx-common-pseries.c b/drivers/crypto/nx/nx-common-pseries.c
> index 60b5049ec9f7..db28a84a826c 100644
> --- a/drivers/crypto/nx/nx-common-pseries.c
> +++ b/drivers/crypto/nx/nx-common-pseries.c
> @@ -967,6 +967,36 @@ static struct attribute_group nx842_attribute_group = {
> .attrs = nx842_sysfs_entries,
> };
>
> +#define nxct_caps_read(_name) \
> +static ssize_t nxct_##_name##_show(struct device *dev, \
> + struct device_attribute *attr, char *buf) \
> +{ \
> + return sprintf(buf, "%lld\n", nx_ct_caps._name); \
> +}
> +
> +#define NXCT_ATTR_RO(_name) \
> + nxct_caps_read(_name); \
> + static struct device_attribute dev_attr_##_name = __ATTR(_name, \
> + 0444, \
> + nxct_##_name##_show, \
> + NULL);
> +
> +NXCT_ATTR_RO(req_max_processed_len);
> +NXCT_ATTR_RO(min_compress_len);
> +NXCT_ATTR_RO(min_decompress_len);
> +
> +static struct attribute *nxct_caps_sysfs_entries[] = {
> + &dev_attr_req_max_processed_len.attr,
> + &dev_attr_min_compress_len.attr,
> + &dev_attr_min_decompress_len.attr,
> + NULL,
> +};
> +
> +static struct attribute_group nxct_caps_attr_group = {
> + .name = "nx_gzip_caps",
> + .attrs = nxct_caps_sysfs_entries,
> +};
> +
> static struct nx842_driver nx842_pseries_driver = {
> .name = KBUILD_MODNAME,
> .owner = THIS_MODULE,
> @@ -1056,6 +1086,16 @@ static int nx842_probe(struct vio_dev *viodev,
> goto error;
> }
>
> + if (caps_feat) {
> + if (sysfs_create_group(&viodev->dev.kobj,
> + &nxct_caps_attr_group)) {
> + dev_err(&viodev->dev,
> + "Could not create sysfs NX capability entries\n");
> + ret = -1;
> + goto error;
> + }
> + }
> +
> return 0;
>
> error_unlock:
> @@ -1075,6 +1115,9 @@ static void nx842_remove(struct vio_dev *viodev)
> pr_info("Removing IBM Power 842 compression device\n");
> sysfs_remove_group(&viodev->dev.kobj, &nx842_attribute_group);
>
> + if (caps_feat)
> + sysfs_remove_group(&viodev->dev.kobj, &nxct_caps_attr_group);
> +
> crypto_unregister_alg(&nx842_pseries_alg);
>
> spin_lock_irqsave(&devdata_mutex, flags);
> --
> 2.18.2
>
>
>