Jean Guyader
2011-Nov-13 17:40 UTC
[Xen-devel] [PATCH 0/6] IOMMU, vtd and iotlb flush rework (v7)
In one of my previous email I detailed a bug I was seeing when passing
through a Intel GPU on a guest that has more that 4G or RAM.
Allen suggested that I go for the Plan B but after a discussion with Tim
we agreed that Plan B was way to disruptive in term of code change.
This patch series implements Plan A.
http://xen.1045712.n5.nabble.com/VTD-Intel-iommu-IOTLB-flush-really-slow-td4952866.html
Changes between v6 and v7:
- xenmem_add_to_physmap_once can''t take a pointer on xatp
because
it modifies .idx and .gpfn.
- Cancel hypercall continuation in the compat layer if the copy_to_guest
failed.
Changes between v5 and v6:
- Rework the patch queue to make it more readable.
- Modify xatp in place in xenmem_add_to_physmap
- Only check for preemption if we are not at the last iteration
- Copy xatp guest handler back to the guest only in case of continuation
- Add continuation only when dealing with the new xenmem space
(XENMAPSPACE_gmfn_range).
Changes between v4 and v5:
- Fix hypercall continuation for add_to_physmap in compat mode.
Changes between v3 and v4:
- Move the loop for gmfn_range from arch_memory_op to
xenmem_add_to_physmap.
- Add a comment to comment to explain the purpose of
iommu_dont_flush_iotlb.
Changes between v2 and v3:
- Check for the presence iotlb_flush_all callback before calling it.
Changes between v1 and v2:
- Move size in struct xen_add_to_physmap in padding between .domid and
.space.
- Store iommu_dont_flush per cpu
- Change the code in hvmloader to relocate by batch of 64K, .size is now
16 bits.
Jean Guyader (6):
vtd: Refactor iotlb flush code
iommu: Introduce iommu_flush and iommu_flush_all.
add_to_physmap: Move the code for XENMEM_add_to_physmap
mm: New XENMEM space, XENMAPSPACE_gmfn_range
hvmloader: Change memory relocation loop when overlap with PCI hole
Introduce per cpu flag (iommu_dont_flush_iotlb) to avoid unnecessary
iotlb flush
tools/firmware/hvmloader/pci.c | 20 ++-
xen/arch/x86/mm.c | 231 ++++++++++++++++++++++-------------
xen/arch/x86/x86_64/compat/mm.c | 14 ++
xen/drivers/passthrough/iommu.c | 25 ++++
xen/drivers/passthrough/vtd/iommu.c | 100 +++++++++-------
xen/include/public/memory.h | 4 +
xen/include/xen/iommu.h | 17 +++
7 files changed, 279 insertions(+), 132 deletions(-)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Factorize the iotlb flush code from map_page and unmap_page into it''s own function. Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> Acked-by: Allen M Kay <allen.m.kay@intel.com> --- xen/drivers/passthrough/vtd/iommu.c | 86 +++++++++++++++++----------------- 1 files changed, 43 insertions(+), 43 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jean Guyader
2011-Nov-13 17:40 UTC
[Xen-devel] [PATCH 2/6] iommu: Introduce iommu_flush and iommu_flush_all.
Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> Acked-by: Allen M Kay <allen.m.kay@intel.com> --- xen/drivers/passthrough/iommu.c | 20 ++++++++++++++++++++ xen/drivers/passthrough/vtd/iommu.c | 12 ++++++++++++ xen/include/xen/iommu.h | 5 +++++ 3 files changed, 37 insertions(+), 0 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jean Guyader
2011-Nov-13 17:40 UTC
[Xen-devel] [PATCH 3/6] add_to_physmap: Move the code for XENMEM_add_to_physmap
Move the code for the XENMEM_add_to_physmap case into it''s own function (xenmem_add_to_physmap). Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> --- xen/arch/x86/mm.c | 189 ++++++++++++++++++++++++++++------------------------- 1 files changed, 100 insertions(+), 89 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jean Guyader
2011-Nov-13 17:40 UTC
[Xen-devel] [PATCH 4/6] mm: New XENMEM space, XENMAPSPACE_gmfn_range
XENMAPSPACE_gmfn_range is like XENMAPSPACE_gmfn but it runs on a range of pages. The size of the range is defined in a new field. This new field .size is located in the 16 bits padding between .domid and .space in struct xen_add_to_physmap to stay compatible with older versions. Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> --- xen/arch/x86/mm.c | 48 ++++++++++++++++++++++++++++++++++++-- xen/arch/x86/x86_64/compat/mm.c | 14 +++++++++++ xen/include/public/memory.h | 4 +++ 3 files changed, 63 insertions(+), 3 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jean Guyader
2011-Nov-13 17:40 UTC
[Xen-devel] [PATCH 5/6] hvmloader: Change memory relocation loop when overlap with PCI hole
Change the way we relocate the memory page if they overlap with pci hole. Use new map space (XENMAPSPACE_gmfn_range) to move the loop into xen. This code usually get triggered when a device is pass through to a guest and the PCI hole has to be extended to have enough room to map the device BARs. The PCI hole will starts lower and it might overlap with some RAM that has been alocated for the guest. That usually happen if the guest has more than 4G of RAM. We have to relocate those pages in high mem otherwise they won''t be accessible. Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> --- tools/firmware/hvmloader/pci.c | 20 ++++++++++++++------ 1 files changed, 14 insertions(+), 6 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jean Guyader
2011-Nov-13 17:40 UTC
[Xen-devel] [PATCH 6/6] Introduce per cpu flag (iommu_dont_flush_iotlb) to avoid unnecessary iotlb flush
Add cpu flag that will be checked by the iommu low level code to skip iotlb flushes. iommu_iotlb_flush shall be called explicitly. Signed-off-by: Jean Guyader <jean.guyader@eu.citrix.com> --- xen/arch/x86/mm.c | 12 ++++++++++++ xen/drivers/passthrough/iommu.c | 5 +++++ xen/drivers/passthrough/vtd/iommu.c | 6 ++++-- xen/include/xen/iommu.h | 12 ++++++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2011-Nov-14 10:04 UTC
[Xen-devel] Re: [PATCH 0/6] IOMMU, vtd and iotlb flush rework (v7)
>>> On 13.11.11 at 18:40, Jean Guyader <jean.guyader@eu.citrix.com> wrote: > In one of my previous email I detailed a bug I was seeing when passing > through a Intel GPU on a guest that has more that 4G or RAM. > > Allen suggested that I go for the Plan B but after a discussion with Tim > we agreed that Plan B was way to disruptive in term of code change. > > This patch series implements Plan A. > > http://xen.1045712.n5.nabble.com/VTD-Intel-iommu-IOTLB-flush-really-slow-td4 > 952866.html > > Changes between v6 and v7: > - xenmem_add_to_physmap_once can''t take a pointer on xatp because > it modifies .idx and .gpfn.This is not an excuse for passing a structure by value. Despite the structure not being huge, I still consider this bad style. Jan> - Cancel hypercall continuation in the compat layer if the copy_to_guest > failed. > > Changes between v5 and v6: > - Rework the patch queue to make it more readable. > - Modify xatp in place in xenmem_add_to_physmap > - Only check for preemption if we are not at the last iteration > - Copy xatp guest handler back to the guest only in case of > continuation > - Add continuation only when dealing with the new xenmem space > (XENMAPSPACE_gmfn_range). > > Changes between v4 and v5: > - Fix hypercall continuation for add_to_physmap in compat mode. > > Changes between v3 and v4: > - Move the loop for gmfn_range from arch_memory_op to > xenmem_add_to_physmap. > - Add a comment to comment to explain the purpose of > iommu_dont_flush_iotlb. > > Changes between v2 and v3: > - Check for the presence iotlb_flush_all callback before calling it. > > Changes between v1 and v2: > - Move size in struct xen_add_to_physmap in padding between .domid > and .space. > - Store iommu_dont_flush per cpu > - Change the code in hvmloader to relocate by batch of 64K, .size is > now 16 bits. > > Jean Guyader (6): > vtd: Refactor iotlb flush code > iommu: Introduce iommu_flush and iommu_flush_all. > add_to_physmap: Move the code for XENMEM_add_to_physmap > mm: New XENMEM space, XENMAPSPACE_gmfn_range > hvmloader: Change memory relocation loop when overlap with PCI hole > Introduce per cpu flag (iommu_dont_flush_iotlb) to avoid unnecessary > iotlb flush > > tools/firmware/hvmloader/pci.c | 20 ++- > xen/arch/x86/mm.c | 231 > ++++++++++++++++++++++------------- > xen/arch/x86/x86_64/compat/mm.c | 14 ++ > xen/drivers/passthrough/iommu.c | 25 ++++ > xen/drivers/passthrough/vtd/iommu.c | 100 +++++++++------- > xen/include/public/memory.h | 4 + > xen/include/xen/iommu.h | 17 +++ > 7 files changed, 279 insertions(+), 132 deletions(-)_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2011-Nov-14 10:11 UTC
[Xen-devel] Re: [PATCH 4/6] mm: New XENMEM space, XENMAPSPACE_gmfn_range
>>> On 13.11.11 at 18:40, Jean Guyader <jean.guyader@eu.citrix.com> wrote: >@@ -4715,10 +4747,20 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg) > return -EPERM; > } > >- rc = xenmem_add_to_physmap(d, xatp); >+ rc = xenmem_add_to_physmap(d, &xatp); > > rcu_unlock_domain(d); > >+ if ( rc )I thought that we had agreed to do the copying back only for the new space.>+ { >+ if ( copy_to_guest(arg, &xatp, 1) ) >+ return -EFAULT; >+ }Also, I personally would prefer avoiding unnecessary extra return points (as they always bare the risk of being overlooked for subsequent changes, and when they aren''t, the resulting patches usually are bigger and less readable). Here this is rather simple: if ( rc && ... == XENMAPSPACE_gmfn_range && copy_to_guest(arg, &xatp, 1) ) rc = -EFAULT; Jan>+ >+ if ( rc == -EAGAIN ) >+ rc = hypercall_create_continuation( >+ __HYPERVISOR_memory_op, "ih", op, arg); >+ > return rc; > } >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel