Xen devel - Dec 2010 - [PATCH 3/4] amd iommu: Large io page support - enablement

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

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


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

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



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

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...


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

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...



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