LinuxLists.cc - [HIFN] Bugfix series.

2008-11-19 14:07:54

Subject: [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.

2008-11-19 14:07:54

by Evgeniy Polyakov

[permalink] [raw]

Subject: [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

2008-11-19 14:07:56

by Evgeniy Polyakov

[permalink] [raw]

Subject: [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

-#define HIFN_D_CMD_RSIZE 24*4
-#define HIFN_D_SRC_RSIZE 80*4
-#define HIFN_D_DST_RSIZE 80*4
-#define HIFN_D_RES_RSIZE 24*4
+#define HIFN_D_CMD_RSIZE 24*1
+#define HIFN_D_SRC_RSIZE 80*1
+#define HIFN_D_DST_RSIZE 80*1
+#define HIFN_D_RES_RSIZE 24*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 = ablkcipher_request_ctx(req);

if (rctx->walk.flags & ASYNC_FLAGS_MISALIGNED) {
unsigned int nbytes = req->nbytes;
@@ -1810,33 +1810,7 @@ static void hifn_process_ready(struct ablkcipher_request *req, int error)
req->base.complete(&req->base, error);
}

-static void hifn_check_for_completion(struct hifn_device *dev, int error)
-{
- int i;
- struct hifn_dma *dma = (struct hifn_dma *)dev->desc_virt;
-
- for (i=0; i<HIFN_D_RES_RSIZE; ++i) {
- struct hifn_desc *d = &dma->resr[i];
-
- if (!(d->l & __cpu_to_le32(HIFN_D_VALID)) && dev->sa[i]) {
- dev->success++;
- dev->reset = 0;
- hifn_process_ready(dev->sa[i], error);
- dev->sa[i] = NULL;
- }
-
- if (d->l & __cpu_to_le32(HIFN_D_DESTOVER | HIFN_D_OVER))
- if (printk_ratelimit())
- printk("%s: overflow detected [d: %u, o: %u] "
- "at %d resr: l: %08x, p: %08x.\n",
- dev->name,
- !!(d->l & __cpu_to_le32(HIFN_D_DESTOVER)),
- !!(d->l & __cpu_to_le32(HIFN_D_OVER)),
- i, d->l, d->p);
- }
-}
-
-static void hifn_clear_rings(struct hifn_device *dev)
+static void hifn_clear_rings(struct hifn_device *dev, int error)
{
struct hifn_dma *dma = (struct hifn_dma *)dev->desc_virt;
int i, u;
@@ -1853,21 +1827,26 @@ static void hifn_clear_rings(struct hifn_device *dev)
if (dma->resr[i].l & __cpu_to_le32(HIFN_D_VALID))
break;

- if (i != HIFN_D_RES_RSIZE)
- u--;
+ if (dev->sa[i]) {
+ dev->success++;
+ dev->reset = 0;
+ hifn_process_ready(dev->sa[i], error);
+ hifn_complete_sa(dev, i);
+ }

- if (++i == (HIFN_D_RES_RSIZE + 1))
+ if (++i == HIFN_D_RES_RSIZE)
i = 0;
+ u--;
}
dma->resk = i; dma->resu = u;

i = dma->srck; u = dma->srcu;
while (u != 0) {
- if (i == HIFN_D_SRC_RSIZE)
- i = 0;
if (dma->srcr[i].l & __cpu_to_le32(HIFN_D_VALID))
break;
- i++, u--;
+ if (++i == HIFN_D_SRC_RSIZE)
+ i = 0;
+ u--;
}
dma->srck = i; dma->srcu = u;

@@ -1875,20 +1854,19 @@ static void hifn_clear_rings(struct hifn_device *dev)
while (u != 0) {
if (dma->cmdr[i].l & __cpu_to_le32(HIFN_D_VALID))
break;
- if (i != HIFN_D_CMD_RSIZE)
- u--;
- if (++i == (HIFN_D_CMD_RSIZE + 1))
+ if (++i == HIFN_D_CMD_RSIZE)
i = 0;
+ u--;
}
dma->cmdk = i; dma->cmdu = u;

i = dma->dstk; u = dma->dstu;
while (u != 0) {
- if (i == HIFN_D_DST_RSIZE)
- i = 0;
if (dma->dstr[i].l & __cpu_to_le32(HIFN_D_VALID))
break;
- i++, u--;
+ if (++i == HIFN_D_DST_RSIZE)
+ i = 0;
+ u--;
}
dma->dstk = i; dma->dstu = u;

@@ -1933,30 +1911,39 @@ static void hifn_work(struct work_struct *work)
} else
dev->active--;

- if (dev->prev_success == dev->success && dev->started)
+ if ((dev->prev_success == dev->success) && dev->started)
reset = 1;
dev->prev_success = dev->success;
spin_unlock_irqrestore(&dev->lock, flags);

if (reset) {
- dprintk("%s: r: %08x, active: %d, started: %d, "
- "success: %lu: reset: %d.\n",
- dev->name, r, dev->active, dev->started,
- dev->success, reset);
-
if (++dev->reset >= 5) {
- dprintk("%s: really hard reset.\n", dev->name);
+ int i;
+ struct hifn_dma *dma = (struct hifn_dma *)dev->desc_virt;
+
+ printk("%s: r: %08x, active: %d, started: %d, "
+ "success: %lu: qlen: %u/%u, reset: %d.\n",
+ dev->name, r, dev->active, dev->started,
+ dev->success, dev->queue.qlen, dev->queue.max_qlen,
+ reset);
+
+ printk("%s: res: ", __func__);
+ for (i=0; i<HIFN_D_RES_RSIZE; ++i) {
+ printk("%x.%p ", dma->resr[i].l, dev->sa[i]);
+ if (dev->sa[i]) {
+ hifn_process_ready(dev->sa[i], -ENODEV);
+ hifn_complete_sa(dev, i);
+ }
+ }
+ printk("\n");
+
hifn_reset_dma(dev, 1);
hifn_stop_device(dev);
hifn_start_device(dev);
dev->reset = 0;
}

- spin_lock_irqsave(&dev->lock, flags);
- hifn_check_for_completion(dev, -EBUSY);
- hifn_clear_rings(dev);
- dev->started = 0;
- spin_unlock_irqrestore(&dev->lock, flags);
+ tasklet_schedule(&dev->tasklet);
}

schedule_delayed_work(&dev->work, HZ);
@@ -1973,8 +1960,8 @@ static irqreturn_t hifn_interrupt(int irq, void *data)
dprintk("%s: 1 dmacsr: %08x, dmareg: %08x, res: %08x [%d], "
"i: %d.%d.%d.%d, u: %d.%d.%d.%d.\n",
dev->name, dmacsr, dev->dmareg, dmacsr & dev->dmareg, dma->cmdi,
- dma->cmdu, dma->srcu, dma->dstu, dma->resu,
- dma->cmdi, dma->srci, dma->dsti, dma->resi);
+ dma->cmdi, dma->srci, dma->dsti, dma->resi,
+ dma->cmdu, dma->srcu, dma->dstu, dma->resu);

if ((dmacsr & dev->dmareg) == 0)
return IRQ_NONE;
@@ -1991,11 +1978,10 @@ static irqreturn_t hifn_interrupt(int irq, void *data)
if (restart) {
u32 puisr = hifn_read_0(dev, HIFN_0_PUISR);

- if (printk_ratelimit())
- printk("%s: overflow: r: %d, d: %d, puisr: %08x, d: %u.\n",
- dev->name, !!(dmacsr & HIFN_DMACSR_R_OVER),
- !!(dmacsr & HIFN_DMACSR_D_OVER),
- puisr, !!(puisr & HIFN_PUISR_DSTOVER));
+ printk(KERN_WARNING "%s: overflow: r: %d, d: %d, puisr: %08x, d: %u.\n",
+ dev->name, !!(dmacsr & HIFN_DMACSR_R_OVER),
+ !!(dmacsr & HIFN_DMACSR_D_OVER),
+ puisr, !!(puisr & HIFN_PUISR_DSTOVER));
if (!!(puisr & HIFN_PUISR_DSTOVER))
hifn_write_0(dev, HIFN_0_PUISR, HIFN_PUISR_DSTOVER);
hifn_write_1(dev, HIFN_1_DMA_CSR, dmacsr & (HIFN_DMACSR_R_OVER |
@@ -2005,12 +1991,11 @@ static irqreturn_t hifn_interrupt(int irq, void *data)
restart = dmacsr & (HIFN_DMACSR_C_ABORT | HIFN_DMACSR_S_ABORT |
HIFN_DMACSR_D_ABORT | HIFN_DMACSR_R_ABORT);
if (restart) {
- if (printk_ratelimit())
- printk("%s: abort: c: %d, s: %d, d: %d, r: %d.\n",
- dev->name, !!(dmacsr & HIFN_DMACSR_C_ABORT),
- !!(dmacsr & HIFN_DMACSR_S_ABORT),
- !!(dmacsr & HIFN_DMACSR_D_ABORT),
- !!(dmacsr & HIFN_DMACSR_R_ABORT));
+ printk(KERN_WARNING "%s: abort: c: %d, s: %d, d: %d, r: %d.\n",
+ dev->name, !!(dmacsr & HIFN_DMACSR_C_ABORT),
+ !!(dmacsr & HIFN_DMACSR_S_ABORT),
+ !!(dmacsr & HIFN_DMACSR_D_ABORT),
+ !!(dmacsr & HIFN_DMACSR_R_ABORT));
hifn_reset_dma(dev, 1);
hifn_init_dma(dev);
hifn_init_registers(dev);
@@ -2023,7 +2008,6 @@ static irqreturn_t hifn_interrupt(int irq, void *data)
}

tasklet_schedule(&dev->tasklet);
- hifn_clear_rings(dev);

return IRQ_HANDLED;
}
@@ -2037,21 +2021,25 @@ static void hifn_flush(struct hifn_device *dev)
struct hifn_dma *dma = (struct hifn_dma *)dev->desc_virt;
int i;

- spin_lock_irqsave(&dev->lock, flags);
for (i=0; i<HIFN_D_RES_RSIZE; ++i) {
struct hifn_desc *d = &dma->resr[i];

if (dev->sa[i]) {
hifn_process_ready(dev->sa[i],
(d->l & __cpu_to_le32(HIFN_D_VALID))?-ENODEV:0);
+ hifn_complete_sa(dev, i);
}
}

+ spin_lock_irqsave(&dev->lock, flags);
while ((async_req = crypto_dequeue_request(&dev->queue))) {
ctx = crypto_tfm_ctx(async_req->tfm);
req = container_of(async_req, struct ablkcipher_request, base);
+ spin_unlock_irqrestore(&dev->lock, flags);

hifn_process_ready(req, -ENODEV);
+
+ spin_lock_irqsave(&dev->lock, flags);
}
spin_unlock_irqrestore(&dev->lock, flags);
}
@@ -2568,7 +2556,7 @@ static void hifn_tasklet_callback(unsigned long data)
* (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);
+ hifn_clear_rings(dev, 0);

if (dev->started < HIFN_QUEUE_LENGTH && dev->queue.qlen)
hifn_process_queue(dev);
--
1.5.2.5

2008-11-19 14:07:54

by Evgeniy Polyakov

[permalink] [raw]

Subject: [PATCH] Remove some unused cruft.

2008-11-19 14:07:57

by Evgeniy Polyakov

[permalink] [raw]

Subject: [PATCH] Have HW invalidate src and dest descriptors after processing

From: Patrick McHardy <[email protected]>

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]>
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 320d08d..4d22b21 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_NOINVALID | HIFN_D_LAST);
+ HIFN_D_MASKDONEIRQ | 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_NOINVALID);
+ * HIFN_D_LAST);
*/

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_NOINVALID | HIFN_D_LAST);
+ HIFN_D_MASKDONEIRQ | 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_NOINVALID);
+ HIFN_D_LAST);
idx = 0;
}
dma->dsti = idx;
--
1.5.2.5

