Hi all, I am trying to use the VT-d support with a PV driver domain which performs network I/O on behalf of guest domains. When I initiate any network I/O it results in I/O page faults. On taking a closer look I find that iommu_map_page() is called only for writable pages from __gnttab_map_grant_ref(). This results in page faults during packet transmission since pages are mapped read-only in this case. When I remove this restriction, I can get it working without any page faults. Is this a bug? or am I missing something here? I am using the latest unstable version of Xen. This part of the source code hasn''t changed in a long time. Thanks! -Kaushik _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
It looks like the person who implemented that never needed the read-only case. We need a function to map pages into the iommu read-only and to call that from the grant code. A patch to just call iommu_map_page() for any kind of grant mapping would be acceptable for now, if you want to submit a patch. -- Keir On 27/05/2010 21:37, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote:> Hi all, > > I am trying to use the VT-d support with a PV driver domain which performs > network I/O on behalf of guest domains. > When I initiate any network I/O it results in I/O page faults. On taking a > closer look I find that iommu_map_page() is called only for writable pages > from __gnttab_map_grant_ref(). This results in page faults during packet > transmission since pages are mapped read-only in this case. > When I remove this restriction, I can get it working without any page faults. > > Is this a bug? or am I missing something here? I am using the latest unstable > version of Xen. This part of the source code hasn''t changed in a long time. > > Thanks! > -Kaushik > _______________________________________________ > 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 May 27, 2010, at 3:46 PM, Keir Fraser wrote:> It looks like the person who implemented that never needed the read-only > case. We need a function to map pages into the iommu read-only and to call > that from the grant code. > > A patch to just call iommu_map_page() for any kind of grant mapping would be > acceptable for now, if you want to submit a patch.OK. Attached. Signed-off-by: Kaushik Kumar Ram <kaushik@rice.edu> diff -r 26c2922da53c xen/common/grant_table.c --- a/xen/common/grant_table.c Thu May 27 09:39:47 2010 +0100 +++ b/xen/common/grant_table.c Thu May 27 15:56:37 2010 -0500 @@ -596,9 +596,7 @@ __gnttab_map_grant_ref( goto undo_out; } - if ( (!is_hvm_domain(ld) && need_iommu(ld)) && - !(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) && - (act_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) ) + if ( !is_hvm_domain(ld) && need_iommu(ld)) { /* Shouldn''t happen, because you can''t use iommu in a HVM * domain. */ @@ -779,9 +777,7 @@ __gnttab_unmap_common( act->pin -= GNTPIN_hstw_inc; } - if ( (!is_hvm_domain(ld) && need_iommu(ld)) && - (old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) && - !(act->pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) ) + if ( !is_hvm_domain(ld) && need_iommu(ld)) { BUG_ON(paging_mode_translate(ld)); if ( iommu_unmap_page(ld, op->frame) ) -Kaushik> On 27/05/2010 21:37, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote: > >> Hi all, >> >> I am trying to use the VT-d support with a PV driver domain which performs >> network I/O on behalf of guest domains. >> When I initiate any network I/O it results in I/O page faults. On taking a >> closer look I find that iommu_map_page() is called only for writable pages >> from __gnttab_map_grant_ref(). This results in page faults during packet >> transmission since pages are mapped read-only in this case. >> When I remove this restriction, I can get it working without any page faults. >> >> Is this a bug? or am I missing something here? I am using the latest unstable >> version of Xen. This part of the source code hasn''t changed in a long time. >> >> Thanks! >> -Kaushik >> _______________________________________________ >> 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 >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 27/05/2010 21:59, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote:> On May 27, 2010, at 3:46 PM, Keir Fraser wrote: > >> It looks like the person who implemented that never needed the read-only >> case. We need a function to map pages into the iommu read-only and to call >> that from the grant code. >> >> A patch to just call iommu_map_page() for any kind of grant mapping would be >> acceptable for now, if you want to submit a patch. > > OK. Attached.Actually I''ve implemented the better fix as xen-unstable:21476. Hope that works okay for you. Thanks, Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On May 28, 2010, at 3:08 AM, Keir Fraser wrote:> On 27/05/2010 21:59, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote: > >> On May 27, 2010, at 3:46 PM, Keir Fraser wrote: >> >>> It looks like the person who implemented that never needed the read-only >>> case. We need a function to map pages into the iommu read-only and to call >>> that from the grant code. >>> >>> A patch to just call iommu_map_page() for any kind of grant mapping would be >>> acceptable for now, if you want to submit a patch. >> >> OK. Attached. > > Actually I''ve implemented the better fix as xen-unstable:21476. Hope that > works okay for you.I don''t see this changeset.... Thanks. -Kaushik _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 29/05/2010 00:20, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote:>> Actually I''ve implemented the better fix as xen-unstable:21476. Hope that >> works okay for you. > > I don''t see this changeset....Should be there now. In general you can also try pulling from: http://xenbits.xensource.com/staging/xen-unstable.hg Which is the intermediate (pre-testing) staging tree. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On May 29, 2010, at 2:29 AM, Keir Fraser wrote:> On 29/05/2010 00:20, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote: > >>> Actually I''ve implemented the better fix as xen-unstable:21476. Hope that >>> works okay for you. >> >> I don''t see this changeset.... > > Should be there now.This should work. I can test it on my machine in a couple of days.> In general you can also try pulling from: > http://xenbits.xensource.com/staging/xen-unstable.hg > Which is the intermediate (pre-testing) staging tree.OK. Thanks. -Kaushik _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On May 28, 2010, at 3:08 AM, Keir Fraser wrote:> On 27/05/2010 21:59, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote: > >> On May 27, 2010, at 3:46 PM, Keir Fraser wrote: >> >>> It looks like the person who implemented that never needed the read-only >>> case. We need a function to map pages into the iommu read-only and to call >>> that from the grant code. >>> >>> A patch to just call iommu_map_page() for any kind of grant mapping would be >>> acceptable for now, if you want to submit a patch. >> >> OK. Attached. > > Actually I''ve implemented the better fix as xen-unstable:21476. Hope that > works okay for you.Checking if act->pin is zero before calling iommu_unmap_page() (in grant_table.c) is not sufficient since there can be multiple active grants all referring to the same mfn. In fact I came across iommu page faults because pages were getting unmapped from the IOMMU when active grants referring to these pages were still around. Ideally, there needs to be a per-page count of how many IOMMU mappings exists for a page. I can''t think of an obvious fix for this problem. For my purposes, I hacked page_info to add another counter which I guess is not an acceptable solution! -Kaushik _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 10/06/2010 22:59, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote:>> Actually I''ve implemented the better fix as xen-unstable:21476. Hope that >> works okay for you. > > > Checking if act->pin is zero before calling iommu_unmap_page() (in > grant_table.c) is not sufficient since there can be multiple > active grants all referring to the same mfn. In fact I came across iommu page > faults because pages were > getting unmapped from the IOMMU when active grants referring to these pages > were still around. > Ideally, there needs to be a per-page count of how many IOMMU mappings exists > for a page. > I can''t think of an obvious fix for this problem. For my purposes, I hacked > page_info to add another counter which I guess is not an acceptable solution!See if xen-unstable:21597 works for you. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Jun 11, 2010, at 3:32 AM, Keir Fraser wrote:> On 10/06/2010 22:59, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote: >> >> Checking if act->pin is zero before calling iommu_unmap_page() (in >> grant_table.c) is not sufficient since there can be multiple >> active grants all referring to the same mfn. In fact I came across iommu page >> faults because pages were >> getting unmapped from the IOMMU when active grants referring to these pages >> were still around. >> Ideally, there needs to be a per-page count of how many IOMMU mappings exists >> for a page. >> I can''t think of an obvious fix for this problem. For my purposes, I hacked >> page_info to add another counter which I guess is not an acceptable solution! > > See if xen-unstable:21597 works for you.Keir, I finally found some time to test your patch. While it seems to fix the problem, it *significantly* degrades performance. On running netperf, there is a 10X reduction in throughput to a guest VM. On profiling, I find a significant number of cycles being spent in mapcount(). In the current solution, the entire grant table is searched every time (right ?). The mapping info ought to be stored in some per-page location... for efficient access. -Kaushik _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 02/07/2010 07:22, "Kaushik Kumar Ram" <kaushik@rice.edu> wrote:>> See if xen-unstable:21597 works for you. > > Keir, > > I finally found some time to test your patch. While it seems to fix the > problem, it *significantly* degrades performance. > On running netperf, there is a 10X reduction in throughput to a guest VM. On > profiling, I find a significant number of cycles > being spent in mapcount(). In the current solution, the entire grant table is > searched every time (right ?). The mapping info > ought to be stored in some per-page location... for efficient access.It needs to be a per-mapping-domain location. Like the VT-d pte itself. Patches welcome. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 02/07/2010 08:13, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:>> I finally found some time to test your patch. While it seems to fix the >> problem, it *significantly* degrades performance. >> On running netperf, there is a 10X reduction in throughput to a guest VM. On >> profiling, I find a significant number of cycles >> being spent in mapcount(). In the current solution, the entire grant table is >> searched every time (right ?). The mapping info >> ought to be stored in some per-page location... for efficient access. > > It needs to be a per-mapping-domain location. Like the VT-d pte itself. > Patches welcome.Scrub that. It would be better to allocate a chunk of otherwised unused machine address space (e.g., 1GB starting at max(4GB, top_of_ram)), and allocate each grant entry that needs an iommu mapping a page in that range, map at that address and return that address as dev_bus_addr from the grant-map hypercall. Then we would have no IOMMU pte sharing between grants, and no need for any extra reference counting. Also it would be easy to batch iommu flushes in this scenario, if you don''t care about finished-with grant mappings being flushed a bit lazily. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel