Re: [PATCH] SCSI: amd_iommu dma_boundary overflow

[Joerg Roedel]

Hi Eddie,

 On Tue, 2013-02-19 at 18:30 -0800, Eddie Wai wrote:
  The code seems correct as it make sense to impose the same hardware
  segment boundary limit on both the blk queue and the DMA code.  It would
  be an easy alternative to simply prevent the shost-dma_boundary from
  being set to DMA_BIT_MASK(64), but it seems more correct to fix the
  amd_iommu code itself to detect and handle this max 64-bit mask condition.

Thanks for tracking this problem down. It turns out that this code does
not only exist in the AMD IOMMU driver but also in other ones (Calgary
and GART at least, havn't checked all).

  --- a/drivers/iommu/amd_iommu.c
  +++ b/drivers/iommu/amd_iommu.c
  @@ -1526,11 +1526,14 @@ static unsigned long dma_ops_area_alloc(struct 
  device *dev,
  unsigned long boundary_size;
  unsigned long address = -1;
  unsigned long limit;
  +   unsigned long mask;
   
  next_bit = PAGE_SHIFT;
   
  -   boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
  -   PAGE_SIZE)  PAGE_SHIFT;

Given that there is a BUG_ON() in the iommu-helpers which checks for
!is_power_of_2(boundary_size) I think we can simplify the this macro and
avoid the overflow in a more clever way:

boundary_size = (dma_get_seg_boundary(dev)  PAGE_SHIFT) + 1;

This should work because dma_get_seg_boundary(dev) really needs to be a
bitmask which becomes a power_of_2 on incrementing.


Regards,

Joerg


-- 
You received this message because you are subscribed to the Google Groups 
open-iscsi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [PATCH] SCSI: amd_iommu dma_boundary overflow

[James Bottomley]

This is a bit tricky, since AMD laid off the team who were maintaining
this, but I added to the cc' one of the original maintainers in the
hopes they can take a look.

James


On Tue, 2013-02-19 at 18:30 -0800, Eddie Wai wrote:
 Hello,
 
 For a 64-bit DMA capable PCIe storage HBA running under the 64-bit
 AMD-VI IOMMU environment, the amd_iommu code was observed to hit an
 overflow when it tries to page align the dma_parms-segment_boundary_mask.
 This overflow would eventually trigger the BUG_ON in the iommu-helper's
 iommu_is_span_boundary is_power_of_2 check.
 
 A little back tracking shows that since the device is 64-bit DMA capable,
 the pcidev-dma_mask was correctly set to DMA_BIT_MASK(64).  This dma_mask
 was then transferred to the SCSI host's dma_boundary param (by the iSCSI
 driver) and was eventually used to populate the q-limits.seg_boundary_mask
 (via blk_queue_segment_boundary) and the dma_parms-segment_boundary_mask
 (via dma_set_seg_boundary) during the scsi queue allocation.
 
 The code seems correct as it make sense to impose the same hardware
 segment boundary limit on both the blk queue and the DMA code.  It would
 be an easy alternative to simply prevent the shost-dma_boundary from
 being set to DMA_BIT_MASK(64), but it seems more correct to fix the
 amd_iommu code itself to detect and handle this max 64-bit mask condition.
 
 Please let me know your comments.
 
 Thanks,
 Eddie
 
 Signed-off-by: Eddie Wai eddie@broadcom.com
 ---
  drivers/iommu/amd_iommu.c |7 +--
  1 files changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
 index d90a421..63185a1 100644
 --- a/drivers/iommu/amd_iommu.c
 +++ b/drivers/iommu/amd_iommu.c
 @@ -1526,11 +1526,14 @@ static unsigned long dma_ops_area_alloc(struct device 
 *dev,
   unsigned long boundary_size;
   unsigned long address = -1;
   unsigned long limit;
 + unsigned long mask;
  
   next_bit = PAGE_SHIFT;
  
 - boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
 - PAGE_SIZE)  PAGE_SHIFT;
 + mask = dma_get_seg_boundary(dev);
 + boundary_size = mask + 1 ?
 + ALIGN(mask + 1, PAGE_SIZE)  PAGE_SHIFT :
 + 1UL  (BITS_PER_LONG - PAGE_SHIFT);
  
   for (;i  max_index; ++i) {
   unsigned long offset = dom-aperture[i]-offset  PAGE_SHIFT;

-- 
You received this message because you are subscribed to the Google Groups 
open-iscsi group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.