All of lore.kernel.org
 help / color / mirror / Atom feed
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
Subject: Re: [PATCH v5 16/17] crypto/nx: Get NX capabilities for GZIP coprocessor type
Date: Mon, 14 Jun 2021 17:01:00 -0700	[thread overview]
Message-ID: <d4939c7b31626ea73322a4f2476f9999e6e35ab1.camel@linux.ibm.com> (raw)
In-Reply-To: <1623641787.0rdwnv3k4u.astroid@bobo.none>

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


WARNING: multiple messages have this Message-ID (diff)
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
Subject: Re: [PATCH v5 16/17] crypto/nx: Get NX capabilities for GZIP coprocessor type
Date: Mon, 14 Jun 2021 17:01:00 -0700	[thread overview]
Message-ID: <d4939c7b31626ea73322a4f2476f9999e6e35ab1.camel@linux.ibm.com> (raw)
In-Reply-To: <1623641787.0rdwnv3k4u.astroid@bobo.none>

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


  reply	other threads:[~2021-06-15  0:01 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-13 10:53 [PATCH v5 00/17] Enable VAS and NX-GZIP support on PowerVM Haren Myneni
2021-06-13 10:53 ` Haren Myneni
2021-06-13 10:54 ` [PATCH v5 01/17] powerpc/powernv/vas: Release reference to tgid during window close Haren Myneni
2021-06-13 10:54   ` Haren Myneni
2021-06-14  2:11   ` Nicholas Piggin
2021-06-14  2:11     ` Nicholas Piggin
2021-06-13 10:55 ` [PATCH v5 02/17] powerpc/vas: Move VAS API to book3s common platform Haren Myneni
2021-06-13 10:55   ` Haren Myneni
2021-06-14  2:10   ` Nicholas Piggin
2021-06-14  2:10     ` Nicholas Piggin
2021-06-13 10:56 ` [PATCH v5 03/17] powerpc/powernv/vas: Rename register/unregister functions Haren Myneni
2021-06-13 10:56   ` Haren Myneni
2021-06-13 10:57 ` [PATCH v5 04/17] powerpc/vas: Add platform specific user window operations Haren Myneni
2021-06-13 10:57   ` Haren Myneni
2021-06-14  2:24   ` Nicholas Piggin
2021-06-14  2:24     ` Nicholas Piggin
2021-06-15  6:37     ` Haren Myneni
2021-06-15  6:37       ` Haren Myneni
2021-06-16  9:11       ` Nicholas Piggin
2021-06-16  9:11         ` Nicholas Piggin
2021-06-13 10:57 ` [PATCH v5 05/17] powerpc/vas: Create take/drop pid and mm reference functions Haren Myneni
2021-06-13 10:57   ` Haren Myneni
2021-06-14  2:26   ` Nicholas Piggin
2021-06-14  2:26     ` Nicholas Piggin
2021-06-13 10:58 ` [PATCH v5 06/17] powerpc/vas: Move update_csb/dump_crb to common book3s platform Haren Myneni
2021-06-13 10:58   ` Haren Myneni
2021-06-14  3:34   ` Nicholas Piggin
2021-06-14  3:34     ` Nicholas Piggin
2021-06-13 10:58 ` [PATCH v5 07/17] powerpc/vas: Define and use common vas_window struct Haren Myneni
2021-06-13 10:58   ` Haren Myneni
2021-06-14  2:28   ` Nicholas Piggin
2021-06-14  2:28     ` Nicholas Piggin
2021-06-13 10:59 ` [PATCH v5 08/17] powerpc/pseries/vas: Define VAS/NXGZIP hcalls and structs Haren Myneni
2021-06-13 10:59   ` Haren Myneni
2021-06-14  2:32   ` Nicholas Piggin
2021-06-14  2:32     ` Nicholas Piggin
2021-06-13 11:00 ` [PATCH v5 09/17] powerpc/vas: Define QoS credit flag to allocate window Haren Myneni
2021-06-13 11:00   ` Haren Myneni
2021-06-14  2:32   ` Nicholas Piggin
2021-06-14  2:32     ` Nicholas Piggin
2021-06-13 11:00 ` [PATCH v5 10/17] powerpc/pseries/vas: Add hcall wrappers for VAS handling Haren Myneni
2021-06-13 11:00   ` Haren Myneni
2021-06-13 11:01 ` [PATCH v5 11/17] powerpc/pseries/vas: Implement getting capabilities from hypervisor Haren Myneni
2021-06-13 11:01   ` Haren Myneni
2021-06-14  2:35   ` Nicholas Piggin
2021-06-14  2:35     ` Nicholas Piggin
2021-06-13 11:02 ` [PATCH v5 12/17] powerpc/pseries/vas: Integrate API with open/close windows Haren Myneni
2021-06-13 11:02   ` Haren Myneni
2021-06-14  2:55   ` Nicholas Piggin
2021-06-14  2:55     ` Nicholas Piggin
2021-06-15  6:51     ` Haren Myneni
2021-06-15  6:51       ` Haren Myneni
2021-06-15  7:26     ` Haren Myneni
2021-06-15  7:26       ` Haren Myneni
2021-06-16  9:38       ` Nicholas Piggin
2021-06-16  9:38         ` Nicholas Piggin
2021-06-13 11:02 ` [PATCH v5 13/17] powerpc/pseries/vas: Setup IRQ and fault handling Haren Myneni
2021-06-13 11:02   ` Haren Myneni
2021-06-14  3:07   ` Nicholas Piggin
2021-06-14  3:07     ` Nicholas Piggin
2021-06-15  9:01     ` Haren Myneni
2021-06-15  9:01       ` Haren Myneni
2021-06-16  9:40       ` Nicholas Piggin
2021-06-16  9:40         ` Nicholas Piggin
2021-06-13 11:03 ` [PATCH v5 14/17] crypto/nx: Rename nx-842-pseries file name to nx-common-pseries Haren Myneni
2021-06-13 11:03   ` Haren Myneni
2021-06-14  3:07   ` Nicholas Piggin
2021-06-14  3:07     ` Nicholas Piggin
2021-06-13 11:04 ` [PATCH v5 15/17] crypto/nx: Register and unregister VAS interface on PowerVM Haren Myneni
2021-06-13 11:04   ` Haren Myneni
2021-06-14  3:35   ` Nicholas Piggin
2021-06-14  3:35     ` Nicholas Piggin
2021-06-13 11:04 ` [PATCH v5 16/17] crypto/nx: Get NX capabilities for GZIP coprocessor type Haren Myneni
2021-06-13 11:04   ` Haren Myneni
2021-06-14  3:39   ` Nicholas Piggin
2021-06-14  3:39     ` Nicholas Piggin
2021-06-15  0:01     ` Haren Myneni [this message]
2021-06-15  0:01       ` Haren Myneni
2021-06-13 11:05 ` [PATCH v5 17/17] crypto/nx: Add sysfs interface to export NX capabilities Haren Myneni
2021-06-13 11:05   ` Haren Myneni
2021-06-14  3:46   ` Nicholas Piggin
2021-06-14  3:46     ` Nicholas Piggin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d4939c7b31626ea73322a4f2476f9999e6e35ab1.camel@linux.ibm.com \
    --to=haren@linux.ibm.com \
    --cc=haren@us.ibm.com \
    --cc=hbabu@us.ibm.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.