[openib-general] [PATCH 0/7] AMSO1100 RNIC Driver
Tom Tucker
tom at opengridcomputing.com
Fri Mar 10 09:44:58 PST 2006
Here is my current understanding of memspace issue...
The c2_mq data structure describes both the request queue and the reply
queue -- they have the same format. A request queue, however, has a
shared index that resides in adapter DDR, while the reply queue's shared
index resides in host DDR. This is why the tool complains -- because the
__iomem qualifier is only correct when the c2_mq structure is used to
define the request queue, and is incorrect for the reply queue. This
fundamental problem extends to the functions used to manipulate these
queues and the parameters that are passed to them.
So... to fix this issue I will perform a spate of strange and unnatural
acts, such as creating two functions with different arguments depending
on whether the queue is a request or response queue, and properly
qualifying arguments to functions where the type ends up getting
promoted due to an expression in the argument (e.g. this +that, etc...)
To check if this works, however, I need the sparse tool. I downloaded
the beast and compiled it, however, it doesn't have a usage statement
and the README was profoundly unhelpful. Can you tell me what arguments
you used to the tool and provide me a link to where you obtained the
tool (to be sure I've got the correct one).
Thanks,
Tom
On Wed, 2006-03-08 at 14:06 -0800, Roland Dreier wrote:
> I ran sparse against the amso1100 driver, and came up with a bunch of
> cleanups. This includes at least one fix for a minor memory leak:
> c2_cleanup_qp_table() was not calling c2_array_cleanup() for the QP
> table.
>
> This leaves the following warnings, which are harder to untangle. The
> fundamental problem is that the c2_mq stuff is broken: for example,
> req_vq has its pool in system memory, while rep_vq has its pool in
> iomem. Fixing this will also require fixing things like qp_wr_post(),
> which right now does a memcpy to iomem.
>
> drivers/infiniband/hw/amso1100/c2_rnic.c:501:16: warning: incorrect type in argument 5 (different address spaces)
> drivers/infiniband/hw/amso1100/c2_rnic.c:501:16: expected unsigned char [usertype] *pool_start
> drivers/infiniband/hw/amso1100/c2_rnic.c:501:16: got void [noderef] *[assigned] mmio_regs<asn:2>
> drivers/infiniband/hw/amso1100/c2_qp.c:365:11: warning: incorrect type in argument 5 (different address spaces)
> drivers/infiniband/hw/amso1100/c2_qp.c:365:11: expected unsigned char [usertype] *pool_start
> drivers/infiniband/hw/amso1100/c2_qp.c:365:11: got void [noderef] *[assigned] mmap<asn:2>
> drivers/infiniband/hw/amso1100/c2_qp.c:384:11: warning: incorrect type in argument 5 (different address spaces)
> drivers/infiniband/hw/amso1100/c2_qp.c:384:11: expected unsigned char [usertype] *pool_start
> drivers/infiniband/hw/amso1100/c2_qp.c:384:11: got void [noderef] *[assigned] mmap<asn:2>
>
> - R.
>
> Various sparse annotation fixes etc. for amso1100.
>
> Signed-off-by: Roland Dreier <rolandd at cisco.com>
>
> --- infiniband/hw/amso1100/c2_mq.c (revision 5693)
> +++ infiniband/hw/amso1100/c2_mq.c (working copy)
> @@ -69,7 +69,7 @@ void c2_mq_produce(struct c2_mq *q)
> BUMP(q, q->priv);
> q->hint_count++;
> /* Update peer's offset. */
> - q->peer->shared = cpu_to_be16(q->priv);
> + writew(cpu_to_be16(q->priv), &q->peer->shared);
> }
> }
>
> @@ -112,7 +112,7 @@ void c2_mq_free(struct c2_mq *q)
> #endif
> BUMP(q, q->priv);
> /* Update peer's offset. */
> - q->peer->shared = cpu_to_be16(q->priv);
> + writew(cpu_to_be16(q->priv), &q->peer->shared);
> }
> }
>
> @@ -148,9 +148,8 @@ u32 c2_mq_count(struct c2_mq *q)
> return (u32) count;
> }
>
> -void
> -c2_mq_init(struct c2_mq *q, u32 index, u32 q_size,
> - u32 msg_size, u8 * pool_start, u16 * peer, u32 type)
> +void c2_mq_init(struct c2_mq *q, u32 index, u32 q_size, u32 msg_size,
> + u8 *pool_start, u16 __iomem *peer, u32 type)
> {
> assert(q->shared);
>
> @@ -159,7 +158,7 @@ c2_mq_init(struct c2_mq *q, u32 index, u
> q->q_size = q_size;
> q->msg_size = msg_size;
> q->msg_pool = pool_start;
> - q->peer = (struct c2_mq_shared *) peer;
> + q->peer = (struct c2_mq_shared __iomem *) peer;
> q->magic = C2_MQ_MAGIC;
> q->type = type;
> q->priv = 0;
> --- infiniband/hw/amso1100/c2_qp.c (revision 5693)
> +++ infiniband/hw/amso1100/c2_qp.c (working copy)
> @@ -66,6 +66,7 @@ static const u8 c2_opcode[] = {
> [IB_WR_ATOMIC_FETCH_AND_ADD] = NO_SUPPORT,
> };
>
> +#if 0
> void c2_qp_event(struct c2_dev *c2dev, u32 qpn, enum ib_event_type event_type)
> {
> struct c2_qp *qp;
> @@ -91,6 +92,7 @@ void c2_qp_event(struct c2_dev *c2dev, u
> if (atomic_dec_and_test(&qp->refcount))
> wake_up(&qp->wait);
> }
> +#endif
>
> static int to_c2_state(enum ib_qp_state ib_state)
> {
> @@ -258,7 +260,7 @@ int c2_alloc_qp(struct c2_dev *c2dev,
> struct c2_cq *recv_cq = to_c2cq(qp_attrs->recv_cq);
> unsigned long peer_pa;
> u32 q_size, msg_size, mmap_size;
> - void *mmap;
> + void __iomem *mmap;
> int err;
>
> qp->qpn = c2_alloc(&c2dev->qp_table.alloc);
> @@ -348,7 +350,7 @@ int c2_alloc_qp(struct c2_dev *c2dev,
> /* Initialize the SQ MQ */
> q_size = be32_to_cpu(reply->sq_depth);
> msg_size = be32_to_cpu(reply->sq_msg_size);
> - peer_pa = (unsigned long) (c2dev->pa + be32_to_cpu(reply->sq_mq_start));
> + peer_pa = c2dev->pa + be32_to_cpu(reply->sq_mq_start);
> mmap_size = PAGE_ALIGN(sizeof(struct c2_mq_shared) + msg_size * q_size);
> mmap = ioremap_nocache(peer_pa, mmap_size);
> if (!mmap) {
> @@ -367,7 +369,7 @@ int c2_alloc_qp(struct c2_dev *c2dev,
> /* Initialize the RQ mq */
> q_size = be32_to_cpu(reply->rq_depth);
> msg_size = be32_to_cpu(reply->rq_msg_size);
> - peer_pa = (unsigned long) (c2dev->pa + be32_to_cpu(reply->rq_mq_start));
> + peer_pa = c2dev->pa + be32_to_cpu(reply->rq_mq_start);
> mmap_size = PAGE_ALIGN(sizeof(struct c2_mq_shared) + msg_size * q_size);
> mmap = ioremap_nocache(peer_pa, mmap_size);
> if (!mmap) {
> @@ -836,4 +838,5 @@ int __devinit c2_init_qp_table(struct c2
> void __devexit c2_cleanup_qp_table(struct c2_dev *c2dev)
> {
> c2_alloc_cleanup(&c2dev->qp_table.alloc);
> + c2_array_cleanup(&c2dev->qp_table.qp, c2dev->max_qp);
> }
> --- infiniband/hw/amso1100/c2.c (revision 5693)
> +++ infiniband/hw/amso1100/c2.c (working copy)
> @@ -81,8 +81,6 @@ static int c2_change_mtu(struct net_devi
> static void c2_reset(struct c2_port *c2_port);
> static struct net_device_stats *c2_get_stats(struct net_device *netdev);
>
> -extern void c2_rnic_interrupt(struct c2_dev *c2dev);
> -
> static struct pci_device_id c2_pci_table[] = {
> {0x18b8, 0xb001, PCI_ANY_ID, PCI_ANY_ID},
> {0}
> @@ -121,7 +119,7 @@ static int c2_tx_ring_alloc(struct c2_ri
> dma_addr_t base, void __iomem * mmio_txp_ring)
> {
> struct c2_tx_desc *tx_desc;
> - struct c2_txp_desc *txp_desc;
> + struct c2_txp_desc __iomem *txp_desc;
> struct c2_element *elem;
> int i;
>
> @@ -139,7 +137,7 @@ static int c2_tx_ring_alloc(struct c2_ri
> /* Set TXP_HTXD_UNINIT */
> writeq(cpu_to_be64(0x1122334455667788ULL),
> (void __iomem *) txp_desc + C2_TXP_ADDR);
> - writew(cpu_to_be16(0), (void *) txp_desc + C2_TXP_LEN);
> + writew(cpu_to_be16(0), (void __iomem *) txp_desc + C2_TXP_LEN);
> writew(cpu_to_be16(TXP_HTXD_UNINIT),
> (void __iomem *) txp_desc + C2_TXP_FLAGS);
>
> @@ -170,7 +168,7 @@ static int c2_rx_ring_alloc(struct c2_ri
> dma_addr_t base, void __iomem * mmio_rxp_ring)
> {
> struct c2_rx_desc *rx_desc;
> - struct c2_rxp_desc *rxp_desc;
> + struct c2_rxp_desc __iomem *rxp_desc;
> struct c2_element *elem;
> int i;
>
> @@ -187,13 +185,13 @@ static int c2_rx_ring_alloc(struct c2_ri
>
> /* Set RXP_HRXD_UNINIT */
> writew(cpu_to_be16(RXP_HRXD_OK),
> - (void *) rxp_desc + C2_RXP_STATUS);
> - writew(cpu_to_be16(0), (void *) rxp_desc + C2_RXP_COUNT);
> - writew(cpu_to_be16(0), (void *) rxp_desc + C2_RXP_LEN);
> + (void __iomem *) rxp_desc + C2_RXP_STATUS);
> + writew(cpu_to_be16(0), (void __iomem *) rxp_desc + C2_RXP_COUNT);
> + writew(cpu_to_be16(0), (void __iomem *) rxp_desc + C2_RXP_LEN);
> writeq(cpu_to_be64(0x99aabbccddeeffULL),
> - (void *) rxp_desc + C2_RXP_ADDR);
> + (void __iomem *) rxp_desc + C2_RXP_ADDR);
> writew(cpu_to_be16(RXP_HRXD_UNINIT),
> - (void *) rxp_desc + C2_RXP_FLAGS);
> + (void __iomem *) rxp_desc + C2_RXP_FLAGS);
>
> elem->skb = NULL;
> elem->ht_desc = rx_desc;
> @@ -1178,7 +1176,7 @@ static int __devinit c2_probe(struct pci
> }
>
> /* Remap the PCI registers in adapter BAR4 to kernel VA space */
> - c2dev->pa = (void *) (reg4_start + C2_PCI_REGS_OFFSET);
> + c2dev->pa = reg4_start + C2_PCI_REGS_OFFSET;
> c2dev->kva = ioremap_nocache(reg4_start + C2_PCI_REGS_OFFSET,
> kva_map_size);
> if (c2dev->kva == 0UL) {
> --- infiniband/hw/amso1100/c2_mq.h (revision 5693)
> +++ infiniband/hw/amso1100/c2_mq.h (working copy)
> @@ -68,7 +68,7 @@ struct c2_mq {
> u8 *msg_pool;
> u16 hint_count;
> u16 priv;
> - struct c2_mq_shared *peer;
> + struct c2_mq_shared __iomem *peer;
> u16 *shared;
> u32 q_size;
> u32 msg_size;
> @@ -95,7 +95,7 @@ extern void c2_mq_produce(struct c2_mq *
> extern void *c2_mq_consume(struct c2_mq *q);
> extern void c2_mq_free(struct c2_mq *q);
> extern u32 c2_mq_count(struct c2_mq *q);
> -extern void c2_mq_init(struct c2_mq *q, u32 index, u32 q_size,
> - u32 msg_size, u8 * pool_start, u16 * peer, u32 type);
> +extern void c2_mq_init(struct c2_mq *q, u32 index, u32 q_size, u32 msg_size,
> + u8 *pool_start, u16 __iomem *peer, u32 type);
>
> #endif /* _C2_MQ_H_ */
> --- infiniband/hw/amso1100/c2.h (revision 5693)
> +++ infiniband/hw/amso1100/c2.h (working copy)
> @@ -263,7 +263,7 @@ struct c2_array {
> */
> struct sp_chunk {
> struct sp_chunk *next;
> - u32 gfp_mask;
> + gfp_t gfp_mask;
> u16 head;
> u16 shared_ptr[0];
> };
> @@ -288,7 +288,7 @@ struct c2_qp_table {
> struct c2_element {
> struct c2_element *next;
> void *ht_desc; /* host descriptor */
> - void *hw_desc; /* hardware descriptor */
> + void __iomem *hw_desc; /* hardware descriptor */
> struct sk_buff *skb;
> dma_addr_t mapaddr;
> u32 maplen;
> @@ -319,7 +319,7 @@ struct c2_dev {
> u32 vendor_id;
> u32 vendor_part_id;
> void __iomem *kva; /* KVA device memory */
> - void __iomem *pa; /* PA device memory */
> + unsigned long pa; /* PA device memory */
> void **qptr_array;
>
> kmem_cache_t *host_msg_cache;
> @@ -514,6 +514,7 @@ extern int c2_register_device(struct c2_
> extern void c2_unregister_device(struct c2_dev *c2dev);
> extern int c2_rnic_init(struct c2_dev *c2dev);
> extern void c2_rnic_term(struct c2_dev *c2dev);
> +extern void c2_rnic_interrupt(struct c2_dev *c2dev);
> int c2_del_addr(struct c2_dev *c2dev, u32 inaddr, u32 inmask);
> int c2_add_addr(struct c2_dev *c2dev, u32 inaddr, u32 inmask);
>
> @@ -569,10 +570,11 @@ extern u32 c2_alloc(struct c2_alloc *all
> extern void c2_free(struct c2_alloc *alloc, u32 obj);
> extern int c2_alloc_init(struct c2_alloc *alloc, u32 num, u32 reserved);
> extern void c2_alloc_cleanup(struct c2_alloc *alloc);
> -extern int c2_init_mqsp_pool(unsigned int gfp_mask, struct sp_chunk **root);
> +extern int c2_init_mqsp_pool(gfp_t gfp_mask, struct sp_chunk **root);
> extern void c2_free_mqsp_pool(struct sp_chunk *root);
> extern u16 *c2_alloc_mqsp(struct sp_chunk *head);
> extern void c2_free_mqsp(u16 * mqsp);
> +extern void c2_array_cleanup(struct c2_array *array, int nent);
> extern int c2_array_init(struct c2_array *array, int nent);
> extern void c2_array_clear(struct c2_array *array, int index);
> extern int c2_array_set(struct c2_array *array, int index, void *value);
> --- infiniband/hw/amso1100/c2_alloc.c (revision 5693)
> +++ infiniband/hw/amso1100/c2_alloc.c (working copy)
> @@ -165,7 +165,7 @@ void c2_array_cleanup(struct c2_array *a
> kfree(array->page_list);
> }
>
> -static int c2_alloc_mqsp_chunk(unsigned int gfp_mask, struct sp_chunk **head)
> +static int c2_alloc_mqsp_chunk(gfp_t gfp_mask, struct sp_chunk **head)
> {
> int i;
> struct sp_chunk *new_head;
> @@ -192,7 +192,7 @@ static int c2_alloc_mqsp_chunk(unsigned
> return 0;
> }
>
> -int c2_init_mqsp_pool(unsigned int gfp_mask, struct sp_chunk **root)
> +int c2_init_mqsp_pool(gfp_t gfp_mask, struct sp_chunk **root)
> {
> return c2_alloc_mqsp_chunk(gfp_mask, root);
> }
> @@ -224,13 +224,13 @@ u16 *c2_alloc_mqsp(struct sp_chunk *head
> head->head = head->shared_ptr[mqsp];
> break;
> } else
> - return 0;
> + return NULL;
> } else
> head = head->next;
> }
> if (head)
> return &(head->shared_ptr[mqsp]);
> - return 0;
> + return NULL;
> }
>
> void c2_free_mqsp(u16 * mqsp)
> --- infiniband/hw/amso1100/c2_provider.c (revision 5693)
> +++ infiniband/hw/amso1100/c2_provider.c (working copy)
> @@ -576,7 +576,7 @@ static int c2_disconnect(struct iw_cm_id
> /* Disassociate the QP from this cm_id */
> cm_id->provider_id = 0;
> qp = to_c2qp(ib_qp);
> - qp->cm_id = 0;
> + qp->cm_id = NULL;
>
> memset(&attr, 0, sizeof(struct ib_qp_attr));
> if (abrupt)
> @@ -816,10 +816,10 @@ int c2_register_device(struct c2_dev *de
> dev->ibdev.reg_user_mr = c2_reg_user_mr;
> dev->ibdev.dereg_mr = c2_dereg_mr;
>
> - dev->ibdev.alloc_fmr = 0;
> - dev->ibdev.unmap_fmr = 0;
> - dev->ibdev.dealloc_fmr = 0;
> - dev->ibdev.map_phys_fmr = 0;
> + dev->ibdev.alloc_fmr = NULL;
> + dev->ibdev.unmap_fmr = NULL;
> + dev->ibdev.dealloc_fmr = NULL;
> + dev->ibdev.map_phys_fmr = NULL;
>
> dev->ibdev.attach_mcast = c2_multicast_attach;
> dev->ibdev.detach_mcast = c2_multicast_detach;
> --- infiniband/hw/amso1100/c2_cq.c (revision 5693)
> +++ infiniband/hw/amso1100/c2_cq.c (working copy)
> @@ -198,30 +198,27 @@ int c2_poll_cq(struct ib_cq *ibcq, int n
>
> int c2_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify notify)
> {
> - struct c2_mq_shared volatile *shared;
> + struct c2_mq_shared __iomem *shared;
> struct c2_cq *cq;
>
> cq = to_c2cq(ibcq);
> shared = cq->mq.peer;
>
> if (notify == IB_CQ_NEXT_COMP)
> - shared->notification_type = C2_CQ_NOTIFICATION_TYPE_NEXT;
> + writeb(C2_CQ_NOTIFICATION_TYPE_NEXT, &shared->notification_type);
> else if (notify == IB_CQ_SOLICITED)
> - shared->notification_type = C2_CQ_NOTIFICATION_TYPE_NEXT_SE;
> + writeb(C2_CQ_NOTIFICATION_TYPE_NEXT_SE, &shared->notification_type);
> else
> return -EINVAL;
>
> - shared->armed = CQ_WAIT_FOR_DMA | CQ_ARMED;
> + writeb(CQ_WAIT_FOR_DMA | CQ_ARMED, &shared->armed);
>
> /*
> * Now read back shared->armed to make the PCI
> * write synchronous. This is necessary for
> * correct cq notification semantics.
> */
> - {
> - volatile char c;
> - c = shared->armed;
> - }
> + readb(&shared->armed);
>
> return 0;
> }
> @@ -250,7 +247,7 @@ static int c2_alloc_cq_buf(struct c2_mq
> q_size,
> msg_size,
> (u8 *) pool_start,
> - 0, /* peer (currently unknown) */
> + NULL, /* peer (currently unknown) */
> C2_MQ_HOST_TARGET);
>
> return 0;
> @@ -320,8 +317,7 @@ int c2_init_cq(struct c2_dev *c2dev, int
> cq->adapter_handle = reply->cq_handle;
> cq->mq.index = be32_to_cpu(reply->mq_index);
>
> - peer_pa =
> - (unsigned long) (c2dev->pa + be32_to_cpu(reply->adapter_shared));
> + peer_pa = c2dev->pa + be32_to_cpu(reply->adapter_shared);
> cq->mq.peer = ioremap_nocache(peer_pa, PAGE_SIZE);
> if (!cq->mq.peer) {
> err = -ENOMEM;
More information about the general
mailing list