| Message ID | 1406542610-18724-3-git-send-email-stefano.stabellini@eu.citrix.com |
|---|---|
| State | New |
| Headers | show |
Hi Stefano, On 07/28/2014 11:16 AM, Stefano Stabellini wrote: > The flag specifies that the hypervisor maps a grant page to guest > physical address == machine address of the page in addition to the > normal grant mapping address. > > Frontends are allowed to map the same page multiple times using multiple > grant references. On the backend side it can be difficult to find out > the physical address corresponding to a particular machine address, > especially at the completation of a dma operation. To simplify address completion [..] > if ( err ) > { > @@ -941,9 +951,18 @@ __gnttab_unmap_common( > int err = 0; > mapcount(lgt, rd, op->frame, &wrc, &rdc); > if ( (wrc + rdc) == 0 ) > - err = iommu_unmap_page(ld, op->frame); > - else if ( wrc == 0 ) > - err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable); > + { > + if ( is_domain_direct_mapped(ld) ) > + err = arch_grant_unmap_page_identity(ld, op->frame); > + else > + err = iommu_unmap_page(ld, op->frame); > + } else if ( wrc == 0 ) > + { > + if ( is_domain_direct_mapped(ld) ) > + err = arch_grant_map_page_identity(ld, op->frame, 0); > + else > + err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable); With this solution, iommu_map_page will never be called on ARM. It's because gnttab_need_iommu_mapping is checking the domain is using direct mapping. So, I think you can drop the iommu_map_page callback of the SMMU in patch #2. Regards,
On Mon, 28 Jul 2014, Julien Grall wrote: > Hi Stefano, > > On 07/28/2014 11:16 AM, Stefano Stabellini wrote: > > The flag specifies that the hypervisor maps a grant page to guest > > physical address == machine address of the page in addition to the > > normal grant mapping address. > > > > Frontends are allowed to map the same page multiple times using multiple > > grant references. On the backend side it can be difficult to find out > > the physical address corresponding to a particular machine address, > > especially at the completation of a dma operation. To simplify address > > completion > > [..] > > > if ( err ) > > { > > @@ -941,9 +951,18 @@ __gnttab_unmap_common( > > int err = 0; > > mapcount(lgt, rd, op->frame, &wrc, &rdc); > > if ( (wrc + rdc) == 0 ) > > - err = iommu_unmap_page(ld, op->frame); > > - else if ( wrc == 0 ) > > - err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable); > > + { > > + if ( is_domain_direct_mapped(ld) ) > > + err = arch_grant_unmap_page_identity(ld, op->frame); > > + else > > + err = iommu_unmap_page(ld, op->frame); > > + } else if ( wrc == 0 ) > > + { > > + if ( is_domain_direct_mapped(ld) ) > > + err = arch_grant_map_page_identity(ld, op->frame, 0); > > + else > > + err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable); > > With this solution, iommu_map_page will never be called on ARM. It's > because gnttab_need_iommu_mapping is checking the domain is using direct > mapping. > > So, I think you can drop the iommu_map_page callback of the SMMU in > patch #2. OK. Should I keep your acked-by on that patch?
On Mon, 28 Jul 2014, Stefano Stabellini wrote: > On Mon, 28 Jul 2014, Julien Grall wrote: > > Hi Stefano, > > > > On 07/28/2014 11:16 AM, Stefano Stabellini wrote: > > > The flag specifies that the hypervisor maps a grant page to guest > > > physical address == machine address of the page in addition to the > > > normal grant mapping address. > > > > > > Frontends are allowed to map the same page multiple times using multiple > > > grant references. On the backend side it can be difficult to find out > > > the physical address corresponding to a particular machine address, > > > especially at the completation of a dma operation. To simplify address > > > > completion > > > > [..] > > > > > if ( err ) > > > { > > > @@ -941,9 +951,18 @@ __gnttab_unmap_common( > > > int err = 0; > > > mapcount(lgt, rd, op->frame, &wrc, &rdc); > > > if ( (wrc + rdc) == 0 ) > > > - err = iommu_unmap_page(ld, op->frame); > > > - else if ( wrc == 0 ) > > > - err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable); > > > + { > > > + if ( is_domain_direct_mapped(ld) ) > > > + err = arch_grant_unmap_page_identity(ld, op->frame); > > > + else > > > + err = iommu_unmap_page(ld, op->frame); > > > + } else if ( wrc == 0 ) > > > + { > > > + if ( is_domain_direct_mapped(ld) ) > > > + err = arch_grant_map_page_identity(ld, op->frame, 0); > > > + else > > > + err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable); > > > > With this solution, iommu_map_page will never be called on ARM. It's > > because gnttab_need_iommu_mapping is checking the domain is using direct > > mapping. > > > > So, I think you can drop the iommu_map_page callback of the SMMU in > > patch #2. > > OK. Should I keep your acked-by on that patch? Actually it has to be done on this patch, otherwise the changes are not bisectable.
On 07/28/2014 04:21 PM, Stefano Stabellini wrote: > On Mon, 28 Jul 2014, Julien Grall wrote: >> Hi Stefano, >> >> On 07/28/2014 11:16 AM, Stefano Stabellini wrote: >>> The flag specifies that the hypervisor maps a grant page to guest >>> physical address == machine address of the page in addition to the >>> normal grant mapping address. >>> >>> Frontends are allowed to map the same page multiple times using multiple >>> grant references. On the backend side it can be difficult to find out >>> the physical address corresponding to a particular machine address, >>> especially at the completation of a dma operation. To simplify address >> >> completion >> >> [..] >> >>> if ( err ) >>> { >>> @@ -941,9 +951,18 @@ __gnttab_unmap_common( >>> int err = 0; >>> mapcount(lgt, rd, op->frame, &wrc, &rdc); >>> if ( (wrc + rdc) == 0 ) >>> - err = iommu_unmap_page(ld, op->frame); >>> - else if ( wrc == 0 ) >>> - err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable); >>> + { >>> + if ( is_domain_direct_mapped(ld) ) >>> + err = arch_grant_unmap_page_identity(ld, op->frame); >>> + else >>> + err = iommu_unmap_page(ld, op->frame); >>> + } else if ( wrc == 0 ) >>> + { >>> + if ( is_domain_direct_mapped(ld) ) >>> + err = arch_grant_map_page_identity(ld, op->frame, 0); >>> + else >>> + err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable); >> >> With this solution, iommu_map_page will never be called on ARM. It's >> because gnttab_need_iommu_mapping is checking the domain is using direct >> mapping. >> >> So, I think you can drop the iommu_map_page callback of the SMMU in >> patch #2. > > OK. Should I keep your acked-by on that patch? Yes, I'm fine with one of 2 solutions (i.e either keep as it is or removing the {,un}map_page callback). Regards,
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index 464007e..68df49d 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -738,13 +738,23 @@ __gnttab_map_grant_ref( !(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) ) { if ( wrc == 0 ) - err = iommu_map_page(ld, frame, frame, - IOMMUF_readable|IOMMUF_writable); + { + if ( is_domain_direct_mapped(ld) ) + err = arch_grant_map_page_identity(ld, frame, 1); + else + err = iommu_map_page(ld, frame, frame, + IOMMUF_readable|IOMMUF_writable); + } } else if ( act_pin && !old_pin ) { if ( (wrc + rdc) == 0 ) - err = iommu_map_page(ld, frame, frame, IOMMUF_readable); + { + if ( is_domain_direct_mapped(ld) ) + err = arch_grant_map_page_identity(ld, frame, 0); + else + err = iommu_map_page(ld, frame, frame, IOMMUF_readable); + } } if ( err ) { @@ -941,9 +951,18 @@ __gnttab_unmap_common( int err = 0; mapcount(lgt, rd, op->frame, &wrc, &rdc); if ( (wrc + rdc) == 0 ) - err = iommu_unmap_page(ld, op->frame); - else if ( wrc == 0 ) - err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable); + { + if ( is_domain_direct_mapped(ld) ) + err = arch_grant_unmap_page_identity(ld, op->frame); + else + err = iommu_unmap_page(ld, op->frame); + } else if ( wrc == 0 ) + { + if ( is_domain_direct_mapped(ld) ) + err = arch_grant_map_page_identity(ld, op->frame, 0); + else + err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable); + } if ( err ) { rc = GNTST_general_error; diff --git a/xen/common/kernel.c b/xen/common/kernel.c index 7e83353..fa40a36 100644 --- a/xen/common/kernel.c +++ b/xen/common/kernel.c @@ -325,6 +325,8 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg) break; } #endif + if ( is_domain_direct_mapped(d) ) + fi.submap |= 1U << XENFEAT_grant_map_identity; break; default: return -EINVAL; diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h index eac8a70..47147ce 100644 --- a/xen/include/asm-arm/grant_table.h +++ b/xen/include/asm-arm/grant_table.h @@ -33,8 +33,7 @@ static inline int replace_grant_supported(void) ( ((i >= nr_grant_frames(d->grant_table)) && \ (i < max_nr_grant_frames)) ? 0 : (d->arch.grant_table_gpfn[i])) -#define gnttab_need_iommu_mapping(d) \ - (is_domain_direct_mapped(d) && need_iommu(d)) +#define gnttab_need_iommu_mapping(d) (is_domain_direct_mapped(d)) #endif /* __ASM_GRANT_TABLE_H__ */ /* diff --git a/xen/include/public/features.h b/xen/include/public/features.h index a149aa6..ba753a0 100644 --- a/xen/include/public/features.h +++ b/xen/include/public/features.h @@ -94,6 +94,9 @@ /* operation as Dom0 is supported */ #define XENFEAT_dom0 11 +/* Xen also maps grant references at pfn = mfn */ +#define XENFEAT_grant_map_identity 12 + #define XENFEAT_NR_SUBMAPS 1 #endif /* __XEN_PUBLIC_FEATURES_H__ */
The flag specifies that the hypervisor maps a grant page to guest physical address == machine address of the page in addition to the normal grant mapping address. Frontends are allowed to map the same page multiple times using multiple grant references. On the backend side it can be difficult to find out the physical address corresponding to a particular machine address, especially at the completation of a dma operation. To simplify address translations, we introduce a second mapping of the grant at physical address == machine address so that dom0 can issue cache maintenance operations without having to find the pfn. Call arch_grant_map_page_identity and arch_grant_unmap_page_identity from __gnttab_map_grant_ref and __gnttab_unmap_common to introduce the second mapping if the domain is directly mapped. To do so we also need to change gnttab_need_iommu_mapping to just be defined as is_domain_direct_mapped on arm. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- Changes in v4: - add XENFEAT_grant_map_identity if is_domain_direct_mapped; - remove gnttab_need_identity_mapping, check is_domain_direct_mapped instead; - define gnttab_need_iommu_mapping as is_domain_direct_mapped on arm. Changes in v3: - introduce gnttab_need_identity_mapping; - check gnttab_need_identity_mapping in __gnttab_map_grant_ref and __gnttab_unmap_common. Changes in v2: - rename XENFEAT_grant_map_11 to XENFEAT_grant_map_identity; - remove superfluous ifdef CONFIG_ARM in xen/common/kernel.c; - don't modify gnttab_need_iommu_mapping; - call arch_grant_map_page_identity and arch_grant_unmap_page_identity from grant_table functions. --- xen/common/grant_table.c | 31 +++++++++++++++++++++++++------ xen/common/kernel.c | 2 ++ xen/include/asm-arm/grant_table.h | 3 +-- xen/include/public/features.h | 3 +++ 4 files changed, 31 insertions(+), 8 deletions(-)