Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <d4939c7b31626ea73322a4f2476f9999e6e35ab1.camel@linux.ibm.com>
Subject: Re: [PATCH v5 16/17] crypto/nx: Get NX capabilities for GZIP
 coprocessor type
From:   Haren Myneni <haren@linux.ibm.com>
To:     Nicholas Piggin <npiggin@gmail.com>, herbert@gondor.apana.org.au,
        linux-crypto@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
        mpe@ellerman.id.au
Cc:     haren@us.ibm.com, hbabu@us.ibm.com
Date:   Mon, 14 Jun 2021 17:01:00 -0700
In-Reply-To: <1623641787.0rdwnv3k4u.astroid@bobo.none>
References: <ed7a09822cf3a2e463f942e5a37309a2365c9d79.camel@linux.ibm.com>
         <726de270eb20e0898f4391a0b0e7077697db66ab.camel@linux.ibm.com>
         <1623641787.0rdwnv3k4u.astroid@bobo.none>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.2 (3.36.2-1.fc32) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

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
> > 
> > 
> >