[PATCH 3/4] amd iommu: Large io page support - enablement

[PATCH 3/4] amd iommu: Large io page support - enablement

[Wei Wang2]

[-- Attachment #1: Type: text/plain, Size: 297 bytes --]

Enable super iommu support on amd systems.
Thanks,
We
Signed-off-by: Wei Wang <wei.wang2@amd.com>
--
Legal Information:
Advanced Micro Devices GmbH
Sitz: Dornach, Gemeinde Aschheim, 
Landkreis München Registergericht München, 
HRB Nr. 43632
Geschäftsführer:
Alberto Bozzo, Andrew Bowd

[-- Attachment #2: iommu_spage_enable.patch --]
[-- Type: text/x-diff, Size: 2622 bytes --]

diff -r 81606e1d202e xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c	Wed Dec 01 16:13:30 2010 +0100
+++ b/xen/arch/x86/mm/p2m.c	Wed Dec 01 17:11:16 2010 +0100
@@ -1381,7 +1381,16 @@ p2m_set_entry(struct p2m_domain *p2m, un
          && (gfn + (1UL << page_order) - 1 > p2m->max_mapped_pfn) )
         p2m->max_mapped_pfn = gfn + (1UL << page_order) - 1;
 
-    if ( iommu_enabled && need_iommu(p2m->domain) )
+    if ( iommu_enabled && iommu_super_page )
+    {
+        if ( p2mt == p2m_ram_rw )
+            iommu_map_pages(p2m->domain, gfn, mfn_x(mfn),
+                            page_order, IOMMUF_readable|IOMMUF_writable);
+        else
+            iommu_unmap_pages(p2m->domain, gfn, page_order);
+    }
+
+    else if ( iommu_enabled && need_iommu(p2m->domain) )
     {
         if ( p2mt == p2m_ram_rw )
             for ( i = 0; i < (1UL << page_order); i++ )
diff -r 81606e1d202e xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c	Wed Dec 01 16:13:30 2010 +0100
+++ b/xen/drivers/passthrough/iommu.c	Wed Dec 01 17:11:16 2010 +0100
@@ -111,6 +111,9 @@ void __init iommu_dom0_init(struct domai
     if ( !iommu_enabled )
         return;
 
+    if ( hd->platform_ops->map_pages == NULL )
+        iommu_super_page = 0;
+
     d->need_iommu = !!iommu_dom0_strict;
     if ( need_iommu(d) )
     {
@@ -177,7 +180,11 @@ int assign_device(struct domain *d, u8 b
     if ( has_arch_pdevs(d) && !need_iommu(d) )
     {
         d->need_iommu = 1;
-        rc = iommu_populate_page_table(d);
+        /* Populate iommu page table when hardware does not
+        support super IO page */
+        if ( (hd->platform_ops->map_pages == NULL) ||
+             (!iommu_super_page) )
+            rc = iommu_populate_page_table(d);
         goto done;
     }
 done:    
@@ -420,6 +427,33 @@ void iommu_suspend()
         ops->suspend();
 }
 
+int iommu_map_pages(struct domain *d, unsigned long gfn,
+    unsigned long mfn, unsigned int order,  unsigned int flags)
+{
+    struct hvm_iommu *hd = domain_hvm_iommu(d);
+
+    if ( !iommu_enabled || !hd->platform_ops )
+        return 0;
+
+    if ( hd->platform_ops->map_pages != NULL )
+        return hd->platform_ops->map_pages(d, gfn, mfn, order, flags);
+
+    return 0;
+}
+
+int iommu_unmap_pages(struct domain *d, unsigned long gfn, unsigned int order)
+{
+    struct hvm_iommu *hd = domain_hvm_iommu(d);
+
+    if ( !iommu_enabled || !hd->platform_ops )
+        return 0;
+
+    if ( hd->platform_ops->map_pages != NULL )
+        return hd->platform_ops->unmap_pages(d, gfn, order);
+
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 3/4] amd iommu: Large io page support - enablement

[Keir Fraser]

Well, let's see. The change to p2m_set_entry() now allows (superpage) calls
to the iommu mapping functions even if !need_iommu(). That seems a semantic
change. Also I don't want the superpage command-line parameter, but I see
why you added it, as you couldn't be bothered to fix up the Intel side to
work properly, so you left in the onld p2m_set_entry() code for that case.
That's unacceptable. Get rid of the command-line parameter, get rid of the
new [un]map_pages iommu hooks and extend the existing map/unmap hooks
instead, and naturally do that for the Intel side as well as the AMD side
(of course, on the Intel side you can just do a dumb implementation of
map_pages which just loops over the existing single-page-at-a-time code --
you need to keep the Intel side working, but you don't have to do the work
to make it faster than it is already).

I'm going to stop reading this patch series now and wait for a better one.

 -- Keir

On 03/12/2010 08:03, "Wei Wang2" <wei.wang2@amd.com> wrote:

> Enable super iommu support on amd systems.
> Thanks,
> We
> Signed-off-by: Wei Wang <wei.wang2@amd.com>
> --
> Legal Information:
> Advanced Micro Devices GmbH
> Sitz: Dornach, Gemeinde Aschheim,
> Landkreis München Registergericht München,
> HRB Nr. 43632
> Geschäftsführer:
> Alberto Bozzo, Andrew Bowd
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 3/4] amd iommu: Large io page support - enablement

[Wei Wang2]

On Friday 03 December 2010 17:24:53 Keir Fraser wrote:
> Well, let's see. The change to p2m_set_entry() now allows (superpage) calls
> to the iommu mapping functions even if !need_iommu(). That seems a semantic
> change. 
That is because we have iommu_populate_page_table() which will delay io page 
table construction until device assignment. But this function can only 
updates io page table with 4k entries. I didn't find a better way to tracking 
page orders after page allocation (Q: could we extend struct page_info to 
cache page orders?). So my thought is to update IO page table earlier. And 
therefore, enabling super io page will also disable lazy io page table 
construction.  

Also, without need_iommu() checking both passthru and non-passthru guests will 
get io page table allocation. Since super paging will highly reduce io page 
table size, we might not waste too much memories here...

> Also I don't want the superpage command-line parameter, but I see 
> why you added it, as you couldn't be bothered to fix up the Intel side to
> work properly, so you left in the onld p2m_set_entry() code for that case.
> That's unacceptable. Get rid of the command-line parameter, get rid of the
> new [un]map_pages iommu hooks and extend the existing map/unmap hooks
> instead, and naturally do that for the Intel side as well as the AMD side
> (of course, on the Intel side you can just do a dumb implementation of
> map_pages which just loops over the existing single-page-at-a-time code --
> you need to keep the Intel side working, but you don't have to do the work
> to make it faster than it is already).
Understood..It will be fixed.

> I'm going to stop reading this patch series now and wait for a better one.
Yes, a better one will be given soon..

Thanks,
Wei
>  -- Keir
>
> On 03/12/2010 08:03, "Wei Wang2" <wei.wang2@amd.com> wrote:
> > Enable super iommu support on amd systems.
> > Thanks,
> > We
> > Signed-off-by: Wei Wang <wei.wang2@amd.com>
> > --
> > Legal Information:
> > Advanced Micro Devices GmbH
> > Sitz: Dornach, Gemeinde Aschheim,
> > Landkreis München Registergericht München,
> > HRB Nr. 43632
> > Geschäftsführer:
> > Alberto Bozzo, Andrew Bowd
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel

Re: [PATCH 3/4] amd iommu: Large io page support - enablement

[Keir Fraser]

On 03/12/2010 08:45, "Wei Wang2" <wei.wang2@amd.com> wrote:

> On Friday 03 December 2010 17:24:53 Keir Fraser wrote:
>> Well, let's see. The change to p2m_set_entry() now allows (superpage) calls
>> to the iommu mapping functions even if !need_iommu(). That seems a semantic
>> change. 
> That is because we have iommu_populate_page_table() which will delay io page
> table construction until device assignment. But this function can only
> updates io page table with 4k entries. I didn't find a better way to tracking
> page orders after page allocation (Q: could we extend struct page_info to
> cache page orders?). So my thought is to update IO page table earlier. And
> therefore, enabling super io page will also disable lazy io page table
> construction.  

How about hiding the superpage mapping stuff entirely within the existing
iommu_[un]map_page() hooks? If you have 9 spare bits per iommu pde (seems
very likely), you could cache in the page-directory entry how many entries
one level down currently are suitable for coalescing into a superpage
mapping. When a new iommu pte/pde is written, if it is a candidate for
coalescing, increment the parent pde's count. If the count ==
2^superpage_order, then coalesce. You can maintain such counts in every pde
up the hierarchy, for 2MB, 1GB, ... superpages.

Personally I think we could do similar for ordinary host p2m maintenance as
well, if the bits are available. With 64-bit entries, we probably have
sufficient bits (we only need 9 spare bits). What we have now for host p2m
maintenance I can't say I love very much, and I don't think we need follow
that as a model for how we introduce superpage mappings to iommu pagetables.

Anyway, this would make your patch only touch AMD code. Similar could be
done on the Intel side later, and for bonus points at that point perhaps
this coalescing/uncoalescing logic could be pulled out to some degree into
shared code.

 -- Keir

> Also, without need_iommu() checking both passthru and non-passthru guests will
> get io page table allocation. Since super paging will highly reduce io page
> table size, we might not waste too much memories here...

Re: [PATCH 3/4] amd iommu: Large io page support - enablement

[Wei Wang2]

On Friday 03 December 2010 19:28:17 Keir Fraser wrote:
> On 03/12/2010 08:45, "Wei Wang2" <wei.wang2@amd.com> wrote:
> > On Friday 03 December 2010 17:24:53 Keir Fraser wrote:
> >> Well, let's see. The change to p2m_set_entry() now allows (superpage)
> >> calls to the iommu mapping functions even if !need_iommu(). That seems a
> >> semantic change.
> >
> > That is because we have iommu_populate_page_table() which will delay io
> > page table construction until device assignment. But this function can
> > only updates io page table with 4k entries. I didn't find a better way to
> > tracking page orders after page allocation (Q: could we extend struct
> > page_info to cache page orders?). So my thought is to update IO page
> > table earlier. And therefore, enabling super io page will also disable
> > lazy io page table construction.
>
> How about hiding the superpage mapping stuff entirely within the existing
> iommu_[un]map_page() hooks? If you have 9 spare bits per iommu pde (seems
> very likely), you could cache in the page-directory entry how many entries
> one level down currently are suitable for coalescing into a superpage
> mapping. When a new iommu pte/pde is written, if it is a candidate for
> coalescing, increment the parent pde's count. If the count ==
> 2^superpage_order, then coalesce. You can maintain such counts in every pde
> up the hierarchy, for 2MB, 1GB, ... superpages.

This looks good to me. According to iommu specification, bit 63 ＋ bit 1-8 in 
pde entry should be free to use. I will implement this algorithm for the next 
version.
Thanks,
Wei

> Personally I think we could do similar for ordinary host p2m maintenance as
> well, if the bits are available. With 64-bit entries, we probably have
> sufficient bits (we only need 9 spare bits). What we have now for host p2m
> maintenance I can't say I love very much, and I don't think we need follow
> that as a model for how we introduce superpage mappings to iommu
> pagetables.
>
> Anyway, this would make your patch only touch AMD code. Similar could be
> done on the Intel side later, and for bonus points at that point perhaps
> this coalescing/uncoalescing logic could be pulled out to some degree into
> shared code.
>
>  -- Keir
>
> > Also, without need_iommu() checking both passthru and non-passthru guests
> > will get io page table allocation. Since super paging will highly reduce
> > io page table size, we might not waste too much memories here...