Re: [PATCH v2 6/9] xen/arm: add P2M type parameter in guest_physmap_add_pages

From: Julien Grall <julien@xen.org>
To: Penny Zheng <Penny.Zheng@arm.com>, xen-devel@lists.xenproject.org
Cc: wei.chen@arm.com, Stefano Stabellini <sstabellini@kernel.org>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [PATCH v2 6/9] xen/arm: add P2M type parameter in guest_physmap_add_pages
Date: Sat, 7 May 2022 10:00:41 +0100	[thread overview]
Message-ID: <442a17fb-b859-8067-417e-a1286dfcec80@xen.org> (raw)
In-Reply-To: <20220506072502.2177828-7-Penny.Zheng@arm.com>

Hi,

On 06/05/2022 08:24, Penny Zheng wrote:
> In order to cover the scenario where users intend to set up guest
> p2m foreign mapping with nr_pages, this commit adds a new P2M type
> parameter in guest_physmap_add_pages.

With this patch, guest_physmap_add_pages() and guest_physmap_add_page() 
will behave differently. The former will be able to deal with any type 
whilst the latter can only handle RAM type.

I think we need to stay consistent with the name. Looking at the 
existing helper, we already have one that should suit you (see 
map_regions_p2mt()). So this patch looks unnecessary.

Cheers,

> 
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> ---
> v2 change:
> - no change
> ---
>   xen/arch/arm/domain_build.c    | 5 +++--
>   xen/arch/arm/include/asm/p2m.h | 5 +++--
>   2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index b3ba0c501d..089b9e99fc 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -507,7 +507,7 @@ static bool __init append_static_memory_to_bank(struct domain *d,
>       else
>           sgfn = gaddr_to_gfn(mfn_to_maddr(smfn));
>   
> -    res = guest_physmap_add_pages(d, sgfn, smfn, nr_pages);
> +    res = guest_physmap_add_pages(d, sgfn, smfn, nr_pages, p2m_ram_rw);
>       if ( res )
>       {
>           dprintk(XENLOG_ERR, "Failed to map pages to DOMU: %d", res);
> @@ -850,7 +850,8 @@ static int __init allocate_shared_memory(struct domain *d,
>       nr_pages = PFN_DOWN(psize);
>       if ( d != dom_io )
>       {
> -        ret = guest_physmap_add_pages(d, gaddr_to_gfn(gbase), smfn, PFN_DOWN(psize));
> +        ret = guest_physmap_add_pages(d, gaddr_to_gfn(gbase), smfn, nr_pages,
> +                                      p2m_ram_rw);
>           if ( ret )
>           {
>               printk(XENLOG_ERR
> diff --git a/xen/arch/arm/include/asm/p2m.h b/xen/arch/arm/include/asm/p2m.h
> index 8cce459b67..58590145b0 100644
> --- a/xen/arch/arm/include/asm/p2m.h
> +++ b/xen/arch/arm/include/asm/p2m.h
> @@ -317,9 +317,10 @@ guest_physmap_add_page(struct domain *d, gfn_t gfn, mfn_t mfn,
>   static inline int guest_physmap_add_pages(struct domain *d,
>                                             gfn_t gfn,
>                                             mfn_t mfn,
> -                                          unsigned int nr_pages)
> +                                          unsigned int nr_pages,
> +                                          p2m_type_t t)
>   {
> -    return p2m_insert_mapping(d, gfn, nr_pages, mfn, p2m_ram_rw);
> +    return p2m_insert_mapping(d, gfn, nr_pages, mfn, t);
>   }
>   
>   mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn);

-- 
Julien Grall