Re: [PATCH] 2.4 use __dma_buffer in scsi.h

Re: [PATCH] 2.4 use __dma_buffer in scsi.h

[James Bottomley]

> Use __dma_buffer macro to align sense_buffer member of Scsi_Cmnd.
[...]

> -       unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE];              /* obtained by REQUEST SENSE
> +       unsigned char sense_buffer[SCSI_SENSE_BUFFERSIZE] __dma_buffer;         /* obtained by REQUEST SENSE

This is actually unnecessary.  We've already had this discussion on the parisc 
mailing lists (over a year ago, I think).

The SCSI subsystem is designed with this type of cache incoherency problem in 
mind.  The Scsi_Cmnd structure has an ownership model to forestall cache line 
bouncing.  The idea is that when you want to dma to/from components of a 
Scsi_Cmnd, you can only do it when the ownership is SCSI_OWNER_LOW_LEVEL, 
which guarantees that nothing in the mid layer will touch the command and 
trigger an incoherency (at least until it times out).  The low level driver is 
supposed to know about the cache incoherency problem, and so avoids touching 
the structure between cache line invalidation/writeback and DMA completion, 
thus, with a correctly implemented driver, there should be no possibility of 
DMA corruption.

Each of the Scsi_Cmnd blocks is individually allocated with kmalloc, so 
they're guaranteed to be non-interfering when it comes to DMA.

Incidentally, if you're really going to insist on padding the structure, some 
drivers also use the cmnd element to DMA the command from, so that should be 
aligned as well.  But I think the best course of action is to fix any low 
level drivers that are not following the cache rules rather than expanding 
Scsi_Cmnd.

James Bottomley

Re: [PATCH] 2.4 use __dma_buffer in scsi.h

[Roland Dreier]

>>>>> "James" == James Bottomley <James.Bottomley@steeleye.com> writes:

    James> This is actually unnecessary.  We've already had this
    James> discussion on the parisc mailing lists (over a year ago, I
    James> think).

Fair enough, the only reason I came up with this patch is that I
looked at all unaligned DMA done by USB, and
drivers/usb/storage/transport.c does DMA into sense_buffer.

    James> The SCSI subsystem is designed with this type of cache
    James> incoherency problem in mind.  The Scsi_Cmnd structure has
    James> an ownership model to forestall cache line bouncing.  The
    James> idea is that when you want to dma to/from components of a
    James> Scsi_Cmnd, you can only do it when the ownership is
    James> SCSI_OWNER_LOW_LEVEL, which guarantees that nothing in the
    James> mid layer will touch the command and trigger an incoherency
    James> (at least until it times out).  The low level driver is
    James> supposed to know about the cache incoherency problem, and
    James> so avoids touching the structure between cache line
    James> invalidation/writeback and DMA completion, thus, with a
    James> correctly implemented driver, there should be no
    James> possibility of DMA corruption.

Out of curiousity, how do you deal with someone writing to Scsi_Cmnd
_before_ the DMA and having that data lost when pci_map_single() with
PCI_DMA_FROMDEVICE invalidates the cache before writeback?

Best,
  Roland

Re: [PATCH] 2.4 use __dma_buffer in scsi.h

[James Bottomley]

roland@topspin.com said:
> Out of curiousity, how do you deal with someone writing to Scsi_Cmnd
> _before_ the DMA and having that data lost when pci_map_single() with
> PCI_DMA_FROMDEVICE invalidates the cache before writeback? 

The the pci_map_single with PCI_DMA_FROMDEVICE is supposed to do a writeback 
invalidate cycle on the cache when pci_sync_single is called (at least this is 
how it is implemented on parisc) so any writes up to the sync point are 
flushed to memory before the cache line is trashed.

James