Re: [Xen-devel] [PATCH v4 3/7] arch, arm: let map_mmio_regions() take pfn as parameters

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 25 Mar 2014, Arianna Avanzini wrote:
> Currently, the map_mmio_regions() function, defined for the ARM
> architecture, has parameters with paddr_t type. This interface,
> however, needs caller functions to correctly page-align addresses
> given as parameters to map_mmio_regions(). This commit changes the
> function's interface to accept page frame numbers as parameters.
> This commit also modifies caller functions in an attempt to adapt
> them to the new interface.
> 
> NOTE: platform-specific code has not been tested.
> 
> Signed-off-by: Arianna Avanzini <avanzini.arianna@xxxxxxxxx>
> Cc: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> Cc: Paolo Valente <paolo.valente@xxxxxxxxxx>
> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxxxxx>
> Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
> Cc: Jan Beulich <JBeulich@xxxxxxxx>
> Cc: Keir Fraser <keir@xxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> Cc: Eric Trudeau <etrudeau@xxxxxxxxxxxx>
> Cc: Viktor Kleinik <viktor.kleinik@xxxxxxxxxxxxxxx>
> ---
>  xen/arch/arm/domain_build.c          |  7 ++++---
>  xen/arch/arm/gic.c                   | 21 ++++++++++++---------
>  xen/arch/arm/p2m.c                   | 13 ++++++++-----
>  xen/arch/arm/platforms/exynos5.c     | 13 ++++++++-----
>  xen/arch/arm/platforms/omap5.c       | 25 ++++++++++++++++---------
>  xen/arch/arm/platforms/xgene-storm.c |  4 +++-
>  xen/include/asm-arm/p2m.h            | 11 ++++++-----
>  7 files changed, 57 insertions(+), 37 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 4a0411c..10f508d 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -744,9 +744,10 @@ static int map_device(struct domain *d, const struct 
> dt_device_node *dev)
>                     addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);
>              return res;
>          }
> -        res = map_mmio_regions(d, addr & PAGE_MASK,
> -                               PAGE_ALIGN(addr + size) - 1,
> -                               addr & PAGE_MASK);
> +        res = map_mmio_regions(d,
> +                               paddr_to_pfn(addr & PAGE_MASK),
> +                               paddr_to_pfn(PAGE_ALIGN(addr + size - 1)),
> +                               paddr_to_pfn(addr & PAGE_MASK));
>          if ( res )
>          {
>              printk(XENLOG_ERR "Unable to map 0x%"PRIx64
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 074624e..55354d4 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -882,20 +882,23 @@ int gicv_setup(struct domain *d)
>       * The second page is always mapped at +4K irrespective of the
>       * GIC_64K_STRIDE quirk. The DTB passed to the guest reflects this.
>       */
> -    ret = map_mmio_regions(d, d->arch.vgic.cbase,
> -                           d->arch.vgic.cbase + PAGE_SIZE - 1,
> -                           gic.vbase);
> +    ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase),
> +                           paddr_to_pfn(PAGE_ALIGN(d->arch.vgic.cbase +
> +                                        PAGE_SIZE - 1)),
> +                           paddr_to_pfn(gic.vbase));
>      if (ret)
>          return ret;
>  
>      if ( !platform_has_quirk(PLATFORM_QUIRK_GIC_64K_STRIDE) )
> -        ret = map_mmio_regions(d, d->arch.vgic.cbase + PAGE_SIZE,
> -                               d->arch.vgic.cbase + (2 * PAGE_SIZE) - 1,
> -                               gic.vbase + PAGE_SIZE);
> +        ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase + 
> PAGE_SIZE),
> +                               paddr_to_pfn(PAGE_ALIGN(d->arch.vgic.cbase +
> +                                         (2 * PAGE_SIZE) - 1)),
> +                               paddr_to_pfn(gic.vbase + PAGE_SIZE));
>      else
> -        ret = map_mmio_regions(d, d->arch.vgic.cbase + PAGE_SIZE,
> -                               d->arch.vgic.cbase + (2 * PAGE_SIZE) - 1,
> -                               gic.vbase + 16*PAGE_SIZE);
> +        ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase + 
> PAGE_SIZE),
> +                               paddr_to_pfn(PAGE_ALIGN(d->arch.vgic.cbase +
> +                                         (2 * PAGE_SIZE) - 1)),
> +                               paddr_to_pfn(gic.vbase + 16*PAGE_SIZE));
>  
>      return ret;
>  
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index bb0db16..c8e77b9 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -469,12 +469,15 @@ int p2m_populate_ram(struct domain *d,
>  }
>  
>  int map_mmio_regions(struct domain *d,
> -                     paddr_t start_gaddr,
> -                     paddr_t end_gaddr,
> -                     paddr_t maddr)
> +                     unsigned long start_gfn,
> +                     unsigned long end_gfn,
> +                     unsigned long mfn)
>  {
> -    return apply_p2m_changes(d, INSERT, start_gaddr, end_gaddr,
> -                             maddr, MATTR_DEV, p2m_mmio_direct);
> +    return apply_p2m_changes(d, INSERT,
> +                             pfn_to_paddr(start_gfn),
> +                             pfn_to_paddr(end_gfn),
> +                             pfn_to_paddr(mfn),
> +                             MATTR_DEV, p2m_mmio_direct);
>  }

This doesn't make any sense. Shouldn't you modify apply_p2m_changes to
take pfns as parameters too?

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