Re: [PATCH v4 6/7] Revert "xen/arm: introduce XENFEAT_grant_map_identity"

[Julien Grall <julien.grall@linaro.org>]

Hi Stefano,

On 10/10/2014 12:43, Stefano Stabellini wrote:
> Revert commit id 8d09ef6906ca0a9957e21334ad2c3eed626abe63.
> Just keep the definition of XENFEAT_grant_map_identity.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

>
> ---
>
> Changes in v2:
>
> - comment out the definition of XENFEAT_grant_map_identity.
> ---
>   xen/common/grant_table.c           |   30 +++++-------------------------
>   xen/common/kernel.c                |    2 --
>   xen/drivers/passthrough/arm/smmu.c |   33 +++++++++++++++++++++++++++++++++
>   xen/include/asm-arm/grant_table.h  |    3 ++-
>   xen/include/public/features.h      |    4 +++-
>   5 files changed, 43 insertions(+), 29 deletions(-)
>
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index a989d81..118d8c7 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -765,23 +765,13 @@ __gnttab_map_grant_ref(
>                !(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
>           {
>               if ( wrc == 0 )
> -            {
> -                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);
> -            }
> +                err = iommu_map_page(ld, frame, frame,
> +                                     IOMMUF_readable|IOMMUF_writable);
>           }
>           else if ( act_pin && !old_pin )
>           {
>               if ( (wrc + rdc) == 0 )
> -            {
> -                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);
> -            }
> +                err = iommu_map_page(ld, frame, frame, IOMMUF_readable);
>           }
>           if ( err )
>           {
> @@ -978,19 +968,9 @@ __gnttab_unmap_common(
>           int err = 0;
>           mapcount(lgt, rd, op->frame, &wrc, &rdc);
>           if ( (wrc + rdc) == 0 )
> -        {
> -            if ( is_domain_direct_mapped(ld) )
> -                err = arch_grant_unmap_page_identity(ld, op->frame);
> -            else
> -                err = iommu_unmap_page(ld, op->frame);
> -        }
> +            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);
> -        }
> +            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 ce65486..d23c422 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -332,8 +332,6 @@ 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/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
> index 3cbd206..9a95ac9 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -1536,6 +1536,37 @@ 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)
> +{
> +    /* 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;
> +
> +    return arch_grant_map_page_identity(d, mfn, flags & IOMMUF_writable);
> +}
> +
> +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;
> +
> +    return arch_grant_unmap_page_identity(d, gfn);
> +}
> +
>   static const struct iommu_ops arm_smmu_iommu_ops = {
>       .init = arm_smmu_iommu_domain_init,
>       .hwdom_init = arm_smmu_iommu_hwdom_init,
> @@ -1544,6 +1575,8 @@ 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/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
> index 47147ce..eac8a70 100644
> --- a/xen/include/asm-arm/grant_table.h
> +++ b/xen/include/asm-arm/grant_table.h
> @@ -33,7 +33,8 @@ 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))
> +#define gnttab_need_iommu_mapping(d)                    \
> +    (is_domain_direct_mapped(d) && need_iommu(d))
>
>   #endif /* __ASM_GRANT_TABLE_H__ */
>   /*
> diff --git a/xen/include/public/features.h b/xen/include/public/features.h
> index b7bf83f..16d92aa 100644
> --- a/xen/include/public/features.h
> +++ b/xen/include/public/features.h
> @@ -94,8 +94,10 @@
>   /* operation as Dom0 is supported */
>   #define XENFEAT_dom0                      11
>
> -/* Xen also maps grant references at pfn = mfn */
> +/* Xen also maps grant references at pfn = mfn.
> + * This feature flag is deprecated and should not be used.
>   #define XENFEAT_grant_map_identity        12
> + */
>
>   #define XENFEAT_NR_SUBMAPS 1
>
>

-- 
Julien Grall