Re: [patch -mm][Intel-IOMMU] Optimize sg map/unmap calls

2007-08-02 Thread Keshavamurthy, Anil S
On Wed, Aug 01, 2007 at 04:45:54PM -0700, Andrew Morton wrote:
> On Wed, 1 Aug 2007 13:06:23 -0700
> "Keshavamurthy, Anil S" <[EMAIL PROTECTED]> wrote:
> 
> > +/* Computes the padding size required, to make the
> > + * the start address naturally aligned on its size
> > + */
> > +static int
> > +iova_get_pad_size(int size, unsigned int limit_pfn)
> > +{
> > +   unsigned int pad_size = 0;
> > +   unsigned int order = ilog2(size);
> > +
> > +   if (order)
> > +   pad_size = (limit_pfn + 1) % (1 << order);
> > +
> > +   return pad_size;
> > +}
> 
> This isn't obviously doing the right thing for non-power-of-2 inputs.
> ilog2() rounds down...
Andrew,
The call chain to iova_get_pad_size() is like this
alloc_iova()--->__alloc_iova_range()--->iova_get_pad_size().

Inside the alloc_iova() we are rounding the size to __roundup_pow_of_two(size)
iff the caller of alloc_iova() request by setting size_aligned bool. And
in every call to iova_get_pad_size() we check the size_aligned bool before 
calling
iova_get_pad_size.  Hence I don;t see any issues. If you want I can insert a 
BUG_ON() statement inside the above iova_get_pad_size() function. 
Please do let me know.

thanks,
Anil
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch -mm][Intel-IOMMU] Optimize sg map/unmap calls

2007-08-02 Thread Keshavamurthy, Anil S
On Wed, Aug 01, 2007 at 04:45:54PM -0700, Andrew Morton wrote:
 On Wed, 1 Aug 2007 13:06:23 -0700
 Keshavamurthy, Anil S [EMAIL PROTECTED] wrote:
 
  +/* Computes the padding size required, to make the
  + * the start address naturally aligned on its size
  + */
  +static int
  +iova_get_pad_size(int size, unsigned int limit_pfn)
  +{
  +   unsigned int pad_size = 0;
  +   unsigned int order = ilog2(size);
  +
  +   if (order)
  +   pad_size = (limit_pfn + 1) % (1  order);
  +
  +   return pad_size;
  +}
 
 This isn't obviously doing the right thing for non-power-of-2 inputs.
 ilog2() rounds down...
Andrew,
The call chain to iova_get_pad_size() is like this
alloc_iova()---__alloc_iova_range()---iova_get_pad_size().

Inside the alloc_iova() we are rounding the size to __roundup_pow_of_two(size)
iff the caller of alloc_iova() request by setting size_aligned bool. And
in every call to iova_get_pad_size() we check the size_aligned bool before 
calling
iova_get_pad_size.  Hence I don;t see any issues. If you want I can insert a 
BUG_ON() statement inside the above iova_get_pad_size() function. 
Please do let me know.

thanks,
Anil
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [patch -mm][Intel-IOMMU] Optimize sg map/unmap calls

2007-08-01 Thread Andrew Morton
On Wed, 1 Aug 2007 13:06:23 -0700
"Keshavamurthy, Anil S" <[EMAIL PROTECTED]> wrote:

> +/* Computes the padding size required, to make the
> + * the start address naturally aligned on its size
> + */
> +static int
> +iova_get_pad_size(int size, unsigned int limit_pfn)
> +{
> + unsigned int pad_size = 0;
> + unsigned int order = ilog2(size);
> +
> + if (order)
> + pad_size = (limit_pfn + 1) % (1 << order);
> +
> + return pad_size;
> +}

This isn't obviously doing the right thing for non-power-of-2 inputs.
ilog2() rounds down...

Please check that this, and all the other ilog2()s which have been added
are doing the right thing if they can be presented with non-power-of-2
inputs?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[patch -mm][Intel-IOMMU] Optimize sg map/unmap calls

