RDMA/nes: Remove volatile qualifier from struct nes_hw_cq.cq_vbase

Remove the volatile qualifier from the cq_vbase member of struct
nes_hw_cq, and add an rmb() in the one place where it looks like
access order might make a difference.  As usual, removing a volatile
qualifier in a declaration is actually a bug fix, since a volatile
qualifier is not sufficient to make sure that aggressively
out-of-order CPUs don't reorder things and cause incorrect results.

For example, a CPU might speculatively execute reads of other cqe
fields before the NIC hardware has written those fields and before it
has set the NES_CQE_VALID bit (even though those reads come after the
test of the NES_CQE_VALID bit in program order), but then when the CPU
actually executes the conditional test of the NES_CQE_VALID, the bit
has been set, and the CPU will proceed with the results of the earlier
speculative execution and end up using bogus data.

This also gets rid of the warning:

    drivers/infiniband/hw/nes/nes_verbs.c: In function 'nes_destroy_cq':
    drivers/infiniband/hw/nes/nes_verbs.c:1978: warning: passing argument 3 of 'pci_free_consistent' discards qualifiers from pointer target type

Signed-off-by: Roland Dreier <rolandd@cisco.com>

diff --git a/drivers/infiniband/hw/nes/nes_hw.h b/drivers/infiniband/hw/nes/nes_hw.h
index b7e2844f096b620489c0b338e65e400937c16c4a..8f36e231bdf5db1e54e54a2707fb0bd12b165d0c 100644 (file)
--- a/drivers/infiniband/hw/nes/nes_hw.h
+++ b/drivers/infiniband/hw/nes/nes_hw.h
@@ -905,7 +905,7 @@ struct nes_hw_qp {
 };
 
 struct nes_hw_cq {
-       struct nes_hw_cqe volatile *cq_vbase;   /* PCI memory for host rings */
+       struct nes_hw_cqe *cq_vbase;    /* PCI memory for host rings */
        void (*ce_handler)(struct nes_device *nesdev, struct nes_hw_cq *cq);
        dma_addr_t cq_pbase;    /* PCI memory for host rings */
        u16 cq_head;

diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
index f9a5d43908923081526b50e4eed3a709c5b4b8e0..ee74f7c7a6da85b3b163f0f1a5ab9171691705a5 100644 (file)
--- a/drivers/infiniband/hw/nes/nes_verbs.c
+++ b/drivers/infiniband/hw/nes/nes_verbs.c
@@ -1976,7 +1976,7 @@ static int nes_destroy_cq(struct ib_cq *ib_cq)
 
        if (nescq->cq_mem_size)
                pci_free_consistent(nesdev->pcidev, nescq->cq_mem_size,
-                               (void *)nescq->hw_cq.cq_vbase, nescq->hw_cq.cq_pbase);
+                                   nescq->hw_cq.cq_vbase, nescq->hw_cq.cq_pbase);
        kfree(nescq);
 
        return ret;
@@ -3610,6 +3610,12 @@ static int nes_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *entry)
        while (cqe_count < num_entries) {
                if (le32_to_cpu(nescq->hw_cq.cq_vbase[head].cqe_words[NES_CQE_OPCODE_IDX]) &
                                NES_CQE_VALID) {
+                       /*
+                        * Make sure we read CQ entry contents *after*
+                        * we've checked the valid bit.
+                        */
+                       rmb();
+
                        cqe = nescq->hw_cq.cq_vbase[head];
                        nescq->hw_cq.cq_vbase[head].cqe_words[NES_CQE_OPCODE_IDX] = 0;
                        u32temp = le32_to_cpu(cqe.cqe_words[NES_CQE_COMP_COMP_CTX_LOW_IDX]);