libata+SGIO: is .dma_boundary respected?

Jens / Jeff,

Each libata driver registers a .dma_boundary field with SCSI. This field is used to prevent merging of bio segments across a hardware limitation boundary, usually 0xffff.

This looks like it works for regular block I/O, but I'm not so sure about SGIO originated requests.

Any thoughts, or code you can point me to?

Quoted 6 lines · Mark Lord

Jens / Jeff,

Each libata driver registers a .dma_boundary field with SCSI. This field is used to prevent merging of bio segments across a hardware limitation boundary, usually 0xffff.

Quoted 4 lines

This looks like it works for regular block I/O, but I'm not so sure about SGIO originated requests.

Any thoughts, or code you can point me to?

Everything goes through the block layer, including SG_IO, so everyone agrees on the boundaries that must be respected.

scsi sets blk_queue_segment_boundary() then gets out of the way, for the most part. BIOVEC_SEG_BOUNDARY() is the macro that accesses this. Trace back and forth from there. You will probably run into a call to blk_recount_segments() in __bio_add_page(), or maybe you'll check the seg boundary from another path.

Quoted 11 lines · Jeff Garzik

Quoted 9 lines · Mark Lord

Jens / Jeff,

Each libata driver registers a .dma_boundary field with SCSI. This field is used to prevent merging of bio segments across a hardware limitation boundary, usually 0xffff.

This looks like it works for regular block I/O, but I'm not so sure about SGIO originated requests.

Quoted 6 lines

Quoted 1 line

Any thoughts, or code you can point me to?

Everything goes through the block layer, including SG_IO, so everyone agrees on the boundaries that must be respected.

scsi sets blk_queue_segment_boundary() then gets out of the way, for the most part. BIOVEC_SEG_BOUNDARY() is the macro that accesses this. Trace back and forth from there. You will probably run into a call to blk_recount_segments() in __bio_add_page(), or maybe you'll check the seg boundary from another path.

Yeah, I'm familiar with that part, and thanks for the note about SGIO.