2007-08-01 Thread Keshavamurthy, Anil S
This patch adds PageSelectiveInvalidation support 
replacing existing DomainSelectiveInvalidation for 
intel_{map/unmap}_sg() calls and also enables
to mapping one big contiguous DMA virtual address
which is mapped to discontiguous physical address
for SG map/unmap calls.


"Doamin selective invalidations" wipes out the IOMMU address
translation cache based on domain ID where as "Page selective
invalidations" wipes out the IOMMU address translation cache for
that address mask range which is more cache friendly when
compared to Domain selective invalidations.


Here is how it is done.
1) changes to iova.c
alloc_iova() now takes a bool size_aligned argument, which
when when set, returns the io virtual address that is 
naturally aligned to 2 ^ x, where x is the order 
of the size requested.

Returning this io vitual address which is naturally 
aligned helps iommu to do the "page selective 
invalidations" which is IOMMU cache friendly
over "domain selective invalidations".

2) Changes to driver/pci/intel-iommu.c
Clean up intel_{map/unmap}_{single/sg} () calls so that
s/g map/unamp calls is no more dependent on 
intel_{map/unmap}_single()

intel_map_sg() now computes the total DMA virtual address
required and allocates the size aligned total DMA virtual address
and maps the discontiguous physical address to the allocated
contiguous DMA virtual address.

In the intel_unmap_sg() case since the DMA virtual address
is contiguous and size_aligned, PageSelectiveInvalidation
is used replacing earlier DomainSelectiveInvalidations.

Andrew,
Please add this patch to you mm queue.

Signed-off-by: Anil S Keshavamurthy <[EMAIL PROTECTED]>

---
 drivers/pci/intel-iommu.c |  325 +-
 drivers/pci/iova.c|   63 +++-
 drivers/pci/iova.h|3 
 3 files changed, 231 insertions(+), 160 deletions(-)

Index: work/drivers/pci/intel-iommu.c
===
--- work.orig/drivers/pci/intel-iommu.c 2007-08-01 11:18:12.0 -0700
+++ work/drivers/pci/intel-iommu.c  2007-08-01 11:18:31.0 -0700
@@ -665,24 +665,10 @@
non_present_entry_flush);
 }
 
-static int iommu_get_alignment(u64 base, unsigned int size)
-{
-   int t = 0;
-   u64 end;
-
-   end = base + size - 1;
-   while (base != end) {
-   t++;
-   base >>= 1;
-   end >>= 1;
-   }
-   return t;
-}
-
 static int iommu_flush_iotlb_psi(struct intel_iommu *iommu, u16 did,
u64 addr, unsigned int pages, int non_present_entry_flush)
 {
-   unsigned int align;
+   unsigned int mask;
 
BUG_ON(addr & (~PAGE_MASK_4K));
BUG_ON(pages == 0);
@@ -696,16 +682,13 @@
 * PSI requires page size to be 2 ^ x, and the base address is naturally
 * aligned to the size
 */
-   align = iommu_get_alignment(addr >> PAGE_SHIFT_4K, pages);
+   mask = ilog2(__roundup_pow_of_two(pages));
/* Fallback to domain selective flush if size is too big */
-   if (align > cap_max_amask_val(iommu->cap))
+   if (mask > cap_max_amask_val(iommu->cap))
return iommu_flush_iotlb_dsi(iommu, did,
non_present_entry_flush);
 
-   addr >>= PAGE_SHIFT_4K + align;
-   addr <<= PAGE_SHIFT_4K + align;
-
-   return __iommu_flush_iotlb(iommu, did, addr, align,
+   return __iommu_flush_iotlb(iommu, did, addr, mask,
DMA_TLB_PSI_FLUSH, non_present_entry_flush);
 }
 
@@ -1772,78 +1755,103 @@
 }
 
 struct iova *
