G2 and dcbi

[Neil Ludban <nludban%ydi.com@localhost>]

Hello,

According to many footnotes in the G2 PowerPC Core Reference Manual,
"the dcbi instruction should never be used on the G2 core".  The only
explanation given is here:

        3.2.6.3.1   Supervisor-Level Cache Management Instruction

        The supervisor-level cache management instruction in the PowerPC
        architecture, dcbi, should not be used on the G2 core. If it is
        used it can cause a data storage interrupt. The user-level dcbf
        instruction, described in Section 3.2.5.3, "Memory Control
        Instructions--VEA" and Section 4.8, "Cache Control Instructions,"
        should be used when the program needs to invalidate cache blocks.

This change in sys/arch/powerpc/powerpc/bus_dma.c:_bus_dmamap_sync():

        case BUS_DMASYNC_PREREAD:
                [ dcbf partial leading cache line ]
                [ dcbf partial trailing cache line ]
                /* FALLTHROUGH */
        case BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE:
        case BUS_DMASYNC_POSTREAD:
                /*
                 * The contents will have changed, make sure to remove
                 * them from the cache.  Note: some implementation
                 * implement dcbi identically to dcbf.  Thus if the
                 * cacheline has data, it will be written to memory.
                 * If the DMA is updating the same cacheline at the
                 * time, bad things can happen.
                 */
-               dcbi(addr, seglen, dcache_line_size);
+               dcbf(addr, seglen, dcache_line_size);
                break;

appears to have fixed a whole range of stability issues on an MPC8272
based system including corrupt kernel memory while mounting NFS root,
wireless cards hanging, DIAGNOSTIC assertions failing (usually related
to pmap and pool_cache), core dumps and MCHK exceptions when compiling.

Does this look like a correct fix?  I'm still suspicious because dcbi
is apparently working for others (right?), and it's presumably
slower to flush the cache than invalidate, so maybe this is just
avoiding a race condition somewhere else.

-Neil