libata NCQ implementation questions

libata NCQ implementation questions

[Mikael Pettersson]

I've started working on NCQ support for sata_promise,
and I have a few questions regarding NCQ and libata:

1. Can I rely not seeing any non-NCQ commands while there
   are uncompleted NCQ commands on a port?

   I _believe_, from the comment at ata_std_qc_defer(), that
   this is the case, but I'd like to get that confirmed.

2. Assuming NCQ and non-NCQ commands cannot be mixed, is there
   a defined way to detect when a port transitions from non-NCQ
   to NCQ mode and vice versa, or do I have to detect that myself?

   The mode affects the programming of the Promise SATA "sequence
   counter control" registers.

3. Is qc->tag defined (to zero for instance) for non-NCQ commands?

   For sata_promise NCQ I need to allocate packets and prds from
   per-port pools indexed by tag. To handle non-NCQ commands it's
   easiest to assume they map to tag 0, but the code to set a tag
   conditionally based on qc->tf.protocol is a bit ugly.

4. What are these "internal commands" that map to ATA_TAG_INTERNAL?
   Are they NCQ or not?
   Does the existence of ATA_TAG_INTERNAL limit queue depth for NCQ?

5. Does dmam_alloc_coherent() give the same alignment guarantees
   that pci_alloc_consistent() does? That is, both CPU and bus
   addresses will be aligned to the smallest PAGE_SIZE order that
   fits the requested size.

   On x86, pci_alloc_consistent() is implemented directly on top of
   dma_alloc_coherent(), which would imply that dma_alloc_coherent()
   has at least as strong alignment guarantees as pci_alloc_consistent().
   And since dmam_alloc_coherent() is documented to be exactly like
   dma_alloc_coherent() except for being managed, similar alignment
   guarantees for dmam_alloc_coherent() should be true. However, this
   refers to one platform's implementation; I can't find any generic
   documentation about alignment for dma_alloc_coherent().

   I've looked at pci_set_dma_mask(), but as far as I can tell, it
   can only express size limits of addresses, it cannot express alignment
   constraints.

   The issue is that NCQ on Promise SATA is done by an FPDMA engine
   that unfortunately has a rather annoying addressing limitation:
   it requires that all NCQ packet and SG data for all active tags
   on a port is located in a single physical memory region that does
   not cross a 64KB physical memory boundary.

/Mikael

Re: libata NCQ implementation questions

[Grant Grundler]

On Sun, May 11, 2008 at 4:19 PM, Mikael Pettersson <mikpe@it.uu.se> wrote:
> I've started working on NCQ support for sata_promise,
> and I have a few questions regarding NCQ and libata:
...
> 5. Does dmam_alloc_coherent() give the same alignment guarantees
>   that pci_alloc_consistent() does? That is, both CPU and bus
>   addresses will be aligned to the smallest PAGE_SIZE order that
>   fits the requested size.
>
>   On x86, pci_alloc_consistent() is implemented directly on top of
>   dma_alloc_coherent(), which would imply that dma_alloc_coherent()
>   has at least as strong alignment guarantees as pci_alloc_consistent().
>   And since dmam_alloc_coherent() is documented to be exactly like
>   dma_alloc_coherent() except for being managed, similar alignment
>   guarantees for dmam_alloc_coherent() should be true. However, this
>   refers to one platform's implementation; I can't find any generic
>   documentation about alignment for dma_alloc_coherent().

