Re: [PATCH V3 6/6] crypto/nx: Add P9 NX support for 842 compression engine
On Tue, 2017-08-29 at 14:54 -0700, Haren Myneni wrote: > Opening send window for each crypto transform (crypto_alloc, > compression/decompression, ..., crypto_free) so that does not have to > wait for the previous copy/paste complete. VAS will map send and > receive windows, and can cache in send windows (up to 128). So I > thought using the same send window (per chip) for more requests (say > 1000) may be adding overhead. > > I will make changes if you prefer using 1 send window per chip. Did you check the cost of opening/closing a window ? Cheers, Ben.
Re: [PATCH V3 6/6] crypto/nx: Add P9 NX support for 842 compression engine
On Tue, 2017-08-29 at 09:58 -0400, Dan Streetman wrote:
> > +
> > + ret = -EINVAL;
> > + if (coproc && coproc->vas.rxwin) {
> > + wmem->txwin = nx842_alloc_txwin(coproc);
>
> this is wrong. the workmem is scratch memory that's valid only for
> the duration of a single operation.
>
> do you actually need a txwin per crypto transform? or do you need a
> txwin per coprocessor? or txwin per processor? either per-coproc or
> per-cpu should be created at driver init and held separately
> (globally) instead of a per-transform txwin. I really don't see why
> you would need a txwin per transform, because the coproc should not
> care how many different transforms there are.
We should only need a single window for the whole kernel really, plus
one per user process who wants direct access but that's not relevant
here.
Cheers,
Ben.
Re: [PATCH] crypto: vmx: Remove dubiously licensed crypto code
On Fri, 2017-05-05 at 15:52 +0200, Michal Suchánek wrote: > > So you have an e-mail message from one of the authors of the code. > Andy Polyakov wrote most of the code but there are probably other > contributors who never gave explicit consent for using their code > outside of OpenSSL. The only contributions to that file in OpenSSL that isn't from Andy are a whitespace fix and the addition of the licence :-) If that makes you feel better we could undo the whitespace fix :-) I've checked the other PowerPC related files in there and the situation is identical (aes-ppc.pl does have also a spelling fix in comments). At this point, it's pretty clear that this code is authored solely by Andy who gave permission to use it. Cheers, Ben.
Re: [PATCH v2] drivers/crypto/nx: saves chaining value from co-processor
On Wed, 2013-08-07 at 18:15 -0500, Fionnuala Gunter wrote: This patch fixes a bug that is triggered when cts(cbc(aes)) is used with nx-crypto driver on input larger than 32 bytes. The chaining value from co-processor was not being saved. This value is needed because it is used as the IV by cts(cbc(aes)). Signed-off-by: Fionnuala Gunter f...@linux.vnet.ibm.com Reviewed-by: Marcelo Cerri mhce...@linux.vnet.ibm.com Herbert, I assume you will handle this along with all the other NX fixes and I can safely take them out of linuxppc patchwork ? Cheers, Ben. --- v2. changed signed-off-by to reviewed-by and added more details to description This bug appeared in the original submission (v3.5) --- drivers/crypto/nx/nx-aes-cbc.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/crypto/nx/nx-aes-cbc.c b/drivers/crypto/nx/nx-aes-cbc.c index 35d483f..a2f99a9 100644 --- a/drivers/crypto/nx/nx-aes-cbc.c +++ b/drivers/crypto/nx/nx-aes-cbc.c @@ -95,6 +95,7 @@ static int cbc_aes_nx_crypt(struct blkcipher_desc *desc, if (rc) goto out; + memcpy(desc-info, csbcpb-cpb.aes_cbc.cv, AES_BLOCK_SIZE); atomic_inc((nx_ctx-stats-aes_ops)); atomic64_add(csbcpb-csb.processed_byte_count, (nx_ctx-stats-aes_bytes)); -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] drivers/crypto/nx: fixes when input data is too large
On Fri, 2013-07-26 at 14:08 -0300, Marcelo Cerri wrote: This series of patches fixes two bugs that are triggered when the input data is too large. The first one is caused by the miscalculation of physical addresses and the second one by some limits that the co-processor has to the input data. BTW. Are these supposed to go upstream via my tree or via crypto ? They are not part of my latest pull request to Linus because they were not CC'ed to linuxppc-dev so I didn't see them while collecting patches from patchwork. If you intend to have them go via the crypto tree that's fine, but if you intend to have them go via powerpc, then please resend with the correct mailing list on CC. Cheers, Ben. Marcelo Cerri (2): drivers/crypto/nx: fix physical addresses added to sg lists drivers/crypto/nx: fix limits to sg lists for SHA-2 drivers/crypto/nx/nx-sha256.c | 108 +++- drivers/crypto/nx/nx-sha512.c | 113 -- drivers/crypto/nx/nx.c| 22 ++-- 3 files changed, 148 insertions(+), 95 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] drivers/crypto/nx: fix limits to sg lists for SHA-2
On Fri, 2013-07-26 at 14:08 -0300, Marcelo Cerri wrote: Signed-off-by: Fionnuala Gunter f...@linux.vnet.ibm.com Signed-off-by: Joel Schopp jsch...@linux.vnet.ibm.com Signed-off-by: Joy Latten jmlat...@linux.vnet.ibm.com Signed-off-by: Marcelo Cerri mhce...@linux.vnet.ibm.com --- Why that enormous S-O-B list ? Did every of these people actually carry the patch ? If it's just acks or reviews, please use the corresponding Acked-by or Reviewed-by. Cheers, Ben. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] drivers/crypto/nx: fix limits to sg lists for SHA-2
On Fri, 2013-07-26 at 14:08 -0300, Marcelo Cerri wrote: --- drivers/crypto/nx/nx-sha256.c | 108 +++- drivers/crypto/nx/nx-sha512.c | 113 -- 2 files changed, 129 insertions(+), 92 deletions(-) What about the other nx drivers ? They are not affected ? Cheers, Ben. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/crypto/nx: fixes for multiple issues
On Mon, 2013-05-06 at 14:24 -0500, Kent Yoder wrote: Hi Ben, just a friendly reminder to please apply. Oh, I assumed that stuff was going via some drivers/crypto maintainer... I can apply. Cheers, Ben. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 03/17] powerpc: Add PFO support to the VIO bus
Hrm... I don't like that much:
+ if (op-timeout)
+ deadline = jiffies + msecs_to_jiffies(op-timeout);
+
+ while (true) {
+ hret = plpar_hcall_norets(H_COP, op-flags,
+ vdev-resource_id,
+ op-in, op-inlen, op-out,
+ op-outlen, op-csbcpb);
+
+ if (hret == H_SUCCESS ||
+ (hret != H_NOT_ENOUGH_RESOURCES
+ hret != H_BUSY hret != H_RESOURCE) ||
+ (op-timeout time_after(deadline, jiffies)))
+ break;
+
+ dev_dbg(dev, %s: hcall ret(%ld), retrying.\n, __func__, hret);
+ }
+
Is this meant to be called in atomic context ? If not, maybe it should
at the very least do a cond_resched() ?
Else, what about ceding the processor ? Or at the very least reducing
the thread priority for a bit ?
Shouldn't we also enforce to always have a timeout ? IE. Something like
30s or so if nothing specified to avoid having the kernel just hard
lock...
In general I don't like that sort of synchronous code, I'd rather return
the busy status up the chain which gives a chance to the caller to take
more appropriate measures depending on what it's doing, but that really
depends what you use that synchronous call for. I suppose if it's for
configuration type operations, it's ok...
Cheers,
Ben.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 03/17] powerpc: Add PFO support to the VIO bus
Else, what about ceding the processor ? Or at the very least reducing the thread priority for a bit ? Shouldn't we also enforce to always have a timeout ? IE. Something like 30s or so if nothing specified to avoid having the kernel just hard lock... In general I don't like that sort of synchronous code, I'd rather return the busy status up the chain which gives a chance to the caller to take more appropriate measures depending on what it's doing, but that really depends what you use that synchronous call for. I suppose if it's for configuration type operations, it's ok... In any case, don't resend the whole series, just that one patch. Cheers, Ben. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/17] powerpc: crypto: nx driver code supporting nx encryption
On Wed, 2012-03-21 at 15:15 -0700, Greg KH wrote: Really? vio drivers are supposed to look like this with the .name and .owner field manually being set in the static initialization of the driver? That's sad, and should be fixed, the vio core should do this type of thing for you. Yeah, they still do it the old way, nobody got to fix that yet, should be pretty easy though. Cheers, Ben. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/17] powerpc: crypto: nx driver code supporting nx encryption
On Thu, 2012-03-22 at 12:50 +1100, Benjamin Herrenschmidt wrote:
On Wed, 2012-03-21 at 15:15 -0700, Greg KH wrote:
Really? vio drivers are supposed to look like this with the .name and
.owner field manually being set in the static initialization of the
driver? That's sad, and should be fixed, the vio core should do this
type of thing for you.
Yeah, they still do it the old way, nobody got to fix that yet, should
be pretty easy though.
Kent, Dave care to try this ? It's only compile tested.
powerpc+sparc/vio: Modernize driver registration
This makes vio_register_driver() get the module owner name at compile
time like PCI drivers do, and adds a name pointer directly in struct
vio_driver to avoid having to explicitly initialize the embedded
struct device.
Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---
arch/powerpc/include/asm/vio.h | 10 +-
arch/powerpc/kernel/vio.c |9 +++--
arch/sparc/include/asm/vio.h |9 -
arch/sparc/kernel/ds.c |5 +
arch/sparc/kernel/vio.c|9 +++--
drivers/block/sunvdc.c |5 +
drivers/net/ethernet/ibm/ibmveth.c |7 ++-
drivers/net/ethernet/sun/sunvnet.c |5 +
drivers/scsi/ibmvscsi/ibmvfc.c |7 ++-
drivers/scsi/ibmvscsi/ibmvscsi.c |7 ++-
drivers/scsi/ibmvscsi/ibmvstgt.c |5 +
drivers/tty/hvc/hvc_vio.c |7 ++-
drivers/tty/hvc/hvcs.c |5 +
13 files changed, 44 insertions(+), 46 deletions(-)
diff --git a/arch/powerpc/include/asm/vio.h b/arch/powerpc/include/asm/vio.h
index 0a290a1..6bfd5ff 100644
--- a/arch/powerpc/include/asm/vio.h
+++ b/arch/powerpc/include/asm/vio.h
@@ -69,6 +69,7 @@ struct vio_dev {
};
struct vio_driver {
+ const char *name;
const struct vio_device_id *id_table;
int (*probe)(struct vio_dev *dev, const struct vio_device_id *id);
int (*remove)(struct vio_dev *dev);
@@ -76,10 +77,17 @@ struct vio_driver {
* be loaded in a CMO environment if it uses DMA.
*/
unsigned long (*get_desired_dma)(struct vio_dev *dev);
+ const struct dev_pm_ops *pm;
struct device_driver driver;
};
-extern int vio_register_driver(struct vio_driver *drv);
+extern int __vio_register_driver(struct vio_driver *drv, struct module *owner,
+const char *mod_name);
+/*
+ * vio_register_driver must be a macro so that KBUILD_MODNAME can be expanded
+ */
+#define vio_register_driver(driver)\
+ __vio_register_driver(driver, THIS_MODULE, KBUILD_MODNAME)
extern void vio_unregister_driver(struct vio_driver *drv);
extern int vio_cmo_entitlement_update(size_t);
diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
index bca3fc4..879dd25 100644
--- a/arch/powerpc/kernel/vio.c
+++ b/arch/powerpc/kernel/vio.c
@@ -1159,17 +1159,22 @@ static int vio_bus_remove(struct device *dev)
* vio_register_driver: - Register a new vio driver
* @drv: The vio_driver structure to be registered.
*/
-int vio_register_driver(struct vio_driver *viodrv)
+int __vio_register_driver(struct vio_driver *viodrv, struct module *owner,
+ const char *mod_name)
{
printk(KERN_DEBUG %s: driver %s registering\n, __func__,
viodrv-driver.name);
/* fill in 'struct driver' fields */
+ viodrv-driver.name = viodrv-name;
+ viodrv-driver.pm = viodrv-pm;
viodrv-driver.bus = vio_bus_type;
+ viodrv-driver.owner = owner;
+ viodrv-driver.mod_name = mod_name;
return driver_register(viodrv-driver);
}
-EXPORT_SYMBOL(vio_register_driver);
+EXPORT_SYMBOL(__vio_register_driver);
/**
* vio_unregister_driver - Remove registration of vio driver.
diff --git a/arch/sparc/include/asm/vio.h b/arch/sparc/include/asm/vio.h
index 9d83d3b..432afa8 100644
--- a/arch/sparc/include/asm/vio.h
+++ b/arch/sparc/include/asm/vio.h
@@ -284,6 +284,7 @@ struct vio_dev {
};
struct vio_driver {
+ const char *name;
struct list_headnode;
const struct vio_device_id *id_table;
int (*probe)(struct vio_dev *dev, const struct vio_device_id *id);
@@ -371,7 +372,13 @@ do { if (vio-debug VIO_DEBUG_##TYPE) \
vio-vdev-channel_id, ## a); \
} while (0)
-extern int vio_register_driver(struct vio_driver *drv);
+extern int __vio_register_driver(struct vio_driver *drv, struct module *owner,
+const char *mod_name);
+/*
+ * vio_register_driver must be a macro so that KBUILD_MODNAME can be expanded
+ */
+#define vio_register_driver(driver)\
+ __vio_register_driver(driver, THIS_MODULE, KBUILD_MODNAME)
extern void vio_unregister_driver(struct vio_driver *drv);
static inline struct vio_driver *to_vio_driver(struct device_driver *drv)
diff --git a/arch/sparc/kernel
Re: [PATCH 14/17] powerpc: crypto: nx driver code supporting nx encryption
On Wed, 2012-03-21 at 20:39 -0700, Greg KH wrote:
On Thu, Mar 22, 2012 at 01:57:30PM +1100, Benjamin Herrenschmidt wrote:
+int __vio_register_driver(struct vio_driver *viodrv, struct module *owner,
+ const char *mod_name)
{
viodrv-driver.bus = vio_bus_type;
+ viodrv-driver.name = viodrv-name;
+ viodrv-driver.bus = vio_bus_type;
+ viodrv-driver.owner = owner;
+ viodrv-driver.mod_name = mod_name;
Any reason you set .bus twice?
Nope, just a typo. I'll fix it up, when I have feedback from Dave.
Cheers,
Ben.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
