svn commit: r216363 - head/sys/dev/mps
Kenneth D. Merry
ken at FreeBSD.org
Fri Dec 10 21:45:10 UTC 2010
Author: ken
Date: Fri Dec 10 21:45:10 2010
New Revision: 216363
URL: http://svn.freebsd.org/changeset/base/216363
Log:
Fix an event handling bug with the mps(4) driver.
This bug manifested itself after repeated device arrivals and
departures. The root of the problem was that the last entry in the
reply array wasn't initialized/allocated. So every time we got
around to that event, we had a bogus address.
There were a couple more problems with the code that are also fixed:
- The reply mechanism was being treated as sequential (indexed by
sc->replycurindex) even though the spec says that the driver
should use the ReplyFrameAddress field of the post queue
descriptor to figure out where the reply is. There is no
guarantee that the reply descriptors will be used in sequential
order.
- The second word of the reply post queue descriptor wasn't being
checked in mps_intr_locked() to make sure that it wasn't
0xffffffff. So the driver could potentially come across a
partially DMAed descriptor.
- The number of replies allocated was one less than the actual
size of the queue. Instead, it was the size of the number of
replies that can be used at one time. (Which is one less than
the size of the queue.)
mps.c: When initializing the entries in the reply free
queue, make sure we initialize the full number that
we tell the chip we have (sc->fqdepth), not the
number that can be used at any one time (sc->num_replies).
When allocating replies, make sure we allocate the
number of replies that we've told the chip exist,
not just the number that can be used simultaneously.
Use the ReplyFrameAddress field of the post queue
descriptor to figure out which reply is being
referenced. This is what the spec says to do, and
the spec doesn't guarantee that the replies will be
used in order.
Put a check in to verify that the reply address passed
back from the card is valid. (Panic if it isn't, we'll
panic when we try to deference the reply pointer in any
case.)
In mps_intr_locked(), verify that the second word of the
post queue descriptor is not 0xffffffff in addition to
verifying that the unused flag is not set, so we can
make sure we didn't get a partially DMAed descriptor.
Remove references to sc->replycurindex, it isn't needed
now.
mpsvar.h: Remove replycurindex from the softc, it isn't needed now.
Reviewed by: scottl
Modified:
head/sys/dev/mps/mps.c
head/sys/dev/mps/mpsvar.h
Modified: head/sys/dev/mps/mps.c
==============================================================================
--- head/sys/dev/mps/mps.c Fri Dec 10 21:43:20 2010 (r216362)
+++ head/sys/dev/mps/mps.c Fri Dec 10 21:45:10 2010 (r216363)
@@ -43,6 +43,7 @@ __FBSDID("$FreeBSD$");
#include <sys/malloc.h>
#include <sys/uio.h>
#include <sys/sysctl.h>
+#include <sys/endian.h>
#include <machine/bus.h>
#include <machine/resource.h>
@@ -607,9 +608,16 @@ mps_alloc_queues(struct mps_softc *sc)
static int
mps_alloc_replies(struct mps_softc *sc)
{
- int rsize;
+ int rsize, num_replies;
- rsize = sc->facts->ReplyFrameSize * sc->num_replies * 4;
+ /*
+ * sc->num_replies should be one less than sc->fqdepth. We need to
+ * allocate space for sc->fqdepth replies, but only sc->num_replies
+ * replies can be used at once.
+ */
+ num_replies = max(sc->fqdepth, sc->num_replies);
+
+ rsize = sc->facts->ReplyFrameSize * num_replies * 4;
if (bus_dma_tag_create( sc->mps_parent_dmat, /* parent */
4, 0, /* algnmnt, boundary */
BUS_SPACE_MAXADDR_32BIT,/* lowaddr */
@@ -782,11 +790,19 @@ mps_init_queues(struct mps_softc *sc)
memset((uint8_t *)sc->post_queue, 0xff, sc->pqdepth * 8);
+ /*
+ * According to the spec, we need to use one less reply than we
+ * have space for on the queue. So sc->num_replies (the number we
+ * use) should be less than sc->fqdepth (allocated size).
+ */
if (sc->num_replies >= sc->fqdepth)
return (EINVAL);
- for (i = 0; i < sc->num_replies; i++)
- sc->free_queue[i] = sc->reply_busaddr + i * sc->facts->ReplyFrameSize * 4;
+ /*
+ * Initialize all of the free queue entries.
+ */
+ for (i = 0; i < sc->fqdepth; i++)
+ sc->free_queue[i] = sc->reply_busaddr + (i * sc->facts->ReplyFrameSize * 4);
sc->replyfreeindex = sc->num_replies;
return (0);
@@ -907,7 +923,6 @@ mps_attach(struct mps_softc *sc)
* replies.
*/
sc->replypostindex = 0;
- sc->replycurindex = 0;
mps_regwrite(sc, MPI2_REPLY_FREE_HOST_INDEX_OFFSET, sc->replyfreeindex);
mps_regwrite(sc, MPI2_REPLY_POST_HOST_INDEX_OFFSET, 0);
@@ -1211,7 +1226,8 @@ mps_intr_locked(void *data)
desc = &sc->post_queue[pq];
flags = desc->Default.ReplyFlags &
MPI2_RPY_DESCRIPT_FLAGS_TYPE_MASK;
- if (flags == MPI2_RPY_DESCRIPT_FLAGS_UNUSED)
+ if ((flags == MPI2_RPY_DESCRIPT_FLAGS_UNUSED)
+ || (desc->Words.High == 0xffffffff))
break;
switch (flags) {
@@ -1224,9 +1240,36 @@ mps_intr_locked(void *data)
uint32_t baddr;
uint8_t *reply;
+ /*
+ * Re-compose the reply address from the address
+ * sent back from the chip. The ReplyFrameAddress
+ * is the lower 32 bits of the physical address of
+ * particular reply frame. Convert that address to
+ * host format, and then use that to provide the
+ * offset against the virtual address base
+ * (sc->reply_frames).
+ */
+ baddr = le32toh(desc->AddressReply.ReplyFrameAddress);
reply = sc->reply_frames +
- sc->replycurindex * sc->facts->ReplyFrameSize * 4;
- baddr = desc->AddressReply.ReplyFrameAddress;
+ (baddr - ((uint32_t)sc->reply_busaddr));
+ /*
+ * Make sure the reply we got back is in a valid
+ * range. If not, go ahead and panic here, since
+ * we'll probably panic as soon as we deference the
+ * reply pointer anyway.
+ */
+ if ((reply < sc->reply_frames)
+ || (reply > (sc->reply_frames +
+ (sc->fqdepth * sc->facts->ReplyFrameSize * 4)))) {
+ printf("%s: WARNING: reply %p out of range!\n",
+ __func__, reply);
+ printf("%s: reply_frames %p, fqdepth %d, "
+ "frame size %d\n", __func__,
+ sc->reply_frames, sc->fqdepth,
+ sc->facts->ReplyFrameSize * 4);
+ printf("%s: baddr %#x,\n", __func__, baddr);
+ panic("Reply address out of range");
+ }
if (desc->AddressReply.SMID == 0) {
mps_dispatch_event(sc, baddr,
(MPI2_EVENT_NOTIFICATION_REPLY *) reply);
@@ -1236,8 +1279,6 @@ mps_intr_locked(void *data)
cm->cm_reply_data =
desc->AddressReply.ReplyFrameAddress;
}
- if (++sc->replycurindex >= sc->fqdepth)
- sc->replycurindex = 0;
break;
}
case MPI2_RPY_DESCRIPT_FLAGS_TARGETASSIST_SUCCESS:
Modified: head/sys/dev/mps/mpsvar.h
==============================================================================
--- head/sys/dev/mps/mpsvar.h Fri Dec 10 21:43:20 2010 (r216362)
+++ head/sys/dev/mps/mpsvar.h Fri Dec 10 21:45:10 2010 (r216363)
@@ -135,7 +135,6 @@ struct mps_softc {
TAILQ_HEAD(, mps_command) tm_list;
int replypostindex;
int replyfreeindex;
- int replycurindex;
struct resource *mps_regs_resource;
bus_space_handle_t mps_bhandle;
More information about the svn-src-head
mailing list