-iommu_alloc_iova(struct dmar_domain *domain, void *host_addr, size_t size,
-   u64 start, u64 end)
+iommu_alloc_iova(struct dmar_domain *domain, size_t size, u64 end)
 {
-   u64 start_addr;
struct iova *piova;
 
/* Make sure it's in range */
-   if ((start > DOMAIN_MAX_ADDR(domain->gaw)) || end < start)
-   return NULL;
-
end = min_t(u64, DOMAIN_MAX_ADDR(domain->gaw), end);
-   start_addr = PAGE_ALIGN_4K(start);
-   size = aligned_size((u64)host_addr, size);
-   if (!size || (start_addr + size > end))
+   if (!size || (IOVA_START_ADDR + size > end))
return NULL;
 
piova = alloc_iova(>iovad,
-   size >> PAGE_SHIFT_4K, IOVA_PFN(end));
-
+   size >> PAGE_SHIFT_4K, IOVA_PFN(end), 1);
return piova;
 }
 
-static dma_addr_t __intel_map_single(struct device *dev, void *addr,
-   size_t size, int dir, u64 *flush_addr, unsigned int *flush_size)
+static struct iova *
+__intel_alloc_iova(struct device *dev, struct dmar_domain *domain,
+   size_t size)
 {
-   struct dmar_domain *domain;
struct pci_dev *pdev = to_pci_dev(dev);
-   int ret;
-   int prot = 0;
struct iova *iova = NULL;
-   u64 start_addr;
-
-   addr = (void *)virt_to_phys(addr);
-
-   domain = get_domain_for_dev(pdev,
-

[patch -mm][Intel-IOMMU] Optimize sg map/unmap calls

2007-08-01 Thread Keshavamurthy, Anil S
This patch adds PageSelectiveInvalidation support 
replacing existing DomainSelectiveInvalidation for 
intel_{map/unmap}_sg() calls and also enables
to mapping one big contiguous DMA virtual address
which is mapped to discontiguous physical address
for SG map/unmap calls.


Doamin selective invalidations wipes out the IOMMU address
translation cache based on domain ID where as Page selective
invalidations wipes out the IOMMU address translation cache for
that address mask range which is more cache friendly when
compared to Domain selective invalidations.


Here is how it is done.
1) changes to iova.c
alloc_iova() now takes a bool size_aligned argument, which
when when set, returns the io virtual address that is 
naturally aligned to 2 ^ x, where x is the order 
of the size requested.

Returning this io vitual address which is naturally 
aligned helps iommu to do the page selective 
invalidations which is IOMMU cache friendly
over domain selective invalidations.

2) Changes to driver/pci/intel-iommu.c
Clean up intel_{map/unmap}_{single/sg} () calls so that
s/g map/unamp calls is no more dependent on 
intel_{map/unmap}_single()

intel_map_sg() now computes the total DMA virtual address
required and allocates the size aligned total DMA virtual address
and maps the discontiguous physical address to the allocated
contiguous DMA virtual address.

In the intel_unmap_sg() case since the DMA virtual address
is contiguous and size_aligned, PageSelectiveInvalidation
is used replacing earlier DomainSelectiveInvalidations.

Andrew,
Please add this patch to you mm queue.

Signed-off-by: Anil S Keshavamurthy [EMAIL PROTECTED]

---
 drivers/pci/intel-iommu.c |  325 +-
 drivers/pci/iova.c|   63 +++-
 drivers/pci/iova.h|3 
 3 files changed, 231 insertions(+), 160 deletions(-)

Index: work/drivers/pci/intel-iommu.c
===
--- work.orig/drivers/pci/intel-iommu.c 2007-08-01 11:18:12.0 -0700
+++ work/drivers/pci/intel-iommu.c  2007-08-01 11:18:31.0 -0700
@@ -665,24 +665,10 @@
non_present_entry_flush);
 }
 
