cell: Fix off-by-one error in spu_dcache_fetch_unaligned
authorIan Romanick <idr@us.ibm.com>
Sat, 23 Feb 2008 01:51:55 +0000 (17:51 -0800)
committerIan Romanick <idr@us.ibm.com>
Tue, 26 Feb 2008 00:18:08 +0000 (16:18 -0800)
This time the off-by-one error caused an extra qword to be fetched
under certain circumstances when the source ea was not qword aligned.

src/gallium/drivers/cell/spu/spu_dcache.c

index 3baeaea998183b826d5af46adec76d6e988aba26..a1701d80d18f57fb4cd297ca76d5cf0843e6fec5 100644 (file)
 
 /**
  * Fetch between arbitrary number of bytes from an unaligned address
+ *
+ * \param dst   Destination data buffer
+ * \param ea    Main memory effective address of source data
+ * \param size  Number of bytes to read
+ *
+ * \warning
+ * As is hinted by the type of the \c dst pointer, this function writes
+ * multiples of 16-bytes.
  */
 void
 spu_dcache_fetch_unaligned(qword *dst, unsigned ea, unsigned size)
 {
    const int shift = ea & 0x0f;
-   const unsigned aligned_start_ea = ea & ~0x0f;
-   const unsigned aligned_end_ea = ROUNDUP16(ea + size);
-   const unsigned num_entries = (aligned_end_ea - aligned_start_ea) / 16;
+   const unsigned read_size = ROUNDUP16(size + shift);
+   const unsigned last_read = ROUNDUP16(ea + size);
+   const qword *const last_write = dst + (ROUNDUP16(size) / 16);
    unsigned i;
 
 
    if (shift == 0) {
       /* Data is already aligned.  Fetch directly into the destination buffer.
        */
-      for (i = 0; i < num_entries; i++) {
-         dst[i] = cache_rd(data, ea + (i * 16));
+      for (i = 0; i < size; i += 16) {
+         *(dst++) = cache_rd(data, ea + i);
       }
    } else {
-      qword tmp[2] ALIGN16_ATTRIB;
-
+      qword hi;
 
-      tmp[0] = cache_rd(data, (ea & ~0x0f));
-      for (i = 0; i < (num_entries & ~1); i++) {
-         const unsigned curr = i & 1;
-         const unsigned next = curr ^ 1;
 
-         tmp[next] = cache_rd(data, (ea & ~0x0f) + (next * 16));
-
-         dst[i] = si_or((qword) spu_slqwbyte(tmp[curr], shift),
-                        (qword) spu_rlmaskqwbyte(tmp[next], shift - 16));
+      /* Please exercise extreme caution when modifying this code.  This code
+       * must not read past the end of the page containing the source data,
+       * and it must not write more than ((size + 15) / 16) qwords to the
+       * destination buffer.
+       */
+      ea &= ~0x0f;
+      hi = cache_rd(data, ea);
+      for (i = 16; i < read_size; i += 16) {
+         qword lo = cache_rd(data, ea + i);
+
+         *(dst++) = si_or((qword) spu_slqwbyte(hi, shift),
+                          (qword) spu_rlmaskqwbyte(lo, shift - 16));
+         hi = lo;
       }
 
-      if (i < num_entries) {
-         dst[i] = si_or((qword) spu_slqwbyte(tmp[(i & 1)], shift),
-                        si_il(0));
+      if (dst != last_write) {
+         *(dst++) = si_or((qword) spu_slqwbyte(hi, shift), si_il(0));
       }
    }
+   
+   ASSERT((ea + i) == last_read);
+   ASSERT(dst == last_write);
 }