Re: [Xen-devel] [PATCH v2 12/13] [RFC] iommu: VT-d: Squash map_pages/unmap_pages with map_page/unmap_page

[Oleksandr Tyshchenko]

Hi, Kevin

On Wed, Sep 20, 2017 at 11:54 AM, Tian, Kevin  wrote:
> this patch alone looks OK to me. but I haven't found time to review
> the whole series to judge whether below change is necessary.

Let me explain.

Here [1] I touched common IOMMU code, and as the result I had to
modify all existing IOMMU platform drivers and left TODO regarding
future optimization.
I did this patch as I was interested in adding IPMMU-VMSA IOMMU on ARM
which supported super-pages.

Current patch as well as the following [2] are attempts to *reduce*
the scope of the TODO for x86-related IOMMUs.
This is a maximum I could do without knowledge in subject.

But the ideal patch would be to use large pages whenever possible in
the case that hardware supports them.

[1] [PATCH v2 02/13] iommu: Add extra order argument to the IOMMU APIs
and platform callbacks
https://www.mail-archive.com/xen-devel@lists.xen.org/msg115910.html

[2] [PATCH v2 13/13] [RFC] iommu: AMD-Vi: Squash map_pages/unmap_pages
with map_page/unmap_page
https://patchwork.kernel.org/patch/9862571/

