CPU Cache and busdma usage in USB

[Hans Petter Selasky]

Hi,

On Wednesday 08 July 2009 12:16:39 Piotr Zięcik wrote:
> Wednesday 08 July 2009 11:03:43 Hans Petter Selasky napisał(a):
> > And what about my patch suggestion in my previous e-mail having the same
> > subject. Does it work?
>
> I have tested it and it does not work. By the way Writeback before
> Writeback Invalidate did not change cache behaviour too much. Writeback
> invalidate means flush all modified cache lines and then invalidate cache.

By flush you mean write from CPU cache to RAM, right. And nothing else? You 
don't mean discard CPU cache by "flush" ???

>
> However looking into logs which I have sent you yesterdat I see one
> difference which may be significant. My patch changes Invalidate into
> Writeback Invalidate. In original code if driver write something to memory
> and then invalidate cache

If that is the case I'm very surprised. Could you make another printout for 
me. Compile the kernel with KDB, and add a call to "kdb_backtrace()" in 
bus_dmamap_sync() before printing out the writeback and/or invalidate, so that 
I can see the backtrace.

> , the write will be lost.

I'm aware about that.

> With my patch after
> change will be written to memory and then cache will be invalidated. What
> do you think ?

I still cannot see that I'm using the wrong flags when calling 
bus_dmamap_sync().

You are absoultely sure that your USB code is indentical to the now stock 8-
current USB code, and that there are no integration issues lurking?


In the USB "usb_pc_cpu_flush()" is used after CPU write operations to flush 
the CPU cache to RAM. "usb_pc_cpu_invalidate()" is used before CPU read 
operations. So far so good?

You are arguing that:

BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD

flags do not cause a CPU invalidate?

Whilst the busdma manual is clear, that these flags are used after "an update 
of host memory by the device and prior to CPU access to host memory".

             BUS_DMASYNC_POSTREAD   Perform any synchronization required after
                                    an update of host memory by the device and
                                    prior to CPU access to host memory.

             BUS_DMASYNC_POSTWRITE  Perform any synchronization required after
                                    device access to host memory.

And that:

             BUS_DMASYNC_PREREAD    Perform any synchronization required prior
                                    to an update of host memory by the device.

             BUS_DMASYNC_PREWRITE   Perform any synchronization required after
                                    an update of host memory by the CPU and
                                    prior to device access to host memory.

flags do not cause a CPU flush prior to device acccess.


/*------------------------------------------------------------------------*
 *      usb_pc_cpu_invalidate - invalidate CPU cache
 *------------------------------------------------------------------------*/
void
usb_pc_cpu_invalidate(struct usb_page_cache *pc)
{
        if (pc->page_offset_end == pc->page_offset_buf) {
                /* nothing has been loaded into this page cache! */
                return;
        }
        bus_dmamap_sync(pc->tag, pc->map,
            BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD);
}

/*------------------------------------------------------------------------*
 *      usb_pc_cpu_flush - flush CPU cache
 *------------------------------------------------------------------------*/
void
usb_pc_cpu_flush(struct usb_page_cache *pc)
{
        if (pc->page_offset_end == pc->page_offset_buf) {
                /* nothing has been loaded into this page cache! */
                return;
        }
        bus_dmamap_sync(pc->tag, pc->map,
            BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
}

Thanks for being patient. If there is a bug, it will be fixed.

--HPS