2008-11-19 14:07:56

by Evgeniy Polyakov

[permalink] [raw]

Subject: [PATCH] Fix request context corruption.

From: Patrick McHardy <[email protected]>

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.

Signed-off-by: Patrick McHardy <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/crypto/hifn_795x.c | 100 ++++++++++++++++++++++++-------------------
1 files changed, 56 insertions(+), 44 deletions(-)

diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index 13152f1..9a9e7ea 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -657,9 +657,15 @@ 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 int keysize, ivsize;
+ unsigned int keysize;
+};
+
+struct hifn_request_context
+{
+ u8 *iv;
+ unsigned int ivsize;
u8 op, type, mode, unused;
struct ablkcipher_walk walk;
};
@@ -1167,7 +1173,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;
@@ -1178,7 +1185,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;
@@ -1195,15 +1202,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;
@@ -1220,7 +1227,7 @@ static int hifn_setup_cmd_desc(struct hifn_device *dev,
goto err_out;
}

- switch (ctx->type) {
+ switch (rctx->type) {
case ACRYPTO_TYPE_AES_128:
if (ctx->keysize != 16)
goto err_out;
@@ -1255,7 +1262,7 @@ static int hifn_setup_cmd_desc(struct hifn_device *dev,

buf_pos += hifn_setup_crypto_command(dev, buf_pos,
nbytes, nbytes, ctx->key, ctx->keysize,
- ctx->iv, ctx->ivsize, md);
+ rctx->iv, rctx->ivsize, md);
}

dev->sa[sa_idx] = priv;
@@ -1265,7 +1272,7 @@ static int hifn_setup_cmd_desc(struct hifn_device *dev,
HIFN_D_LAST | HIFN_D_MASKDONEIRQ);

if (++dma->cmdi == HIFN_D_CMD_RSIZE) {
- dma->cmdr[dma->cmdi].l = __cpu_to_le32(HIFN_MAX_COMMAND |
+ dma->cmdr[dma->cmdi].l = __cpu_to_le32(
HIFN_D_VALID | HIFN_D_LAST |
HIFN_D_MASKDONEIRQ | HIFN_D_JUMP);
dma->cmdi = 0;
@@ -1369,7 +1376,8 @@ static void hifn_setup_dst_desc(struct hifn_device *dev, struct page *page,
}
}

-static int hifn_setup_dma(struct hifn_device *dev, struct hifn_context *ctx,
+static int hifn_setup_dma(struct hifn_device *dev,
+ struct hifn_context *ctx, struct hifn_request_context *rctx,
struct scatterlist *src, struct scatterlist *dst,
unsigned int nbytes, void *priv)
{
@@ -1385,18 +1393,18 @@ static int hifn_setup_dma(struct hifn_device *dev, struct hifn_context *ctx,
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);
+ "priv: %p, rctx: %p.\n",
+ dev->name, spage, soff, nbytes, priv, rctx);
hifn_setup_src_desc(dev, spage, soff, len, n - len == 0);

src++;
n -= len;
}

- t = &ctx->walk.cache[0];
+ t = &rctx->walk.cache[0];
n = nbytes;
while (n) {
- if (t->length) {
+ if (t->length && rctx->walk.flags & ASYNC_FLAGS_MISALIGNED) {
dpage = sg_page(t);
doff = 0;
len = t->length;
@@ -1408,8 +1416,8 @@ static int hifn_setup_dma(struct hifn_device *dev, struct hifn_context *ctx,
len = min(len, n);

dprintk("%s: dpage: %p, doffset: %u, nbytes: %u, "
- "priv: %p, ctx: %p.\n",
- dev->name, dpage, doff, nbytes, priv, ctx);
+ "priv: %p, rctx: %p.\n",
+ dev->name, dpage, doff, nbytes, priv, rctx);
hifn_setup_dst_desc(dev, dpage, doff, len, n - len == 0);

dst++;
@@ -1417,7 +1425,7 @@ static int hifn_setup_dma(struct hifn_device *dev, struct hifn_context *ctx,
n -= len;
}

- hifn_setup_cmd_desc(dev, ctx, priv, nbytes);
+ hifn_setup_cmd_desc(dev, ctx, rctx, priv, nbytes);
hifn_setup_res_desc(dev);
return 0;
}
@@ -1580,16 +1588,17 @@ static int ablkcipher_walk(struct ablkcipher_request *req,
static int hifn_setup_session(struct ablkcipher_request *req)
{
struct hifn_context *ctx = crypto_tfm_ctx(req->base.tfm);
+ struct hifn_request_context *rctx = ablkcipher_request_ctx(req);
struct hifn_device *dev = ctx->dev;
unsigned long dlen, flags;
unsigned int nbytes = req->nbytes, idx = 0;
int err = -EINVAL, sg_num;
struct scatterlist *dst;

- if (ctx->iv && !ctx->ivsize && ctx->mode != ACRYPTO_MODE_ECB)
+ if (rctx->iv && !rctx->ivsize && rctx->mode != ACRYPTO_MODE_ECB)
goto err_out_exit;

- ctx->walk.flags = 0;
+ rctx->walk.flags = 0;

while (nbytes) {
dst = &req->dst[idx];
@@ -1597,19 +1606,19 @@ static int hifn_setup_session(struct ablkcipher_request *req)

if (!IS_ALIGNED(dst->offset, HIFN_D_DST_DALIGN) ||
!IS_ALIGNED(dlen, HIFN_D_DST_DALIGN))
- ctx->walk.flags |= ASYNC_FLAGS_MISALIGNED;
+ rctx->walk.flags |= ASYNC_FLAGS_MISALIGNED;

nbytes -= dlen;
idx++;
}

- if (ctx->walk.flags & ASYNC_FLAGS_MISALIGNED) {
- err = ablkcipher_walk_init(&ctx->walk, idx, GFP_ATOMIC);
+ if (rctx->walk.flags & ASYNC_FLAGS_MISALIGNED) {
+ err = ablkcipher_walk_init(&rctx->walk, idx, GFP_ATOMIC);
if (err < 0)
return err;
}

- sg_num = ablkcipher_walk(req, &ctx->walk);
+ sg_num = ablkcipher_walk(req, &rctx->walk);
if (sg_num < 0) {
err = sg_num;
goto err_out_exit;
@@ -1624,7 +1633,7 @@ static int hifn_setup_session(struct ablkcipher_request *req)
dev->snum++;
dev->started++;

- err = hifn_setup_dma(dev, ctx, req->src, req->dst, req->nbytes, req);
+ err = hifn_setup_dma(dev, ctx, rctx, req->src, req->dst, req->nbytes, req);
if (err)
goto err_out;

@@ -1639,9 +1648,9 @@ err_out_exit:
if (err)
dprintk("%s: iv: %p [%d], key: %p [%d], mode: %u, op: %u, "
"type: %u, err: %d.\n",
- dev->name, ctx->iv, ctx->ivsize,
+ dev->name, rctx->iv, rctx->ivsize,
ctx->key, ctx->keysize,
- ctx->mode, ctx->op, ctx->type, err);
+ rctx->mode, rctx->op, rctx->type, err);

return err;
}
@@ -1651,6 +1660,7 @@ static int hifn_test(struct hifn_device *dev, int encdec, u8 snum)
int n, err;
u8 src[16];
struct hifn_context ctx;
+ struct hifn_request_context rctx;
u8 fips_aes_ecb_from_zero[16] = {
0x66, 0xE9, 0x4B, 0xD4,
0xEF, 0x8A, 0x2C, 0x3B,
@@ -1663,16 +1673,16 @@ static int hifn_test(struct hifn_device *dev, int encdec, u8 snum)

ctx.dev = dev;
ctx.keysize = 16;
- ctx.ivsize = 0;
- ctx.iv = NULL;
- ctx.op = (encdec)?ACRYPTO_OP_ENCRYPT:ACRYPTO_OP_DECRYPT;
- ctx.mode = ACRYPTO_MODE_ECB;
- ctx.type = ACRYPTO_TYPE_AES_128;
- ctx.walk.cache[0].length = 0;
+ rctx.ivsize = 0;
+ rctx.iv = NULL;
+ rctx.op = (encdec)?ACRYPTO_OP_ENCRYPT:ACRYPTO_OP_DECRYPT;
+ rctx.mode = ACRYPTO_MODE_ECB;
+ rctx.type = ACRYPTO_TYPE_AES_128;
+ rctx.walk.cache[0].length = 0;

sg_init_one(&sg, &src, sizeof(src));

- err = hifn_setup_dma(dev, &ctx, &sg, &sg, sizeof(src), NULL);
+ err = hifn_setup_dma(dev, &ctx, &rctx, &sg, &sg, sizeof(src), NULL);
if (err)
goto err_out;

@@ -1758,9 +1768,10 @@ static int ablkcipher_get(void *saddr, unsigned int *srestp, unsigned int offset
static void hifn_process_ready(struct ablkcipher_request *req, int error)
{
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.\n", __func__, req, ctx);
+ dprintk("%s: req: %p, ctx: %p rctx: %p.\n", __func__, req, ctx, rctx);

dev = ctx->dev;
dprintk("%s: req: %p, started: %d.\n", __func__, req, dev->started);
@@ -1768,14 +1779,14 @@ static void hifn_process_ready(struct ablkcipher_request *req, int error)
if (--dev->started < 0)
BUG();

- if (ctx->walk.flags & ASYNC_FLAGS_MISALIGNED) {
+ if (rctx->walk.flags & ASYNC_FLAGS_MISALIGNED) {
unsigned int nbytes = req->nbytes;
int idx = 0, err;
struct scatterlist *dst, *t;
void *saddr;

while (nbytes) {
- t = &ctx->walk.cache[idx];
+ t = &rctx->walk.cache[idx];
dst = &req->dst[idx];

dprintk("\n%s: sg_page(t): %p, t->length: %u, "
@@ -1803,7 +1814,7 @@ static void hifn_process_ready(struct ablkcipher_request *req, int error)
kunmap_atomic(saddr, KM_IRQ1);
}

- ablkcipher_walk_exit(&ctx->walk);
+ ablkcipher_walk_exit(&rctx->walk);
}

req->base.complete(&req->base, error);
@@ -2109,6 +2120,7 @@ static int hifn_setup_crypto_req(struct ablkcipher_request *req, u8 op,
u8 type, u8 mode)
{
struct hifn_context *ctx = crypto_tfm_ctx(req->base.tfm);
+ struct hifn_request_context *rctx = ablkcipher_request_ctx(req);
unsigned ivsize;

ivsize = crypto_ablkcipher_ivsize(crypto_ablkcipher_reqtfm(req));
@@ -2129,11 +2141,11 @@ static int hifn_setup_crypto_req(struct ablkcipher_request *req, u8 op,
type = ACRYPTO_TYPE_AES_256;
}

- ctx->op = op;
- ctx->mode = mode;
- ctx->type = type;
- ctx->iv = req->info;
- ctx->ivsize = ivsize;
+ rctx->op = op;
+ rctx->mode = mode;
+ rctx->type = type;
+ rctx->iv = req->info;
+ rctx->ivsize = ivsize;

/*
* HEAVY TODO: needs to kick Herbert XU to write documentation.
@@ -2484,7 +2496,7 @@ static int hifn_cra_init(struct crypto_tfm *tfm)
struct hifn_context *ctx = crypto_tfm_ctx(tfm);

ctx->dev = ha->dev;

2008-11-19 14:07:55

by Evgeniy Polyakov

[permalink] [raw]

Subject: [PATCH] Don't copy src sg list.

2008-11-19 14:07:57

by Evgeniy Polyakov

[permalink] [raw]

Subject: [PATCH] Fix DMA setup.

From: Patrick McHardy <[email protected]>

Signed-off-by: Patrick McHardy <[email protected]>
Signed-off-by: Evgeniy Polyakov <[email protected]>
---
drivers/crypto/hifn_795x.c | 165 +++++++++++++++++++++----------------------
1 files changed, 81 insertions(+), 84 deletions(-)

diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index 4d22b21..89e2e9a 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -662,7 +662,6 @@ struct hifn_context
unsigned int keysize, ivsize;
u8 op, type, mode, unused;
struct ablkcipher_walk walk;
- atomic_t sg_num;
};

#define crypto_alg_to_hifn(a) container_of(a, struct hifn_crypto_alg, alg)
@@ -1284,7 +1283,7 @@ err_out:
}

static int hifn_setup_src_desc(struct hifn_device *dev, struct page *page,
- unsigned int offset, unsigned int size)
+ unsigned int offset, unsigned int size, int last)
{
struct hifn_dma *dma = (struct hifn_dma *)dev->desc_virt;
int idx;
@@ -1296,12 +1295,12 @@ 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 | (last ? HIFN_D_LAST : 0));

if (++idx == HIFN_D_SRC_RSIZE) {
dma->srcr[idx].l = __cpu_to_le32(HIFN_D_VALID |
- HIFN_D_JUMP |
- HIFN_D_MASKDONEIRQ | HIFN_D_LAST);
+ HIFN_D_JUMP | HIFN_D_MASKDONEIRQ |
+ (last ? HIFN_D_LAST : 0));
idx = 0;
}

@@ -1342,7 +1341,7 @@ static void hifn_setup_res_desc(struct hifn_device *dev)
}

static void hifn_setup_dst_desc(struct hifn_device *dev, struct page *page,
- unsigned offset, unsigned size)
+ unsigned offset, unsigned size, int last)
{
struct hifn_dma *dma = (struct hifn_dma *)dev->desc_virt;
int idx;
@@ -1353,12 +1352,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 | (last ? HIFN_D_LAST : 0));

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);
+ (last ? HIFN_D_LAST : 0));
idx = 0;
}
dma->dsti = idx;
@@ -1370,16 +1369,47 @@ static void hifn_setup_dst_desc(struct hifn_device *dev, struct page *page,
}
}

-static int hifn_setup_dma(struct hifn_device *dev, struct page *spage, unsigned int soff,
- struct page *dpage, unsigned int doff, unsigned int nbytes, void *priv,
- struct hifn_context *ctx)
+static int hifn_setup_dma(struct hifn_device *dev, struct hifn_context *ctx,
+ struct scatterlist *src, struct scatterlist *dst,
+ unsigned int nbytes, void *priv)
{
- dprintk("%s: spage: %p, soffset: %u, dpage: %p, doffset: %u, nbytes: %u, priv: %p, ctx: %p.\n",
+ struct scatterlist *t;
+ struct page *spage, *dpage;
+ unsigned int soff, doff;
+ unsigned int n, len;
+
+ t = &ctx->walk.cache[0];
+ n = nbytes;
+ while (n) {
+ if (t->length) {
+ spage = dpage = sg_page(t);
+ soff = 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, nbytes);
+ hifn_setup_src_desc(dev, spage, soff, len, n - len == 0);
+ hifn_setup_dst_desc(dev, dpage, doff, len, n - len == 0);
+
+ src++;
+ dst++;
+ t++;
+ n -= len;
+ }
+
hifn_setup_cmd_desc(dev, ctx, priv, nbytes);
- hifn_setup_dst_desc(dev, dpage, doff, nbytes);
hifn_setup_res_desc(dev);
return 0;
}
@@ -1560,11 +1590,10 @@ static int hifn_setup_session(struct ablkcipher_request *req)
{
struct hifn_context *ctx = crypto_tfm_ctx(req->base.tfm);
struct hifn_device *dev = ctx->dev;
- struct page *spage, *dpage;
- unsigned long soff, doff, dlen, flags;
- unsigned int nbytes = req->nbytes, idx = 0, len;
+ unsigned long dlen, flags;
+ unsigned int nbytes = req->nbytes, idx = 0;
int err = -EINVAL, sg_num;
- struct scatterlist *src, *dst, *t;
+ struct scatterlist *dst;

if (ctx->iv && !ctx->ivsize && ctx->mode != ACRYPTO_MODE_ECB)
goto err_out_exit;
@@ -1589,15 +1618,11 @@ static int hifn_setup_session(struct ablkcipher_request *req)
return err;
}

- nbytes = req->nbytes;
- 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);

spin_lock_irqsave(&dev->lock, flags);
if (dev->started + sg_num > HIFN_QUEUE_LENGTH) {
@@ -1606,36 +1631,11 @@ static int hifn_setup_session(struct ablkcipher_request *req)
}

dev->snum++;
- dev->started += sg_num;
-
- while (nbytes) {
- src = &req->src[idx];
- dst = &req->dst[idx];
- t = &ctx->walk.cache[idx];
-
- if (t->length) {
- spage = dpage = sg_page(t);
- soff = doff = 0;
- len = t->length;
- } else {
- spage = sg_page(src);
- soff = src->offset;
-
- dpage = sg_page(dst);
- doff = dst->offset;
-
- len = dst->length;
- }
-
- idx++;
+ dev->started++;

- err = hifn_setup_dma(dev, spage, soff, dpage, doff, nbytes,
- req, ctx);
- if (err)
- goto err_out;
-
- nbytes -= min(len, nbytes);
- }
+ err = hifn_setup_dma(dev, ctx, req->src, req->dst, req->nbytes, req);
+ if (err)
+ goto err_out;

dev->active = HIFN_DEFAULT_ACTIVE_NUM;
spin_unlock_irqrestore(&dev->lock, flags);
@@ -1665,6 +1665,7 @@ static int hifn_test(struct hifn_device *dev, int encdec, u8 snum)
0xEF, 0x8A, 0x2C, 0x3B,
0x88, 0x4C, 0xFA, 0x59,
0xCA, 0x34, 0x2B, 0x2E};
+ struct scatterlist sg;

memset(src, 0, sizeof(src));
memset(ctx.key, 0, sizeof(ctx.key));
@@ -1676,12 +1677,11 @@ static int hifn_test(struct hifn_device *dev, int encdec, u8 snum)
ctx.op = (encdec)?ACRYPTO_OP_ENCRYPT:ACRYPTO_OP_DECRYPT;
ctx.mode = ACRYPTO_MODE_ECB;
ctx.type = ACRYPTO_TYPE_AES_128;
- atomic_set(&ctx.sg_num, 1);
+ ctx.walk.cache[0].length = 0;

- err = hifn_setup_dma(dev,
- virt_to_page(src), offset_in_page(src),
- virt_to_page(src), offset_in_page(src),
- sizeof(src), NULL, &ctx);
+ sg_init_one(&sg, &src, sizeof(src));
+
+ err = hifn_setup_dma(dev, &ctx, &sg, &sg, sizeof(src), NULL);
if (err)
goto err_out;

@@ -1772,53 +1772,50 @@ static void hifn_process_ready(struct ablkcipher_request *req, int error)
dprintk("%s: req: %p, ctx: %p.\n", __func__, req, ctx);

dev = ctx->dev;
- dprintk("%s: req: %p, started: %d, sg_num: %d.\n",
- __func__, req, dev->started, atomic_read(&ctx->sg_num));
+ dprintk("%s: req: %p, started: %d.\n", __func__, req, dev->started);

if (--dev->started < 0)
BUG();

- if (atomic_dec_and_test(&ctx->sg_num)) {
+ if (ctx->walk.flags & ASYNC_FLAGS_MISALIGNED) {
unsigned int nbytes = req->nbytes;
int idx = 0, err;
struct scatterlist *dst, *t;
void *saddr;

- if (ctx->walk.flags & ASYNC_FLAGS_MISALIGNED) {
- while (nbytes) {
- t = &ctx->walk.cache[idx];
- dst = &req->dst[idx];
+ while (nbytes) {
+ t = &ctx->walk.cache[idx];
+ dst = &req->dst[idx];

- dprintk("\n%s: sg_page(t): %p, t->length: %u, "
- "sg_page(dst): %p, dst->length: %u, "
- "nbytes: %u.\n",
- __func__, sg_page(t), t->length,
- sg_page(dst), dst->length, nbytes);
-
- if (!t->length) {
- nbytes -= min(dst->length, nbytes);
- idx++;
- continue;
- }
+ dprintk("\n%s: sg_page(t): %p, t->length: %u, "
+ "sg_page(dst): %p, dst->length: %u, "
+ "nbytes: %u.\n",
+ __func__, sg_page(t), t->length,
+ sg_page(dst), dst->length, nbytes);

- saddr = kmap_atomic(sg_page(t), KM_IRQ1);
+ if (!t->length) {
+ nbytes -= min(dst->length, nbytes);
+ idx++;
+ continue;
+ }

- err = ablkcipher_get(saddr, &t->length, t->offset,
- dst, nbytes, &nbytes);
- if (err < 0) {
- kunmap_atomic(saddr, KM_IRQ1);
- break;
- }
+ saddr = kmap_atomic(sg_page(t), KM_IRQ1);

- idx += err;
+ err = ablkcipher_get(saddr, &t->length, t->offset,
+ dst, nbytes, &nbytes);
+ if (err < 0) {
kunmap_atomic(saddr, KM_IRQ1);
+ break;
}

- ablkcipher_walk_exit(&ctx->walk);
+ idx += err;
+ kunmap_atomic(saddr, KM_IRQ1);
}

- req->base.complete(&req->base, error);
+ ablkcipher_walk_exit(&ctx->walk);
}
+
+ req->base.complete(&req->base, error);
}

static void hifn_check_for_completion(struct hifn_device *dev, int error)
--
1.5.2.5

2008-11-19 14:07:54

by Evgeniy Polyakov

[permalink] [raw]

Subject: [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

2008-11-20 02:27:10

by Andreas Gerlich

[permalink] [raw]

Subject: Re: [PATCH] short benchmarks

Hello,

here some short benchmarks with the new HIFN 795x driver:
---------------------------------------------------------
Hardware which I use: net5501-70 with vpn1401 (PCI-Card)
(Users with vpn1411 mini-pci-card, please
report it if the driver works with this card.)

Software which I use: Latest stable kernel 2.6.27.6 from
http://www.kernel.org.

Chiper: aes-cbc-essiv:sha256, keysize: 256 bits

With hifn_795x driver:
----------------------------------------------------------------------
(/misc is mounted to the crypted partition)

piepmatz:/misc# time dd if=/dev/zero of=xxx.img bs=1024 count=700000
700000+0 Datens?tze ein
700000+0 Datens?tze aus
716800000 Bytes (717 MB) kopiert, 68,0707 s, 10,5 MB/s

real 1m9.206s
user 0m0.536s
sys 0m7.312s
piepmatz:/misc# time cat xxx.img >/dev/null

real 1m12.737s
user 0m0.080s
sys 0m2.596s
piepmatz:/misc#

----------------------------------------------------------------------

===========================================================================

As referece the software implementation of AES:

---------------------------------------------------------------------------
piepmatz:/misc# time dd if=/dev/zero of=xxx.img bs=1024 count=700000
700000+0 Datens?tze ein
700000+0 Datens?tze aus
716800000 Bytes (717 MB) kopiert, 128,616 s, 5,6 MB/s

real 2m8.711s
user 0m0.616s
sys 0m6.652s
piepmatz:/misc# time cat xxx.img >/dev/null

real 2m2.761s
user 0m0.080s
sys 0m2.368s
piepmatz:/misc#
---------------------------------------------------------------------------

With the HIFN 7955 crypto engine the throughput is 10,5 MB/s, nearly
the double of the software implementation of AES. Therefore the time
is nearly the half.

Regards
Andreas Gerlich
--
Andreas Gerlich, University of Ulm, Germany
open source project --> http://yaze-ag.de/ (Yet Another Z80 Emulator)

2008-11-20 13:27:46

by Herbert Xu

[permalink] [raw]

Subject: Re: [PATCH] short benchmarks

On Thu, Nov 20, 2008 at 03:26:16AM +0100, Andreas Gerlich wrote:
> Hello,
>
> here some short benchmarks with the new HIFN 795x driver:

Awesome!

Now if only I could get those hifn bug fixes in a form that I
could apply :)

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-11-20 14:26:00

by Evgeniy Polyakov

[permalink] [raw]

Subject: Re: [PATCH] short benchmarks

On Thu, Nov 20, 2008 at 09:27:26PM +0800, Herbert Xu ([email protected]) wrote:
> On Thu, Nov 20, 2008 at 03:26:16AM +0100, Andreas Gerlich wrote:
> > Hello,
> >
> > here some short benchmarks with the new HIFN 795x driver:
>
> Awesome!
>
> Now if only I could get those hifn bug fixes in a form that I
> could apply :)

Hey, I sent you a series of perfectly split patches, even with cool
descriptions and other bureaucratic parts like signed-off, from: and so
on :)

I also tested that patches apply clearly against current mainline
(current as of yesterday).

--
Evgeniy Polyakov

2008-11-20 14:33:34

by Herbert Xu

[permalink] [raw]

Subject: Re: [PATCH] short benchmarks

On Thu, Nov 20, 2008 at 05:25:56PM +0300, Evgeniy Polyakov wrote:
>
> Hey, I sent you a series of perfectly split patches, even with cool
> descriptions and other bureaucratic parts like signed-off, from: and so
> on :)

Ah, there seems to be something wrong with my emails as I haven't
got them in my mailbox. But I've found them on linux-crypto now.

Thanks Evgeniy!
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-11-24 13:53:55

by Herbert Xu

[permalink] [raw]

Subject: Re: [PATCH] Have HW invalidate src and dest descriptors after processing

On Wed, Nov 19, 2008 at 02:03:27PM +0000, Evgeniy Polyakov wrote:
> From: Patrick McHardy <[email protected]>
>
> 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]>
> 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 320d08d..4d22b21 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_NOINVALID | HIFN_D_LAST);
> + HIFN_D_MASKDONEIRQ | HIFN_D_LAST);

Something is not right here. This patch is an exact reversal
of the previous patch in this series. Could you please take a
look?

Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-11-24 14:00:55

by Patrick McHardy

[permalink] [raw]

Subject: Re: [PATCH] Have HW invalidate src and dest descriptors after processing

commit 97ab2aef0c736582ef953596abbc15980cce1a54
Author: Patrick McHardy <[email protected]>
Date: Wed May 7 13:15:36 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.

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

diff --git a/drivers/crypto/hifn_795x.c b/drivers/crypto/hifn_795x.c
index d6f0423..c9fe18d 100644
--- a/drivers/crypto/hifn_795x.c
+++ b/drivers/crypto/hifn_795x.c
@@ -1168,109 +1168,15 @@ static int hifn_setup_crypto_command(struct hifn_device *dev,
return cmd_len;
}

-static int hifn_setup_src_desc(struct hifn_device *dev, struct page *page,
- unsigned int offset, unsigned int size)
-{
- struct hifn_dma *dma = (struct hifn_dma *)dev->desc_virt;
- int idx;
- dma_addr_t addr;
-
- addr = pci_map_page(dev->pdev, page, offset, size, PCI_DMA_TODEVICE);
-
- idx = dma->srci;
-
- dma->srcr[idx].p = __cpu_to_le32(addr);
- dma->srcr[idx].l = __cpu_to_le32(size | HIFN_D_VALID |
- 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 |
- HIFN_D_JUMP |
- HIFN_D_MASKDONEIRQ | HIFN_D_LAST);
- idx = 0;
- }
-
- dma->srci = idx;
- dma->srcu++;
-
- if (!(dev->flags & HIFN_FLAG_SRC_BUSY)) {
- hifn_write_1(dev, HIFN_1_DMA_CSR, HIFN_DMACSR_S_CTRL_ENA);
- dev->flags |= HIFN_FLAG_SRC_BUSY;
- }
-
- return size;
-}
-
-static void hifn_setup_res_desc(struct hifn_device *dev)
-{
- struct hifn_dma *dma = (struct hifn_dma *)dev->desc_virt;
-
- dma->resr[dma->resi].l = __cpu_to_le32(HIFN_USED_RESULT |
- 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_NOINVALID);
- */
-
- if (++dma->resi == HIFN_D_RES_RSIZE) {
- dma->resr[HIFN_D_RES_RSIZE].l = __cpu_to_le32(HIFN_D_VALID |
- HIFN_D_JUMP | HIFN_D_MASKDONEIRQ | HIFN_D_LAST);
- dma->resi = 0;
- }
-
- dma->resu++;
-
- if (!(dev->flags & HIFN_FLAG_RES_BUSY)) {
- hifn_write_1(dev, HIFN_1_DMA_CSR, HIFN_DMACSR_R_CTRL_ENA);
- dev->flags |= HIFN_FLAG_RES_BUSY;
- }
-}
-
-static void hifn_setup_dst_desc(struct hifn_device *dev, struct page *page,
- unsigned offset, unsigned size)
-{
- struct hifn_dma *dma = (struct hifn_dma *)dev->desc_virt;
- int idx;
- dma_addr_t addr;
-
- addr = pci_map_page(dev->pdev, page, offset, size, PCI_DMA_FROMDEVICE);
-
- 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_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_NOINVALID);
- idx = 0;
- }
- dma->dsti = idx;
- dma->dstu++;
-
- if (!(dev->flags & HIFN_FLAG_DST_BUSY)) {
- hifn_write_1(dev, HIFN_1_DMA_CSR, HIFN_DMACSR_D_CTRL_ENA);
- dev->flags |= HIFN_FLAG_DST_BUSY;
- }
-}
-
-static int hifn_setup_dma(struct hifn_device *dev, struct page *spage, unsigned int soff,
- struct page *dpage, unsigned int doff, unsigned int nbytes, void *priv,
- struct hifn_context *ctx)
+static int hifn_setup_cmd_desc(struct hifn_device *dev,
+ struct hifn_context *ctx, void *priv, unsigned int nbytes)
{
struct hifn_dma *dma = (struct hifn_dma *)dev->desc_virt;
int cmd_len, sa_idx;
u8 *buf, *buf_pos;
u16 mask;

- 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);
-
- sa_idx = dma->resi;
-
- hifn_setup_src_desc(dev, spage, soff, nbytes);
-
+ sa_idx = dma->cmdi;
buf_pos = buf = dma->command_bufs[dma->cmdi];

mask = 0;
@@ -1372,16 +1278,113 @@ static int hifn_setup_dma(struct hifn_device *dev, struct page *spage, unsigned
hifn_write_1(dev, HIFN_1_DMA_CSR, HIFN_DMACSR_C_CTRL_ENA);
dev->flags |= HIFN_FLAG_CMD_BUSY;
}
-
- hifn_setup_dst_desc(dev, dpage, doff, nbytes);
- hifn_setup_res_desc(dev);
-
return 0;

err_out:
return -EINVAL;
}

+static int hifn_setup_src_desc(struct hifn_device *dev, struct page *page,
+ unsigned int offset, unsigned int size)
+{
+ struct hifn_dma *dma = (struct hifn_dma *)dev->desc_virt;
+ int idx;
+ dma_addr_t addr;
+
+ addr = pci_map_page(dev->pdev, page, offset, size, PCI_DMA_TODEVICE);
+
+ idx = dma->srci;
+
+ dma->srcr[idx].p = __cpu_to_le32(addr);
+ dma->srcr[idx].l = __cpu_to_le32(size | HIFN_D_VALID |
+ 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 |
+ HIFN_D_JUMP |
+ HIFN_D_MASKDONEIRQ | HIFN_D_LAST);
+ idx = 0;
+ }
+
+ dma->srci = idx;
+ dma->srcu++;
+
+ if (!(dev->flags & HIFN_FLAG_SRC_BUSY)) {
+ hifn_write_1(dev, HIFN_1_DMA_CSR, HIFN_DMACSR_S_CTRL_ENA);
+ dev->flags |= HIFN_FLAG_SRC_BUSY;
+ }
+
+ return size;
+}
+
+static void hifn_setup_res_desc(struct hifn_device *dev)
+{
+ struct hifn_dma *dma = (struct hifn_dma *)dev->desc_virt;
+
+ dma->resr[dma->resi].l = __cpu_to_le32(HIFN_USED_RESULT |
+ 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_NOINVALID);
+ */
+
+ if (++dma->resi == HIFN_D_RES_RSIZE) {
+ dma->resr[HIFN_D_RES_RSIZE].l = __cpu_to_le32(HIFN_D_VALID |
+ HIFN_D_JUMP | HIFN_D_MASKDONEIRQ | HIFN_D_LAST);
+ dma->resi = 0;
+ }
+
+ dma->resu++;
+
+ if (!(dev->flags & HIFN_FLAG_RES_BUSY)) {
+ hifn_write_1(dev, HIFN_1_DMA_CSR, HIFN_DMACSR_R_CTRL_ENA);
+ dev->flags |= HIFN_FLAG_RES_BUSY;
+ }
+}
+
+static void hifn_setup_dst_desc(struct hifn_device *dev, struct page *page,
+ unsigned offset, unsigned size)
+{
+ struct hifn_dma *dma = (struct hifn_dma *)dev->desc_virt;
+ int idx;
+ dma_addr_t addr;
+
+ addr = pci_map_page(dev->pdev, page, offset, size, PCI_DMA_FROMDEVICE);
+
+ 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_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_NOINVALID);
+ idx = 0;
+ }
+ dma->dsti = idx;
+ dma->dstu++;
+
+ if (!(dev->flags & HIFN_FLAG_DST_BUSY)) {
+ hifn_write_1(dev, HIFN_1_DMA_CSR, HIFN_DMACSR_D_CTRL_ENA);
+ dev->flags |= HIFN_FLAG_DST_BUSY;
+ }
+}
+
+static int hifn_setup_dma(struct hifn_device *dev, struct page *spage, unsigned int soff,
+ struct page *dpage, unsigned int doff, unsigned int nbytes, void *priv,
+ struct hifn_context *ctx)
+{
+ 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, nbytes);
+ hifn_setup_cmd_desc(dev, ctx, priv, nbytes);
+ hifn_setup_dst_desc(dev, dpage, doff, nbytes);
+ hifn_setup_res_desc(dev);
+ return 0;
+}
+
static int ablkcipher_walk_init(struct ablkcipher_walk *w,
int num, gfp_t gfp_flags)
{

Attachments:

x (7.24 kB)

2008-11-24 14:05:37

by Herbert Xu

[permalink] [raw]

Subject: Re: [HIFN] Bugfix series.

On Wed, Nov 19, 2008 at 02:03:25PM +0000, Evgeniy Polyakov wrote:
>
> 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.

All applied except for the first two.

Thanks!
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-11-24 14:09:40

by Herbert Xu

[permalink] [raw]

Subject: Re: [PATCH] Have HW invalidate src and dest descriptors after processing

On Mon, Nov 24, 2008 at 03:00:45PM +0100, Patrick McHardy wrote:
>
> It seems the previous patch got mixed up, this is what I have in
> my old tree (just for reference, I don't know whether Evgeniy made
> any changes):

Thanks Patrick. Evgeniy, please let us know if this move is
still needed or not. The rest of your seies actually applied
without this but I'll hold onto it for a while so you can see
if this is intended or not.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-11-24 14:23:24

by Evgeniy Polyakov

[permalink] [raw]

Subject: Re: [PATCH] Have HW invalidate src and dest descriptors after processing

Hi.

On Mon, Nov 24, 2008 at 10:09:34PM +0800, Herbert Xu ([email protected]) wrote:
> > It seems the previous patch got mixed up, this is what I have in
> > my old tree (just for reference, I don't know whether Evgeniy made
> > any changes):
>
> Thanks Patrick. Evgeniy, please let us know if this move is
> still needed or not. The rest of your seies actually applied
> without this but I'll hold onto it for a while so you can see
> if this is intended or not.

Weird. patch/diff and hands all messed up. In the version which was
tested there is no that changes, so just drop it.

--
Evgeniy Polyakov

2008-11-24 14:34:34

by Herbert Xu

[permalink] [raw]

Subject: Re: [PATCH] Have HW invalidate src and dest descriptors after processing

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.

Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2008-11-24 14:52:42

by Evgeniy Polyakov

[permalink] [raw]

Subject: [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

2008-11-24 14:53:12

by Evgeniy Polyakov

[permalink] [raw]

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

2008-11-25 15:21:46

by Herbert Xu

[permalink] [raw]

Subject: Re: [HIFN] Use softirq kernel mapping in bh context.

On Mon, Nov 24, 2008 at 05:52:40PM +0300, Evgeniy Polyakov wrote:
> 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]>

Patch applied. Thanks Evgeniy!
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt