Hi Marc.
On Thu, Jun 28, 2007 at 01:49:13PM -0600, Marc St-Jean (stjeanma@xxxxxxxxxxxxxx) 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
