| Message ID | 1404834794-16055-1-git-send-email-stefano.stabellini@eu.citrix.com |
|---|---|
| State | New |
| Headers | show |
Hi Stefano, You forgot to add Jan Beulich for the generic IOMMU part. Regards, On 07/08/2014 04:53 PM, Stefano Stabellini wrote: > From: Julien Grall <julien.grall@citrix.com> > > From: Julien Grall <julien.grall@citrix.com> > > Introduce two arch specific iommu grant mapping and unmapping functions, > they are called from __gnttab_map_grant_ref and __gnttab_unmap_common. > > The x86 implementation simply calls iommu_(un)map_page. > The ARM implementation is based on arm_smmu_(un)map_page. > > Signed-off-by: Julien Grall <julien.grall@citrix.com> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > xen/common/grant_table.c | 10 +++---- > xen/drivers/passthrough/arm/iommu.c | 49 +++++++++++++++++++++++++++++++++++ > xen/drivers/passthrough/arm/smmu.c | 42 ------------------------------ > xen/drivers/passthrough/x86/iommu.c | 11 ++++++++ > xen/include/xen/iommu.h | 4 +++ > 5 files changed, 69 insertions(+), 47 deletions(-) > > diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c > index 464007e..6ea1c2f 100644 > --- a/xen/common/grant_table.c > +++ b/xen/common/grant_table.c > @@ -738,13 +738,13 @@ __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); > + err = arch_iommu_grant_map_page(ld, frame, > + IOMMUF_readable|IOMMUF_writable); > } > else if ( act_pin && !old_pin ) > { > if ( (wrc + rdc) == 0 ) > - err = iommu_map_page(ld, frame, frame, IOMMUF_readable); > + err = arch_iommu_grant_map_page(ld, frame, IOMMUF_readable); > } > if ( err ) > { > @@ -941,9 +941,9 @@ __gnttab_unmap_common( > int err = 0; > mapcount(lgt, rd, op->frame, &wrc, &rdc); > if ( (wrc + rdc) == 0 ) > - err = iommu_unmap_page(ld, op->frame); > + err = arch_iommu_grant_unmap_page(ld, op->frame); > else if ( wrc == 0 ) > - err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable); > + err = arch_iommu_grant_map_page(ld, op->frame, IOMMUF_readable); > if ( err ) > { > rc = GNTST_general_error; > diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c > index 3007b99..6460b7e 100644 > --- a/xen/drivers/passthrough/arm/iommu.c > +++ b/xen/drivers/passthrough/arm/iommu.c > @@ -18,6 +18,8 @@ > #include <xen/lib.h> > #include <xen/iommu.h> > #include <xen/device_tree.h> > +#include <xen/sched.h> > +#include <asm/p2m.h> > #include <asm/device.h> > > static const struct iommu_ops *iommu_ops; > @@ -68,3 +70,50 @@ void arch_iommu_domain_destroy(struct domain *d) > { > iommu_dt_domain_destroy(d); > } > + > +int arch_iommu_grant_map_page(struct domain *d, unsigned long frame, > + unsigned flags) > +{ > + p2m_type_t t; > + > + /* Grant mappings can be used for DMA requests. The dev_bus_addr returned by > + * the hypercall is the MFN (not the IPA). For device protected by > + * an IOMMU, Xen needs to add a 1:1 mapping in the domain p2m to > + * allow DMA request to work. > + * This is only valid when the domain is directed mapped. > + */ > + BUG_ON(!is_domain_direct_mapped(d)); > + > + /* We only support readable and writable flags */ > + if ( !(flags & (IOMMUF_readable | IOMMUF_writable)) ) > + return -EINVAL; > + > + t = (flags & IOMMUF_writable) ? p2m_iommu_map_rw : p2m_iommu_map_ro; > + > + /* The function guest_physmap_add_entry replaces the current mapping > + * if there is already one... > + */ > + return guest_physmap_add_entry(d, frame, frame, 0, t); > +} > + > +int arch_iommu_grant_unmap_page(struct domain *d, unsigned long frame) > +{ > + /* This function should only be used by gnttab code when the domain > + * is direct mapped > + */ > + if ( !is_domain_direct_mapped(d) ) > + return -EINVAL; > + > + guest_physmap_remove_page(d, frame, frame, 0); > + > + return 0; > +} > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c > index f4eb2a2..21b4572 100644 > --- a/xen/drivers/passthrough/arm/smmu.c > +++ b/xen/drivers/passthrough/arm/smmu.c > @@ -1536,46 +1536,6 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d) > xfree(smmu_domain); > } > > -static int arm_smmu_map_page(struct domain *d, unsigned long gfn, > - unsigned long mfn, unsigned int flags) > -{ > - p2m_type_t t; > - > - /* Grant mappings can be used for DMA requests. The dev_bus_addr returned by > - * the hypercall is the MFN (not the IPA). For device protected by > - * an IOMMU, Xen needs to add a 1:1 mapping in the domain p2m to > - * allow DMA request to work. > - * This is only valid when the domain is directed mapped. Hence this > - * function should only be used by gnttab code with gfn == mfn. > - */ > - BUG_ON(!is_domain_direct_mapped(d)); > - BUG_ON(mfn != gfn); > - > - /* We only support readable and writable flags */ > - if ( !(flags & (IOMMUF_readable | IOMMUF_writable)) ) > - return -EINVAL; > - > - t = (flags & IOMMUF_writable) ? p2m_iommu_map_rw : p2m_iommu_map_ro; > - > - /* The function guest_physmap_add_entry replaces the current mapping > - * if there is already one... > - */ > - return guest_physmap_add_entry(d, gfn, mfn, 0, t); > -} > - > -static int arm_smmu_unmap_page(struct domain *d, unsigned long gfn) > -{ > - /* This function should only be used by gnttab code when the domain > - * is direct mapped > - */ > - if ( !is_domain_direct_mapped(d) ) > - return -EINVAL; > - > - guest_physmap_remove_page(d, gfn, gfn, 0); > - > - return 0; > -} > - > static const struct iommu_ops arm_smmu_iommu_ops = { > .init = arm_smmu_iommu_domain_init, > .hwdom_init = arm_smmu_iommu_hwdom_init, > @@ -1584,8 +1544,6 @@ static const struct iommu_ops arm_smmu_iommu_ops = { > .iotlb_flush_all = arm_smmu_iotlb_flush_all, > .assign_dt_device = arm_smmu_attach_dev, > .reassign_dt_device = arm_smmu_reassign_dt_dev, > - .map_page = arm_smmu_map_page, > - .unmap_page = arm_smmu_unmap_page, > }; > > static int __init smmu_init(struct dt_device_node *dev, > diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c > index ce0ca5a..a88626a 100644 > --- a/xen/drivers/passthrough/x86/iommu.c > +++ b/xen/drivers/passthrough/x86/iommu.c > @@ -135,6 +135,17 @@ void arch_iommu_domain_destroy(struct domain *d) > } > } > > +int arch_iommu_grant_map_page(struct domain *d, unsigned long frame, > + unsigned flags) > +{ > + return iommu_map_page(d, frame, frame, flags); > +} > + > +int arch_iommu_grant_unmap_page(struct domain *d, unsigned long frame) > +{ > + return iommu_unmap_page(d, frame); > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h > index 8eb764a..0dd6987 100644 > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -76,6 +76,10 @@ int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn, > unsigned int flags); > int iommu_unmap_page(struct domain *d, unsigned long gfn); > > +int arch_iommu_grant_map_page(struct domain *d, unsigned long frame, > + unsigned flags); > +int arch_iommu_grant_unmap_page(struct domain *d, unsigned long frame); > + > enum iommu_feature > { > IOMMU_FEAT_COHERENT_WALK, >
On Tue, 2014-07-08 at 16:53 +0100, Stefano Stabellini wrote: > +int arch_iommu_grant_map_page(struct domain *d, unsigned long frame, > + unsigned flags) > +{ > + p2m_type_t t; > + > + /* Grant mappings can be used for DMA requests. The dev_bus_addr returned by > + * the hypercall is the MFN (not the IPA). For device protected by > + * an IOMMU, Xen needs to add a 1:1 mapping in the domain p2m to > + * allow DMA request to work. > + * This is only valid when the domain is directed mapped. > + */ > + BUG_ON(!is_domain_direct_mapped(d)); What stops a non-1:1 guest from trying to map a grant reference and ending up here? Ian.
Hi Ian, On 16/07/14 16:56, Ian Campbell wrote: > On Tue, 2014-07-08 at 16:53 +0100, Stefano Stabellini wrote: >> +int arch_iommu_grant_map_page(struct domain *d, unsigned long frame, >> + unsigned flags) >> +{ >> + p2m_type_t t; >> + >> + /* Grant mappings can be used for DMA requests. The dev_bus_addr returned by >> + * the hypercall is the MFN (not the IPA). For device protected by >> + * an IOMMU, Xen needs to add a 1:1 mapping in the domain p2m to >> + * allow DMA request to work. >> + * This is only valid when the domain is directed mapped. >> + */ >> + BUG_ON(!is_domain_direct_mapped(d)); > > What stops a non-1:1 guest from trying to map a grant reference and > ending up here? The function is protected by gnttab_need_iommu_mapping which check if the domain use direct mapping for the memory. Regards,
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index 464007e..6ea1c2f 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -738,13 +738,13 @@ __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); + err = arch_iommu_grant_map_page(ld, frame, + IOMMUF_readable|IOMMUF_writable); } else if ( act_pin && !old_pin ) { if ( (wrc + rdc) == 0 ) - err = iommu_map_page(ld, frame, frame, IOMMUF_readable); + err = arch_iommu_grant_map_page(ld, frame, IOMMUF_readable); } if ( err ) { @@ -941,9 +941,9 @@ __gnttab_unmap_common( int err = 0; mapcount(lgt, rd, op->frame, &wrc, &rdc); if ( (wrc + rdc) == 0 ) - err = iommu_unmap_page(ld, op->frame); + err = arch_iommu_grant_unmap_page(ld, op->frame); else if ( wrc == 0 ) - err = iommu_map_page(ld, op->frame, op->frame, IOMMUF_readable); + err = arch_iommu_grant_map_page(ld, op->frame, IOMMUF_readable); if ( err ) { rc = GNTST_general_error; diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c index 3007b99..6460b7e 100644 --- a/xen/drivers/passthrough/arm/iommu.c +++ b/xen/drivers/passthrough/arm/iommu.c @@ -18,6 +18,8 @@ #include <xen/lib.h> #include <xen/iommu.h> #include <xen/device_tree.h> +#include <xen/sched.h> +#include <asm/p2m.h> #include <asm/device.h> static const struct iommu_ops *iommu_ops; @@ -68,3 +70,50 @@ void arch_iommu_domain_destroy(struct domain *d) { iommu_dt_domain_destroy(d); } + +int arch_iommu_grant_map_page(struct domain *d, unsigned long frame, + unsigned flags) +{ + p2m_type_t t; + + /* Grant mappings can be used for DMA requests. The dev_bus_addr returned by + * the hypercall is the MFN (not the IPA). For device protected by + * an IOMMU, Xen needs to add a 1:1 mapping in the domain p2m to + * allow DMA request to work. + * This is only valid when the domain is directed mapped. + */ + BUG_ON(!is_domain_direct_mapped(d)); + + /* We only support readable and writable flags */ + if ( !(flags & (IOMMUF_readable | IOMMUF_writable)) ) + return -EINVAL; + + t = (flags & IOMMUF_writable) ? p2m_iommu_map_rw : p2m_iommu_map_ro; + + /* The function guest_physmap_add_entry replaces the current mapping + * if there is already one... + */ + return guest_physmap_add_entry(d, frame, frame, 0, t); +} + +int arch_iommu_grant_unmap_page(struct domain *d, unsigned long frame) +{ + /* This function should only be used by gnttab code when the domain + * is direct mapped + */ + if ( !is_domain_direct_mapped(d) ) + return -EINVAL; + + guest_physmap_remove_page(d, frame, frame, 0); + + return 0; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c index f4eb2a2..21b4572 100644 --- a/xen/drivers/passthrough/arm/smmu.c +++ b/xen/drivers/passthrough/arm/smmu.c @@ -1536,46 +1536,6 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d) xfree(smmu_domain); } -static int arm_smmu_map_page(struct domain *d, unsigned long gfn, - unsigned long mfn, unsigned int flags) -{ - p2m_type_t t; - - /* Grant mappings can be used for DMA requests. The dev_bus_addr returned by - * the hypercall is the MFN (not the IPA). For device protected by - * an IOMMU, Xen needs to add a 1:1 mapping in the domain p2m to - * allow DMA request to work. - * This is only valid when the domain is directed mapped. Hence this - * function should only be used by gnttab code with gfn == mfn. - */ - BUG_ON(!is_domain_direct_mapped(d)); - BUG_ON(mfn != gfn); - - /* We only support readable and writable flags */ - if ( !(flags & (IOMMUF_readable | IOMMUF_writable)) ) - return -EINVAL; - - t = (flags & IOMMUF_writable) ? p2m_iommu_map_rw : p2m_iommu_map_ro; - - /* The function guest_physmap_add_entry replaces the current mapping - * if there is already one... - */ - return guest_physmap_add_entry(d, gfn, mfn, 0, t); -} - -static int arm_smmu_unmap_page(struct domain *d, unsigned long gfn) -{ - /* This function should only be used by gnttab code when the domain - * is direct mapped - */ - if ( !is_domain_direct_mapped(d) ) - return -EINVAL; - - guest_physmap_remove_page(d, gfn, gfn, 0); - - return 0; -} - static const struct iommu_ops arm_smmu_iommu_ops = { .init = arm_smmu_iommu_domain_init, .hwdom_init = arm_smmu_iommu_hwdom_init, @@ -1584,8 +1544,6 @@ static const struct iommu_ops arm_smmu_iommu_ops = { .iotlb_flush_all = arm_smmu_iotlb_flush_all, .assign_dt_device = arm_smmu_attach_dev, .reassign_dt_device = arm_smmu_reassign_dt_dev, - .map_page = arm_smmu_map_page, - .unmap_page = arm_smmu_unmap_page, }; static int __init smmu_init(struct dt_device_node *dev, diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c index ce0ca5a..a88626a 100644 --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -135,6 +135,17 @@ void arch_iommu_domain_destroy(struct domain *d) } } +int arch_iommu_grant_map_page(struct domain *d, unsigned long frame, + unsigned flags) +{ + return iommu_map_page(d, frame, frame, flags); +} + +int arch_iommu_grant_unmap_page(struct domain *d, unsigned long frame) +{ + return iommu_unmap_page(d, frame); +} + /* * Local variables: * mode: C diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index 8eb764a..0dd6987 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -76,6 +76,10 @@ int iommu_map_page(struct domain *d, unsigned long gfn, unsigned long mfn, unsigned int flags); int iommu_unmap_page(struct domain *d, unsigned long gfn); +int arch_iommu_grant_map_page(struct domain *d, unsigned long frame, + unsigned flags); +int arch_iommu_grant_unmap_page(struct domain *d, unsigned long frame); + enum iommu_feature { IOMMU_FEAT_COHERENT_WALK,