Re: [PATCH v7 04/10] arch/arm: let map_mmio_regions() use start and count

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

Hi Arianna,

Thank you for the patch.

On 05/05/14 16:54, Arianna Avanzini wrote:
> Currently, the arguments given to the function map_mmio_regions() to
> describe the memory range to be mapped are the start and end page frame
> numbers of the range to be mapped. However, this could give rise to
> issues due to the range being inclusive or exclusive of the end gfn
> given as parameter. This commit changes the interface of the function
> to accept the start gfn and the number of gfns to be mapped.
>
> Signed-off-by: Arianna Avanzini <avanzini.arianna@gmail.com>
> Cc: Dario Faggioli <dario.faggioli@citrix.com>
> Cc: Paolo Valente <paolo.valente@unimore.it>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Julien Grall <julien.grall@citrix.com>
> Cc: Ian Campbell <Ian.Campbell@eu.citrix.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Eric Trudeau <etrudeau@broadcom.com>
> Cc: Viktor Kleinik <viktor.kleinik@globallogic.com>
> ---
>   xen/arch/arm/domain_build.c          |  2 +-
>   xen/arch/arm/gic.c                   | 11 +++--------
>   xen/arch/arm/p2m.c                   |  4 ++--
>   xen/arch/arm/platforms/exynos5.c     |  6 ++----
>   xen/arch/arm/platforms/omap5.c       | 12 ++++--------
>   xen/arch/arm/platforms/xgene-storm.c |  2 +-
>   xen/include/asm-arm/p2m.h            |  7 ++++---
>   7 files changed, 17 insertions(+), 27 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index fc3f110..23874a8 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -753,7 +753,7 @@ static int map_device(struct domain *d, const struct dt_device_node *dev)
>           }
>           res = map_mmio_regions(d,
>                                  paddr_to_pfn(addr & PAGE_MASK),
> -                               paddr_to_pfn_aligned(addr + size),
> +                               size / PAGE_SIZE + (size % PAGE_SIZE ? 1 : 0),

Would not it be easier to use paddr_to_pfn(PAGE_ALIGN(size))?

>       return 0;
> diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
> index f3662fd..a7f4c8b 100644
> --- a/xen/arch/arm/platforms/xgene-storm.c
> +++ b/xen/arch/arm/platforms/xgene-storm.c
> @@ -48,7 +48,7 @@ static int map_one_mmio(struct domain *d, const char *what,
>       printk("Additional MMIO %"PRIpaddr"-%"PRIpaddr" (%s)\n",
>              start, end, what);
>       ret = map_mmio_regions(d, paddr_to_pfn(start),
> -                           paddr_to_pfn_aligned(end),
> +                           paddr_to_pfn_aligned(end) - paddr_to_pfn(start) + 1,

I would directly give the pfn in argument of map_one_mmio.

Regards,

-- 
Julien Grall