So therefore, code to manage the dma_boundary is NOT necessary in sata drivers. Right? Currently we have in sata_mv.c:

    MV_DMA_BOUNDARY = 0xffff;
                while (sg_len) {
                        offset = addr & MV_DMA_BOUNDARY;
                        len = sg_len;
                        if ((offset + sg_len) > 0x10000)
                                len = 0x10000 - offset;
                ...

That whole block should be able to go, then.

Quoted 11 lines · Mark Lord

So therefore, code to manage the dma_boundary is NOT necessary in sata drivers. Right? Currently we have in sata_mv.c:

    MV_DMA_BOUNDARY = 0xffff;
                while (sg_len) {
                        offset = addr & MV_DMA_BOUNDARY;
                        len = sg_len;
                        if ((offset + sg_len) > 0x10000)
                                len = 0x10000 - offset;
                ...

Quoted 1 line

That whole block should be able to go, then.

Incorrect. :)

The idiot IOMMU layer may merge too aggressively, which is the reason for this code and similar code in ata_fill_sg(). The IOMMU stuff always happens at pci_map_sg() time, after the block layer gets out of the way.

Quoted 1 line · Jeff Garzik

The idiot IOMMU layer may merge too aggressively, which is the reason for this code and similar code in ata_fill_sg(). The IOMMU stuff always happens at pci_map_sg() time, after the block layer gets out of the way.

Ahh.. then how does the low-level driver know what to use for ".sg_tablesize"?

It cannot use the real hardware/driver value, because it may need to do request splitting. I wonder what the worst case number of splits required is, for each sg[] entry?

Quoted 10 lines · Mark Lord

Quoted 1 line · Jeff Garzik

The idiot IOMMU layer may merge too aggressively, which is the reason for this code and similar code in ata_fill_sg(). The IOMMU stuff always happens at pci_map_sg() time, after the block layer gets out of the way.

Ahh.. then how does the low-level driver know what to use for ".sg_tablesize"?

It cannot use the real hardware/driver value, because it may need to do request splitting. I wonder what the worst case number of splits required is, for each sg[] entry?

Mmmm. I suppose the answer is that the block layer guarantees no more than .sg_tablesize entries, and the IOMMU layer may reduce the segment count, but never increase it.

So the low-level driver should be able to safely use it's own internal hardware/driver limit when registering .sg_tablesize.

Mark Lord wrote:

Quoted 11 lines

Quoted 5 lines

Ahh.. then how does the low-level driver know what to use for ".sg_tablesize"?

It cannot use the real hardware/driver value, because it may need to do request splitting. I wonder what the worst case number of splits required is, for each sg[] entry?

Mmmm. I suppose the answer is that the block layer guarantees no more than .sg_tablesize entries, and the IOMMU layer may reduce the segment count, but never increase it.

Quoted 2 lines

So the low-level driver should be able to safely use it's own internal hardware/driver limit when registering .sg_tablesize.

The IOMMU layer can merge across 64k boundaries, yet still produce a worst case s/g entry count. Thus, you wind up with sg_tablesize entries, and splits still to be done.

That's why drivers that worry about 64k boundary have to give a false sg_tablesize to the SCSI layer: to reserve sufficient "true" s/g entries for the worst case IOMMU split.

But (as I replied to myself earlier), I think it is a non issue, because the IOMMU merging cannot produce more SG entries than there were originally. It may produce less, and the driver may then end up splitting them apart again, but it will never exceed what the block layer permitted in the first place.

Quoted 5 lines · Mark Lord

But (as I replied to myself earlier), I think it is a non issue, because the IOMMU merging cannot produce more SG entries than there were originally. It may produce less, and the driver may then end up splitting them apart again, but it will never exceed what the block layer permitted in the first place.

That says nothing about the boundaries upon which the IOMMU layer will or will not merge. Without the fix, the problem case happens when (for example) the IOMMU output produces sg_tablesize segments, but some of those segments cross a 64k boundary and need to be split.

Quoted 11 lines · Jeff Garzik

Quoted 5 lines · Mark Lord

But (as I replied to myself earlier), I think it is a non issue, because the IOMMU merging cannot produce more SG entries than there were originally. It may produce less, and the driver may then end up splitting them apart again, but it will never exceed what the block layer permitted in the first place.

That says nothing about the boundaries upon which the IOMMU layer will or will not merge. Without the fix, the problem case happens when (for example) the IOMMU output produces sg_tablesize segments, but some of those segments cross a 64k boundary and need to be split.

Yes, but the merging happens *after* the block layer has already guaranteed an sg list that respects what the driver told it.

So worst case, the IOMMU merges the entire sg list into a single multi-megabyte segment, and then the driver's fill_sg() function splits it all apart again while making up it's PRD list.

Even in that worst case, the number of segments for the PRD list will *always* be less than or equal to the size of the original block layer sg list.

So no need for hocus-pocus divide by 2.
Good thing, too, because "divide by 2" would also fail if the above stuff were not true.

Think about it some more.

Jens, you know more about this stuff than most folks:
What am I missing?

Seems to me that your reasoning is correct. It's a fact that the original block mapped sg lists satisfies all requirements of the device driver and/or hardware, otherwise would be a bug. The iommu may go nuts of course, but logically that new sg list should be choppable into the same requirements.

It would be much nicer if the iommu actually had some more knowledge, ideally the same requirements that the block layer is faced with. No driver should have to check the mapped sg list.

Jens Axboe wrote:
..

Quoted 5 lines

Seems to me that your reasoning is correct. It's a fact that the original block mapped sg lists satisfies all requirements of the device driver and/or hardware, otherwise would be a bug. The iommu may go nuts of course, but logically that new sg list should be choppable into the same requirements.

I just finished going through all of the arch implementations and, as near as I can tell, they only ever *merge* sg list items, and never create additional sg entries.

So low-level drivers (at present) can safely report their real limits, and then in their fill_sg() routines they can run around and split up any IOMMU merges that their hardware cannot tolerate.

Quoted 3 lines

It would be much nicer if the iommu actually had some more knowledge, ideally the same requirements that the block layer is faced with. No driver should have to check the mapped sg list.

Yup. Absolutely. So long as they continue to never *add* new sg entries (only doing merges instead), then I believe they just need to know the device's .dma_boundary parameter. We could pass this to them as an extra parameters, or perhaps embed it into the sg_list data structure somehow.

In the case of sata_mv on the Marvell 6081 (which I'm looking at this week) it's hardware limit is actually 0xffffffff rather than 0xffff.

I wonder how well Linux drivers in general deal with that on a 64-bit machine?

Quoted 3 lines

In the case of sata_mv on the Marvell 6081 (which I'm looking at this week) it's hardware limit is actually 0xffffffff rather than 0xffff.

If the limit is not 0xffff, then there's no need for any of this limitation junk. No s/g entry splitting after pci_map_sg(), no artificial sg_tablesize limitation, etc.

Quoted 5 lines · Jeff Garzik

Quoted 3 lines

In the case of sata_mv on the Marvell 6081 (which I'm looking at this week) it's hardware limit is actually 0xffffffff rather than 0xffff.

If the limit is not 0xffff, then there's no need for any of this limitation junk. No s/g entry splitting after pci_map_sg(), no artificial sg_tablesize limitation, etc.

Not even for a merged IOMMU segment that crosses the 4GB "boundary" ?

Quoted 10 lines · Mark Lord

Quoted 8 lines · Jeff Garzik

Quoted 4 lines

In the case of sata_mv on the Marvell 6081 (which I'm looking at this week)
it's hardware limit is actually 0xffffffff rather than 0xffff.

If the limit is not 0xffff, then there's no need for any of this limitation junk. No s/g entry splitting after pci_map_sg(), no artificial sg_tablesize limitation, etc.

Quoted 1 line

Not even for a merged IOMMU segment that crosses the 4GB "boundary" ?

Clarification: this is a 64-bit PCI(e/X) device, and the above query applies mainly to it's use in a 64-bit slot on a 64-bit kernel.

It's not clear to me whether this can be an issue on a 32-bit kernel on 36-bit hardware, though.

My explanation was for the block layer part of course, I'm hoping (did not check) that the iommu has similar sane defaults.

But this still really wants a unification of the dma restrictions...

Quoted 2 lines · Jens Axboe

My explanation was for the block layer part of course, I'm hoping (did not check) that the iommu has similar sane defaults.

Part of the problem is that the iommu doesn't know as much as the block layer.

Quoted 1 line

But this still really wants a unification of the dma restrictions...

Strongly agreed. ISTR JamesB had some concrete thoughts in that direction, but they never made it beyond an IRC channel and/or a few emails.

Strongly agreed. ISTR JamesB had some concrete thoughts in that direction, but they never made it beyond an IRC channel and/or a few emails.

Quoted 3 lines · Jeff Garzik

Strongly agreed. ISTR JamesB had some concrete thoughts in that direction, but they never made it beyond an IRC channel and/or a few emails.

Actually, I had a patch for it ... but it never really went anywhere. The argument being that the machines which would actually need it didn't use IDE anyway ...

Do <insert random device here> really never have segment or boundary restrictions outside of IDE? Seems to me that supporting that would be the conservative and sane thing to do.

Quoted 3 lines · Jens Axboe

Do <insert random device here> really never have segment or boundary restrictions outside of IDE? Seems to me that supporting that would be the conservative and sane thing to do.

Well the only machines that actually turn on virtual merging are sparc and parisc. They have a small list of "certified" devices for them, none of which seems to have aribtrary segment boundary restrictions (although most of them have the standard 4GB one).

When I brought this up the last time it degenerated into a slanging match over the value of virtual merging (which no-one can seem to provide a definitive answer to).

Jens Axboe wrote:
..

Quoted 5 lines

Seems to me that your reasoning is correct. It's a fact that the original block mapped sg lists satisfies all requirements of the device driver and/or hardware, otherwise would be a bug. The iommu may go nuts of course, but logically that new sg list should be choppable into the same requirements.

I just finished going through all of the arch implementations and, as near as I can tell, they only ever *merge* sg list items, and never create additional sg entries.

So low-level drivers (at present) can safely report their real limits, and then in their fill_sg() routines they can run around and split up any IOMMU merges that their hardware cannot tolerate.

Quoted 3 lines

It would be much nicer if the iommu actually had some more knowledge, ideally the same requirements that the block layer is faced with. No driver should have to check the mapped sg list.

Yup. Absolutely. So long as they continue to never *add* new sg entries (only doing merges instead), then I believe they just need to know the device's .dma_boundary parameter. We could pass this to them as an extra parameters, or perhaps embed it into the sg_list data structure somehow.

In the case of sata_mv on the Marvell 6081 (which I'm looking at this week)
it's hardware limit is actually 0xffffffff rather than 0xffff.

I wonder how well Linux drivers in general deal with that on a 64-bit machine?

Quoted 1 line · Mark Lord

Jens Axboe wrote:

Quoted 7 lines

Quoted 3 lines

It would be much nicer if the iommu actually had some more knowledge, ideally the same requirements that the block layer is faced with. No driver should have to check the mapped sg list.

Yup. Absolutely. So long as they continue to never *add* new sg entries (only doing merges instead), then I believe they just need to know the device's .dma_boundary parameter. We could pass this to them as an extra parameters, or perhaps embed it into the sg_list data structure somehow.

You want max size as well, so boundary and max size should be enough.

Oh, now there's a thought. How do we specify "max segment size" today? I'll need to make sure that sata_mv still does that (64KB), even though it doesn't care about crossing 64KB boundaries.

Quoted 3 lines · Mark Lord

Oh, now there's a thought. How do we specify "max segment size" today? I'll need to make sure that sata_mv still does that (64KB), even though it doesn't care about crossing 64KB boundaries.

Agreed. 50xx is the same: 64k segment size limit, dma boundary is really 0xffffffff. Good catch, I had made the mistaken assumption that there was the standard IDE 64k boundary as well.

Jens Axboe wrote:
..

Quoted 5 lines

Seems to me that your reasoning is correct. It's a fact that the original block mapped sg lists satisfies all requirements of the device driver and/or hardware, otherwise would be a bug. The iommu may go nuts of course, but logically that new sg list should be choppable into the same requirements.

I just finished going through all of the arch implementations and, as near as I can tell, they only ever *merge* sg list items, and never create additional sg entries.

One question though. Do IOMMU's preserve alignment? ie. Do they align 33k block on 64k boundary? I guess they do, just wanna make sure.

Quoted 18 lines · Jeff Garzik

Quoted 11 lines · Mark Lord

So therefore, code to manage the dma_boundary is NOT necessary in sata drivers. Right? Currently we have in sata_mv.c:

    MV_DMA_BOUNDARY = 0xffff;
                while (sg_len) {
                        offset = addr & MV_DMA_BOUNDARY;
                        len = sg_len;
                        if ((offset + sg_len) > 0x10000)
                                len = 0x10000 - offset;
                ...

Quoted 1 line

That whole block should be able to go, then.

Incorrect. :)

Quoted 1 line

The idiot IOMMU layer may merge too aggressively, which is the reason for this code and similar code in ata_fill_sg(). The IOMMU stuff always happens at pci_map_sg() time, after the block layer gets out of the way.

Ahh.. then how does the low-level driver know what to use for ".sg_tablesize"?

It cannot use the real hardware/driver value, because it may need to do request splitting. I wonder what the worst case number of splits required is, for each sg[] entry?