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 <ha...@linux.ibm.com>
> > Acked-by: Herbert Xu <herb...@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
> > 
> > 
> > 

Reply via email to