On 3/13/2015 8:37 PM, Tom Lendacky wrote:
> On 03/13/2015 12:16 PM, Horia Geanta wrote:
>> I was running into situations where the hardware FIFO was filling up, and
>> the code was returning EAGAIN to dm-crypt and just dropping the submitted
>> crypto request.
>>
>> This adds support in talitos for a software backlog queue. When requests
>> can't be queued to the hardware immediately EBUSY is returned. The queued
>> requests are dispatched to the hardware in received order as hardware FIFO
>> slots become available.
>>
>> Signed-off-by: Martin Hicks <m...@bork.org>
>> Signed-off-by: Horia Geanta <horia.gea...@freescale.com>
>> ---
>>   drivers/crypto/talitos.c | 107 
>> +++++++++++++++++++++++++++++++++++++++++------
>>   drivers/crypto/talitos.h |   2 +
>>   2 files changed, 97 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
>> index c184987dfcc7..d4679030d23c 100644
>> --- a/drivers/crypto/talitos.c
>> +++ b/drivers/crypto/talitos.c
>> @@ -197,23 +197,41 @@ static struct talitos_request *to_talitos_req(struct 
>> crypto_async_request *areq)
>>      }
>>   }
>>
>> -int talitos_submit(struct device *dev, int ch,
>> -               struct crypto_async_request *areq)
>> +/*
>> + * Enqueue to HW queue a request, coming either from upper layer or taken 
>> from
>> + * SW queue. When drawing from SW queue, check if there are backlogged 
>> requests
>> + * and notify their producers.
>> + */
>> +int __talitos_handle_queue(struct device *dev, int ch,
>> +                       struct crypto_async_request *areq,
>> +                       unsigned long *irq_flags)
>>   {
>>      struct talitos_private *priv = dev_get_drvdata(dev);
>>      struct talitos_request *request;
>> -    unsigned long flags;
>>      int head;
>>
>> -    spin_lock_irqsave(&priv->chan[ch].head_lock, flags);
>> -
>>      if (!atomic_inc_not_zero(&priv->chan[ch].submit_count)) {
>>              /* h/w fifo is full */
>> -            spin_unlock_irqrestore(&priv->chan[ch].head_lock, flags);
>> -            return -EAGAIN;
>> +            if (!areq)
>> +                    return -EBUSY;
>> +
>> +            /* Try to backlog request (if allowed) */
>> +            return crypto_enqueue_request(&priv->chan[ch].queue, areq);
> 
> I'd remembered something about how hardware drivers should use their
> own list element for queuing, searched back and found this:
> 
> http://marc.info/?l=linux-crypto-vger&m=137609769605139&w=2

The crypto_async_request "list" field is used only for queuing using the
crypto API functions - crypto_{enqueue,dequeue}_request.

I am not sure I understand what is the problem.
Are crypto_{enqueue,dequeue}_request part of *internal* crypto API?

>>      }
>>
>> -    head = priv->chan[ch].head;
>> +    if (!areq) {
>> +            struct crypto_async_request *backlog =
>> +                    crypto_get_backlog(&priv->chan[ch].queue);
>> +
>> +            /* Dequeue the oldest request */
>> +            areq = crypto_dequeue_request(&priv->chan[ch].queue);
>> +            if (!areq)
>> +                    return 0;
>> +
>> +            /* Mark a backlogged request as in-progress */
>> +            if (backlog)
>> +                    backlog->complete(backlog, -EINPROGRESS);
>> +    }
>>
>>      request = to_talitos_req(areq);
>>      if (IS_ERR(request))
>> @@ -224,6 +242,7 @@ int talitos_submit(struct device *dev, int ch,
>>                                         DMA_BIDIRECTIONAL);
>>
>>      /* increment fifo head */
>> +    head = priv->chan[ch].head;
>>      priv->chan[ch].head = (priv->chan[ch].head + 1) & (priv->fifo_len - 1);
>>
>>      smp_wmb();
>> @@ -236,14 +255,66 @@ int talitos_submit(struct device *dev, int ch,
>>      out_be32(priv->chan[ch].reg + TALITOS_FF_LO,
>>               lower_32_bits(request->dma_desc));
>>
>> +    return -EINPROGRESS;
>> +}
>> +
>> +int talitos_submit(struct device *dev, int ch,
>> +               struct crypto_async_request *areq)
>> +{
>> +    struct talitos_private *priv = dev_get_drvdata(dev);
>> +    unsigned long flags;
>> +    int ret;
>> +
>> +    spin_lock_irqsave(&priv->chan[ch].head_lock, flags);
>> +
>> +    /*
>> +     * Hidden assumption: we maintain submission order separately for
>> +     * requests that may be backlogged and those that may not. For e.g. even
>> +     * if SW queue has some requests, we won't drop an incoming request that
>> +     * may not be backlogged, but enqueue it in the HW queue (in front of
>> +     * pending ones).
>> +     */
>> +    if (areq->flags & CRYPTO_TFM_REQ_MAY_BACKLOG &&
>> +        priv->chan[ch].queue.qlen) {
>> +            /*
>> +             * There are pending requests in the SW queue. Since we want to
>> +             * maintain the order of requests, we cannot enqueue in the HW
>> +             * queue. Thus put this new request in SW queue and dispatch
>> +             * the oldest backlogged request to the hardware.
>> +             */
>> +            ret = crypto_enqueue_request(&priv->chan[ch].queue, areq);
>> +            __talitos_handle_queue(dev, ch, NULL, &flags);
>> +    } else {
>> +            ret = __talitos_handle_queue(dev, ch, areq, &flags);
>> +    }
>> +
>>      spin_unlock_irqrestore(&priv->chan[ch].head_lock, flags);
>>
>> -    return -EINPROGRESS;
>> +    return ret;
>>   }
>>   EXPORT_SYMBOL(talitos_submit);
>>
>> +static void talitos_handle_queue(struct device *dev, int ch)
>> +{
>> +    struct talitos_private *priv = dev_get_drvdata(dev);
>> +    unsigned long flags;
>> +    int ret = -EINPROGRESS;
>> +
>> +    if (!priv->chan[ch].queue.qlen)
>> +            return;
>> +
>> +    spin_lock_irqsave(&priv->chan[ch].head_lock, flags);
>> +
>> +    /* Queue backlogged requests as long as the hardware has room */
>> +    while (priv->chan[ch].queue.qlen && ret == -EINPROGRESS)
>> +            ret = __talitos_handle_queue(dev, ch, NULL, &flags);
>> +
>> +    spin_unlock_irqrestore(&priv->chan[ch].head_lock, flags);
>> +}
>> +
>>   /*
>>    * process what was done, notify callback of error if not
>> + * when done with HW queue, check SW queue (backlogged requests)
>>    */
>>   static void flush_channel(struct device *dev, int ch, int error, int 
>> reset_ch)
>>   {
>> @@ -293,6 +364,8 @@ static void flush_channel(struct device *dev, int ch, 
>> int error, int reset_ch)
>>      }
>>
>>      spin_unlock_irqrestore(&priv->chan[ch].tail_lock, flags);
>> +
>> +    talitos_handle_queue(dev, ch);
>>   }
>>
>>   /*
>> @@ -1069,7 +1142,8 @@ static int ipsec_esp(struct aead_request *areq, u64 
>> seq,
>>      req_ctx->req.callback = callback;
>>      req_ctx->req.context = areq;
>>      ret = talitos_submit(dev, ctx->ch, &areq->base);
>> -    if (ret != -EINPROGRESS) {
>> +    if (ret != -EINPROGRESS &&
>> +        !(ret == -EBUSY && areq->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)) {
>>              ipsec_esp_unmap(dev, edesc, areq);
>>              kfree(edesc->link_tbl);
>>      }
>> @@ -1451,7 +1525,8 @@ static int common_nonsnoop(struct ablkcipher_request 
>> *areq,
>>      req_ctx->req.callback = callback;
>>      req_ctx->req.context = areq;
>>      ret = talitos_submit(dev, ctx->ch, &areq->base);
>> -    if (ret != -EINPROGRESS) {
>> +    if (ret != -EINPROGRESS &&
>> +        !(ret == -EBUSY && areq->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)) {
>>              common_nonsnoop_unmap(dev, edesc, areq);
>>              kfree(edesc->link_tbl);
>>      }
>> @@ -1634,7 +1709,8 @@ static int common_nonsnoop_hash(struct ahash_request 
>> *areq, unsigned int length,
>>      req_ctx->req.callback = callback;
>>      req_ctx->req.context = areq;
>>      ret = talitos_submit(dev, ctx->ch, &areq->base);
>> -    if (ret != -EINPROGRESS) {
>> +    if (ret != -EINPROGRESS &&
>> +        !(ret == -EBUSY && areq->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)) {
>>              common_nonsnoop_hash_unmap(dev, edesc, areq);
>>              kfree(edesc->link_tbl);
>>      }
>> @@ -2753,6 +2829,13 @@ static int talitos_probe(struct platform_device 
>> *ofdev)
>>
>>              atomic_set(&priv->chan[i].submit_count,
>>                         -(priv->chfifo_len - 1));
>> +
>> +            /*
>> +             * The crypto_queue is used to manage the backlog only. While
>> +             * the hardware FIFO has space requests are dispatched
>> +             * immediately.
>> +             */
>> +            crypto_init_queue(&priv->chan[i].queue, 0);
>>      }
>>
>>      dma_set_mask(dev, DMA_BIT_MASK(36));
>> diff --git a/drivers/crypto/talitos.h b/drivers/crypto/talitos.h
>> index ebae5c3ef0fb..68ea26a4309d 100644
>> --- a/drivers/crypto/talitos.h
>> +++ b/drivers/crypto/talitos.h
>> @@ -118,6 +118,8 @@ struct talitos_channel {
>>      /* index to next free descriptor request */
>>      int head;
>>
>> +    struct crypto_queue queue;
>> +
>>      /* request release (tail) lock */
>>      spinlock_t tail_lock ____cacheline_aligned;
>>      /* index to next in-progress/done descriptor request */
>>
> 
> 


-- 
Please avoid sending me Word or PowerPoint attachments.
See http://www.gnu.org/philosophy/no-word-attachments.html

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

Reply via email to