Re: [PATCH v3]crypto:hifn_795x.c Fix warning: variable 'ctx' set but not used
On Tue, Jul 06, 2010 at 11:36:37AM -0700, Justin P. Mattock (justinmatt...@gmail.com) wrote: The below patch gets rid of an unused variable ctx reported by GCC when building the kernel. CC [M] drivers/crypto/hifn_795x.o drivers/crypto/hifn_795x.c: In function 'hifn_flush': drivers/crypto/hifn_795x.c:2021:23: warning: variable 'ctx' set but not used drivers/crypto/hifn_795x.c: In function 'hifn_process_queue': drivers/crypto/hifn_795x.c:2142:23: warning: variable 'ctx' set but not used Signed-off-by: Justin P. Mattock justinmatt...@gmail.com Looks good, thank you. I can only wonder how I wanted to use it in that context... -- Evgeniy Polyakov -- 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: hifn_795x module on a 64 bit system
On Sat, Feb 27, 2010 at 03:04:13PM +0100, Michael Bohn (boh...@googlemail.com) wrote: is there a way to use the hifn_795x module on a 64 bit system. ( with 4GB RAM ) ? I get this error message if I try to load this module. [3.466307] HIFN supports only 32-bit addresses. [ 1147.645181] HIFN supports only 32-bit addresses. That's it - card only works in 32bit mode, it is limited by descriptor registers. The only way is to work this around is to allocate temporal buffer and copy data here and there, which is unlikely to be faster than software encryption. -- Evgeniy Polyakov -- 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: [RFC] [PATCH v2 0/4] Parallel IPsec
Hi. On Fri, Apr 24, 2009 at 12:24:51PM +0200, Steffen Klassert (steffen.klass...@secunet.com) wrote: This patchset adds the 'pcrypt' parallel crypto template. With this template it is possible to process the crypto requests of a transform in parallel without getting request reorder. This is in particular interesting for IPsec. Why can't it be used by default for all crypto operations instead of synchronous one? -- Evgeniy Polyakov -- 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: [RFC] [PATCH v2 0/4] Parallel IPsec
On Sat, Apr 25, 2009 at 05:21:42PM +0800, Herbert Xu (herb...@gondor.apana.org.au) wrote: Why can't it be used by default for all crypto operations instead of synchronous one? PCI-based drivers will not benefit from spreading the requests across CPUs. If anything they will suffer from the synchronisation. What's the deal PCI drivers have with CPUs data comes from/to? They do not touch cache, just run DMA transfer and complete the request. -- Evgeniy Polyakov -- 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: New kernel hifn795x driver does not play nice with luks
On Tue, Mar 17, 2009 at 11:33:32PM +0100, markus reichelt (m...@mareichelt.de) wrote: I have encountered a problem with the new HiFn driver in 2.6.27, cryptsetup and luks. the mailinglist you posted to mainly deals with loop-aes these days, the dm-crypt/luks/mainline loop-crypto guys for some obscure reason opted to start a mailinglist of the same name at linux-crypto@vger.kernel.org, so you are better off asking there (blame them for confusing people). CC added, though I guess you need to subscribe to their holy shrine, anyway. And keep in mind that dm-crypt/yada is still beta-code, it's nowhere near stable. What was that? Also, m...@mareichelt.de email was not in the proper header, so it does not appear in copy list. I am able to unlock my luks devices fine, but attempting to mount the device will result in mount hanging forever (ps shows it as D+, presumably it's waiting for I/O from the card), forcing me to manually kill the process. If i then try to close the luks partition luksClose informs me that the drive is in use. The hifn_795x module will be locked as well in the process. However, as soon as I disable/remove the hifn_795x module prior to mounting the device, everything works as it should yet again. Please send a dmesg output when card is blocked. Thank you. -- Evgeniy Polyakov -- 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: Runaway loop with the current git.
On Sun, Dec 07, 2008 at 05:01:08PM +, Alan Cox ([EMAIL PROTECTED]) wrote: Wrong. First at least because tty/console stuff is built in. Are you two people or just two names and email addresses for the same ? Depending on who you are asking for. /dev/console is a logical mapping to a device which may well be different, loaded after PCI is initialised and dependant on PCI. Yup. It may. But in showed case it is not. In showed case it showed a bug. Which you call a feature. But we alredy got, that you decided that Debian's initramfs-tools (0.92e) are allowed not to boot with some kernel configs. Yes. They won't boot if you don't include any disk drivers. They won't boot if you don't include any file systems. They wont boot in a million other cases. That is the user space not the kernel at fault. But right now _everything_ is presented. All things are in places. Disk drivers, filesystem, and even that stupid console/tty driver. It _IS_ in the kernel. The kernel is doing precisely what it is supposed to. It is even logging the user space bug and stopping trying to get stuck in a loop loading a module in order to attempt to cope with the user space bug. If you want to complain then file a debian bug, go fix the user space. You introduce so naive rules for the initramfs userspace... It should not use console, it should not load modules, which may load another one... What next, not to use syscalls? Sigh, instead of thinking on how to fix the weak boot process so that next time similar problem arises, we ended up with pointing a finger one to another... Hope we will not get another similar cases though. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Runaway loop with the current git.
On Sun, Dec 07, 2008 at 05:52:45PM +, Alan Cox ([EMAIL PROTECTED]) wrote: Alan, let's make some progress on this fingerpointing. If Herbert's patch fixes the crypto loading problem, it will find its way upstream for the current tree, and in the merge window Kay's patch may be applied and widely tested. Thoughts? I have no intention of applying Kay's patch because it is wrong and it will only break things not fix them. And you are sure it breaks something based on what? -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v4] crypto: sha512 - Move message schedule W[80] to static percpu area
Hi.
On Sun, Dec 07, 2008 at 11:17:28PM +0100, Adrian-Ken Rueegsegger ([EMAIL
PROTECTED]) wrote:
The message schedule W (u64[80]) is too big for the stack. In order
for this algorithm to be used with shash it is moved to a static
percpu area.
Signed-off-by: Adrian-Ken Rueegsegger [EMAIL PROTECTED]
+static DEFINE_PER_CPU(u64[80], msg_schedule);
+
static inline u64 Ch(u64 x, u64 y, u64 z)
{
return z ^ (x (y ^ z));
@@ -89,11 +90,12 @@ static inline void BLEND_OP(int I, u64 *W)
}
static void
-sha512_transform(u64 *state, u64 *W, const u8 *input)
+sha512_transform(u64 *state, const u8 *input)
{
u64 a, b, c, d, e, f, g, h, t1, t2;
int i;
+ u64 *W = get_cpu_var(msg_schedule);
This should be protected by preempt_disable(), shouldn't it?
--
Evgeniy Polyakov
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v4] crypto: sha512 - Move message schedule W[80] to static percpu area
On Mon, Dec 08, 2008 at 08:24:35AM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote: + u64 *W = get_cpu_var(msg_schedule); This should be protected by preempt_disable(), shouldn't it? get_cpu_var does it implicitly. Yeah, that simple! -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 回复：Re : vgdisplay problem with hifn795x
On Sat, Dec 06, 2008 at 10:09:55PM +0800, Lingbo Tang (SINA) ([EMAIL PROTECTED]) wrote: In some cases the non-debug version can work with mkfs.xfs, but it could not in most case. I tried to disable most services in the system, but it seems have no related to the system load. So I suppose it is easy to reproduce problem. Please show 'dmesg' output for the hung case when debug is turned off. Did you see register dump message when that happened? Please also try attached patch and show the hung dmesg (with tured off heavy debug). Thank you. -- Evgeniy Polyakov diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c index 6e9b39c..fc29bec 100644 --- a/drivers/crypto/hifn_795x.c +++ b/drivers/crypto/hifn_795x.c @@ -2093,6 +2093,10 @@ static int hifn_handle_req(struct ablkcipher_request *req) spin_unlock_irqrestore(dev-lock, flags); } + if (err) + printk(%s: nbytes: %u, started: %u, qlen: %d, err: %d.\n, + __func__, req-nbytes, dev-started, dev-queue.qlen, err); + return err; }
Re: Updated Openssl Patch to support Linux CryptoAPI v3
On Mon, Dec 01, 2008 at 04:28:49PM -0800, Shasi Pulijala ([EMAIL PROTECTED])
wrote:
This Openssl patch is the version 3 which incorporates some changes suggested
by the Linux Crypto Maintainer. Openssl still needs to be patched with OCF
first to use the linux cryptodev interface. The major changes in this patch
include:
1) Addition of a header file that defines the crypto and hash algorithm/modes
as a bitmap.
2) The structures session_op and crypt_op need to be included from the linux
kernel headers.
There are other problems mentioned prviously which were not included
here: http://marc.info/?l=linux-crypto-vgerm=122727693310351w=2
/* NB: deprecated */
+#ifndef CRYPTODEV_LINUX
struct session_op {
u_int32_t cipher; /* ie. CRYPTO_DES_CBC */
u_int32_t mac;/* ie. CRYPTO_MD5_HMAC */
@@ -187,8 +189,9 @@
int mackeylen; /* mac key */
caddr_t mackey;
If this strcuture is shared between kernelspace and userspace things are
very broken: pointer types may have different sizes in kernel and
userspace.
- u_int32_t ses;/* returns: session # */
+ u_int32_t ses;/* returns: session # */
};
+#endif
struct session2_op {
u_int32_t cipher; /* ie. CRYPTO_DES_CBC */
@@ -199,11 +202,12 @@
int mackeylen; /* mac key */
caddr_t mackey;
The same.
- u_int32_t ses;/* returns: session # */
+ u_int32_t ses;/* returns: session # */
int crid; /* driver id + flags (rw) */
int pad[4]; /* for future expansion */
};
+#ifndef CRYPTODEV_LINUX
struct crypt_op {
u_int32_t ses;
u_int16_t op; /* i.e. COP_ENCRYPT */
@@ -217,7 +221,7 @@
caddr_t mac;/* must be big enough for chosen MAC */
caddr_t iv;
The same.
Please provide full patch next time, it is hard to tell if there are
other problem places without looking at the code.
--
Evgeniy Polyakov
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: vgdisplay problem with hifn795x
Hi. On Sat, Nov 29, 2008 at 08:46:28PM +0800, [EMAIL PROTECTED] ([EMAIL PROTECTED]) wrote: I'm testing one hifn card with cryptsetup, but the vgdisplay will hang after I created the LV by cryptsetup. If I reboot the system, the system will hang during activating LVs at boot time. The system is CentOS5.2+2.6.27.4 kernel. I used all default options to create the encryption disk. I do not know what is vgdisplay, but HIFN driver got several bug fixes recently, so if problem is related to the driver, likely it was fixed. You may try pulling driver from the cryptodev repo [1] into your kernel build system and compile it, or wait for the kernel update from your vendor. Likely this bugfixes will be pushed into mainline tree in 2.6.29 merge window. 1. cryptodev tree http://git.kernel.org/?p=linux/kernel/git/herbert/cryptodev-2.6.git -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[HIFN] Use softirq kernel mapping in bh context.
Use KM_SOFTIRQ instead of KM_IRQ in tasklet context.
Added bug_on on input no-page condition.
Signed-off-by: Evgeniy Polyakov [EMAIL PROTECTED]
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index 26384e7..6e9b39c 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -1393,10 +1393,12 @@ static int hifn_setup_dma(struct hifn_device *dev,
n = nbytes;
while (n) {
if (t-length rctx-walk.flags ASYNC_FLAGS_MISALIGNED) {
+ BUG_ON(!sg_page(t));
dpage = sg_page(t);
doff = 0;
len = t-length;
} else {
+ BUG_ON(!sg_page(dst));
dpage = sg_page(dst);
doff = dst-offset;
len = dst-length;
@@ -1791,17 +1793,17 @@ static void hifn_process_ready(struct
ablkcipher_request *req, int error)
continue;
}
- saddr = kmap_atomic(sg_page(t), KM_IRQ1);
+ saddr = kmap_atomic(sg_page(t), KM_SOFTIRQ0);
err = ablkcipher_get(saddr, t-length, t-offset,
dst, nbytes, nbytes);
if (err 0) {
- kunmap_atomic(saddr, KM_IRQ1);
+ kunmap_atomic(saddr, KM_SOFTIRQ0);
break;
}
idx += err;
- kunmap_atomic(saddr, KM_IRQ1);
+ kunmap_atomic(saddr, KM_SOFTIRQ0);
}
ablkcipher_walk_exit(rctx-walk);
--
Evgeniy Polyakov
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Have HW invalidate src and dest descriptors after processing
On Mon, Nov 24, 2008 at 10:34:27PM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote: On Mon, Nov 24, 2008 at 05:23:22PM +0300, Evgeniy Polyakov wrote: Weird. patch/diff and hands all messed up. In the version which was tested there is no that changes, so just drop it. OK I've applied everything bar the first two and pushed it out. Please let me know if there's anything broken. Thank you. I will check it and report back if there are mismatches. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1 v8] Add CryptoAPI User Interface Support v8
Hi. On Mon, Nov 17, 2008 at 03:31:42PM -0800, Shasi Pulijala ([EMAIL PROTECTED]) wrote: This patch v8 includes the code that prevents synchronous calls from freeing data when request completion is interrupted, while the data may still be accessed by the crypto code in parallel. From: Shasi Pulijala [EMAIL PROTECTED] Looks ok, I did not find any obvious error. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Updated Openssl Patch to support Linux CryptoAPI
';
+ memcpy(op-data + op-algo_size, key, op-key_size);
+
+ if (ioctl(fd, CIOCGSESSION, op) != -1)
nids[count++] = ciphers[i].nid;
I thought this is a fatal error, doesn't?
+ close(fd);
}
- close(fd);
if (count 0)
*cnids = nids;
@@ -318,26 +375,34 @@
get_cryptodev_digests(const int **cnids)
{
static int nids[CRYPTO_ALGORITHM_MAX];
- struct session_op sess;
+ char data[100];
+ struct session_op *op = (struct session_op *)data;
int fd, i, count = 0;
+ char *mackey = NULL;
- if ((fd = get_dev_crypto()) 0) {
- *cnids = NULL;
- return (0);
- }
- memset(sess, 0, sizeof(sess));
- sess.mackey = (caddr_t)123456789abcdefghijklmno;
for (i = 0; digests[i].id count CRYPTO_ALGORITHM_MAX; i++) {
if (digests[i].nid == NID_undef)
continue;
- sess.mac = digests[i].id;
- sess.mackeylen = digests[i].keylen;
- sess.cipher = 0;
- if (ioctl(fd, CIOCGSESSION, sess) != -1
- ioctl(fd, CIOCFSESSION, sess.ses) != -1)
+ if ((fd = open_cryptodev_fd()) 0) {
+ *cnids = NULL;
+ return (0);
+ }
+
+ memset(op, 0, sizeof(struct session_op));
+ op-algo_size = strlen(algo_map_tbl[digests[i].id]);
+ op-key_size = 0;
+ op-hmackey_size = digests[i].keylen;
+ memcpy(op-data, algo_map_tbl[digests[i].id], op-algo_size);
+ if (op-hmackey_size)
+ mackey = (caddr_t) 123456789abcdefghijklmno;
+ op-data[op-algo_size++] = '\0';
+ memcpy(op-data + op-algo_size, mackey, op-hmackey_size);
+
+ if (ioctl(fd, CIOCGSESSION, op) != -1)
nids[count++] = digests[i].nid;
Same here and in other places where ioctl fails without returning error
to the callers.
--
Evgeniy Polyakov
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Fix queue processing.
From: Patrick McHardy [EMAIL PROTECTED]
Signed-off-by: Patrick McHardy [EMAIL PROTECTED]
Signed-off-by: Evgeniy Polyakov [EMAIL PROTECTED]
---
drivers/crypto/hifn_795x.c |9 -
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index 9a9e7ea..47952d8 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -2158,7 +2158,7 @@ static int hifn_setup_crypto_req(struct
ablkcipher_request *req, u8 op,
static int hifn_process_queue(struct hifn_device *dev)
{
- struct crypto_async_request *async_req;
+ struct crypto_async_request *async_req, *backlog;
struct hifn_context *ctx;
struct ablkcipher_request *req;
unsigned long flags;
@@ -2166,12 +2166,16 @@ static int hifn_process_queue(struct hifn_device *dev)
while (dev-started HIFN_QUEUE_LENGTH) {
spin_lock_irqsave(dev-lock, flags);
+ backlog = crypto_get_backlog(dev-queue);
async_req = crypto_dequeue_request(dev-queue);
spin_unlock_irqrestore(dev-lock, flags);
if (!async_req)
break;
+ if (backlog)
+ backlog-complete(backlog, -EINPROGRESS);
+
ctx = crypto_tfm_ctx(async_req-tfm);
req = container_of(async_req, struct ablkcipher_request, base);
@@ -2575,6 +2579,9 @@ static void hifn_tasklet_callback(unsigned long data)
* context or update is atomic (like setting dev-sa[i] to NULL).
*/
hifn_check_for_completion(dev, 0);
+
+ if (dev-started HIFN_QUEUE_LENGTH dev-queue.qlen)
+ hifn_process_queue(dev);
}
static int hifn_probe(struct pci_dev *pdev, const struct pci_device_id *id)
--
1.5.2.5
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[HIFN] Bugfix series.
This series contains HIFN driver fixes based on Patrick McHardy's work and additional queue management fix. This set also bumps HIFN performance almost two times compared to old code. Please apply. -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Move command descriptor setup to seperate function as preparation
From: Patrick McHardy [EMAIL PROTECTED]
Note 1: also fix a harmless typo while moving it: sa_idx is initialized
to dma-resi instead of dma-cmdi.
Note 2: errors from command descriptor setup are not propagated back,
anymore, they can't be handled anyway and all conditions leading
to errors should be checked earlier.
Signed-off-by: Patrick McHardy [EMAIL PROTECTED]
Signed-off-by: Evgeniy Polyakov [EMAIL PROTECTED]
---
drivers/crypto/hifn_795x.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index 4d22b21..320d08d 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -1296,7 +1296,7 @@ static int hifn_setup_src_desc(struct hifn_device *dev,
struct page *page,
dma-srcr[idx].p = __cpu_to_le32(addr);
dma-srcr[idx].l = __cpu_to_le32(size | HIFN_D_VALID |
- HIFN_D_MASKDONEIRQ | HIFN_D_LAST);
+ HIFN_D_MASKDONEIRQ | HIFN_D_NOINVALID | HIFN_D_LAST);
if (++idx == HIFN_D_SRC_RSIZE) {
dma-srcr[idx].l = __cpu_to_le32(HIFN_D_VALID |
@@ -1324,7 +1324,7 @@ static void hifn_setup_res_desc(struct hifn_device *dev)
HIFN_D_VALID | HIFN_D_LAST);
/*
* dma-resr[dma-resi].l = __cpu_to_le32(HIFN_MAX_RESULT |
HIFN_D_VALID |
-* HIFN_D_LAST);
+* HIFN_D_LAST | HIFN_D_NOINVALID);
*/
if (++dma-resi == HIFN_D_RES_RSIZE) {
@@ -1353,12 +1353,12 @@ static void hifn_setup_dst_desc(struct hifn_device
*dev, struct page *page,
idx = dma-dsti;
dma-dstr[idx].p = __cpu_to_le32(addr);
dma-dstr[idx].l = __cpu_to_le32(size | HIFN_D_VALID |
- HIFN_D_MASKDONEIRQ | HIFN_D_LAST);
+ HIFN_D_MASKDONEIRQ | HIFN_D_NOINVALID | HIFN_D_LAST);
if (++idx == HIFN_D_DST_RSIZE) {
dma-dstr[idx].l = __cpu_to_le32(HIFN_D_VALID |
HIFN_D_JUMP | HIFN_D_MASKDONEIRQ |
- HIFN_D_LAST);
+ HIFN_D_LAST | HIFN_D_NOINVALID);
idx = 0;
}
dma-dsti = idx;
--
1.5.2.5
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Don't copy src sg list.
From: Patrick McHardy [EMAIL PROTECTED]
Signed-off-by: Patrick McHardy [EMAIL PROTECTED]
Signed-off-by: Evgeniy Polyakov [EMAIL PROTECTED]
---
drivers/crypto/hifn_795x.c | 75 +++
1 files changed, 33 insertions(+), 42 deletions(-)
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index 89e2e9a..13152f1 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -1378,32 +1378,40 @@ static int hifn_setup_dma(struct hifn_device *dev,
struct hifn_context *ctx,
unsigned int soff, doff;
unsigned int n, len;
+ n = nbytes;
+ while (n) {
+ spage = sg_page(src);
+ soff = src-offset;
+ len = min(src-length, n);
+
+ dprintk(%s: spage: %p, soffset: %u, nbytes: %u,
+ priv: %p, ctx: %p.\n,
+ dev-name, spage, soff, nbytes, priv, ctx);
+ hifn_setup_src_desc(dev, spage, soff, len, n - len == 0);
+
+ src++;
+ n -= len;
+ }
+
t = ctx-walk.cache[0];
n = nbytes;
while (n) {
if (t-length) {
- spage = dpage = sg_page(t);
- soff = doff = 0;
+ dpage = sg_page(t);
+ doff = 0;
len = t-length;
} else {
- spage = sg_page(src);
- soff = src-offset;
-
dpage = sg_page(dst);
doff = dst-offset;
-
len = dst-length;
}
len = min(len, n);
- dprintk(%s: spage: %p, soffset: %u, dpage: %p, doffset: %u,
- nbytes: %u, priv: %p, ctx: %p.\n,
- dev-name, spage, soff, dpage, doff, nbytes, priv, ctx);
-
- hifn_setup_src_desc(dev, spage, soff, len, n - len == 0);
+ dprintk(%s: dpage: %p, doffset: %u, nbytes: %u,
+ priv: %p, ctx: %p.\n,
+ dev-name, dpage, doff, nbytes, priv, ctx);
hifn_setup_dst_desc(dev, dpage, doff, len, n - len == 0);
- src++;
dst++;
t++;
n -= len;
@@ -1454,32 +1462,26 @@ static void ablkcipher_walk_exit(struct ablkcipher_walk
*w)
w-num = 0;
}
-static int ablkcipher_add(void *daddr, unsigned int *drestp, struct
scatterlist *src,
+static int ablkcipher_add(unsigned int *drestp, struct scatterlist *dst,
unsigned int size, unsigned int *nbytesp)
{
unsigned int copy, drest = *drestp, nbytes = *nbytesp;
int idx = 0;
- void *saddr;
if (drest size || size nbytes)
return -EINVAL;
while (size) {
- copy = min(drest, min(size, src-length));
-
- saddr = kmap_atomic(sg_page(src), KM_SOFTIRQ1);
- memcpy(daddr, saddr + src-offset, copy);
- kunmap_atomic(saddr, KM_SOFTIRQ1);
+ copy = min(drest, min(size, dst-length));
size -= copy;
drest -= copy;
nbytes -= copy;
- daddr += copy;
dprintk(%s: copy: %u, size: %u, drest: %u, nbytes: %u.\n,
__func__, copy, size, drest, nbytes);
- src++;
+ dst++;
idx++;
}
@@ -1492,8 +1494,7 @@ static int ablkcipher_add(void *daddr, unsigned int
*drestp, struct scatterlist
static int ablkcipher_walk(struct ablkcipher_request *req,
struct ablkcipher_walk *w)
{
- struct scatterlist *src, *dst, *t;
- void *daddr;
+ struct scatterlist *dst, *t;
unsigned int nbytes = req-nbytes, offset, copy, diff;
int idx, tidx, err;
@@ -1503,26 +1504,22 @@ static int ablkcipher_walk(struct ablkcipher_request
*req,
if (idx = w-num (w-flags ASYNC_FLAGS_MISALIGNED))
return -EINVAL;
- src = req-src[idx];
dst = req-dst[idx];
- dprintk(\n%s: slen: %u, dlen: %u, soff: %u, doff: %u, offset:
%u,
- nbytes: %u.\n,
- __func__, src-length, dst-length, src-offset,
- dst-offset, offset, nbytes);
+ dprintk(\n%s: dlen: %u, doff: %u, offset: %u, nbytes: %u.\n,
+ __func__, dst-length, dst-offset, offset, nbytes);
if (!IS_ALIGNED(dst-offset, HIFN_D_DST_DALIGN) ||
!IS_ALIGNED(dst-length, HIFN_D_DST_DALIGN) ||
offset) {
- unsigned slen = min(src-length - offset, nbytes);
+ unsigned slen = min(dst-length - offset, nbytes);
unsigned dlen = PAGE_SIZE
[PATCH] Fix queue management.
Fix queue management. Change ring size and perform its check not
one after another descriptor, but using stored pointers to the last
checked descriptors.
Signed-off-by: Evgeniy Polyakov [EMAIL PROTECTED]
---
drivers/crypto/hifn_795x.c | 174 ---
1 files changed, 81 insertions(+), 93 deletions(-)
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index 73aef49..26384e7 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -360,14 +360,14 @@ static atomic_t hifn_dev_number;
#define HIFN_NAMESIZE 32
#define HIFN_MAX_RESULT_ORDER 5
-#defineHIFN_D_CMD_RSIZE24*4
-#defineHIFN_D_SRC_RSIZE80*4
-#defineHIFN_D_DST_RSIZE80*4
-#defineHIFN_D_RES_RSIZE24*4
+#defineHIFN_D_CMD_RSIZE24*1
+#defineHIFN_D_SRC_RSIZE80*1
+#defineHIFN_D_DST_RSIZE80*1
+#defineHIFN_D_RES_RSIZE24*1
#define HIFN_D_DST_DALIGN 4
-#define HIFN_QUEUE_LENGTH HIFN_D_CMD_RSIZE-1
+#define HIFN_QUEUE_LENGTH (HIFN_D_CMD_RSIZE - 1)
#define AES_MIN_KEY_SIZE 16
#define AES_MAX_KEY_SIZE 32
@@ -1256,6 +1256,7 @@ static int hifn_setup_cmd_desc(struct hifn_device *dev,
}
dev-sa[sa_idx] = priv;
+ dev-started++;
cmd_len = buf_pos - buf;
dma-cmdr[dma-cmdi].l = __cpu_to_le32(cmd_len | HIFN_D_VALID |
@@ -1382,9 +1383,6 @@ static int hifn_setup_dma(struct hifn_device *dev,
soff = src-offset;
len = min(src-length, n);
- dprintk(%s: spage: %p, soffset: %u, nbytes: %u,
- priv: %p, rctx: %p.\n,
- dev-name, spage, soff, nbytes, priv, rctx);
hifn_setup_src_desc(dev, spage, soff, len, n - len == 0);
src++;
@@ -1405,9 +1403,6 @@ static int hifn_setup_dma(struct hifn_device *dev,
}
len = min(len, n);
- dprintk(%s: dpage: %p, doffset: %u, nbytes: %u,
- priv: %p, rctx: %p.\n,
- dev-name, dpage, doff, nbytes, priv, rctx);
hifn_setup_dst_desc(dev, dpage, doff, len, n - len == 0);
dst++;
@@ -1620,13 +1615,12 @@ static int hifn_setup_session(struct ablkcipher_request
*req)
goto err_out;
}
- dev-snum++;
- dev-started++;
-
err = hifn_setup_dma(dev, ctx, rctx, req-src, req-dst, req-nbytes,
req);
if (err)
goto err_out;
+ dev-snum++;
+
dev-active = HIFN_DEFAULT_ACTIVE_NUM;
spin_unlock_irqrestore(dev-lock, flags);
@@ -1635,12 +1629,13 @@ static int hifn_setup_session(struct ablkcipher_request
*req)
err_out:
spin_unlock_irqrestore(dev-lock, flags);
err_out_exit:
- if (err)
- dprintk(%s: iv: %p [%d], key: %p [%d], mode: %u, op: %u,
+ if (err) {
+ printk(%s: iv: %p [%d], key: %p [%d], mode: %u, op: %u,
type: %u, err: %d.\n,
dev-name, rctx-iv, rctx-ivsize,
ctx-key, ctx-keysize,
rctx-mode, rctx-op, rctx-type, err);
+ }
return err;
}
@@ -1676,6 +1671,7 @@ static int hifn_test(struct hifn_device *dev, int encdec,
u8 snum)
if (err)
goto err_out;
+ dev-started = 0;
msleep(200);
dprintk(%s: decoded: , dev-name);
@@ -1702,6 +1698,7 @@ static int hifn_start_device(struct hifn_device *dev)
{
int err;
+ dev-started = dev-active = 0;
hifn_reset_dma(dev, 1);
err = hifn_enable_crypto(dev);
@@ -1755,19 +1752,22 @@ static int ablkcipher_get(void *saddr, unsigned int
*srestp, unsigned int offset
return idx;
}
-static void hifn_process_ready(struct ablkcipher_request *req, int error)
+static inline void hifn_complete_sa(struct hifn_device *dev, int i)
{
- struct hifn_context *ctx = crypto_tfm_ctx(req-base.tfm);
- struct hifn_request_context *rctx = ablkcipher_request_ctx(req);
- struct hifn_device *dev;
-
- dprintk(%s: req: %p, ctx: %p rctx: %p.\n, __func__, req, ctx, rctx);
+ unsigned long flags;
- dev = ctx-dev;
- dprintk(%s: req: %p, started: %d.\n, __func__, req, dev-started);
+ spin_lock_irqsave(dev-lock, flags);
+ dev-sa[i] = NULL;
+ dev-started--;
+ if (dev-started 0)
+ printk(%s: started: %d.\n, __func__, dev-started);
+ spin_unlock_irqrestore(dev-lock, flags);
+ BUG_ON(dev-started 0);
+}
- if (--dev-started 0)
- BUG();
+static void hifn_process_ready(struct ablkcipher_request *req, int error)
+{
+ struct hifn_request_context *rctx
Re: HIFN driver update.
Hi Herbert. On Thu, Nov 13, 2008 at 09:58:54PM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote: Please send them as separate patches like Patrick's original work. Having everything mashed together makes maintainence difficult. Not sure if it is possible, since half of the Patrick's patches were not applied because of already made cleanups. I can split to Patrick's and 'the rest' though. New features (like variable ring size) will be in the new patches. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dm-crypt using hifn_795x - still broken
Hi Tobias. On Wed, Nov 12, 2008 at 05:10:41PM +0100, Tobias Geiger ([EMAIL PROTECTED]) wrote: hifn0: r: , active: 0, started: 20, success: 4247: qlen: 0/1, reset: 1. Please try to disable watchdog for now, its logic to detect stall hardware is a bit subtle: --- drivers/crypto/hifn_795x.c~ 2008-11-12 19:20:18.0 +0300 +++ drivers/crypto/hifn_795x.c 2008-11-12 19:20:50.0 +0300 @@ -2689,7 +2689,7 @@ goto err_out_unregister_rng; INIT_DELAYED_WORK(dev-work, hifn_work); - schedule_delayed_work(dev-work, HZ); + //schedule_delayed_work(dev-work, HZ); dprintk(HIFN crypto accelerator card at %s has been successfully registered as %s.\n, -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dm-crypt using hifn_795x - still broken
, - PAGE_SIZE HIFN_MAX_RESULT_ORDER, - PCI_DMA_FROMDEVICE); - free_pages(dev-result_mem, HIFN_MAX_RESULT_ORDER); for (i=0; i3; ++i) if (dev-bar[i]) iounmap(dev-bar[i]); -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
HIFN driver update.
Hi.
Updated version is based on old Patrick McHardy's patches, and contains
several bug fixes from me.
Great thanks to Andreas Gerlich (agl_online.de), who provided remote
access to the machine with the HIFN card in.
Now it is very stable and passed lots of stress tests for the last several days.
Patch includes unaligned fixes, ring size changes, watchdog update,
backlog processing fixes, dma cleanups.
Signed-off-by: Evgeniy Polyakov [EMAIL PROTECTED]
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index 4d22b21..3900a3c 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -38,9 +38,6 @@
#include asm/kmap_types.h
-#undef dprintk
-
-#define HIFN_TEST
//#define HIFN_DEBUG
#ifdef HIFN_DEBUG
@@ -363,14 +360,14 @@ static atomic_t hifn_dev_number;
#define HIFN_NAMESIZE 32
#define HIFN_MAX_RESULT_ORDER 5
-#defineHIFN_D_CMD_RSIZE24*4
-#defineHIFN_D_SRC_RSIZE80*4
-#defineHIFN_D_DST_RSIZE80*4
-#defineHIFN_D_RES_RSIZE24*4
+#defineHIFN_D_CMD_RSIZE24
+#defineHIFN_D_SRC_RSIZE80
+#defineHIFN_D_DST_RSIZE80
+#defineHIFN_D_RES_RSIZE24
#define HIFN_D_DST_DALIGN 4
-#define HIFN_QUEUE_LENGTH HIFN_D_CMD_RSIZE-1
+#define HIFN_QUEUE_LENGTH HIFN_D_CMD_RSIZE
#define AES_MIN_KEY_SIZE 16
#define AES_MAX_KEY_SIZE 32
@@ -406,8 +403,6 @@ struct hifn_dma {
u8
command_bufs[HIFN_D_CMD_RSIZE][HIFN_MAX_COMMAND];
u8 result_bufs[HIFN_D_CMD_RSIZE][HIFN_MAX_RESULT];
- u64 test_src, test_dst;
-
/*
* Our current positions for insertion and removal from the descriptor
* rings.
@@ -434,9 +429,6 @@ struct hifn_device
struct pci_dev *pdev;
void __iomem*bar[3];
- unsigned long result_mem;
- dma_addr_t dst;
-
void*desc_virt;
dma_addr_t desc_dma;
@@ -446,8 +438,6 @@ struct hifn_device
spinlock_t lock;
- void*priv;
-
u32 flags;
int active, started;
struct delayed_work work;
@@ -657,12 +647,17 @@ struct ablkcipher_walk
struct hifn_context
{
- u8 key[HIFN_MAX_CRYPT_KEY_LENGTH], *iv;
+ u8 key[HIFN_MAX_CRYPT_KEY_LENGTH];
struct hifn_device *dev;
- unsigned intkeysize, ivsize;
+ unsigned intkeysize;
+};
+
+struct hifn_request_context
+{
+ u8 *iv;
+ unsigned intivsize;
u8 op, type, mode, unused;
struct ablkcipher_walk walk;
- atomic_tsg_num;
};
#define crypto_alg_to_hifn(a) container_of(a, struct hifn_crypto_alg, alg)
@@ -1168,7 +1163,8 @@ static int hifn_setup_crypto_command(struct hifn_device
*dev,
}
static int hifn_setup_cmd_desc(struct hifn_device *dev,
- struct hifn_context *ctx, void *priv, unsigned int nbytes)
+ struct hifn_context *ctx, struct hifn_request_context *rctx,
+ void *priv, unsigned int nbytes)
{
struct hifn_dma *dma = (struct hifn_dma *)dev-desc_virt;
int cmd_len, sa_idx;
@@ -1179,7 +1175,7 @@ static int hifn_setup_cmd_desc(struct hifn_device *dev,
buf_pos = buf = dma-command_bufs[dma-cmdi];
mask = 0;
- switch (ctx-op) {
+ switch (rctx-op) {
case ACRYPTO_OP_DECRYPT:
mask = HIFN_BASE_CMD_CRYPT | HIFN_BASE_CMD_DECODE;
break;
@@ -1196,15 +1192,15 @@ static int hifn_setup_cmd_desc(struct hifn_device *dev,
buf_pos += hifn_setup_base_command(dev, buf_pos, nbytes,
nbytes, mask, dev-snum);
- if (ctx-op == ACRYPTO_OP_ENCRYPT || ctx-op == ACRYPTO_OP_DECRYPT) {
+ if (rctx-op == ACRYPTO_OP_ENCRYPT || rctx-op == ACRYPTO_OP_DECRYPT) {
u16 md = 0;
if (ctx-keysize)
md |= HIFN_CRYPT_CMD_NEW_KEY;
- if (ctx-iv ctx-mode != ACRYPTO_MODE_ECB)
+ if (rctx-iv rctx-mode != ACRYPTO_MODE_ECB)
md |= HIFN_CRYPT_CMD_NEW_IV;
- switch (ctx-mode) {
+ switch (rctx-mode) {
case ACRYPTO_MODE_ECB:
md |= HIFN_CRYPT_CMD_MODE_ECB;
break;
@@ -1221,7 +1217,7 @@ static int hifn_setup_cmd_desc(struct hifn_device *dev,
goto err_out;
}
- switch (ctx-type) {
+ switch (rctx
Re: Can anyone explain the AEAD implementation
Hi. On Mon, Nov 03, 2008 at 11:01:16AM +, Dean Jenkins ([EMAIL PROTECTED]) wrote: I observe that the ESP module uses an AEAD crypto API. AEAD means Authentication Encryption with Associated Data. It seems Encryption and HMAC are combined into a single API. Am I correct ? Please can anyone explain how individual Encryption algorithms and Hashing algorithms are selected for the AEAD API ? For example, I want to use a particular AES and SHA-1 implementations in a hardware accelerator. You can find precise examples in crypto/tcrypt.c test module. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IPsec books and how to add async hardware encryption ?
Hi Dean. On Mon, Nov 03, 2008 at 10:50:59AM +, Dean Jenkins ([EMAIL PROTECTED]) wrote: cryptd is an software engine example of how hardware driver could be implemented. Are you saying that to implement an async hardware driver I could use the APIs used by cryptd and create my own hwcryptd ? Yes for the APIs used in cryptd, but usually hardware driver does not need to have any threads attached, since completion of the event is handled in the interrupt handler. If yes, is there any documentation for the APIs used by cryptd ? No, there is no documentation except source code. In some files you can even find this comments: * HEAVY TODO: needs to kick Herbert XU to write documentation. Herbert is a crypto maintainer who created async crypto interfaces you found in cryptd. You can also check hardware crypto drivers in drivers/crypto/ directory. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: hifn_795x and geode-aes module
Hi. On Sat, Nov 01, 2008 at 06:22:35PM +0100, Andreas Gerlich ([EMAIL PROTECTED]) wrote: When I use the software modules (aes_i586, aes_generic) all works. The hifn_795x module without geode-aes module works NOT! It seems when both modules hifn_795x and goede-aes are loaded only the geode-aes is used (only with keysize 128). Apparently hifn does not work correctly in your case which requires a little bit of debugging we have discussed in private. When both modules are loaded, only one with higher prio is used (until you specify exactly needed module), geode-aes for specified chain mode (cbc, ecb and so on) is higher for geode aes (400 vs 300 for hifn), so it is used. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: hifn_7955 and Linux
Hi. On Fri, Oct 31, 2008 at 02:36:16PM +0100, Andreas Gerlich ([EMAIL PROTECTED]) wrote: But it doesn't work. I get errors when I try tu use the AES-engine of the hifn_795x/Chip/Module. Can you tell me what the parameters are nessesary for cryptsetup to use the hifn_7955? Please show the dmesg output for the failed command and optionally (but very helpful) enable debug and send us its output. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IPsec books and how to add async hardware encryption ?
Hi. On Fri, Oct 31, 2008 at 03:08:48PM +, Dean Jenkins ([EMAIL PROTECTED]) wrote: I am having difficulty finding a book that describes IPsec that includes IKEv2 and is preferably aimed at using Linux crypto. Can anyone recommend such a book ? There is no such book I believe. Also, how do I find info on the cypto APIs eg. how to use cryptd (2.6.24 kernel) ? I'm working on a project that uses an embedded hardware encryption engine that needs to use async off-load. Is cryptd the entity for async hardware crypto off-loading ? cryptd is an software engine example of how hardware driver could be implemented. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1 v7] Add Crypto API User Interface Support
Hi. On Fri, Oct 24, 2008 at 03:16:52PM -0700, Loc Ho ([EMAIL PROTECTED]) wrote: I don't understand what you are referring to here. Let's assume that operation is submitted synchronously to cryptodev, cryptodev in turn will submit to the underlying driver. If the underlying driver is software based, it will completed synchronous and return without calling wait_for_completion. If the underlying driver is hardware based or asynchronous, it will wait for completion via the callback function signaling the event wait object. Now, let's assume that operation is submitted asynchronously to cryptodev, crypto in turn will submit to the underlying driver. If the underlying driver is software based and completed, it just return. If the underlying driver is asynchronous such as hardware, it will return immediately without waiting. CryptoDev will call the AIO callback function when the crypto driver call cryptodev callback function. Therefore, what is the issue? With async driver crypto_ablkcipher_encrypt() (and other crypto processing functions) will return immediately (with -EINPROGRESS return value in case of HIFN driver for example), but your code will wait for request completion at wait_for_completion_interruptible() point. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1 v7] Add Crypto API User Interface Support
Hi.
On Fri, Oct 24, 2008 at 01:54:23PM -0700, Shasi Pulijala ([EMAIL PROTECTED])
wrote:
This patch v7 includes a few coding style changes and benchmark comparison
between for tcrypt and cryptodev. These are just a rough estimate and not the
exact numbers for cbc-aes. CryptoDev interface will always be slight more as
it includes crypto setup and a few other housekeeping tasks. Tcrypt is just
the time for the encrypt or decrypt function calls only.
Tcrypt Cryptodev
16 byte 47us132us
32 byte 51us141us
64 byte 68us150us
Can you also run cryptodev under profiler to determine, why it is, well,
noticebly, slower than tcrypt?
With regard to forward declaration, they are needed.
Why? Can't you reorder functions to eliminate that?
+ }
+ sg_set_page(sg[x], page, bufsize, offset);
+ rop = PAGE_SIZE - sg[x].offset;
+ if (bufsize rop) {
+ sg[x].length = rop;
+ bufsize = bufsize - rop;
+ }
+ offset = 0;
What if bufsize is smaller than 'rop', but there are several pages?
This will initialize wrong buffers, but probably there is a check somewhere
above this layer.
Yes, there is a check above for the number of pages returned. It returns an
error if the number of pages returned is greater/less than requested.
With regard to Still this is not an async crypto, it is asynchronous
interface if called via AIO. For vector write, it will turn into synchronous
AIO within the kernel AIO framework.
It does not mean it is async, since crypto processing itself is
synchronous. I.e. you can submit requests asynchronously, but they are
processed by the crypto code one-by-one with waiting after each request.
This actually can explain your numbers: instead of having flow of
requests, you have to do really lots of work per-request. I do not say
this is wrong (although imho it should be done differently), since it is
your code and likely it fits your needs.
Plus, you did not figure what happens when request completion is
interrupted with regard to freeing data, which may be accesed by the
crypto code in parallel.
+ aead_request_set_crypt(req, ssg, dsg, bufsize, cop-iv);
+ aead_request_set_assoc(req, adata, cop-assoc_len);
+
+ atomic_inc(ses_ptr-refcnt);
+
+ if (cop-eop == COP_ENCRYPT)
+ ret = crypto_aead_encrypt(req);
+ else
+ ret = crypto_aead_decrypt(req);
+
+ switch (ret) {
+ case 0:
+ if (!iocb)
+ atomic_dec(result-opcnt);
+ break;
+ case -EINPROGRESS:
+ case -EBUSY:
+ if (iocb) {
+ CDPRINTK(2, KERN_INFO,
+ Async Call AEAD:Returning Now\n);
+ return -EIOCBQUEUED;
+ }
+ ret = wait_for_completion_interruptible(
+ result-crypto_completion);
+ if (!ret)
+ ret = result-err;
+ if (!ret) {
+ INIT_COMPLETION(result-crypto_completion);
+ break;
+ }
I.e. let's suppose it was interrupted here, while crypto driver performs
a processing on given pages.
+ /* fall through */
+ default:
+ printk(KERN_ERR PFX sid %p enc/dec failed error %d\n,
+ ses_ptr, -ret);
+ if (!iocb)
+ atomic_dec(result-opcnt);
+ break;
+ }
+
+ if (nopin !ret) {
+ if (copy_to_user(dst, data, enc ? bufsize + authsize :
+ bufsize - authsize))
+ printk(KERN_ERR PFX
+ failed to copy encrypted data
+ to user space\n);
+ CD_HEXDUMP(data, enc ? bufsize + authsize :
+ bufsize - authsize);
+ }
+
+ /* Check if last reference */
+ if (atomic_dec_and_test(ses_ptr-refcnt))
+ cryptodev_destroy_session(ses_ptr);
+ if (dst_flag)
+ cryptodev_release_pages(result-dpages, nr_dpages);
+out_spages:
+ cryptodev_release_pages(result-spages, nr_spages);
Now you have freed pages, which are still accessible by the request
crypto processing code somewhere in the underlying driver.
--
Evgeniy Polyakov
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: hifn_795x in Linux-2.6.27-rc7
Hi Patrick. On Wed, Sep 24, 2008 at 06:40:29PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: There are a few known problems left in the HIFN driver that I have patches for but unfortunately didn't manage to clean them up and submit them yet. Looking at my queue, what's still broken is roughly: [HIFN]: Have HW invalidate src and dest descriptors after processing The descriptors need to be invalidated after processing for ring cleanup to work properly and to avoid using an old destination descriptor when the src and cmd descriptors are already set up and the dst descriptor isn't. Signed-off-by: Patrick McHardy [EMAIL PROTECTED] That one is actually probably OK but I didn't send it upstream yet because I didn't finish all follow-up patches that depend on how this works. Further: [HIFN]: Fix DMA setup without a changelog :) IIRC the problem is that HIFN sets up each DMA descriptor for the full request length, even though it should only cover on SG entry. The result is memory corruption. Can not tell without patch itself, but code operates on the number of bytes provided from the higher levels, not full dma size (0x3ff bytes). [HIFN]: Don't copy src sg list also without a changelog. I think this one was just an optimization, the source descriptors don't have alignment or size restrictions and thus we don't need to linearize it first. Also can not say for sure, but there should not be src linearization, instead we copy data from src to aligned dst and then perform crypto processin in place, and then copy data back. [HIFN]: Fix request context corruption HIFN uses the transform context to store per-request data, which breaks when more than one request is outstanding. Move per request members from struct hifn_context to a new struct hifn_request_context and convert the code to use this. Its per-tfm, so things like key and iv are ok, and others should only be accessed under the lock, but there may be problems. and: [HIFN]: Fix queue processing again without a changelog. The problem this patch fixed was missing crypto backlog handling, causing crashes with dm_crypt under load. Yup, that's messy :) If someone is interested in picking this up I could send my old patches, I probably won't manage to do it myself in the next time. Please do, if they work for you, we can just apply them, but I would like first to check them out. Thank you! -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1 v6] Add CryptoAPI User Interface Support Patch v6
) {
+ if (copy_to_user(dst, data, enc ? bufsize + authsize :
+ bufsize - authsize))
+ printk(KERN_ERR PFX
+ failed to copy encrypted data
+ to user space\n);
+ CD_HEXDUMP(data, enc ? bufsize + authsize :
+ bufsize - authsize);
+ }
+
+ /* Check if last reference */
+ if (atomic_dec_and_test(ses_ptr-refcnt))
+ cryptodev_destroy_session(ses_ptr);
+ if (dst_flag)
+ cryptodev_release_pages(result-dpages, nr_dpages);
+out_spages:
+ cryptodev_release_pages(result-spages, nr_spages);
+
+out_tmp:
+ if (atomic_dec_and_test(result-opcnt))
+ cryptodev_destroy_res(result);
+ return ret;
+}
Still this is not an async crypto.
Is it what you expect it to be? Since it is protected by the per-ctx
lock and waits for the crypto operation completion. Also, since waiting
is interruptible, it is possible to destroy session and/or release
pages, so actual crypto processing will crash the system.
Are there any benchmark of this approach?
--
Evgeniy Polyakov
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: hifn_795x in Linux-2.6.27-rc7
On Wed, Sep 24, 2008 at 07:13:36PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: Please do, if they work for you, we can just apply them, but I would like first to check them out. I'm not sure whether they're ready for applying. IIRC I still got stalls occasionally, but I don't remember the details. They made things better for me though :) Anyways, patches coming up in a minute. I will review them and push to Herbert if things are ok. Thanks a lot, Patrick! -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: hifn_795x in Linux-2.6.27-rc7
Hi Dimitri. On Mon, Sep 22, 2008 at 07:39:23PM +0200, Dimitri Puzin ([EMAIL PROTECTED]) wrote: nommu_map_single: overflow 1fe047e00+512 of device mask Does attached patch fix the problem? diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c index 81f3f95..b288bbd 100644 --- a/drivers/crypto/hifn_795x.c +++ b/drivers/crypto/hifn_795x.c @@ -2593,7 +2593,7 @@ static int hifn_probe(struct pci_dev *pdev, const struct pci_device_id *id) return err; pci_set_master(pdev); - err = pci_set_dma_mask(pdev, DMA_32BIT_MASK); + err = pci_set_dma_mask(pdev, DMA_64BIT_MASK); if (err) goto err_out_disable_pci_device; -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1 v5] Add CryptoAPI User Interface Patch v5
Hi. Sorry for late reply. On Wed, Aug 27, 2008 at 02:23:31PM -0700, Shasi Pulijala ([EMAIL PROTECTED]) wrote: Hi Evgeniy, This is Linux CryptoAPI user space interface support patch v5. This version adds: -Preventing race conditions in session creation. -Crypto operations are now issued from user space using writev(sync) and AIO write(async) instead of ioctls. There is number of issues I found with this version. 1. It does not apply since your mailer damaged the patch. 2. It does not follow codying style in several places (like spaces, empty lines and other similar small stuff). 3. There are way _too_ many allocations. I think it does not matter too much in case of expensive crypto processing, but I think it can be reduced. 4. You should not pass double pointer to csession in allocation path. Instead return allocated structure as a result value, and use ERR_PTR, PTR_ERR and IS_ERR functions to check that returned pointer is valid or error. 5. Main issue: it is not asynchronous crypto layer. I did not check aio_write() processing, but otherwise you wait in the submission fuction for the operation to complete. Also I'm not sure it works as expected, since waiting in wait_for_completion_interruptible(result-crypto_completion) can be interrupted and storage freed (tmp variable and thus result variable), so eventually when crypto core will invoke async completion callback, it will oops. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OpenSSL patch to support Linux CryptoAPI.
Hi. On Mon, Aug 11, 2008 at 11:53:41AM -0700, Loc Ho ([EMAIL PROTECTED]) wrote: You do not need to pass multiple pointers, but instead a multiple data. You can do that via single area provided to the kernel and multiple size fields, where each one corresponds to the size of the contiguous sectors in the provided data. [Loc Ho] It sounds like the solution is to format the data (parameter values, src, dst) into a single buffer. This will require memory copy of the source and destination data as follow: 1. User space formating the user space buffer that includes: 1a) size parameter fields (this is required regardless) 2b) parameter data such as IV, adata, and etc (this is required regardless) 3c) copy the source data from application buffer into this single buffer (this is extra memcpy) 2. Kernel operation of the single user buffer (this is required regardless) 3. User space copy the destination data buffer into its appropriate memory pointer 3a) For hash, it is just the hash 3b) For crypto, it is the entire buffer (= to source length) 3c) For aead, it is the entire buffer + aead ICV As you can see, there is two extra memcpy's. There is noticeable performance on our SoC (AMCC 4xx) which memory copy is performed. It will be buried in noise compared to crypto processing time, but you still can try to optimize it. I was thinking. What do you think about passing multiple data pointer using writev (vector write)? And possible a similar idea for AIO. Yes, that's a good approach. Another one is to use a dedicated syscall. It is up to you as developer decide, since all of them have advantages. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1 v4] Add CryptoAPI User Interface Support
Hi. On Mon, Aug 04, 2008 at 10:43:13AM -0700, Loc Ho ([EMAIL PROTECTED]) wrote: We wasn't plan on protecting this. It is the caller responsibity to call in the proper order. If the caller want to change the key, it musts do so before issue run or after all run operations completed. Actually, I think it is better if we drop setkey. If the caller want to operate on a different key, create another transform. Are you concern that the underlying driver might have problem handling key change? Fair enough. If caller is not responsible to protect against simultaneous runs it is not a task for kernel to help him. But problem still exists, since there can be a leak in cryptodev_ioctl(CIOCGSESSION): if two or more threads simultaneously entered cryptdev_user_create_session() and each one allocated own session, only the last one will be assigned to the file-private_data and will be eventually freed, others will leak. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1 v4] Add CryptoAPI User Interface Support
Hi. On Wed, Jul 30, 2008 at 04:53:01PM -0700, Loc Ho ([EMAIL PROTECTED]) wrote: This is Linux CryptoAPI user space interface support patch v4. This version includes: * Add async support using Linux AIO. * Interafce to choose Copy to/from user and pinning pages based on size of data. * Asynchronous operations are issued through aio write function call. And result data is read back in the same context using the retry aio operations. * The crypto operation structure is passed via the data parameter of the write function. Internally, it will perform a copy from user call just as the synchronous I/O control version. What does prevent from simultaneous command execution? Or setkey() vs. run() race? Do you rely on BKL which 'guards' ioctl execution? How is -write() protected from ioctl()? Please also remove forward declarations and rearrange code if needed. You can also drop looong lines from comments. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regarding integration with cryptodev
Hi. On Fri, Jul 11, 2008 at 09:00:12AM +0530, Manish RATHI ([EMAIL PROTECTED]) wrote: Is there any effort done to make linux crypto api to work with cryptodev? No. How can I use kernel space linux crypto with user space libraries? You can not. Does Linux crypto framework provide async APIs like OCF? What are the pros and cons of Linux crypto in comparison to OCF? Cryptoapi does much more than OCF has. Has anybody tested openswan KLIPS patch with 2.6.24 kernel so that ipsec use OCF? Unlikely... There is a work to implement userspace interface for cryptoapi, which will use all its features, be asynchronous (there is a patch to implement aio_read/aio_write with crypto file descriptors), although work is in early development, it promises interesting capabilities. Check work from Shasi Pulijala [EMAIL PROTECTED] and Loc Ho [EMAIL PROTECTED] -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1 v2] crypto: Add CryptoAPI User Interface Support
Hi. On Thu, Jun 12, 2008 at 01:54:00PM -0700, Loc Ho ([EMAIL PROTECTED]) wrote: This is CryptoAPI user space interface support patch v2. This version includes: 1. a file descriptor per tfm 2. Direct I/O for user data buffer (with option for single mapping) 3. algorithm properties is set via I/O control call 4. Per buffer operation is operated via I/O control call Number of overall moments: * your mailer heavily mangled patch, please fix it or attach patch next time * be ready to defend, why /dev/crypto should be char device and not misc, also why it is not a syscall :) * overall approach of multiple copy_from_user is noticebly slower than single copy_from_user of the whole structure and they parse it according to internal fields * the same applies to multiple allocations: try to avoid it as much as possible and allocate whole structure in one go -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: remove duplicate include of linux/interrupt.h
Hi. On Thu, Jun 19, 2008 at 02:43:08PM +0200, Andre Haupt ([EMAIL PROTECTED]) wrote: Signed-off-by: Andre Haupt [EMAIL PROTECTED] Thank you for the patch, but similar one from Huang Weiyi is in Herbert Xu cryptodev-2.6 tree an will be pushed upstream in upcoming merge window. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG in scatterlist.h when loading tcrypt
Hi. On Mon, Jun 02, 2008 at 11:08:59AM +0200, Eric Sesterhenn ([EMAIL PROTECTED]) wrote: i guess this shouldnt happen, got this when loading the tcrypt module [ 60.113277] testing ecb(seed) decryption across pages (chunking) [ 60.113309] [ 60.113311] testing cts(cbc(aes)) encryption [ 60.120984] test 1 (128 bit key): [ 60.121153] [ cut here ] [ 60.121312] kernel BUG at include/linux/scatterlist.h:65! Fix will be in Linus' tree soon, Herbert just pushed it upstream. http://git.kernel.org/?p=linux/kernel/git/herbert/crypto-2.6.git;a=commit;h=c4913c7b71abc79b008a3c118628cfb59bdb0efc -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] talitos: Freescale integrated security engine (SEC) driver
On Mon, Jun 02, 2008 at 11:50:21AM -0500, Kim Phillips ([EMAIL PROTECTED]) wrote: But can it be changed? You write to it without lock, but read under the one (different for each channel though), so it attracted attention. can you point where in the code your concern is? talitos_submit() writes to hdr (initially I think?) without locks. It is read in flush_channel() under tail lock, but then it is dropped, so I rised a question, if it can be modified during that time, since if it can, status value, calculated from it, can be different and thus error check result can be false. Or this is not an issue? -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] talitos: Freescale integrated security engine (SEC) driver
Hi.
On Thu, May 29, 2008 at 02:12:50PM -0500, Kim Phillips ([EMAIL PROTECTED])
wrote:
+static irqreturn_t talitos_interrupt(int irq, void *data)
+{
+ struct device *dev = data;
+ struct talitos_private *priv = dev_get_drvdata(dev);
+
+ priv-status = in_be32(priv-reg + TALITOS_ISR);
+ priv-status_lo = in_be32(priv-reg + TALITOS_ISR_LO);
+
+ if (unlikely(priv-status ~TALITOS_ISR_CHDONE)) {
+ talitos_error((unsigned long)data);
+ /* ack */
+ out_be32(priv-reg + TALITOS_ICR, priv-status);
+ out_be32(priv-reg + TALITOS_ICR_LO, priv-status_lo);
+ }
+ else
+ {
+ /* ack */
+ out_be32(priv-reg + TALITOS_ICR, priv-status);
+ out_be32(priv-reg + TALITOS_ICR_LO, priv-status_lo);
+
+ if (likely(priv-status TALITOS_ISR_CHDONE))
+ tasklet_schedule(priv-done_task);
+ }
+
+ return (priv-status || priv-status_lo) ? IRQ_HANDLED : IRQ_NONE;
+}
Don't you want to protect against simultaneous access to register space
from different CPUs? Or it is single processor board only?
--
Evgeniy Polyakov
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] talitos: Freescale integrated security engine (SEC) driver
On Fri, May 30, 2008 at 02:41:17PM -0500, Scott Wood ([EMAIL PROTECTED]) wrote: Don't you want to protect against simultaneous access to register space from different CPUs? Or it is single processor board only? Doesn't linux mask the IRQ line for the interrupt currently being serviced, and on all processors? Yes. Could there be interference from non-interrupt driver code on another cpu (or interrupted code), though? Yes, that register space can be assigned from non-interrupt path on different cpu. I saw spin_lock_irqsave() is used in some other places, but not in interrupt handler itself. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Userspace API proposal was: Re: [PATCH 1/1] RFC: Add CryptoAPI User Space Interface Support
On Wed, May 14, 2008 at 01:57:30PM +0200, Sebastian Siewior ([EMAIL PROTECTED]) wrote: Great. Here a few ideas for a new interface: - /dev/crypto: - open file, creates a new ctx which may be one of crypto/hash/... - set type via ioctl / netlink - set key / other attributes via ioctl - put a block for encryption via write() - wait until it is done. poll() could be used to determine this state - read the result via read(). - -final() (hash) could be executed on read() Above but without special device, but syscall instead, which will have all needed parameters like mode string, key, iv and sizes. - cryptofs attempt (somehow inspired by spufs): - 1 syscall to create a special crypto device (that is aes(cbc), hmac(sha1) or what ever the crypto api offers). - returns a handle and creates a unique folder in cryptfs - the folder is RW to the owner - and contains properties of the algorithm. So we write in the file keysize to specify the size of the key and write to the file key to set the key. This properties are based on the class of the algorithm (should be almost equal I guess). - Every crypto request will be created once a file in the request folder is created. Request is fed with data via the write(). - I'm not sure how we signalize that a request is done. Maybe another file pops up and we can track this via inotify. So I put this two for discussion :) I came up with those two a while ago but never wrote code because I had no use case. Well, it might be time to start :) I'm not sure virtual filesystem is needed though, but as well can be a good idea. At least not ioctl hell with /dev/crypto -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Userspace API proposal was: Re: [PATCH 1/1] RFC: Add CryptoAPI User Space Interface Support
Hi. On Wed, May 14, 2008 at 08:40:43AM -0700, Loc Ho ([EMAIL PROTECTED]) wrote: Option #2: 1. Use syscall with algorithm name and its associated parameters 2. Operation such as encrypt, decrypt, hash, and etc are operated via another two system call - crypto_read and crypto_write 3. For this option, how should one handle asynchronous operation??? No need to read/write syscall, initialization one returns a file descriptor, which can be read/written via usual read/write calls. It is also pollable. Option #3: 1. Use syscall to create a special crypto device folder per an algorithm 2. A handle is returned and a crypto filesystem entry is create for that handle 3. Crypto parameter can be set based on read/write on that folder 4. Crypto operation will be based on file created under that folder. It will inherit all crypto attributes of that folder. 5. And etc... (See previous email) 6. This approach is overkill and totally unnecessary. Does this email summary all suggested solution? If not please add to this list. Which one will like be accepted into the crypto tree??? Essentially it is the same as second one, but with aditional eye-candies like simplified key management (some file in some dir written and key is being changed). Belive me, there will be always people, who do not like your interface, whatever one you will create, with second or third approach such number will be smaller, so just create what you like and prove your point is strong. New interfaces is such a tasty ground for empty talks :) -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[HIFN 1/1] Remove duplicated include
Removed duplicated include file linux/interrupt.h. Signed-off-by: Huang Weiyi [EMAIL PROTECTED] Signed-off-by: Evgeniy Polyakov [EMAIL PROTECTED] --- a/drivers/crypto/hifn_795x.c2008-05-08 22:03:12.0 +0800 +++ b/drivers/crypto/hifn_795x.c2008-05-08 22:03:56.0 +0800 @@ -29,7 +29,6 @@ #include linux/dma-mapping.h #include linux/scatterlist.h #include linux/highmem.h -#include linux/interrupt.h #include linux/crypto.h #include linux/hw_random.h #include linux/ktime.h -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [HIFN 05/n]: Fix data alignment checks
On Wed, May 07, 2008 at 02:26:30PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: I'm not entirely sure about the alignmask change at the end of this patch, is an alignmask of 1 correct if no source buffer alignment is required, but the destination buffer should be (doesn't have to be though) 4 byte aligned? commit f76618d53e82c8905214e889a3f79f1816c680fb Author: Patrick McHardy [EMAIL PROTECTED] Date: Wed May 7 12:44:15 2008 +0200 [HIFN]: Fix data alignment checks The check for misalignment of the scatterlist data has two bugs: - the source buffer doesn't need to be aligned at all - the destination buffer and its size needs to be aligned to a multiple of 4, not to the crypto alg blocksize If memory serves me right, both src and dst addresses have to be 4 bytes aligned. Protocol alignment is not needed. If it is not the issue, then I have no objections. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [HIFN 04/n]: Handle ablkcipher_walk errors
On Wed, May 07, 2008 at 02:20:34PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: commit 7e24c849df30a5d2b0bb89165b933ea2faa747bd Author: Patrick McHardy [EMAIL PROTECTED] Date: Wed May 7 12:43:20 2008 +0200 [HIFN]: Handle ablkcipher_walk errors ablkcipher_walk may return a negative error value, handle this properly instead of treating it as a huge number of scatter-gather elements. Signed-off-by: Patrick McHardy [EMAIL PROTECTED] Ooops, ACK :) -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [HIFN 09/n]: Fix max queue length value
On Wed, May 07, 2008 at 02:38:31PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: commit 21b18f9b22c7aa0a458c3f93fc1771a5eb5e70c8 Author: Patrick McHardy [EMAIL PROTECTED] Date: Wed May 7 13:00:10 2008 +0200 [HIFN]: Fix max queue length value All but the last element of the command and result descriptor rings can be used for crypto requests, fix HIFN_QUEUE_LENGTH. Signed-off-by: Patrick McHardy [EMAIL PROTECTED] Yaah, that's right, we can also wrap it into (), although current usage is ok. Thanks Patric. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [HIFN 06/n]: Properly handle requests for less than the full scatterlist
On Wed, May 07, 2008 at 02:30:28PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: [HIFN]: Properly handle requests for less than the full scatterlist The scatterlist may contain more data than the crypto request, causing an underflow of the remaining byte count while walking the list. Use the minimum of the scatterlist element size and the remaining byte count specified in the crypto request to avoid this. Signed-off-by: Patrick McHardy [EMAIL PROTECTED] That one is the nastiest! Thanks a lot Patrick for fixing that. Ack of course. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [HIFN 05/n]: Fix data alignment checks
On Wed, May 07, 2008 at 02:45:15PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: Of the data buffers only the destination buffer needs to be aligned, see the Source Pointer description in 2.2 Source Descriptors in the HIFN documentation. Ok, thanks for the reference. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [HIFN 10/n]: Move command descriptor setup to seperate function
On Wed, May 07, 2008 at 02:50:29PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: commit 163c74fc4f18d086bbb7f114d55bc7dac939d6b7 Author: Patrick McHardy [EMAIL PROTECTED] Date: Wed May 7 13:11:59 2008 +0200 [HIFN]: Move command descriptor setup to seperate function Move command descriptor setup to seperate function as preparation for the following DMA setup fixes. Note 1: also fix a harmless typo while moving it: sa_idx is initialized to dma-resi instead of dma-cmdi. Yep, resource and command indexes should be always equal. Note 2: errors from command descriptor setup are not propagated back, anymore, they can't be handled anyway and all conditions leading to errors should be checked earlier. Signed-off-by: Patrick McHardy [EMAIL PROTECTED] I have no objections, thanks Patrick. Ack. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [HIFN 05/n]: Fix data alignment checks
On Wed, May 07, 2008 at 03:05:48PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: As a side-note: its also not just an overseight in the documentation, not aligning the destination buffer to a multiple of 4 results in the HW freezing, the source buffer alignment changes work fine though. Probably I just mirrored dst to src alignment and thus forced mask to be at least 3. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [HIFN 01/n]: Endianess fixes
On Wed, May 07, 2008 at 03:01:55PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: We can also be safe without volatile annotations. Yes, will also be taken care of by a later patch :) I just receive them in a strange order and do not know when others will arrive :) -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [HIFN 01/n]: Endianess fixes
On Wed, May 07, 2008 at 03:46:12PM +0200, Patrick McHardy ([EMAIL PROTECTED]) wrote: Appologies for imperfect ordering :) The patchset grew while fixing issues as I ran into them and attempts to reorder caused too many conflicts. I believe it is more to mail system, which delivers 5 before 1 and so on, not how patches are organized in the patchset :) The removal of volatile doesn't belong in this patch though. Ok, no problem. -- Evgeniy Polyakov -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HIFN+IPsec crashes in current -git
,
kunmap_atomic(daddr, KM_SOFTIRQ0);
} else {
- nbytes -= src-length;
+ nbytes -= nb;
idx++;
}
@@ -1564,7 +1566,7 @@ static int hifn_setup_session(struct ablkcipher_request
*req)
struct hifn_device *dev = ctx-dev;
struct page *spage, *dpage;
unsigned long soff, doff, flags;
- unsigned int nbytes = req-nbytes, idx = 0, len;
+ unsigned int nbytes = req-nbytes, idx = 0, len, nb;
int err = -EINVAL, sg_num;
struct scatterlist *src, *dst, *t;
unsigned blocksize =
@@ -1581,6 +1583,8 @@ static int hifn_setup_session(struct ablkcipher_request
*req)
src = req-src[idx];
dst = req-dst[idx];
+ nb = min(src-length, nbytes);
+
if (src-length (blocksize - 1) ||
src-offset (alignmask - 1) ||
dst-length (blocksize - 1) ||
@@ -1588,7 +1592,7 @@ static int hifn_setup_session(struct ablkcipher_request
*req)
ctx-walk.flags |= ASYNC_FLAGS_MISALIGNED;
}
- nbytes -= src-length;
+ nbytes -= nb;
idx++;
}
@@ -1602,6 +1606,10 @@ static int hifn_setup_session(struct ablkcipher_request
*req)
idx = 0;
sg_num = ablkcipher_walk(req, ctx-walk);
+ if (sg_num 0) {
+ err = sg_num;
+ goto err_out_exit;
+ }
atomic_set(ctx-sg_num, sg_num);
@@ -1632,10 +1640,12 @@ static int hifn_setup_session(struct ablkcipher_request
*req)
len = dst-length;
}
+
+ nb = min(len, nbytes);
idx++;
- err = hifn_setup_dma(dev, spage, soff, dpage, doff, nbytes,
+ err = hifn_setup_dma(dev, spage, soff, dpage, doff, nb,
req, ctx);
if (err)
goto err_out;
@@ -1652,7 +1662,7 @@ err_out:
spin_unlock_irqrestore(dev-lock, flags);
err_out_exit:
if (err printk_ratelimit())
- dprintk(%s: iv: %p [%d], key: %p [%d], mode: %u, op: %u,
+ printk(%s: iv: %p [%d], key: %p [%d], mode: %u, op: %u,
type: %u, err: %d.\n,
dev-name, ctx-iv, ctx-ivsize,
ctx-key, ctx-keysize,
@@ -1786,7 +1796,7 @@ static void hifn_process_ready(struct ablkcipher_request
*req, int error)
BUG();
if (atomic_dec_and_test(ctx-sg_num)) {
- unsigned int nbytes = req-nbytes;
+ unsigned int nbytes = req-nbytes, nb;
int idx = 0, err;
struct scatterlist *dst, *t;
void *saddr;
@@ -1796,6 +1806,8 @@ static void hifn_process_ready(struct ablkcipher_request
*req, int error)
t = ctx-walk.cache[idx];
dst = req-dst[idx];
+ nb = min(nbytes, dst-length);
+
dprintk(\n%s: sg_page(t): %p, t-length: %u,
sg_page(dst): %p, dst-length: %u,
nbytes: %u.\n,
@@ -1803,7 +1815,7 @@ static void hifn_process_ready(struct ablkcipher_request
*req, int error)
sg_page(dst), dst-length, nbytes);
if (!t-length) {
- nbytes -= dst-length;
+ nbytes -= nb;
idx++;
continue;
}
@@ -1811,7 +1823,7 @@ static void hifn_process_ready(struct ablkcipher_request
*req, int error)
saddr = kmap_atomic(sg_page(t), KM_IRQ1);
err = ablkcipher_get(saddr, t-length,
t-offset,
- dst, nbytes, nbytes);
+ dst, nb, nbytes);
if (err 0) {
kunmap_atomic(saddr, KM_IRQ1);
break;
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HIFN+IPsec crashes in current -git
On Thu, Feb 21, 2008 at 03:20:45PM +0100, Patrick McHardy ([EMAIL PROTECTED])
wrote:
Almost I guess :) There are similar loops in hifn_setup_session().
Additionally we need to check that the return value of ablkcipher_walk()
is not a negative errno code.
Yep. Kind of this one:
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index dfbf24c..c88b4d9 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -1544,7 +1544,10 @@ static int ablkcipher_walk(struct ablkcipher_request
*req,
kunmap_atomic(daddr, KM_SOFTIRQ0);
} else {
- nbytes -= src-length;
+ if (src-length = nbytes)
+ nbytes = 0;
+ else
+ nbytes -= src-length;
idx++;
}
@@ -1588,7 +1591,10 @@ static int hifn_setup_session(struct ablkcipher_request
*req)
ctx-walk.flags |= ASYNC_FLAGS_MISALIGNED;
}
- nbytes -= src-length;
+ if (src-length nbytes)
+ nbytes = 0;
+ else
+ nbytes -= src-length;
idx++;
}
@@ -1602,6 +1608,10 @@ static int hifn_setup_session(struct ablkcipher_request
*req)
idx = 0;
sg_num = ablkcipher_walk(req, ctx-walk);
+ if (sg_num 0) {
+ err = sg_num;
+ goto err_out_exit;
+ }
atomic_set(ctx-sg_num, sg_num);
@@ -1640,7 +1650,10 @@ static int hifn_setup_session(struct ablkcipher_request
*req)
if (err)
goto err_out;
- nbytes -= len;
+ if (len nbytes)
+ nbytes = 0;
+ else
+ nbytes -= len;
}
dev-active = HIFN_DEFAULT_ACTIVE_NUM;
@@ -1803,7 +1816,10 @@ static void hifn_process_ready(struct ablkcipher_request
*req, int error)
sg_page(dst), dst-length, nbytes);
if (!t-length) {
- nbytes -= dst-length;
+ if (dst-length nbytes)
+ nbytes = 0;
+ else
+ nbytes -= dst-length;
idx++;
continue;
}
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HIFN+IPsec crashes in current -git
Hi Patrick.
On Wed, Feb 13, 2008 at 05:44:42PM +0300, Evgeniy Polyakov ([EMAIL PROTECTED])
wrote:
Any chance you can apply following patch and check output for correct
and broken cases (it will produce 2 or 3 debug strings for each crypto
operation)?
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index dfbf24c..b8b088d 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -1558,6 +1558,23 @@ err_out_unmap:
return err;
}
+static void hifn_dump_req(struct ablkcipher_request *req, const char *prefix)
+{
+ int nbytes = (signed)req-nbytes;
+ struct scatterlist *src, *dst;
+ int idx = 0;
+
+ printk(%s: nbytes: %u, , prefix, nbytes);
+ while (nbytes 0) {
+ src = req-src[idx];
+ dst = req-dst[idx];
+
+ printk(%u/%u , src-length, dst-length);
+ nbytes -= src-length;
Ouch, forgot idx++;
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: HIFN+IPsec crashes in current -git
Hi Patric.
On Wed, Feb 13, 2008 at 02:17:45PM +0100, Patrick McHardy ([EMAIL PROTECTED])
wrote:
I'm getting crashes when using HIFN and IPsec (ESP with
AES + MD5) in the current -git tree. I didn't capture the
Oops, but there seem to be a number of problems:
- hifn_setup_session walks over the scatterlist, subtracting
the scatterlist element size from nbytes until nbytes
reaches zero. In my case nbytes is 12 byte smaller than
the scatterlist, so nbytes underflows and it oopses when
walking over the of the scatterlist.
How is it possible? If I understood correctly ablkcipher_request-nbytes
has to have value equal to number of bytes placed into underlying
scatterlists, so if they do not match, hifn driver will not work at all.
I couldn't figure out where in the crypto code the
nbytes decrement by 12 bytes compared to the length
seen when setting up the crypto operation happens
or I might have tried to properly fix it myself.
I'll happily test patches in case someone more
familiar with the code does a proper fix.
Any chance you can apply following patch and check output for correct
and broken cases (it will produce 2 or 3 debug strings for each crypto
operation)?
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index dfbf24c..b8b088d 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -1558,6 +1558,23 @@ err_out_unmap:
return err;
}
+static void hifn_dump_req(struct ablkcipher_request *req, const char *prefix)
+{
+ int nbytes = (signed)req-nbytes;
+ struct scatterlist *src, *dst;
+ int idx = 0;
+
+ printk(%s: nbytes: %u, , prefix, nbytes);
+ while (nbytes 0) {
+ src = req-src[idx];
+ dst = req-dst[idx];
+
+ printk(%u/%u , src-length, dst-length);
+ nbytes -= src-length;
+ }
+ printk(\n);
+}
+
static int hifn_setup_session(struct ablkcipher_request *req)
{
struct hifn_context *ctx = crypto_tfm_ctx(req-base.tfm);
@@ -1572,6 +1589,8 @@ static int hifn_setup_session(struct ablkcipher_request
*req)
unsigned alignmask =
crypto_ablkcipher_alignmask(crypto_ablkcipher_reqtfm(req));
+ hifn_dump_req(req, __func__);
+
if (ctx-iv !ctx-ivsize ctx-mode != ACRYPTO_MODE_ECB)
goto err_out_exit;
@@ -2182,6 +2201,8 @@ static int hifn_process_queue(struct hifn_device *dev)
ctx = crypto_tfm_ctx(async_req-tfm);
req = container_of(async_req, struct ablkcipher_request, base);
+ hifn_dump_req(req, __func__);
+
err = hifn_handle_req(req);
if (err)
break;
@@ -2196,6 +2217,8 @@ static int hifn_setup_crypto(struct ablkcipher_request
*req, u8 op,
int err;
struct hifn_context *ctx = crypto_tfm_ctx(req-base.tfm);
struct hifn_device *dev = ctx-dev;
+
+ hifn_dump_req(req, __func__);
err = hifn_setup_crypto_req(req, op, type, mode);
if (err)
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [build bug] ./drivers/crypto/hifn_795x.c
Hi. On Sat, Jan 26, 2008 at 09:45:45AM +1100, Herbert Xu ([EMAIL PROTECTED]) wrote: On Fri, Jan 25, 2008 at 11:21:50PM +0100, Ingo Molnar wrote: randconfig testing found this (post-v2.6.24) build bug: drivers/built-in.o: In function `hifn_unregister_rng': hifn_795x.c:(.text+0x17bbd9): undefined reference to `hwrng_unregister' drivers/built-in.o: In function `hifn_probe': hifn_795x.c:(.text+0x17df70): undefined reference to `hwrng_register' config attached. Thanks for the report. This is casued by CONFIG_HW_RANDOM=m CONFIG_CRYPTO_DEV_HIFN_795X=y I'll fix it with the following patch. Thanks for fixing that up, if people who do not like select will complain, we can make hifn rng depend on hw_random. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/11] [CRYPTO] blkcipher: Merge ablkcipher and blkcipher into one option/module
On Thu, Nov 22, 2007 at 07:28:15PM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote: On Thu, Nov 22, 2007 at 02:18:07PM +0300, Evgeniy Polyakov wrote: On Thu, Nov 22, 2007 at 04:48:41PM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote: [CRYPTO] blkcipher: Merge ablkcipher and blkcipher into one option/module Now that we have the givcipher type, both blkcipher and ablkcipher algorithms will use it to create givcipher objects. As such it no longer makes sense to split the system between ablkcipher and blkcipher. In particular, both ablkcipher.c and blkcipher.c would need to use the givcipher type which has to reside in ablkcipher.c since it shares much code with it. This patch merges the two Kconfig options as well as the modules into one. This breaks cryptodev tree, since some devices other than cryptd selects CRYPTO_ABLKCIPHER. Are you sure? The bottom hunk of that patch fixes this, no? It looks I missed something :) -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [0/11] Add IV generators and givcrypt
On Thu, Nov 22, 2007 at 07:31:16PM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote: Idea and implementation look very good, I have couple of comments on patches and one generic comment here: you absolutely have to write at least bits of documentation for this new interfaces, how they behave and who and how should use it :) Sorry, when my employer starts paying me to do that or hires someone to do that for me is when better docs will exist :) Somehow you described that to others - just combine things together and put to Documentation/crypto and that will be enough. Until that happens why don't you chip in and contribute some documentation the user? It is only possible if I clearly understand the whole idea, but it is not that simple like it looks from author's point of view :) For example this patchset looks like possible first step in proper chaining mechanism for hardware devices, but if this will be impemented this way, then each hardware completion callback should be wrapped with proper geniv methods (like those which copy iv back to req-info). Is this right approach (for those users who care about correct returned IV), or will it just use software implementation only? -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [0/11] Add IV generators and givcrypt
On Thu, Nov 22, 2007 at 08:09:37PM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote: On Thu, Nov 22, 2007 at 02:57:07PM +0300, Evgeniy Polyakov wrote: Somehow you described that to others - just combine things together and put to Documentation/crypto and that will be enough. Patches are welcome :) I still do not understand thow whole concept. For example this patchset looks like possible first step in proper chaining mechanism for hardware devices, but if this will be impemented this way, then each hardware completion callback should be wrapped with proper geniv methods (like those which copy iv back to req-info). Is this right approach (for those users who care about correct returned IV), or will it just use software implementation only? I'm not sure I understand your question. First of all givcrypt is designed to work for hardware devices too. If they can generate their own IVs then they should directly hook up to the givcrypt method and use the givcipher type. But for example chainiv_givcrypt() will not return correct iv when called fro async device, since when givcrypt() returned operation is not yet completed. If not then they can use one of the precanned geniv wrappers and declare their preference in the in crypto_ablkcipher_alg-geniv. As to chaining, I presume you mean something like encryption followed by hashing? If so then this really doesn't have much to do with chaining at all. Yes, that what I meant. And also other possible crypto modes, which can require iv-based tweaks. I think we don't really need chaining in general because the hardware doesn't do arbitrary chaining. Instead what they do is specific chains that are useful for particular applications. Case in point would be encryption followed by hashing which is designed for IPsec. Therefore instead of having a general chaining abstraction I've chosen to do chaining support on a case-by-case basis. For instance, the above chaining is now supported by the new crypto_aead transform type. It just so happens that people are also designing algorithms to make crypto_aead useful for software as well which is a bonus :) This sheds some light on, thanks. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IV copy strategy
On Mon, Nov 19, 2007 at 07:56:55PM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote: I'm not sure what user will do, when it request chaining, but driver will set CRYPTO_ALG_CIPHER_NOCHAIN itself and return wrong/old in req-info? For IPsec it is not an issue though, but I can not say that for all. Drivers that set CRYPTO_ALG_CIPHER_NOCHAIN won't be returned if the user requests for a chaining cipher. This is the same as when you request for a cipher with no fallback that the system won't return one needing a fallback. If you're worried about users doing chaining we could even create a new frontend type for it. So the user would need to allocate an object of type crypto_chncipher and use that for operations that chain. Well, this looks like a good way forward. Not even getting into account, that it requires essentially zero efforts from driver writers :) -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IV copy strategy
On Sun, Nov 18, 2007 at 02:52:37PM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote: On Fri, Nov 16, 2007 at 02:11:10PM +0300, Evgeniy Polyakov wrote: That's a question - should it copy IV back or not? Currently it is not required by crypto users. OK I've changed my mind :) The reason is CTR, or rather the CTR as used by IPsec. CTR itself should be able to chain, in fact one of the things I'll do later is to move most of the current CTR code into a chainable CTR with just the algorithm parameter, i.e., ctr(aes) which would be what we currently call ctr(0,16,16). We can then put a non- chainable wrapper around that, perhaps ctr-ipsec(aes). In any case, it is clear that some algorithms simply won't be able to chain because the IV exposed to the outside is not the complete IV. So my plan is to add a new flag, CRYPTO_ALG_CIPHER_NOCHAIN that you would set on algorithms that cannot be chained. The semantics is that everything else remains the same except that on encrypt calls, the req-info after completion is undefined. Users requiring chaining would then do crypto_alloc_blkcipher(foo, 0, CRYPTO_ALG_CIPHER_NOCHAIN) Hmm, users who want chaining will set flag _NOCHAIN :) I would call it something more informative... For hardware offload devices such as yours where chaining does not come naturally you could then elect to not implement chaining and just set the flag. Please let me know if you see any problems with this approach. I'm not sure what user will do, when it request chaining, but driver will set CRYPTO_ALG_CIPHER_NOCHAIN itself and return wrong/old in req-info? For IPsec it is not an issue though, but I can not say that for all. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC HIFN 00/02]: RNG support
Hi Patrick. On Sat, Nov 17, 2007 at 08:30:09PM +0100, Patrick McHardy ([EMAIL PROTECTED]) wrote: These two patches add support for using the HIFN rng. Great! The first patch improves the PLL initialization a bit by making the reference clock configurable and its speed known to the driver, which is needed to calculate the amount of time to wait between two RNG reads. Since there is no way to find out the frequency reliably (especially for the external clock), it adds some sane looking defaults and a module parameter to override it. Suggestions how to improve this are welcome. I'm not sure it is possible to get it cleaner anyway. The second patch adds hw_random support. The ugly part is finding out when to allow reads from the RNG. It currently translates the public key engine clock cycles to CPU cycles based on a 4GHz CPU and uses get_cycles(). The problems with this are obvious, it only works on CPUs that actually have some kind of cycle counter, has problems with unsynchronized TSCs and the 4GHz assumption is not very nice either, but I was reluctant to use ktime for this since it seems rather expensive to call ktime_get once per 4 bytes of random. Suggestion for improvement of this are also welcome :) It will not work on arm, but I'm not sure this is relevant... Another option is to directly access xtime without all wrappers in the ktime_get(). Running rngtest on the random number generator indicates that it works properly, with an average failure ratio of about 1:1000 at ~2.5mbit. drivers/crypto/hifn_795x.c | 158 +++- 1 files changed, 156 insertions(+), 2 deletions(-) Patrick McHardy (2): [HIFN]: Improve PLL initialization [HIFN]: Add support for using the random number generator Ack both patches. Thanks a lot Patrick. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IV copy strategy
On Fri, Nov 16, 2007 at 07:25:30PM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote:
On Fri, Nov 16, 2007 at 02:11:10PM +0300, Evgeniy Polyakov wrote:
That's a question - should it copy IV back or not?
Currently it is not required by crypto users.
Well currently we have exactly one crypto user of ablkcipher
in the tree, and that's tcrypt :)
In acrypto days it was ipsec and dmcrypt too :)
I thought you converted dmcrypt to use ablkcipher already.
However, looking at the sync crypto users it would seem that
chaining while not popular is used by at least RXKAD. So I'd
like to preserve this functionality. Although in light of the
fact that on DMA devices touching the encrypted result to copy
the IV may be expensive, we could make it conditional on a flag
inside the request, i.e., something like
CRYPTO_TFM_REQ_COPY_IV
Well, it is doable to copy iv back after data has been processed just
before callback is invoked when this flag is set for tfm.
But the point is that this is something that has to be done by
the algorithm since only it knows in general what/where the IV
is. So if the algorithm doesn't do that then the user can't
easily work around this.
Actually on second thought why don't we change the interface
for ablkcipher so that we allow the IV to be returned by either
copying it to req-info or replacing the req-info pointer?
Better copy I think, since otherwise it has to allocate (in interrupt
context) and free iv for each packet. Even if it will be preallocated
during packet setup (in setiv() for example) it is unneded additional
overhead.
That way if the destination is linear and lowmem at the end we
can just return a pointer to it without touching the data at all.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] HIFN: make Kconfig option depend on PCI
On Mon, Nov 12, 2007 at 01:31:49PM +, Jan Glauber ([EMAIL PROTECTED]) wrote: The HIFN driver is currently selectable on s390 but wont compile. Since it looks like HIFN needs PCI make the Kconfig dependent on PCI, which is not available on s390. Ugh, correct of course. Thanks Jan. ACK. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [1/1] HIFN: schedule callback invocation to tasklet.
On Fri, Nov 09, 2007 at 07:12:14PM +0300, Evgeniy Polyakov ([EMAIL PROTECTED])
wrote:
This patch forces HIFN driver to invoke crypto request callbacks from
tasklet (softirq context) instead of hardirq context, since network
stack expects it to be called from bottom halves.
And first patch is obviously wrong, since it does not stop tasklet on
exit path, please apply given one.
Signed-off-by: Evgeniy Polyakov [EMAIL PROTECTED]
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index e152917..0a7c07b 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -26,6 +26,7 @@
#include linux/delay.h
#include linux/mm.h
#include linux/highmem.h
+#include linux/interrupt.h
#include linux/crypto.h
#include crypto/algapi.h
@@ -425,6 +426,8 @@ struct hifn_device
u8 snum;
+ struct tasklet_struct tasklet;
+
struct crypto_queue queue;
struct list_headalg_list;
};
@@ -1881,7 +1884,7 @@ static irqreturn_t hifn_interrupt(int irq, void *data)
hifn_write_1(dev, HIFN_1_DMA_IER, dev-dmareg);
}
- hifn_check_for_completion(dev, 0);
+ tasklet_schedule(dev-tasklet);
hifn_clear_rings(dev);
return IRQ_HANDLED;
@@ -2414,6 +2417,19 @@ err_out_exit:
return err;
}
+static void hifn_tasklet_callback(unsigned long data)
+{
+ struct hifn_device *dev = (struct hifn_device *)data;
+
+ /*
+* This is ok to call this without lock being held,
+* althogh it modifies some parameters used in parallel,
+* (like dev-success), but they are used in process
+* context or update is atomic (like setting dev-sa[i] to NULL).
+*/
+ hifn_check_for_completion(dev, 0);
+}
+
static int hifn_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
int err, i;
@@ -2495,6 +2511,8 @@ static int hifn_probe(struct pci_dev *pdev, const struct
pci_device_id *id)
pci_set_drvdata(pdev, dev);
+ tasklet_init(dev-tasklet, hifn_tasklet_callback, (unsigned long)dev);
+
crypto_init_queue(dev-queue, 1);
err = request_irq(dev-irq, hifn_interrupt, IRQF_SHARED, dev-name,
dev);
@@ -2530,6 +2548,7 @@ err_out_stop_device:
hifn_stop_device(dev);
err_out_free_irq:
free_irq(dev-irq, dev-name);
+ tasklet_kill(dev-tasklet);
err_out_free_desc:
pci_free_consistent(pdev, sizeof(struct hifn_dma),
dev-desc_virt, dev-desc_dma);
@@ -2569,6 +2588,7 @@ static void hifn_remove(struct pci_dev *pdev)
hifn_stop_device(dev);
free_irq(dev-irq, dev-name);
+ tasklet_kill(dev-tasklet);
hifn_flush(dev);
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Patch to support stream ciphers
Hi. On Fri, Oct 26, 2007 at 01:55:48AM +0800, Tan Swee Heng ([EMAIL PROTECTED]) wrote: Hi all, I am learning CryptoAPI by porting stream ciphers into it (particularly eSTREAM candidates at http://www.ecrypt.eu.org/stream/). As eSTREAM ciphers manipulates IV differently depending on algorithm design, it seems that standard block modes cannot be used: ECB has no IV; CBC and CTR don't let ciphers control how the IV is processed. So I derived stream.c from Herbert's cbc.c and Joy's ctr.c. It wraps around any cipher exporting cia_ivsize and cia_setiv_crypt() from struct cipher_alg. As a test case, I've used Daniel Bernstein's Salsa20 implementation at http://cr.yp.to/snuffle.html as the cipher. My patch against Herbert's cryptodev-2.6 git is attached. Some questions come to mind: 1. Is this the way to go for general stream ciphers? 2. Would a separate struct stream_alg be better than hijacking struct cipher_alg? 3. Currently cia_setiv_crypt() does setiv() and inplace encryption; is this a good design? 4. Should the blocksize be 1 for the stream template? It depends. I do not know any stream cipher, which operates on 1-byte quantity in real life, but stream means 1-byte blocksize is a correct assumption. Your approach is likely the less intrusive for existing block-based design, so I believe this is a way to go. Besides number of codying style issues (like comments, () placement, spaces and the like) it looks good. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [CRYPTO] hifn_795x: Update for new scatterlist API
Hi Herbert. On Fri, Oct 26, 2007 at 08:10:41PM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote: Hi Evgeniy: I've updated the HIFN driver to work with the new scatterlist API. Please have a look to see if I've done anything stupid. Using some script I think, since it also updated debug text :) That does not matter though, so this looks ok. Thanks Herbert. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [1/1 take 3] HIFN 795x driver.
On Thu, Oct 11, 2007 at 06:14:00PM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote:
On Wed, Oct 10, 2007 at 07:21:47PM +0400, Evgeniy Polyakov wrote:
It passed all tests for AES, DES and DES3_EDE except weak test for DES,
since hardware can not determine weak keys.
Patch applied. Thanks Evgeniy!
Thanks!
BTW, we should change it so that the DES algorithm's setkey
function uses the same weak-key test as used by the generic
DES code.
Attached patch fixes that with following tcrypt output:
[71705.695117] test 5 (64 bit key):
[71705.698462] setkey() failed flags=100100
[71705.702499] 015744256a5ed31d
[71705.705703] pass
Signed-off-by: Evgeniy Polyakov [EMAIL PROTECTED]
diff --git a/crypto/des.c b/crypto/des.c
index 1df3a71..37c0858 100644
--- a/crypto/des.c
+++ b/crypto/des.c
@@ -634,7 +634,7 @@ static const u32 S8[64] = {
* Choice 1 has operated on the key.
*
*/
-static unsigned long ekey(u32 *pe, const u8 *k)
+unsigned long des_ekey(u32 *pe, const u8 *k)
{
/* KR: long is at least 32 bits */
unsigned long a, b, c, d, w;
@@ -709,6 +709,7 @@ static unsigned long ekey(u32 *pe, const u8 *k)
/* Zero if weak key */
return w;
}
+EXPORT_SYMBOL_GPL(des_ekey);
/*
* Decryption key expansion
@@ -792,7 +793,7 @@ static int des_setkey(struct crypto_tfm *tfm, const u8 *key,
int ret;
/* Expand to tmp */
- ret = ekey(tmp, key);
+ ret = des_ekey(tmp, key);
if (unlikely(ret == 0) (*flags CRYPTO_TFM_REQ_WEAK_KEY)) {
*flags |= CRYPTO_TFM_RES_WEAK_KEY;
@@ -879,9 +880,9 @@ static int des3_ede_setkey(struct crypto_tfm *tfm, const u8
*key,
return -EINVAL;
}
- ekey(expkey, key); expkey += DES_EXPKEY_WORDS; key += DES_KEY_SIZE;
+ des_ekey(expkey, key); expkey += DES_EXPKEY_WORDS; key += DES_KEY_SIZE;
dkey(expkey, key); expkey += DES_EXPKEY_WORDS; key += DES_KEY_SIZE;
- ekey(expkey, key);
+ des_ekey(expkey, key);
return 0;
}
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index e3d01da..52b5bb4 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -90,9 +90,9 @@ config ZCRYPT_MONOLITHIC
config CRYPTO_DEV_HIFN_795X
tristate Driver HIFN 795x crypto accelerator chips
+ select DES
select CRYPTO_ALGAPI
select CRYPTO_ABLKCIPHER
- select CRYPTO_BLKCIPHER
help
This option allows you to have support for HIFN 795x crypto adapters.
diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index 6c446f3..e152917 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -1915,6 +1915,8 @@ static void hifn_flush(struct hifn_device *dev)
spin_unlock_irqrestore(dev-lock, flags);
}
+extern unsigned long des_ekey(u32 *pe, const u8 *k);
+
static int hifn_setkey(struct crypto_ablkcipher *cipher, const u8 *key,
unsigned int len)
{
@@ -1927,6 +1929,16 @@ static int hifn_setkey(struct crypto_ablkcipher *cipher,
const u8 *key,
return -1;
}
+ if (len == HIFN_DES_KEY_LENGTH) {
+ u32 tmp[32];
+ int ret = des_ekey(tmp, key);
+
+ if (unlikely(ret == 0) (tfm-crt_flags
CRYPTO_TFM_REQ_WEAK_KEY)) {
+ tfm-crt_flags |= CRYPTO_TFM_RES_WEAK_KEY;
+ return -EINVAL;
+ }
+ }
+
dev-flags = ~HIFN_FLAG_OLD_KEY;
memcpy(ctx-key, key, len);
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [1/1 take 3] HIFN 795x driver.
On Thu, Oct 11, 2007 at 06:52:02PM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote: On Thu, Oct 11, 2007 at 02:43:14PM +0400, Evgeniy Polyakov wrote: Attached patch fixes that with following tcrypt output: That was quick :) Unfortunately it doesn't apply because des.c has been renamed to des_generic.c in cryptodev-2.6. Could you please fix that up and also move the prototype into a header file, say include/crypto/des.h? Here is DES update. I will send HIFN river update in the next mail. Signed-off-by: Evgeniy Polyakov [EMAIL PROTECTED] diff --git a/crypto/des_generic.c b/crypto/des_generic.c index ec09e7b..355ecb7 100644 --- a/crypto/des_generic.c +++ b/crypto/des_generic.c @@ -20,13 +20,7 @@ #include linux/crypto.h #include linux/types.h -#define DES_KEY_SIZE 8 -#define DES_EXPKEY_WORDS 32 -#define DES_BLOCK_SIZE 8 - -#define DES3_EDE_KEY_SIZE (3 * DES_KEY_SIZE) -#define DES3_EDE_EXPKEY_WORDS (3 * DES_EXPKEY_WORDS) -#define DES3_EDE_BLOCK_SIZEDES_BLOCK_SIZE +#include crypto/des.h #define ROL(x, r) ((x) = rol32((x), (r))) #define ROR(x, r) ((x) = ror32((x), (r))) diff --git a/include/crypto/des.h b/include/crypto/des.h new file mode 100644 index 000..2971c63 --- /dev/null +++ b/include/crypto/des.h @@ -0,0 +1,19 @@ +/* + * DES Triple DES EDE Cipher Algorithms. + */ + +#ifndef __CRYPTO_DES_H +#define __CRYPTO_DES_H + +#define DES_KEY_SIZE 8 +#define DES_EXPKEY_WORDS 32 +#define DES_BLOCK_SIZE 8 + +#define DES3_EDE_KEY_SIZE (3 * DES_KEY_SIZE) +#define DES3_EDE_EXPKEY_WORDS (3 * DES_EXPKEY_WORDS) +#define DES3_EDE_BLOCK_SIZEDES_BLOCK_SIZE + + +extern unsigned long des_ekey(u32 *pe, const u8 *k); + +#endif /* __CRYPTO_DES_H */ -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [1/1 take 2] HIFN 795x driver.
On Thu, Oct 04, 2007 at 01:49:23PM +0800, Herbert Xu ([EMAIL PROTECTED]) wrote: What is may backlog option? I did not find it in cryptd.c, which I used for reference. I found a backlog variables in the driver, but without any signs for the outside world - queue is initialized and backlog is being set to that queue - since there is a dequeueing code, what is a purpose for backlog in that case? It's not directly in cryptd because it uses the helpers from algapi.c. In particular ablkcipher_enqueue_request has the necessary logic to handle this correctly. You could either use that helper yourself once your hardware queue fills up, or implement your own backlog logic. The key is each tfm object must be guaranteed to be able to queue at least one request. I use ablkcipher_enqueue_request() when hardware can not handle new request, so this should be ok. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [1/1 take 2] HIFN 795x driver.
On Tue, Oct 02, 2007 at 09:05:12PM +0200, Sebastian Siewior ([EMAIL PROTECTED]) wrote: DES currently does not pass tcrypt's multipage test, since HIFN adapter can not work with non-blocksize aligned chunks, although crypto alignmask is set to 3, tcrypt provides a block of size 2 in its multipage test, which fails. I recalled now that I forgot to point this out. You set the align mask to the block size of the algorithm but it is almost unused / useless. Let me explain: - it is ensured that the key you get (in the setkey function) is properly aligned. This doesn't matter in your case because you memcpy() the key away to your private struct. - your private struct is pointer aligned (if I remember correctly). You memcpy() your key away so it looks like you don't need the ctx aligned according to your mask (but it would be possible :)) - the proper alignment of src, dst and the IV that you get is _not_ ensured. This would be the case if you would develop a blkcipher and use blkcipher_walk_init(), blkcipher_walk_virt(), Now. If you pass your AES-tcrypt-test (where you need 16 bytes alignment) than this is pure luck. If you really need aligned src,dst or the IV than you have to do it by yourself :( It has to be multiple of blocksize. I.e. it is impossible to crypt one byte - hardware will stall, DES test provides two bytes as input - this will not work. If that is going to be handled in driver, then it will relocate. I'm not sure it is the right decision. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [1/1] HIFN 795x driver.
Hi.
On Mon, Oct 01, 2007 at 10:22:14PM +0200, Sebastian Siewior ([EMAIL PROTECTED])
wrote:
optimisations. It also refuses to register 'ecb(des)' with min and max
keylen set to the same number, so right now des and 3des are removed.
I don't know if I understood you correctly but keep this in mind:
min and max key size is only important for the output in /proc/crypto.
If you register an algorithm like AES which is specified for 128, 192
and 256 bit keys you have to provide all three sizes within one
algorithm. If you post some code that is not working I could take a
look.
After a quick look I can tell:
- CBC is not working because when you call hifn_setup_session() from
hifn_setup_crypto() you don't use the IV supplied by the crypto API
(tcrypt) but set the IV to NULL and its length to zero. You should use
something like req-base.data, 16 :)
Yep :)
iv sits in req-info, but its size can not be obtained via
crypto_ablkcipher_ivsize(crypto_ablkcipher_reqtfm(req)) for tcrypt at
least, so I added a check if req-info is not NULL and mode is not ecb,
in that case I set ivsize according to algorithm, but that can lead to
undetectible errors - if caller has a bug and iv is not set correctly,
so ivsize will be zero, but code will use some garbage as iv.
- The code looks like you are going to remove
hifn_encrypt_aes_ecb_{16,24,32} and set the appropriate
Yes, I've removed that.
ACRYPTO_TYPE_AES_??? depending on ctx-current_key_len. Good.
- You need a software queue in case your HW queue is full and you receive
a requests which you may not drop. Currently you don't consider
CRYPTO_TFM_REQ_MAY_BACKLOG (it is fine if you can process all requests
no mater what).
That is what I do not like, but will implement.
- You may want to call
crypto_ablkcipher_set_flags(cipher, CRYPTO_TFM_RES_BAD_KEY_LEN);
in hifn_setkey() if the key size is wrong (you may want to move the
check for 16/24/32 from hifn_setup_session() to hifn_setkey()).
Done.
Anyway, it looks fine from what I can say :)
Thanks for review, Sebastian, I will release new version soon with fixed
issues.
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Tcrypt output for HIFN 795x driver.
: 81003a8739c8, tfm: 81003a873f20, ctx: 81003a873f60, keylen: 32. [ 629.310424] hifn_setup_session: start [ 629.314131] cmd: i=6, u=0, k=6 [ 629.317231] src: i=6, u=6, k=0 [ 629.320330] dst: i=6, u=6, k=0 [ 629.323430] res: i=6, u=0, k=6 [ 629.326532] hifn0: iv: [0], key: 81003a873f60 [32], mode: 0, op: 0, type: 2. [ 629.335473] hifn0: 1 dmacsr: 889c, dmareg: 22322023, res: 0010 [7], i: 1.7.7.1, u: 7.7.7.7. [ 629.344573] hifn0: ring cleanup 1: i: 7.7.7.7, u: 1.7.7.1, k: 6.0.0.6. [ 629.351146] hifn0: ring cleanup 2: i: 7.7.7.7, u: 0.7.7.0, k: 7.0.0.7. [ 629.358529] 00112233445566778899aabbccddeeff [ 629.363762] pass [ 629.365742] [ 629.365743] testing ecb(aes) decryption across pages (chunking) [ 629.373421] [ 629.373422] testing cbc(aes) encryption [ 629.379011] hifn_cra_init: tfm: 81003a8739c8, dev: hifn0 [81003dd7c2c8]. [ 629.386560] test 1 (128 bit key): [ 629.390012] hifn_setkey: tfm: 81003a8739c8, ctx: 81003a873a08, dev: hifn0 [81003dd7c2c8], len: 16. [ 629.400170] hifn_setup_crypto: req: 81003a873f20, tfm: 81003a8739c8, ctx: 81003a873a08, keylen: 16. [ 629.410430] hifn_setup_session: start [ 629.414143] cmd: i=7, u=0, k=7 [ 629.417241] src: i=7, u=7, k=0 [ 629.420342] dst: i=7, u=7, k=0 [ 629.423442] res: i=7, u=0, k=7 [ 629.426543] hifn0: iv: [0], key: 81003a873a08 [16], mode: 1, op: 1, type: 0. [ 629.435484] hifn0: 1 dmacsr: 889c, dmareg: 22322023, res: 0010 [8], i: 1.8.8.1, u: 8.8.8.8. [ 629.444585] hifn0: ring cleanup 1: i: 8.8.8.8, u: 1.8.8.1, k: 7.0.0.7. [ 629.451156] hifn0: ring cleanup 2: i: 8.8.8.8, u: 0.8.8.0, k: 8.0.0.8. [ 629.458574] 3b629d77f45eff9817c5849f9a0aba71 [ 629.463817] fail [ 629.465795] test 2 (128 bit key): [ 629.469246] hifn_setkey: tfm: 81003a8739c8, ctx: 81003a873a08, dev: hifn0 [81003dd7c2c8], len: 16. [ 629.479405] hifn_setup_crypto: req: 81003a873f20, tfm: 81003a8739c8, ctx: 81003a873a08, keylen: 16. [ 629.489649] hifn_setup_session: start [ 629.493359] cmd: i=8, u=0, k=8 [ 629.496458] src: i=8, u=8, k=0 [ 629.499558] dst: i=8, u=8, k=0 [ 629.502661] res: i=8, u=0, k=8 [ 629.505760] hifn0: iv: [0], key: 81003a873a08 [16], mode: 1, op: 1, type: 0. [ 629.514704] hifn0: 1 dmacsr: 889c, dmareg: 22322023, res: 0010 [9], i: 1.9.9.1, u: 9.9.9.9. [ 629.523810] hifn0: ring cleanup 1: i: 9.9.9.9, u: 1.9.9.1, k: 8.0.0.8. [ 629.530383] hifn0: ring cleanup 2: i: 9.9.9.9, u: 0.9.9.0, k: 9.0.0.9. [ 629.537784] bd0cb8b2220fab0cf10079d1b48ffde82b8bae025030fb5245010d5b7f1fc8c4 [ 629.546619] fail [ 629.548600] [ 629.548601] testing cbc(aes) encryption across pages (chunking) [ 629.556264] [ 629.556265] testing cbc(aes) decryption [ 629.561849] hifn_cra_init: tfm: 81003a873f20, dev: hifn0 [81003dd7c2c8]. [ 629.569412] test 1 (128 bit key): [ 629.572868] hifn_setkey: tfm: 81003a873f20, ctx: 81003a873f60, dev: hifn0 [81003dd7c2c8], len: 16. [ 629.583026] hifn_setup_crypto: req: 81003a8739c8, tfm: 81003a873f20, ctx: 81003a873f60, keylen: 16. [ 629.593270] hifn_setup_session: start [ 629.596981] cmd: i=9, u=0, k=9 [ 629.600081] src: i=9, u=9, k=0 [ 629.603181] dst: i=9, u=9, k=0 [ 629.606283] res: i=9, u=0, k=9 [ 629.609384] hifn0: iv: [0], key: 81003a873f60 [16], mode: 1, op: 0, type: 0. [ 629.618327] hifn0: 1 dmacsr: 889c, dmareg: 22322023, res: 0010 [10], i: 1.10.10.1, u: 10.10.10.10. [ 629.628039] hifn0: ring cleanup 1: i: 10.10.10.10, u: 1.10.10.1, k: 9.0.0.9. [ 629.635131] hifn0: ring cleanup 2: i: 10.10.10.10, u: 0.10.10.0, k: 10.0.0.10. [ 629.643103] 8d95a3b9e1823aeaff452dc6b285c73c [ 629.648346] fail [ 629.650323] test 2 (128 bit key): [ 629.653776] hifn_setkey: tfm: 81003a873f20, ctx: 81003a873f60, dev: hifn0 [81003dd7c2c8], len: 16. [ 629.663960] hifn_setup_crypto: req: 81003a8739c8, tfm: 81003a873f20, ctx: 81003a873f60, keylen: 16. [ 629.674202] hifn_setup_session: start [ 629.677914] cmd: i=10, u=0, k=10 [ 629.681186] src: i=10, u=10, k=0 [ 629.684460] dst: i=10, u=10, k=0 [ 629.687734] res: i=10, u=0, k=10 [ 629.691008] hifn0: iv: [0], key: 81003a873f60 [16], mode: 1, op: 0, type: 0. [ 629.699951] hifn0: 1 dmacsr: 889c, dmareg: 22322023, res: 0010 [11], i: 1.11.11.1, u: 11.11.11.11. [ 629.709663] hifn0: ring cleanup 1: i: 11.11.11.11, u: 1.11.11.1, k: 10.0.0.10. [ 629.716946] hifn0: ring cleanup 2: i: 11.11.11.11, u: 0.11.11.0, k: 11.0.0.11. [ 629.725063] 23a975b74c30c4d6ce38d6dcf0f57be6101112131415161718191a1b1c1d1e1f [ 629.733895] fail -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PATCH: SEED cipher support
Hi. On Mon, Aug 13, 2007 at 12:11:30AM +0900, Hye-Shik Chang ([EMAIL PROTECTED]) wrote: Hello, Here I propose a patch for linux crypto API to add a support SEED (RFC4269). This patch was generated against git-HEAD as of today. This patch have been used in few VPN appliance vendors in Korea for several years. And it was verified by KISA, who developed the algorithm itself. As its importance in Korean banking industry, it would be great if linux incorporates the support. Besides the only c++ comment in the tcrypt.c this patch looks really good. This is a good submission. As a side note: please do not send gzipped patches, since it is quite hard to show where the problem in the patch is if any when replying. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: status of async crypto
On Mon, Aug 06, 2007 at 12:00:13PM +0300, Ronen Shitrit ([EMAIL PROTECTED]) wrote: Wow, I thought that there was more progress ... BTW: I know that the OCF support the OpenSWAN, does it also support the KLIPS by now? I also noticed that the Acrypto have a patch to support KLIPS, does it also support the OpenSWAN? No and no :) Regards Ronen Shitrit -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: status of async crypto
On Sun, Aug 05, 2007 at 01:50:48PM +0300, Ronen Shitrit ([EMAIL PROTECTED]) wrote: I saw that the current code support async crypto for the dm-crypt, does it also support async crypto for the klips? Is there any support for using the async crypto from the OpenSSL engine library? I saw that the async crypto support block cipher, does it also support digest operations? Can it support encryption + authentication (lets say AES-SHA1) as one operation? Is there any Documentation available? (I guess not) No for all above. What is the todo list for further development of the async crypto support? New drivers and extending functionality if required. Some work is being done in this area, although not that fast. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Looking for comments on a Linux driver for HW accelerated Kasumi+F8+F9 algorithms.
Hi. On Wed, Jul 18, 2007 at 04:33:14PM -0700, tirumalareddy marri ([EMAIL PROTECTED]) wrote: Hi , I have created Linux driver for HW accelerated kasumi, F8 and F9 algorithms. Could you please look at the driver and provide comments about the usage. This driver is acts as a character driver. Encryption API's can be accessed from kernel and user space. My concern is accessing the driver from user space is secure or not. If possible can some one please send me rules to follow when we write a Linux security driver. --- patch for the driver --- Sorry, but your patch looks horrible - your mailer broke alignment, you do not follow Linux kernel coding standards. Please fix that first. According to patch itself, do all 440epx have that engine, since you access generic registers? Patch has too many debug prints in the hottest path, either remove them or wrap into helper which allows to turn them off - no one wants to read a message each time data is ready. Encryption is not protected against simultaneous access - there will be either bug or data corruption if several users simultaneously try to perfrom a processing. There is number of missed checks of the returned values of the functions which can fail. There is number of exported symbols without appropriate in-kernel users - it is not allowed. And userspace interface - I will not argue if it is good or bad, but be ready to listen quite a few phrases about your taste. I would suggest to create a generic userspace interface to deliver crypto events to/frm userspace and bind it to cryptoapi. You also likely want to create a cryptoapi driver to allow in-kernel subsystem to use kasumi at least. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RSA support into kernel?
On Thu, Jul 05, 2007 at 03:48:51PM -0700, Gautam Singaraju ([EMAIL PROTECTED]) wrote: Is there any attempts being made to provide software based RSA cryptographic support in kernel level? I see that 2.6.21 supports Hardware devices such as VIA Padlock ACE. Has anybody had a change to use such a system? VIA padlock engine or RSA? The former is heavily used in the wild, but why would anyone want to use RSA in the kernel? -GS -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RSA support into kernel?
On Fri, Jul 06, 2007 at 04:05:33AM -0700, David Miller ([EMAIL PROTECTED]) wrote: From: Evgeniy Polyakov [EMAIL PROTECTED] Date: Fri, 6 Jul 2007 14:37:31 +0400 On Thu, Jul 05, 2007 at 03:48:51PM -0700, Gautam Singaraju ([EMAIL PROTECTED]) wrote: Is there any attempts being made to provide software based RSA cryptographic support in kernel level? I see that 2.6.21 supports Hardware devices such as VIA Padlock ACE. Has anybody had a change to use such a system? VIA padlock engine or RSA? The former is heavily used in the wild, but why would anyone want to use RSA in the kernel? Automatic SSL done in-kernel on user data for socket I/O, with hardware offload from the crypto layer when available. Solaris has done this for quite some time and it helps a lot for things like the VIA and Niagara. I.e. for userspace stuff? That is obviously the right usage, but Linux cryptoapi does not have userspace interface, so was my question. Actually I was several times already asked after acrypto was closed, how userspace can use new hardware drivers, and frankly I do not know what the best userspace API would look like (in one of the projects I already used all three methods one-by-one and failed to determine the best). Simple char device read/write or ioctl, or blocking/nonblocking syscall over file descriptor, or anything else? -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 12/12] drivers: PMC MSP71xx security engine driver
Hi Marc.
On Thu, Jun 28, 2007 at 01:49:13PM -0600, Marc St-Jean ([EMAIL PROTECTED])
wrote:
+static int
+sec_init_queues(void)
+{
+ int i;
+ struct workq *wq;
+ struct compq *cq;
+
+ /*
+ * Allocate uncached space for hw_ptr values.
+ * NOTE: status ptr value is not currently used.
+ */
+ status_ptr = dma_alloc_coherent(NULL, sizeof(int), status_dma_addr,
+ GFP_KERNEL);
+ DBG_SEC(Allocated status ptr memory at 0x%p (0x%08x)\n,
+ status_ptr, status_dma_addr);
+ if (!status_ptr)
+ return -ENOMEM;
+
+ for (i = 0; i HW_NR_COMP_QUEUES; i++) {
+ void *base; /* slowpath virtual address of base */
+ dma_addr_t base_dma_addr; /* DMA bus address of base */
+
+ base = dma_alloc_coherent(NULL, SEC_COMP_Q_SIZE,
+ base_dma_addr, GFP_KERNEL);
+ DBG_SEC(Allocated CQ%d at 0x%p (0x%08x)\n,
+ i, base, base_dma_addr);
+ if (!base)
+ return -ENOMEM;
This leaks allocations.
+ cq = sec_comp_queues[i];
+
+ cq-compq_lock = SPIN_LOCK_UNLOCKED;
+ cq-cq_regs = sec2_regs-cq[i];
+ cq-base = base;
+ cq-base_dma_addr = base_dma_addr;
+ cq-out = 0;
+
+ cq-cq_regs-ofst_ptr = (unsigned int *)status_dma_addr;
+ cq-cq_regs-base = (unsigned char *)cq-base_dma_addr;
+ cq-cq_regs-size = SEC_COMP_Q_SIZE;
+ cq-cq_regs-in = 0;
+ cq-cq_regs-out = 0;
+ }
+
+ for (i = 0; i HW_NR_WORK_QUEUES; i++) {
+ void *base; /* slowpath virtual address of base */
+ dma_addr_t base_dma_addr; /* DMA bus address of base */
+
+ base = dma_alloc_coherent(NULL, SEC_WORK_Q_SIZE,
+ base_dma_addr, GFP_KERNEL);
+ DBG_SEC(Allocated WQ%d at 0x%p (0x%08x)\n,
+ i, base, base_dma_addr);
+ if (!base)
+ return -ENOMEM;
This too.
+ wq = sec_work_queues[i];
+
+ init_waitqueue_head(wq-space_wait);
+
+ wq-workq_lock = SPIN_LOCK_UNLOCKED;
+ wq-wq_regs = sec2_regs-wq[i];
+ wq-base = base;
+ wq-base_dma_addr = base_dma_addr;
+ wq-in = 0;
+ wq-low_water = SEC_WORK_Q_SIZE 1; /* wake when half full */
+
+ wq-wq_regs-ofst_ptr = (unsigned int *)status_dma_addr;
+ wq-wq_regs-base = (unsigned char *)wq-base_dma_addr;
+ wq-wq_regs-size = SEC_WORK_Q_SIZE;
+ wq-wq_regs-in = 0;
+ wq-wq_regs-out = 0;
+ }
+
+ debug_dump_sec_regs();
+
+ return 0;
+}
+
+static int __init
+msp_secv2_init(void)
Shouldn't this and other places be marked as __devinit?
...
+static irqreturn_t
+msp_secv2_interrupt(int irq, void *dev_id)
+{
+ /*
+ * TODO: This clears all interrupts, and assumes
+ * that the cause was a completion queue update.
+ */
+ unsigned int status;
+
+ status = sec2_regs-sis;
+ sec2_regs-sis = /* ~status */ 0;
+
+ DBG_SEC(interrupt irq %d status was %x\n, irq, status);
+
+ poll_completion();
+
+ return IRQ_HANDLED;
+}
Irqs can not be shared?
...
+static int
+poll_completion(void)
+{
+ struct compq *cq;
+ int flags;
+ int work_ct = 0;
+
+ /*
+ * Check IPSEC engine register to see if at least one
+ * completion element is in completion queue.
+ */
+ cq = sec_comp_queues;
+ spin_lock_irqsave(cq-compq_lock, flags);
This lock seems not to protect against desc_do_work() for example, but
there are register/mmio access under both - what is a locking rules
there?
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 0/2] AES ablkcipher driver for SPUs
On Wed, Jun 27, 2007 at 12:59:52AM +0200, Sebastian Siewior ([EMAIL PROTECTED]) wrote: This driver adds support for AES on SPU. Patch is for review only because some parts of the code are not upstream yet. Patch one contains the main driver (which uses ablkcipher_ctx_cast()), patch two is for clarity (parts of the missing API that is used). Currently only ECB block mode is supported. I plan support for CBC but the way the IV currently handled is unfavorable (later more). Interesting. Do you have any benchmark of the SPU handling AES crypto? -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: combined mode algorithms
On Mon, Jun 25, 2007 at 05:13:58PM -0500, Joy Latten ([EMAIL PROTECTED]) wrote: I have been reading IP Encapsulating Payload-(ESP) RFC4303 where use of combined mode algorithms are mentioned and accommodated for. In trying to determine how I should handle this, I examined the crypto code and could not readily recognize any combined mode algorithms. Are there any current plans to implement combined mode algorithms? I think it should be first supported by ipsec stack at least with state, where SA cold be configured, integrity check for the data/header is not a problem after that changes are stable. sha1/encryption is a poor man's combined algo after all with hash data being ICV :) -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [1/1 take 2] HIFN: preliminary HIFN 795x driver for new async cryptoapi.
Hi Sebastian.
On Tue, Jun 05, 2007 at 02:56:04PM +0200, Sebastian Siewior ([EMAIL PROTECTED])
wrote:
* Evgeniy Polyakov | 2007-06-04 17:42:48 [+0400]:
+struct hifn_device
+{
+struct pci_dev *pdev;
+spinlock_t lock;
+u8 current_key[HIFN_MAX_CRYPT_KEY_LENGTH];
+int current_key_len;
+struct list_headalg_list;
+};
...
This struct looks to me like one per real hardware. What are you doing
if you get two+ crypto users? Or is this struct one per crypto user?
This is what gets allocated by crypto api for every user.
This is tricky, hifn requires to have a key per crypto setup, so this
one must be in a crypto context, but context is not used right now, so
yes, it supports only one user. I will update it to use context, since
it is smaller that hifn_device.
+static void hifn_work(struct work_struct *);
+
+static int hifn_start_device(struct hifn_device *dev)
+{
...
+INIT_DELAYED_WORK(dev-work, hifn_work);
+schedule_delayed_work(dev-work, HZ);
+
+return 0;
+}
...
+static irqreturn_t hifn_interrupt(int irq, void *data)
...
This looks to me like you have to reset the hardware once in a while.
The worker func and the interrupt handler are the only two functions
(exept hifn_remove()) that know about your hardware at run time.
It is a watchdog worker, it checks if there were sessions setup, and if
they were not ready when watchdog fired, it resets hardware and
completes appropriate requests with error.
+static int hifn_setkey(struct crypto_ablkcipher *cipher, const u8 *key,
unsigned int len)
+{
+struct crypto_tfm *tfm = crypto_ablkcipher_tfm(cipher);
+struct hifn_device *dev = crypto_tfm_ctx(tfm);
+
+if (len HIFN_MAX_CRYPT_KEY_LENGTH)
+return -1;
+
+if (!memcmp(dev-current_key, key, len)) {
+dev-flags |= HIFN_FLAG_OLD_KEY;
+return 0;
+}
+
+dev-flags = ~HIFN_FLAG_OLD_KEY;
+
+memcpy(dev-current_key, key, len);
+dev-current_key_len = len;
+
+return 0;
+}
...
dev is allocated by the crypto API but isn't initialized right? Nothing
points to your real hw right?
I (in my case) assign directly key ctx to hw ctx in set_key. This is
crap because more than just one hw could exist. The API chooses the
algo with the highest prio so you can't use more than one device.
A load balancer could be the person in charge for assigning hw ctx to
crypto user ctx.
I should use crypto context here, it will hosts a pointer to hifn device
and key, so far key is stored in hifn_device, which is one per HIFN cpu.
Yes, I know, it is not the right way to do it.
Context should contain pointer to hardware structure and keys.
+static inline int hifn_encrypt_aes_ecb_16(struct ablkcipher_request *req)
+{
+return hifn_setup_crypto(req, ACRYPTO_OP_ENCRYPT, ACRYPTO_TYPE_AES_128,
ACRYPTO_MODE_ECB);
+}
...
This is what I had in mind, as I said look on my skeleton, than you will
see how you can distinguish which algo is requested. Since you have 12
algos I understand now what you meant with it doesn't and your cat :)
HIFN also supports hashes, compressions and multiplication operations...
This will look so horrible, that people will commit suicide, when are trying
to fix something in such driver.
Sebastian
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [1/1 take 2] HIFN: preliminary HIFN 795x driver for new async cryptoapi.
On Tue, Jun 05, 2007 at 05:07:31PM +0400, Evgeniy Polyakov ([EMAIL PROTECTED]) wrote: This is tricky, hifn requires to have a key per crypto setup, so this one must be in a crypto context, but context is not used right now, so yes, it supports only one user. I will update it to use context, since it is smaller that hifn_device. Hmm, I was wrong, it is not possible (from first view). Here is a sequence of events to setup a crypto session: 1. user calls setkey, which results in tfm. It is possible to get a context from tfm. Context can be initialized via cra_init() callback in registered algorithm. But there is no way to provide a private pointer into that callback. So, there is no way to provide a pointer to hifn_device and thus tfm context can not find a hardware device to setup this session. Herbert, could you please elaborate how to provide a private pointer into cra_init without searching via static lists of all devices? -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [1/1] HIFN: preliminary HIFN 795x driver for new async cryptoapi.
On Tue, May 22, 2007 at 05:19:19PM +0200, Sebastian Siewior ([EMAIL PROTECTED]) wrote: * Evgeniy Polyakov | 2007-05-22 16:58:29 [+0400]: Current driver only supports AES ECB encrypt/decrypt, since I do not know how to detect operation mode in runtime (a question). Take a look on my skeleton driver (posted just a few seconds ago). It should solve your problem here. It does not - your code only supposed to work with ecb, since it is what was requested during initialization time. This new scheme with templates helps alot for ciphers/crypto modes which do not support several templates, so I used old scheme with 'cipher' only template, not 'ecb(cipher)', 'cbc(cipher)' and so on. And, btw, do not use mutex in encryption path, it is not supposed to sleep in ipsec. HIFN supports at least 12 different ciphers/mode (3des, des and aes, each one with 4 modes), so it is not a good idea to put them all into separated structures, so I rised a question about it. Sebastian -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [1/1] HIFN: preliminary HIFN 795x driver for new async cryptoapi.
Hi, Sebastian. On Wed, May 23, 2007 at 12:02:44PM +0200, Sebastian Siewior ([EMAIL PROTECTED]) wrote: It is also possible that I interpreted Herbert's code the wrong way. Let me comment the obvious part of the skeleton code which I thing you overlooked (If you didn't than I didn't catch up with in the first place or missed the goal of the async API). Register 12 struct crypto_alg, each with unique functions for aes|3des|.. + ecb|cbc|.. + encrypto|decrypt (I agree with you, that you prefer 4 instead of 12 since most of the attributes are the same). Now, algo_decrypt_ecb_async() is doing just: return enqueue_request(req, algo_request_decrypt); That is what I want to avoid. consider algo_request_decrypt as request_aes_decrypt_ecb. This function (algo_request_decrypt) only calls blkcipher_crypt(, HW_DECRYPT_ECB) which calls the HW directly. You see that way what is requested (AES+ECB+ENCRYPT). Instead of calling a function pointer, you could shorten the code by enqueue_request(..., HW_DECRYPT_ECB) directly and call blkcipher_crypt() from process_requests_thread() with the correct operation type. However the encrypt/decrypt process happens in a seperate kthread. My point is not to introduce a lot of structures and functions, which are essentially (read: exactly) the same, but instead get crypto processing mode from the tfm/whatever structure. Your code only registers ecb, but to fully support crypto capabilities for given device the same structure (but with different functions and template strings) must be registered for every device for every cipher/digest and every mode and probably even for every key size, but I'm not sure about the latter though. That is what I want to avoid. I took a deeper look on your code and it seems to me, that you might still use the sync API. Your .cra_type is still crypto_blkcipher_type. Your code might actually be broken because you set up a struct ablkcipher_alg but the crypto might threat it as struct blkcipher_alg. Check /proc/crypto, your supported keysizes should be wrong. Thanks for pointing, that must be ablkcpiher_type, I've fixed that typo. And, btw, do not use mutex in encryption path, it is not supposed to sleep in ipsec. I am aware of that but again: I might be totally wrong. Herbert introduced a async API. My understanding was that he wants to queue encrypt+decrypt (not setkey) and process it later in a dedicated thread. On the other hand: what is async when still evrything happny sync. Ah, then your code only works with dedicated thread, which is not needed for true hardware, which works in interrupt mode, since register setup is quite fast compared to rescheduling to dedicated thread, so it is not needed, and instead registers setup is performed in -encrypt/-decrypt callbacks and completion function is called (with disabled interrupts) from interrupt handler. *I* have to sleep while handling a crypto request over to the hardware. No, you have not. Check acrypto sources and how it is implemented for example for hifn driver and ipsec stack. My understanding of Herbert's async crypto API was a blessing :) In case I'm about to disagree, last time we talked with Herbert about async cryptoapi design we failed to find a solution, suitable for both points of view. :-) of IPsec I am actually thinking how to fix this and not break anything (in terms of performance and hacky code). In case of async IPsec you might check acrypto sources, which have async ipsec support quite for a while, but it is hacky indeed - I needed to heavily change ipsec processing code both in input and output to make it possible to work without any sleeps and with 'interrupted' crypto processing. It works not slower than native code, although I only did esp4. Sebastian -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine
On Thu, May 03, 2007 at 08:19:52AM +0200, Martin Schiller ([EMAIL PROTECTED])
wrote:
Hi Evgeniy,
Hi Martin.
Sorry for my late answer, but I didn't get your message because I haven't
subscribed to the mailing-list.
I've found it yesterday on the mailing-list archive. So please, could you
reply directly to me and to the mailing-list on any further messages?
I did, but your mail server bounced message.
This message is also sent to you directly, hope it will pass through.
Btw, AMD list bounces too.
I've tested the patch now, but nothing changed. When doing any aes cipher
tests with the tcrypt test module, the machine freezes without any error.
Hmm, that means that main waiting loop of the driver is not an issue.
Since it is PCI, likely bus is locked by incorrect written value into
the space. Can you run with this patch (it will freeze too, but at
least with some useful info), since AMD keeps silence, we will go hard
way:
diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
index 6d3840e..6ddfab9 100644
--- a/drivers/crypto/geode-aes.c
+++ b/drivers/crypto/geode-aes.c
@@ -61,8 +61,10 @@ static inline void
_writefield(u32 offset, void *value)
{
int i;
+ printk(%s: start, offset: %x.\n, __func__, offset);
for(i = 0; i 4; i++)
iowrite32(((u32 *) value)[i], _iobase + offset + (i * 4));
+ printk(%s: finish, offset: %x.\n, __func__, offset);
}
/* Read a 128 bit field (either a writable key or IV) */
@@ -70,8 +72,10 @@ static inline void
_readfield(u32 offset, void *value)
{
int i;
+ printk(%s: start, offset: %x.\n, __func__, offset);
for(i = 0; i 4; i++)
((u32 *) value)[i] = ioread32(_iobase + offset + (i * 4));
+ printk(%s: finish, offset: %x.\n, __func__, offset);
}
static int
@@ -80,19 +84,24 @@ do_crypt(void *src, void *dst, int len, u32 flags)
u32 status;
u32 counter = AES_OP_TIMEOUT;
+ printk(%s: src: %p, dst: %p, len: %d, flags: %x\n, __func__, src,
dst, len, flags);
iowrite32(virt_to_phys(src), _iobase + AES_SOURCEA_REG);
iowrite32(virt_to_phys(dst), _iobase + AES_DSTA_REG);
iowrite32(len, _iobase + AES_LENA_REG);
+ printk(%s: start op, likely will freeze.\n, __func__);
/* Start the operation */
iowrite32(AES_CTRL_START | flags, _iobase + AES_CTRLA_REG);
+ printk(%s: started, but we are stil alive.\n, __func__);
- do
+ do {
status = ioread32(_iobase + AES_INTR_REG);
- while(!(status AES_INTRA_PENDING) --counter);
+ printk(%s: status: %x, counter: %u.\n, __func__, status,
counter);
+ } while(!(status AES_INTRA_PENDING) --counter);
/* Clear the event */
iowrite32((status 0xFF) | AES_INTRA_PENDING, _iobase + AES_INTR_REG);
+ printk(%s: completed.\n, __func__);
return counter ? 0 : 1;
}
@@ -113,6 +122,7 @@ geode_aes_crypt(struct geode_aes_op *op)
/* Start the critical section */
+ printk(%s: start.\n, __func__);
spin_lock_irqsave(lock, iflags);
if (op-mode == AES_MODE_CBC) {
@@ -131,6 +141,7 @@ geode_aes_crypt(struct geode_aes_op *op)
_readfield(AES_WRITEIV0_REG, op-iv);
spin_unlock_irqrestore(lock, iflags);
+ printk(%s: completed.\n, __func__);
return op-len;
}
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine
On Thu, May 03, 2007 at 03:12:43PM +0200, Martin Schiller ([EMAIL PROTECTED]) wrote: The freeze is the result of an invinite while-loop in the function geode_ecb_encrypt() (and maybe also in the geode_ecb_decrypt()). The cause for this seems to be, that the op-src == op-dst whereby the geode_aes_crypt() function does nothing at all, and so the value of nbytes will never be changed. Hm, driver does not perform encryption in-place at all. Since we did not hear from AMD quite for a while, could you please remove src==dst check in geode_aes_crypt() and run tests again. If it is software protection against hardware bug, I doubt such hardware should be used at all... Regards, Martin -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine
On Thu, May 03, 2007 at 08:47:44AM -0600, Jordan Crouse ([EMAIL PROTECTED]) wrote: On 03/05/07 23:49 +1000, Herbert Xu wrote: Evgeniy Polyakov [EMAIL PROTECTED] wrote: Hm, driver does not perform encryption in-place at all. Since we did not hear from AMD quite for a while, could you please remove src==dst check in geode_aes_crypt() and run tests again. If it is software protection against hardware bug, I doubt such hardware should be used at all... I agree. Jordan, could you please see if this can be fixed up? On older versions of the chip, in-place encryption was not possible, even though there was no hardware protection against it. I can't remember if the newer chip version can handle in place encryption or not. I missed out on the context of this thread - does the tcrypt demand in-place encryption? Majority of the in-kernel crypto users require in-place crypto processing. The only way to fix this I see is to allocate a buffer, copy data and then perform crypto processing. But I seriously doubt it will be faster then software encryption/decryption on that processor. Test for possibility for in-place encryption can be done in module load time and in case of failed crypto processing driver should fail into alternative (with allocation) ecryption way (at least similar check I perform in hifn module). Jordan -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] [CRYPTO] cryptd: Add software async crypto daemon
, but it will be requested to do so each time new tfm is being requested, if my analysis correct... Herbert, please clarify this issue. -- Evgeniy Polyakov - To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine
On Fri, Apr 27, 2007 at 06:10:51PM +1000, Herbert Xu ([EMAIL PROTECTED]) wrote:
Has anyone else tried to test the geode-aes driver with the tcrypt module?
I am also not able to use the geode-aes driver with openswan-2.4.7 on kernel
2.6.19 (with patched-in geode driver).
Jordan, do you have any ideas why this is happening?
Could it be compiler problem and broken hardware?
Martin, can you test attached patch?
diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
index 6d3840e..724169b 100644
--- a/drivers/crypto/geode-aes.c
+++ b/drivers/crypto/geode-aes.c
@@ -78,7 +78,7 @@ static int
do_crypt(void *src, void *dst, int len, u32 flags)
{
u32 status;
- u32 counter = AES_OP_TIMEOUT;
+ int counter = AES_OP_TIMEOUT;
iowrite32(virt_to_phys(src), _iobase + AES_SOURCEA_REG);
iowrite32(virt_to_phys(dst), _iobase + AES_DSTA_REG);
@@ -89,7 +89,9 @@ do_crypt(void *src, void *dst, int len, u32 flags)
do
status = ioread32(_iobase + AES_INTR_REG);
- while(!(status AES_INTRA_PENDING) --counter);
+ while(--counter 0 !(status AES_INTRA_PENDING));
+
+ WARN_ON(!counter);
/* Clear the event */
iowrite32((status 0xFF) | AES_INTRA_PENDING, _iobase + AES_INTR_REG);
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Testing the geode-aes driver with the tcrypt module completely freezes the machine
On Fri, Apr 27, 2007 at 01:50:37PM +0400, Evgeniy Polyakov ([EMAIL PROTECTED])
wrote:
On Fri, Apr 27, 2007 at 06:10:51PM +1000, Herbert Xu ([EMAIL PROTECTED])
wrote:
Has anyone else tried to test the geode-aes driver with the tcrypt module?
I am also not able to use the geode-aes driver with openswan-2.4.7 on
kernel
2.6.19 (with patched-in geode driver).
Jordan, do you have any ideas why this is happening?
Could it be compiler problem and broken hardware?
Martin, can you test attached patch?
Or better this one:
diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
index 6d3840e..724169b 100644
--- a/drivers/crypto/geode-aes.c
+++ b/drivers/crypto/geode-aes.c
@@ -78,7 +78,7 @@ static int
do_crypt(void *src, void *dst, int len, u32 flags)
{
u32 status;
- u32 counter = AES_OP_TIMEOUT;
+ int counter = 0x1000;
iowrite32(virt_to_phys(src), _iobase + AES_SOURCEA_REG);
iowrite32(virt_to_phys(dst), _iobase + AES_DSTA_REG);
@@ -89,7 +89,9 @@ do_crypt(void *src, void *dst, int len, u32 flags)
do
status = ioread32(_iobase + AES_INTR_REG);
- while(!(status AES_INTRA_PENDING) --counter);
+ while(--counter 0 !(status AES_INTRA_PENDING));
+
+ WARN_ON(!counter);
/* Clear the event */
iowrite32((status 0xFF) | AES_INTRA_PENDING, _iobase + AES_INTR_REG);
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