-static int iommu_get_alignment(u64 base, unsigned int size)
-{
-   int t = 0;
-   u64 end;
-
-   end = base + size - 1;
-   while (base != end) {
-   t++;
-   base = 1;
-   end = 1;
-   }
-   return t;
-}
-
 static int iommu_flush_iotlb_psi(struct intel_iommu *iommu, u16 did,
u64 addr, unsigned int pages, int non_present_entry_flush)
 {
-   unsigned int align;
+   unsigned int mask;
 
BUG_ON(addr  (~PAGE_MASK_4K));
BUG_ON(pages == 0);
@@ -696,16 +682,13 @@
 * PSI requires page size to be 2 ^ x, and the base address is naturally
 * aligned to the size
 */
-   align = iommu_get_alignment(addr  PAGE_SHIFT_4K, pages);
+   mask = ilog2(__roundup_pow_of_two(pages));
/* Fallback to domain selective flush if size is too big */
-   if (align  cap_max_amask_val(iommu-cap))
+   if (mask  cap_max_amask_val(iommu-cap))
return iommu_flush_iotlb_dsi(iommu, did,
non_present_entry_flush);
 
-   addr = PAGE_SHIFT_4K + align;
-   addr = PAGE_SHIFT_4K + align;
-
-   return __iommu_flush_iotlb(iommu, did, addr, align,
+   return __iommu_flush_iotlb(iommu, did, addr, mask,
DMA_TLB_PSI_FLUSH, non_present_entry_flush);
 }
 
@@ -1772,78 +1755,103 @@
 }
 
 struct iova *
-iommu_alloc_iova(struct dmar_domain *domain, void *host_addr, size_t size,
-   u64 start, u64 end)
+iommu_alloc_iova(struct dmar_domain *domain, size_t size, u64 end)
 {
-   u64 start_addr;
struct iova *piova;
 
/* Make sure it's in range */
-   if ((start  DOMAIN_MAX_ADDR(domain-gaw)) || end  start)
-   return NULL;
-
end = min_t(u64, DOMAIN_MAX_ADDR(domain-gaw), end);
-   start_addr = PAGE_ALIGN_4K(start);
-   size = aligned_size((u64)host_addr, size);
-   if (!size || (start_addr + size  end))
+   if (!size || (IOVA_START_ADDR + size  end))
return NULL;
 
piova = alloc_iova(domain-iovad,
-   size  PAGE_SHIFT_4K, IOVA_PFN(end));
-
+   size  PAGE_SHIFT_4K, IOVA_PFN(end), 1);
return piova;
 }
 
-static dma_addr_t __intel_map_single(struct device *dev, void *addr,
-   size_t size, int dir, u64 *flush_addr, unsigned int *flush_size)
+static struct iova *
+__intel_alloc_iova(struct device *dev, struct dmar_domain *domain,
+   size_t size)
 {
-   struct dmar_domain *domain;
struct pci_dev *pdev = to_pci_dev(dev);
-   int ret;
-   int prot = 0;
struct iova *iova = NULL;
-   u64 start_addr;
-
-   addr = (void *)virt_to_phys(addr);
-
-   domain = get_domain_for_dev(pdev,
-   

Re: [patch -mm][Intel-IOMMU] Optimize sg map/unmap calls

2007-08-01 Thread Andrew Morton
On Wed, 1 Aug 2007 13:06:23 -0700
Keshavamurthy, Anil S [EMAIL PROTECTED] wrote:

 +/* Computes the padding size required, to make the
 + * the start address naturally aligned on its size
 + */
 +static int
 +iova_get_pad_size(int size, unsigned int limit_pfn)
 +{
 + unsigned int pad_size = 0;
 + unsigned int order = ilog2(size);
 +
 + if (order)
 + pad_size = (limit_pfn + 1) % (1  order);
 +
 + return pad_size;
 +}

This isn't obviously doing the right thing for non-power-of-2 inputs.
ilog2() rounds down...

Please check that this, and all the other ilog2()s which have been added
are doing the right thing if they can be presented with non-power-of-2
inputs?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/