There is no guarantee on alignment. :(


>   I've looked at pci_set_dma_mask(), but as far as I can tell, it
>   can only express size limits of addresses, it cannot express alignment
>   constraints.

Correct. The section on dma_pool_create() in Documentation/DMA-API.txt
does provide a means for the driver to express alignment requirements.
But I suspect this only solves part of your problem.

>   The issue is that NCQ on Promise SATA is done by an FPDMA engine
>   that unfortunately has a rather annoying addressing limitation:
>   it requires that all NCQ packet and SG data for all active tags
>   on a port is located in a single physical memory region that does
>   not cross a 64KB physical memory boundary.

Could the driver map 64KB space and test if the DMA mapping is
properly aligned?

I'm not sure if it would have to fall back to a bounce-buffer like scheme
to copy the data to a "well aligned" buffer that has the right alignment.
That's obviously not desirable since it's a huge perf hit.

Anyway, This issue was raised by Fujita Tomonori at LSF2008:
    http://iou.parisc-linux.org/lsf2008/IO-DMA_Representations-fujita_tomonori.pdf

I posted a summary of the discussion here (search for "DMA" in the file):
    http://iou.parisc-linux.org/lsf2008/SUMMARY-Storage.txt

The relevant text is:
    Slide 7 proposes adding a new "struct device_dma_parameters" in order
    to put all the DMA related parameters into one place and make
    them visible to IOMMUs. This idea was rejected as over kill.
    The preferred solution (jejb and others) was to just add the
    missing fields to struct device.  Slide 10 summarized where all
    the various parameters currently live and adding another
    struct to deal with it was just adding more pointers that
    we didn't really need. Most devices do DMA and adding the
    fields directly to "struct device".

I don't have access at the moment to a current git tree but
I'm sure someone else can determine if any of the above has
happened in 2.6.26 yet.

hth,
grant

Re: libata NCQ implementation questions

[Mikael Pettersson]

Grant Grundler writes:
 > On Sun, May 11, 2008 at 4:19 PM, Mikael Pettersson <mikpe@it.uu.se> wrote:
 > > I've started working on NCQ support for sata_promise,
 > > and I have a few questions regarding NCQ and libata:
 > ...
 > > 5. Does dmam_alloc_coherent() give the same alignment guarantees
 > >   that pci_alloc_consistent() does? That is, both CPU and bus
 > >   addresses will be aligned to the smallest PAGE_SIZE order that
 > >   fits the requested size.
 > >
 > >   On x86, pci_alloc_consistent() is implemented directly on top of
 > >   dma_alloc_coherent(), which would imply that dma_alloc_coherent()
 > >   has at least as strong alignment guarantees as pci_alloc_consistent().
 > >   And since dmam_alloc_coherent() is documented to be exactly like
 > >   dma_alloc_coherent() except for being managed, similar alignment
 > >   guarantees for dmam_alloc_coherent() should be true. However, this
 > >   refers to one platform's implementation; I can't find any generic
 > >   documentation about alignment for dma_alloc_coherent().
 > 
 > There is no guarantee on alignment. :(

Thanks, I was afraid of that.

 > >   I've looked at pci_set_dma_mask(), but as far as I can tell, it
 > >   can only express size limits of addresses, it cannot express alignment
 > >   constraints.
 > 
 > Correct. The section on dma_pool_create() in Documentation/DMA-API.txt
 > does provide a means for the driver to express alignment requirements.
 > But I suspect this only solves part of your problem.

Maybe. I'll have a look.

 > >   The issue is that NCQ on Promise SATA is done by an FPDMA engine
 > >   that unfortunately has a rather annoying addressing limitation:
 > >   it requires that all NCQ packet and SG data for all active tags
 > >   on a port is located in a single physical memory region that does
 > >   not cross a 64KB physical memory boundary.
 > 
 > Could the driver map 64KB space and test if the DMA mapping is
 > properly aligned?
 > 
 > I'm not sure if it would have to fall back to a bounce-buffer like scheme
 > to copy the data to a "well aligned" buffer that has the right alignment.
 > That's obviously not desirable since it's a huge perf hit.

I don't think bounce buffers are meaningful for NCQ. Command
execution is delayed and completed in unpredictable order, so
I'd need a largish valid buffer anyway to make that work.

One workaround is to overallocate and do my own alignment.

Or one could just disable NCQ if the special allocation fails
and fall back to the current one-command-at-a-time scheme.

/Mikael

Re: libata NCQ implementation questions

[Mark Lord]

Mikael Pettersson wrote:
> I've started working on NCQ support for sata_promise,
> and I have a few questions regarding NCQ and libata:
> 
> 1. Can I rely not seeing any non-NCQ commands while there
>    are uncompleted NCQ commands on a port?
..

No, you cannot.  So you'll have to code a .qc_defer() method
to hold them off in that case.  For a *really* good example,
see my recently deployed mv_qc_defer() function in sata_mv.c
as of linux-2.6.26-rc2.

> 3. Is qc->tag defined (to zero for instance) for non-NCQ commands?
..

The tag always has a valid value, NCQ or not.

> 4. What are these "internal commands" that map to ATA_TAG_INTERNAL?
>    Are they NCQ or not?
..

Not NCQ, but a valid tag number regardless.
libata reserves one tag for use during error-handling (EH),
primarily for issing the READ_LOG_EXT_10H command to find out which
NCQ command failed and what sector it failed on.


>    Does the existence of ATA_TAG_INTERNAL limit queue depth for NCQ?
..

Yup.  Max depth is 31 instead of 32 for most devices.

> 5. Does dmam_alloc_coherent() give the same alignment guarantees
>    that pci_alloc_consistent() does? That is, both CPU and bus
>    addresses will be aligned to the smallest PAGE_SIZE order that
>    fits the requested size.
..

In sata_mv, I ran into those same concerns, and ended up creating
a couple of pools for local use by the driver.  It uses dmam_pool_create()
(note the extra "m" in "dmam_"), dma_pool_alloc(), and dma_pool_free().

If you just search for "_pool" in sata_mv, you'll find all of the calls
(not many).  The use of "dmam_pool_create()" assures that things will be
cleaned up properly/automatically if the controller ever "goes away".

Cheers

Re: libata NCQ implementation questions

[Mikael Pettersson]

Mark Lord writes:
 > Mikael Pettersson wrote:
 > > I've started working on NCQ support for sata_promise,
 > > and I have a few questions regarding NCQ and libata:
 > > 
 > > 1. Can I rely not seeing any non-NCQ commands while there
 > >    are uncompleted NCQ commands on a port?
 > ..
 > 
 > No, you cannot.  So you'll have to code a .qc_defer() method
 > to hold them off in that case.  For a *really* good example,
 > see my recently deployed mv_qc_defer() function in sata_mv.c
 > as of linux-2.6.26-rc2.

Hmm, so I misinterpreted ata_std_qc_defer()? Ok, I'll look
at sata_mv's version.

...

 > > 5. Does dmam_alloc_coherent() give the same alignment guarantees
 > >    that pci_alloc_consistent() does? That is, both CPU and bus
 > >    addresses will be aligned to the smallest PAGE_SIZE order that
 > >    fits the requested size.
 > ..
 > 
 > In sata_mv, I ran into those same concerns, and ended up creating
 > a couple of pools for local use by the driver.  It uses dmam_pool_create()
 > (note the extra "m" in "dmam_"), dma_pool_alloc(), and dma_pool_free().
 > 
 > If you just search for "_pool" in sata_mv, you'll find all of the calls
 > (not many).  The use of "dmam_pool_create()" assures that things will be
 > cleaned up properly/automatically if the controller ever "goes away".

Thanks, this sounds like a decent solution.

/Mikael

Re: libata NCQ implementation questions

[Tejun Heo]

Mikael Pettersson wrote:
> Mark Lord writes:
>  > Mikael Pettersson wrote:
>  > > I've started working on NCQ support for sata_promise,
>  > > and I have a few questions regarding NCQ and libata:
>  > > 
>  > > 1. Can I rely not seeing any non-NCQ commands while there
>  > >    are uncompleted NCQ commands on a port?
>  > ..
>  > 
>  > No, you cannot.  So you'll have to code a .qc_defer() method
>  > to hold them off in that case.  For a *really* good example,
>  > see my recently deployed mv_qc_defer() function in sata_mv.c
>  > as of linux-2.6.26-rc2.
> 
> Hmm, so I misinterpreted ata_std_qc_defer()? Ok, I'll look
> at sata_mv's version.

ata_std_qc_defer() which is inherited from sata_port_ops does guarantee 
that NCQ and non-NCQ commands don't mix on a single device.  Mark, 
you're talking about PMP, right?

Thanks.

-- 
tejun

Re: libata NCQ implementation questions

[Tejun Heo]

Hello,

Mikael Pettersson wrote:
> I've started working on NCQ support for sata_promise,
> and I have a few questions regarding NCQ and libata:

Great.

> 1. Can I rely not seeing any non-NCQ commands while there
>    are uncompleted NCQ commands on a port?
> 
>    I _believe_, from the comment at ata_std_qc_defer(), that
>    this is the case, but I'd like to get that confirmed.

Yes, you can rely on that.

> 2. Assuming NCQ and non-NCQ commands cannot be mixed, is there
>    a defined way to detect when a port transitions from non-NCQ
>    to NCQ mode and vice versa, or do I have to detect that myself?
> 
>    The mode affects the programming of the Promise SATA "sequence
>    counter control" registers.

There's no defined way.  You'll probably need to add ->last_protocol or 
->was_ncq in LLD private structure.

> 3. Is qc->tag defined (to zero for instance) for non-NCQ commands?
> 
>    For sata_promise NCQ I need to allocate packets and prds from
>    per-port pools indexed by tag. To handle non-NCQ commands it's
>    easiest to assume they map to tag 0, but the code to set a tag
>    conditionally based on qc->tf.protocol is a bit ugly.

For all non-NCQ, non-EH commands, the tag is fixed at 0.

> 4. What are these "internal commands" that map to ATA_TAG_INTERNAL?
>    Are they NCQ or not?
>    Does the existence of ATA_TAG_INTERNAL limit queue depth for NCQ?

Yeah, it reserves tag 31 for EH commands.  We might as well use tag 32 
for EH commands and let LLD map it to whatever tag it can use for EH 
commands but Jeff didn't like using all the tags as it makes 0xffffffff 
a valid status value.

Note that depending on LLD implementation, you might want to remap tag 
31 to different hardware tag.  sata_sil24 does this mainly because it 
made polled execution implementation easier.

Thanks.

-- 
tejun

Re: libata NCQ implementation questions

[Mikael Pettersson]

Tejun Heo writes:
...
 > > 4. What are these "internal commands" that map to ATA_TAG_INTERNAL?
 > >    Are they NCQ or not?
 > >    Does the existence of ATA_TAG_INTERNAL limit queue depth for NCQ?
 > 
 > Yeah, it reserves tag 31 for EH commands.  We might as well use tag 32 
 > for EH commands and let LLD map it to whatever tag it can use for EH 
 > commands but Jeff didn't like using all the tags as it makes 0xffffffff 
 > a valid status value.
 > 
 > Note that depending on LLD implementation, you might want to remap tag 
 > 31 to different hardware tag.  sata_sil24 does this mainly because it 
 > made polled execution implementation easier.

I think I'll remap them to 0 too. The tag ends up in a field
in the ATA packet header that's overloaded for NCQ tags and
non-NCQ packet "delay sequence ids". So far sata_promise has
always set this field to 0, and set up seqid #0 as /dev/null,
so I'm not comfortable allowing non-zero values into this field
for non-NCQ packets.

Thanks,

/Mikael

Re: libata NCQ implementation questions

[Mark Lord]

Tejun Heo wrote:
> Mikael Pettersson wrote:
>> Mark Lord writes:
>>  > Mikael Pettersson wrote:
>>  > > I've started working on NCQ support for sata_promise,
>>  > > and I have a few questions regarding NCQ and libata:
>>  > >  > > 1. Can I rely not seeing any non-NCQ commands while there
>>  > >    are uncompleted NCQ commands on a port?
>>  > ..
>>  >  > No, you cannot.  So you'll have to code a .qc_defer() method
>>  > to hold them off in that case.  For a *really* good example,
>>  > see my recently deployed mv_qc_defer() function in sata_mv.c
>>  > as of linux-2.6.26-rc2.
>>
>> Hmm, so I misinterpreted ata_std_qc_defer()? Ok, I'll look
>> at sata_mv's version.
> 
> ata_std_qc_defer() which is inherited from sata_port_ops does guarantee 
> that NCQ and non-NCQ commands don't mix on a single device.  Mark, 
> you're talking about PMP, right?
..

Ahh.. probably, yes.  I just saw it mixing PIO with NCQ and then
turfed it in favour of a more comprehensive mv_qc_defer().

But that was with a PM connected at the time, and I don't remember if
I also saw it without a PM.

Cheers