>
>> From: Oleksandr Tyshchenko [mailto:olekst...@gmail.com]
>> Sent: Tuesday, September 12, 2017 10:44 PM
>>
>> Hi.
>>
>> Gentle reminder.
>>
>> On Mon, Aug 21, 2017 at 7:44 PM, Oleksandr Tyshchenko
>>  wrote:
>> > Hi, all.
>> >
>> > Any comments?
>> >
>> > On Tue, Jul 25, 2017 at 8:26 PM, Oleksandr Tyshchenko
>> >  wrote:
>> >> From: Oleksandr Tyshchenko 
>> >>
>> >> Reduce the scope of the TODO by squashing single-page stuff with
>> >> multi-page one. Next target is to use large pages whenever possible
>> >> in the case that hardware supports them.
>> >>
>> >> Signed-off-by: Oleksandr Tyshchenko
>> 
>> >> CC: Jan Beulich 
>> >> CC: Kevin Tian 
>> >>
>> >> ---
>> >>Changes in v1:
>> >>   -
>> >>
>> >>Changes in v2:
>> >>   -
>> >> ---
>> >>  xen/drivers/passthrough/vtd/iommu.c | 138 +
>> ---
>> >>  1 file changed, 67 insertions(+), 71 deletions(-)
>> >>
>> >> diff --git a/xen/drivers/passthrough/vtd/iommu.c
>> b/xen/drivers/passthrough/vtd/iommu.c
>> >> index 45d1f36..d20b2f9 100644
>> >> --- a/xen/drivers/passthrough/vtd/iommu.c
>> >> +++ b/xen/drivers/passthrough/vtd/iommu.c
>> >> @@ -1750,15 +1750,24 @@ static void
>> iommu_domain_teardown(struct domain *d)
>> >>  spin_unlock(>arch.mapping_lock);
>> >>  }
>> >>
>> >> -static int __must_check intel_iommu_map_page(struct domain *d,
>> >> - unsigned long gfn,
>> >> - unsigned long mfn,
>> >> - unsigned int flags)
>> >> +static int __must_check intel_iommu_unmap_pages(struct domain *d,
>> >> +unsigned long gfn,
>> >> +unsigned int order);
>> >> +
>> >> +/*
>> >> + * TODO: Optimize by using large pages whenever possible in the case
>> >> + * that hardware supports them.
>> >> + */
>> >> +static int __must_check intel_iommu_map_pages(struct domain *d,
>> >> +  unsigned long gfn,
>> >> +  unsigned long mfn,
>> >> +  unsigned int order,
>> >> +  unsigned int flags)
>> >>  {
>> >>  struct domain_iommu *hd = dom_iommu(d);
>> >> -struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
>> >> -u64 pg_maddr;
>> >>  int rc = 0;
>> >> +unsigned long orig_gfn = gfn;
>> >> +unsigned long i;
>> >>
>> >>  /* Do nothing if VT-d shares EPT page table */
>> >>  if ( iommu_use_hap_pt(d) )
>> >> @@ -1768,78 +1777,60 @@ static int __must_check
>> intel_iommu_map_page(struct domain *d,
>> >>  if ( iommu_passthrough && is_hardware_domain(d) )
>> >>  return 0;
>> >>
>> >> -spin_lock(>arch.mapping_lock);
>> >> -
>> >> -pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn <<
>> PAGE_SHIFT_4K, 1);
>> >> -if ( pg_maddr == 0 )
>> >> +for ( i = 0; i < (1UL << order); i++, gfn++, mfn++ )
>> >>  {
>> >> -spin_unlock(>arch.mapping_lock);
>> >> -return -ENOMEM;
>> >> -}
>> >> -page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
>> >> -pte = page + (gfn & LEVEL_MASK);
>> >> -old = *pte;
>> >> -dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
>> >> -dma_set_pte_prot(new,
>> >> - ((flags & IOMMUF_readable) ? DMA_PTE_READ  : 0) |
>> >> - ((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
>> >> +struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
>> >> +u64 pg_maddr;
>> >>
>> >> -/* Set the SNP on leaf page table if Snoop Control available */
>> >> -if ( iommu_snoop 

Re: [Xen-devel] [PATCH v2 12/13] [RFC] iommu: VT-d: Squash map_pages/unmap_pages with map_page/unmap_page

[Tian, Kevin]

this patch alone looks OK to me. but I haven't found time to review
the whole series to judge whether below change is necessary.

> From: Oleksandr Tyshchenko [mailto:olekst...@gmail.com]
> Sent: Tuesday, September 12, 2017 10:44 PM
> 
> Hi.
> 
> Gentle reminder.
> 
> On Mon, Aug 21, 2017 at 7:44 PM, Oleksandr Tyshchenko
>  wrote:
> > Hi, all.
> >
> > Any comments?
> >
> > On Tue, Jul 25, 2017 at 8:26 PM, Oleksandr Tyshchenko
> >  wrote:
> >> From: Oleksandr Tyshchenko 
> >>
> >> Reduce the scope of the TODO by squashing single-page stuff with
> >> multi-page one. Next target is to use large pages whenever possible
> >> in the case that hardware supports them.
> >>
> >> Signed-off-by: Oleksandr Tyshchenko
> 
> >> CC: Jan Beulich 
> >> CC: Kevin Tian 
> >>
> >> ---
> >>Changes in v1:
> >>   -
> >>
> >>Changes in v2:
> >>   -
> >> ---
> >>  xen/drivers/passthrough/vtd/iommu.c | 138 +
> ---
> >>  1 file changed, 67 insertions(+), 71 deletions(-)
> >>
> >> diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c
> >> index 45d1f36..d20b2f9 100644
> >> --- a/xen/drivers/passthrough/vtd/iommu.c
> >> +++ b/xen/drivers/passthrough/vtd/iommu.c
> >> @@ -1750,15 +1750,24 @@ static void
> iommu_domain_teardown(struct domain *d)
> >>  spin_unlock(>arch.mapping_lock);
> >>  }
> >>
> >> -static int __must_check intel_iommu_map_page(struct domain *d,
> >> - unsigned long gfn,
> >> - unsigned long mfn,
> >> - unsigned int flags)
> >> +static int __must_check intel_iommu_unmap_pages(struct domain *d,
> >> +unsigned long gfn,
> >> +unsigned int order);
> >> +
> >> +/*
> >> + * TODO: Optimize by using large pages whenever possible in the case
> >> + * that hardware supports them.
> >> + */
> >> +static int __must_check intel_iommu_map_pages(struct domain *d,
> >> +  unsigned long gfn,
> >> +  unsigned long mfn,
> >> +  unsigned int order,
> >> +  unsigned int flags)
> >>  {
> >>  struct domain_iommu *hd = dom_iommu(d);
> >> -struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
> >> -u64 pg_maddr;
> >>  int rc = 0;
> >> +unsigned long orig_gfn = gfn;
> >> +unsigned long i;
> >>
> >>  /* Do nothing if VT-d shares EPT page table */
> >>  if ( iommu_use_hap_pt(d) )
> >> @@ -1768,78 +1777,60 @@ static int __must_check
> intel_iommu_map_page(struct domain *d,
> >>  if ( iommu_passthrough && is_hardware_domain(d) )
> >>  return 0;
> >>
> >> -spin_lock(>arch.mapping_lock);
> >> -
> >> -pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn <<
> PAGE_SHIFT_4K, 1);
> >> -if ( pg_maddr == 0 )
> >> +for ( i = 0; i < (1UL << order); i++, gfn++, mfn++ )
> >>  {
> >> -spin_unlock(>arch.mapping_lock);
> >> -return -ENOMEM;
> >> -}
> >> -page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
> >> -pte = page + (gfn & LEVEL_MASK);
> >> -old = *pte;
> >> -dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
> >> -dma_set_pte_prot(new,
> >> - ((flags & IOMMUF_readable) ? DMA_PTE_READ  : 0) |
> >> - ((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
> >> +struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
> >> +u64 pg_maddr;
> >>
> >> -/* Set the SNP on leaf page table if Snoop Control available */
> >> -if ( iommu_snoop )
> >> -dma_set_pte_snp(new);
> >> +spin_lock(>arch.mapping_lock);
> >>
> >> -if ( old.val == new.val )
> >> -{
> >> +pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn <<
> PAGE_SHIFT_4K, 1);
> >> +if ( pg_maddr == 0 )
> >> +{
> >> +spin_unlock(>arch.mapping_lock);
> >> +rc = -ENOMEM;
> >> +goto err;
> >> +}
> >> +page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
> >> +pte = page + (gfn & LEVEL_MASK);
> >> +old = *pte;
> >> +dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
> >> +dma_set_pte_prot(new,
> >> + ((flags & IOMMUF_readable) ? DMA_PTE_READ  : 0) |
> >> + ((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
> >> +
> >> +/* Set the SNP on leaf page table if Snoop Control available */
> >> +if ( iommu_snoop )
> >> +dma_set_pte_snp(new);
> >> +
> >> +if ( old.val == new.val )
> >> +{
> >> +  

Re: [Xen-devel] [PATCH v2 12/13] [RFC] iommu: VT-d: Squash map_pages/unmap_pages with map_page/unmap_page

[Oleksandr Tyshchenko]

Hi.

Gentle reminder.

On Mon, Aug 21, 2017 at 7:44 PM, Oleksandr Tyshchenko
 wrote:
> Hi, all.
>
> Any comments?
>
> On Tue, Jul 25, 2017 at 8:26 PM, Oleksandr Tyshchenko
>  wrote:
>> From: Oleksandr Tyshchenko 
>>
>> Reduce the scope of the TODO by squashing single-page stuff with
>> multi-page one. Next target is to use large pages whenever possible
>> in the case that hardware supports them.
>>
>> Signed-off-by: Oleksandr Tyshchenko 
>> CC: Jan Beulich 
>> CC: Kevin Tian 
>>
>> ---
>>Changes in v1:
>>   -
>>
>>Changes in v2:
>>   -
>> ---
>>  xen/drivers/passthrough/vtd/iommu.c | 138 
>> +---
>>  1 file changed, 67 insertions(+), 71 deletions(-)
>>
>> diff --git a/xen/drivers/passthrough/vtd/iommu.c 
>> b/xen/drivers/passthrough/vtd/iommu.c
>> index 45d1f36..d20b2f9 100644
>> --- a/xen/drivers/passthrough/vtd/iommu.c
>> +++ b/xen/drivers/passthrough/vtd/iommu.c
>> @@ -1750,15 +1750,24 @@ static void iommu_domain_teardown(struct domain *d)
>>  spin_unlock(>arch.mapping_lock);
>>  }
>>
>> -static int __must_check intel_iommu_map_page(struct domain *d,
>> - unsigned long gfn,
>> - unsigned long mfn,
>> - unsigned int flags)
>> +static int __must_check intel_iommu_unmap_pages(struct domain *d,
>> +unsigned long gfn,
>> +unsigned int order);
>> +
>> +/*
>> + * TODO: Optimize by using large pages whenever possible in the case
>> + * that hardware supports them.
>> + */
>> +static int __must_check intel_iommu_map_pages(struct domain *d,
>> +  unsigned long gfn,
>> +  unsigned long mfn,
>> +  unsigned int order,
>> +  unsigned int flags)
>>  {
>>  struct domain_iommu *hd = dom_iommu(d);
>> -struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
>> -u64 pg_maddr;
>>  int rc = 0;
>> +unsigned long orig_gfn = gfn;
>> +unsigned long i;
>>
>>  /* Do nothing if VT-d shares EPT page table */
>>  if ( iommu_use_hap_pt(d) )
>> @@ -1768,78 +1777,60 @@ static int __must_check intel_iommu_map_page(struct 
>> domain *d,
>>  if ( iommu_passthrough && is_hardware_domain(d) )
>>  return 0;
>>
>> -spin_lock(>arch.mapping_lock);
>> -
>> -pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn << PAGE_SHIFT_4K, 1);
>> -if ( pg_maddr == 0 )
>> +for ( i = 0; i < (1UL << order); i++, gfn++, mfn++ )
>>  {
>> -spin_unlock(>arch.mapping_lock);
>> -return -ENOMEM;
>> -}
>> -page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
>> -pte = page + (gfn & LEVEL_MASK);
>> -old = *pte;
>> -dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
>> -dma_set_pte_prot(new,
>> - ((flags & IOMMUF_readable) ? DMA_PTE_READ  : 0) |
>> - ((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
>> +struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
>> +u64 pg_maddr;
>>
>> -/* Set the SNP on leaf page table if Snoop Control available */
>> -if ( iommu_snoop )
>> -dma_set_pte_snp(new);
>> +spin_lock(>arch.mapping_lock);
>>
>> -if ( old.val == new.val )
>> -{
>> +pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn << PAGE_SHIFT_4K, 
>> 1);
>> +if ( pg_maddr == 0 )
>> +{
>> +spin_unlock(>arch.mapping_lock);
>> +rc = -ENOMEM;
>> +goto err;
>> +}
>> +page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
>> +pte = page + (gfn & LEVEL_MASK);
>> +old = *pte;
>> +dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
>> +dma_set_pte_prot(new,
>> + ((flags & IOMMUF_readable) ? DMA_PTE_READ  : 0) |
>> + ((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
>> +
>> +/* Set the SNP on leaf page table if Snoop Control available */
>> +if ( iommu_snoop )
>> +dma_set_pte_snp(new);
>> +
>> +if ( old.val == new.val )
>> +{
>> +spin_unlock(>arch.mapping_lock);
>> +unmap_vtd_domain_page(page);
>> +continue;
>> +}
>> +*pte = new;
>> +
>> +iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
>>  spin_unlock(>arch.mapping_lock);
>>  unmap_vtd_domain_page(page);
>> -return 0;
>> -}
>> -*pte = new;
>> -
>> -iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
>> -spin_unlock(>arch.mapping_lock);
>> -

Re: [Xen-devel] [PATCH v2 12/13] [RFC] iommu: VT-d: Squash map_pages/unmap_pages with map_page/unmap_page

[Oleksandr Tyshchenko]

Hi, all.

Any comments?

On Tue, Jul 25, 2017 at 8:26 PM, Oleksandr Tyshchenko
 wrote:
> From: Oleksandr Tyshchenko 
>
> Reduce the scope of the TODO by squashing single-page stuff with
> multi-page one. Next target is to use large pages whenever possible
> in the case that hardware supports them.
>
> Signed-off-by: Oleksandr Tyshchenko 
> CC: Jan Beulich 
> CC: Kevin Tian 
>
> ---
>Changes in v1:
>   -
>
>Changes in v2:
>   -
> ---
>  xen/drivers/passthrough/vtd/iommu.c | 138 
> +---
>  1 file changed, 67 insertions(+), 71 deletions(-)
>
> diff --git a/xen/drivers/passthrough/vtd/iommu.c 
> b/xen/drivers/passthrough/vtd/iommu.c
> index 45d1f36..d20b2f9 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1750,15 +1750,24 @@ static void iommu_domain_teardown(struct domain *d)
>  spin_unlock(>arch.mapping_lock);
>  }
>
> -static int __must_check intel_iommu_map_page(struct domain *d,
> - unsigned long gfn,
> - unsigned long mfn,
> - unsigned int flags)
> +static int __must_check intel_iommu_unmap_pages(struct domain *d,
> +unsigned long gfn,
> +unsigned int order);
> +
> +/*
> + * TODO: Optimize by using large pages whenever possible in the case
> + * that hardware supports them.
> + */
> +static int __must_check intel_iommu_map_pages(struct domain *d,
> +  unsigned long gfn,
> +  unsigned long mfn,
> +  unsigned int order,
> +  unsigned int flags)
>  {
>  struct domain_iommu *hd = dom_iommu(d);
> -struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
> -u64 pg_maddr;
>  int rc = 0;
> +unsigned long orig_gfn = gfn;
> +unsigned long i;
>
>  /* Do nothing if VT-d shares EPT page table */
>  if ( iommu_use_hap_pt(d) )
> @@ -1768,78 +1777,60 @@ static int __must_check intel_iommu_map_page(struct 
> domain *d,
>  if ( iommu_passthrough && is_hardware_domain(d) )
>  return 0;
>
> -spin_lock(>arch.mapping_lock);
> -
> -pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn << PAGE_SHIFT_4K, 1);
> -if ( pg_maddr == 0 )
> +for ( i = 0; i < (1UL << order); i++, gfn++, mfn++ )
>  {
> -spin_unlock(>arch.mapping_lock);
> -return -ENOMEM;
> -}
> -page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
> -pte = page + (gfn & LEVEL_MASK);
> -old = *pte;
> -dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
> -dma_set_pte_prot(new,
> - ((flags & IOMMUF_readable) ? DMA_PTE_READ  : 0) |
> - ((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
> +struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
> +u64 pg_maddr;
>
> -/* Set the SNP on leaf page table if Snoop Control available */
> -if ( iommu_snoop )
> -dma_set_pte_snp(new);
> +spin_lock(>arch.mapping_lock);
>
> -if ( old.val == new.val )
> -{
> +pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn << PAGE_SHIFT_4K, 
> 1);
> +if ( pg_maddr == 0 )
> +{
> +spin_unlock(>arch.mapping_lock);
> +rc = -ENOMEM;
> +goto err;
> +}
> +page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
> +pte = page + (gfn & LEVEL_MASK);
> +old = *pte;
> +dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
> +dma_set_pte_prot(new,
> + ((flags & IOMMUF_readable) ? DMA_PTE_READ  : 0) |
> + ((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
> +
> +/* Set the SNP on leaf page table if Snoop Control available */
> +if ( iommu_snoop )
> +dma_set_pte_snp(new);
> +
> +if ( old.val == new.val )
> +{
> +spin_unlock(>arch.mapping_lock);
> +unmap_vtd_domain_page(page);
> +continue;
> +}
> +*pte = new;
> +
> +iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
>  spin_unlock(>arch.mapping_lock);
>  unmap_vtd_domain_page(page);
> -return 0;
> -}
> -*pte = new;
> -
> -iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
> -spin_unlock(>arch.mapping_lock);
> -unmap_vtd_domain_page(page);
>
> -if ( !this_cpu(iommu_dont_flush_iotlb) )
> -rc = iommu_flush_iotlb(d, gfn, dma_pte_present(old), 1);
> -
> -return rc;
> -}
> -
> -static int __must_check intel_iommu_unmap_page(struct domain *d,

[Xen-devel] [PATCH v2 12/13] [RFC] iommu: VT-d: Squash map_pages/unmap_pages with map_page/unmap_page

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko 

Reduce the scope of the TODO by squashing single-page stuff with
multi-page one. Next target is to use large pages whenever possible
in the case that hardware supports them.

Signed-off-by: Oleksandr Tyshchenko 
CC: Jan Beulich 
CC: Kevin Tian 

---
   Changes in v1:
  -

   Changes in v2:
  -
---
 xen/drivers/passthrough/vtd/iommu.c | 138 +---
 1 file changed, 67 insertions(+), 71 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index 45d1f36..d20b2f9 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1750,15 +1750,24 @@ static void iommu_domain_teardown(struct domain *d)
 spin_unlock(>arch.mapping_lock);
 }
 
-static int __must_check intel_iommu_map_page(struct domain *d,
- unsigned long gfn,
- unsigned long mfn,
- unsigned int flags)
+static int __must_check intel_iommu_unmap_pages(struct domain *d,
+unsigned long gfn,
+unsigned int order);
+
+/*
+ * TODO: Optimize by using large pages whenever possible in the case
+ * that hardware supports them.
+ */
+static int __must_check intel_iommu_map_pages(struct domain *d,
+  unsigned long gfn,
+  unsigned long mfn,
+  unsigned int order,
+  unsigned int flags)
 {
 struct domain_iommu *hd = dom_iommu(d);
-struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
-u64 pg_maddr;
 int rc = 0;
+unsigned long orig_gfn = gfn;
+unsigned long i;
 
 /* Do nothing if VT-d shares EPT page table */
 if ( iommu_use_hap_pt(d) )
@@ -1768,78 +1777,60 @@ static int __must_check intel_iommu_map_page(struct 
domain *d,
 if ( iommu_passthrough && is_hardware_domain(d) )
 return 0;
 
-spin_lock(>arch.mapping_lock);
-
-pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn << PAGE_SHIFT_4K, 1);
-if ( pg_maddr == 0 )
+for ( i = 0; i < (1UL << order); i++, gfn++, mfn++ )
 {
-spin_unlock(>arch.mapping_lock);
-return -ENOMEM;
-}
-page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
-pte = page + (gfn & LEVEL_MASK);
-old = *pte;
-dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
-dma_set_pte_prot(new,
- ((flags & IOMMUF_readable) ? DMA_PTE_READ  : 0) |
- ((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
+struct dma_pte *page = NULL, *pte = NULL, old, new = { 0 };
+u64 pg_maddr;
 
-/* Set the SNP on leaf page table if Snoop Control available */
-if ( iommu_snoop )
-dma_set_pte_snp(new);
+spin_lock(>arch.mapping_lock);
 
-if ( old.val == new.val )
-{
+pg_maddr = addr_to_dma_page_maddr(d, (paddr_t)gfn << PAGE_SHIFT_4K, 1);
+if ( pg_maddr == 0 )
+{
+spin_unlock(>arch.mapping_lock);
+rc = -ENOMEM;
+goto err;
+}
+page = (struct dma_pte *)map_vtd_domain_page(pg_maddr);
+pte = page + (gfn & LEVEL_MASK);
+old = *pte;
+dma_set_pte_addr(new, (paddr_t)mfn << PAGE_SHIFT_4K);
+dma_set_pte_prot(new,
+ ((flags & IOMMUF_readable) ? DMA_PTE_READ  : 0) |
+ ((flags & IOMMUF_writable) ? DMA_PTE_WRITE : 0));
+
+/* Set the SNP on leaf page table if Snoop Control available */
+if ( iommu_snoop )
+dma_set_pte_snp(new);
+
+if ( old.val == new.val )
+{
+spin_unlock(>arch.mapping_lock);
+unmap_vtd_domain_page(page);
+continue;
+}
+*pte = new;
+
+iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
 spin_unlock(>arch.mapping_lock);
 unmap_vtd_domain_page(page);
-return 0;
-}
-*pte = new;
-
-iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
-spin_unlock(>arch.mapping_lock);
-unmap_vtd_domain_page(page);
 
-if ( !this_cpu(iommu_dont_flush_iotlb) )
-rc = iommu_flush_iotlb(d, gfn, dma_pte_present(old), 1);
-
-return rc;
-}
-
-static int __must_check intel_iommu_unmap_page(struct domain *d,
-   unsigned long gfn)
-{
-/* Do nothing if hardware domain and iommu supports pass thru. */
-if ( iommu_passthrough && is_hardware_domain(d) )
-return 0;
-
-return dma_pte_clear_one(d, (paddr_t)gfn << PAGE_SHIFT_4K);
-}
-
-/* TODO: Optimize by squashing map_pages/unmap_pages with