[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v5 15/17] xen/arm: p2m: Introduce helpers to insert and remove mapping



On Tue, 28 Jun 2016, Julien Grall wrote:
> More the half of the arguments of INSERT and REMOVE are the same for
> each callers. Simplify the callers of apply_p2m_changes by adding new
> helpers which will fill common arguments with default values.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>

I don't see much value in this patch. It looks good because it is before
"Rework the interface of apply_p2m_changes and use typesafe" in this
series (therefore eliminates a bunch of temporary casts), but otherwise
I don't think it would be much of an improvement.


> ---
>     Changes in v5:
>         - Add missing Signed-off-by
> 
>     Changes in v4:
>         - Patch added
> ---
>  xen/arch/arm/p2m.c | 70 
> ++++++++++++++++++++++++++++--------------------------
>  1 file changed, 36 insertions(+), 34 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 0fdd11f..a5b584b 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1158,17 +1158,40 @@ out:
>      return rc;
>  }
>  
> +static inline int p2m_insert_mapping(struct domain *d,
> +                                     gfn_t start_gfn,
> +                                     unsigned long nr,
> +                                     mfn_t mfn,
> +                                     int mattr, p2m_type_t t)
> +{
> +    return apply_p2m_changes(d, INSERT,
> +                             pfn_to_paddr(gfn_x(start_gfn)),
> +                             pfn_to_paddr(gfn_x(start_gfn) + nr),
> +                             pfn_to_paddr(mfn_x(mfn)),
> +                             mattr, 0, t, d->arch.p2m.default_access);
> +}
> +
> +static inline int p2m_remove_mapping(struct domain *d,
> +                                     gfn_t start_gfn,
> +                                     unsigned long nr,
> +                                     mfn_t mfn)
> +{
> +    return apply_p2m_changes(d, REMOVE,
> +                             pfn_to_paddr(gfn_x(start_gfn)),
> +                             pfn_to_paddr(gfn_x(start_gfn) + nr),
> +                             pfn_to_paddr(mfn_x(mfn)),
> +                             /* arguments below not used when removing 
> mapping */
> +                             MATTR_MEM, 0, p2m_invalid,
> +                             d->arch.p2m.default_access);
> +}
> +
>  int map_regions_rw_cache(struct domain *d,
>                           gfn_t gfn,
>                           unsigned long nr,
>                           mfn_t mfn)
>  {
> -    return apply_p2m_changes(d, INSERT,
> -                             pfn_to_paddr(gfn_x(gfn)),
> -                             pfn_to_paddr(gfn_x(gfn) + nr),
> -                             pfn_to_paddr(mfn_x(mfn)),
> -                             MATTR_MEM, 0, p2m_mmio_direct,
> -                             d->arch.p2m.default_access);
> +    return p2m_insert_mapping(d, gfn, nr, mfn,
> +                              MATTR_MEM, p2m_mmio_direct);
>  }
>  
>  int unmap_regions_rw_cache(struct domain *d,
> @@ -1176,12 +1199,7 @@ int unmap_regions_rw_cache(struct domain *d,
>                             unsigned long nr,
>                             mfn_t mfn)
>  {
> -    return apply_p2m_changes(d, REMOVE,
> -                             pfn_to_paddr(gfn_x(gfn)),
> -                             pfn_to_paddr(gfn_x(gfn) + nr),
> -                             pfn_to_paddr(mfn_x(mfn)),
> -                             MATTR_MEM, 0, p2m_invalid,
> -                             d->arch.p2m.default_access);
> +    return p2m_remove_mapping(d, gfn, nr, mfn);
>  }
>  
>  int map_mmio_regions(struct domain *d,
> @@ -1189,12 +1207,8 @@ int map_mmio_regions(struct domain *d,
>                       unsigned long nr,
>                       mfn_t mfn)
>  {
> -    return apply_p2m_changes(d, INSERT,
> -                             pfn_to_paddr(gfn_x(start_gfn)),
> -                             pfn_to_paddr(gfn_x(start_gfn) + nr),
> -                             pfn_to_paddr(mfn_x(mfn)),
> -                             MATTR_DEV, 0, p2m_mmio_direct,
> -                             d->arch.p2m.default_access);
> +    return p2m_insert_mapping(d, start_gfn, nr, mfn,
> +                              MATTR_MEM, p2m_mmio_direct);
>  }
>  
>  int unmap_mmio_regions(struct domain *d,
> @@ -1202,12 +1216,7 @@ int unmap_mmio_regions(struct domain *d,
>                         unsigned long nr,
>                         mfn_t mfn)
>  {
> -    return apply_p2m_changes(d, REMOVE,
> -                             pfn_to_paddr(gfn_x(start_gfn)),
> -                             pfn_to_paddr(gfn_x(start_gfn) + nr),
> -                             pfn_to_paddr(mfn_x(mfn)),
> -                             MATTR_DEV, 0, p2m_invalid,
> -                             d->arch.p2m.default_access);
> +    return p2m_remove_mapping(d, start_gfn, nr, mfn);
>  }
>  
>  int map_dev_mmio_region(struct domain *d,
> @@ -1237,22 +1246,15 @@ int guest_physmap_add_entry(struct domain *d,
>                              unsigned long page_order,
>                              p2m_type_t t)
>  {
> -    return apply_p2m_changes(d, INSERT,
> -                             pfn_to_paddr(gfn_x(gfn)),
> -                             pfn_to_paddr(gfn_x(gfn) + (1 << page_order)),
> -                             pfn_to_paddr(mfn_x(mfn)), MATTR_MEM, 0, t,
> -                             d->arch.p2m.default_access);
> +    return p2m_insert_mapping(d, gfn, (1 << page_order), mfn,
> +                              MATTR_MEM, t);
>  }
>  
>  void guest_physmap_remove_page(struct domain *d,
>                                 gfn_t gfn,
>                                 mfn_t mfn, unsigned int page_order)
>  {
> -    apply_p2m_changes(d, REMOVE,
> -                      pfn_to_paddr(gfn_x(gfn)),
> -                      pfn_to_paddr(gfn_x(gfn) + (1<<page_order)),
> -                      pfn_to_paddr(mfn_x(mfn)), MATTR_MEM, 0, p2m_invalid,
> -                      d->arch.p2m.default_access);
> +    p2m_remove_mapping(d, gfn, (1 << page_order), mfn);
>  }
>  
>  int p2m_alloc_table(struct domain *d)
> -- 
> 1.9.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.