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

From: Penny Zheng <Penny.Zheng@arm.com>
To: Julien Grall <julien@xen.org>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Wei Chen <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 09:20:49 +0000	[thread overview]
Message-ID: <DU2PR08MB73254BF9880B718BA1631D7CF7C49@DU2PR08MB7325.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <442a17fb-b859-8067-417e-a1286dfcec80@xen.org>

Hi julien

> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: Saturday, May 7, 2022 5:01 PM
> To: Penny Zheng <Penny.Zheng@arm.com>; xen-devel@lists.xenproject.org
> Cc: Wei Chen <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
> 
> 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.
> 

Right, map_regions_p2mt has the P2M type as parameter. I'll change to it. Thx~

> 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