Re: [PATCH v5 16/17] crypto/nx: Get NX capabilities for GZIP coprocessor type

Haren Myneni
2021-06-15T00:01:19Z

On Mon, 2021-06-14 at 13:39 +1000, Nicholas Piggin wrote:
> Excerpts from Haren Myneni's message of June 13, 2021 9:04 pm:
> > The hypervisor provides different capabilities that it supports
> > to define the user space NX request. These capabilities are
> > recommended minimum compression / decompression lengths and the
> > maximum request buffer size in bytes.
> >
> > Changes to get NX overall capabilities which points to the
> > specific features that the hypervisor supports. Then retrieve
> > the capabilities for the specific feature (available only
> > for NXGZIP).
>
> So what does this give you which you didn't have before? Should
> this go before the previous patch that enables the interface for
> guests,
> or is there some functional-yet-degraded mode that is available
> without
> this patch?
>
> I would suggest even if this is the case to switch ordering of the
> patches so as to reduce the matrix of functionality that userspace
> sees
> when bisecting. Unless you specifically want this kind of
> bisectability,
> in which case make that explicit in the changelog.

Thanks for your suggestions. I will incorporate them and post next
revision.

The user space request buffer length should not be more than
req_max_processed_len (available through sysfs). Otherwise NX will
return the request with RMA_Reject. Whereasas min_compress_len and
min_decompress_len are recommended values.

We can add this patch and the last one (crypto/nx: Add sysfs interface
to export NX capabilities) before the actual enablement ("crypto/nx:
Register and unregisterVAS interface on PowerVM""

Thanks
Haren

>
> 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 | 86
> > +++++++++++++++++++++++++++
> > 1 file changed, 86 insertions(+)
> >
> > diff --git a/drivers/crypto/nx/nx-common-pseries.c
> > b/drivers/crypto/nx/nx-common-pseries.c
> > index 9a40fca8a9e6..60b5049ec9f7 100644
> > --- a/drivers/crypto/nx/nx-common-pseries.c
> > +++ b/drivers/crypto/nx/nx-common-pseries.c
> > @@ -9,6 +9,7 @@
> > */
> >
> > #include <asm/vio.h>
> > +#include <asm/hvcall.h>
> > #include <asm/vas.h>
> >
> > #include "nx-842.h"
> > @@ -20,6 +21,29 @@ MODULE_DESCRIPTION("842 H/W Compression driver
> > for IBM Power processors");
> > MODULE_ALIAS_CRYPTO("842");
> > MODULE_ALIAS_CRYPTO("842-nx");
> >
> > +/*
> > + * Coprocessor type specific capabilities from the hypervisor.
> > + */
> > +struct hv_nx_ct_caps {
> > + __be64 descriptor;
> > + __be64 req_max_processed_len; /* Max bytes in one GZIP
> > request */
> > + __be64 min_compress_len; /* Min compression size in
> > bytes */
> > + __be64 min_decompress_len; /* Min decompression size
> > in bytes */
> > +} __packed __aligned(0x1000);
> > +
> > +/*
> > + * Coprocessor type specific capabilities.
> > + */
> > +struct nx_ct_caps {
> > + u64 descriptor;
> > + u64 req_max_processed_len; /* Max bytes in one GZIP request */
> > + u64 min_compress_len; /* Min compression in bytes */
> > + u64 min_decompress_len; /* Min decompression in bytes */
> > +};
> > +
> > +static u64 caps_feat;
> > +static struct nx_ct_caps nx_ct_caps;
> > +
> > static struct nx842_constraints nx842_pseries_constraints = {
> > .alignment = DDE_BUFFER_ALIGN,
> > .multiple = DDE_BUFFER_LAST_MULT,
> > @@ -1066,6 +1090,64 @@ static void nx842_remove(struct vio_dev
> > *viodev)
> > kfree(old_devdata);
> > }
> >
> > +/*
> > + * Get NX capabilities from the hypervisor.
> > + * Only NXGZIP capabilities are provided by the hypersvisor right
> > + * now and these values are available to user space with sysfs.
> > + */
> > +static void __init nxct_get_capabilities(void)
> > +{
> > + struct hv_vas_all_caps *hv_caps;
> > + struct hv_nx_ct_caps *hv_nxc;
> > + int rc;
> > +
> > + hv_caps = kmalloc(sizeof(*hv_caps), GFP_KERNEL);
> > + if (!hv_caps)
> > + return;
> > + /*
> > + * Get NX overall capabilities with feature type=0
> > + */
> > + rc = h_query_vas_capabilities(H_QUERY_NX_CAPABILITIES, 0,
> > + (u64)virt_to_phys(hv_caps));
> > + if (rc)
> > + goto out;
> > +
> > + caps_feat = be64_to_cpu(hv_caps->feat_type);
> > + /*
> > + * NX-GZIP feature available
> > + */
> > + if (caps_feat & VAS_NX_GZIP_FEAT_BIT) {
> > + hv_nxc = kmalloc(sizeof(*hv_nxc), GFP_KERNEL);
> > + if (!hv_nxc)
> > + goto out;
> > + /*
> > + * Get capabilities for NX-GZIP feature
> > + */
> > + rc = h_query_vas_capabilities(H_QUERY_NX_CAPABILITIES,
> > + VAS_NX_GZIP_FEAT,
> > + (u64)virt_to_phys(hv_
> > nxc));
> > + } else {
> > + pr_err("NX-GZIP feature is not available\n");
> > + rc = -EINVAL;
> > + }
> > +
> > + if (!rc) {
> > + nx_ct_caps.descriptor = be64_to_cpu(hv_nxc-
> > >descriptor);
> > + nx_ct_caps.req_max_processed_len =
> > + be64_to_cpu(hv_nxc-
> > >req_max_processed_len);
> > + nx_ct_caps.min_compress_len =
> > + be64_to_cpu(hv_nxc->min_compress_len);
> > + nx_ct_caps.min_decompress_len =
> > + be64_to_cpu(hv_nxc-
> > >min_decompress_len);
> > + } else {
> > + caps_feat = 0;
> > + }
> > +
> > + kfree(hv_nxc);
> > +out:
> > + kfree(hv_caps);
> > +}
> > +
> > static const struct vio_device_id nx842_vio_driver_ids[] = {
> > {"ibm,compression-v1", "ibm,compression"},
> > {"", ""},
> > @@ -1093,6 +1175,10 @@ static int __init nx842_pseries_init(void)
> > return -ENOMEM;
> >
> > RCU_INIT_POINTER(devdata, new_devdata);
> > + /*
> > + * Get NX capabilities from the hypervisor.
> > + */
> > + nxct_get_capabilities();
> >
> > ret = vio_register_driver(&nx842_vio_driver);
> > if (ret) {
> > --
> > 2.18.2
> >
> >
> >