Re: [PATCH v3 35/52] xen/arm: map static memory on demand

[Penny Zheng <penny.zheng@xxxxxxx>]

Hi Ayan

On 2023/7/4 23:10, Ayan Kumar Halder wrote:
> Hi Penny,
>
> On 26/06/2023 04:34, Penny Zheng wrote:
> > CAUTION: This message has originated from an External Source. Please 
> > use proper judgment and caution when opening attachments, clicking 
> > links, or responding to this email.
> >
> > In function init_staticmem_pages, we need the access to static memory
> > for proper initialization.
> > It is not a problem in MMU system, as Xen map the whole RAM in
> > setup_mm(). However, with limited MPU memory regions, it is too luxury
> > to map the whole RAM.
> > As a result, we follow the rule of "map on demand", to map static memory
> > temporarily before its initialization, and unmap immediately after its
> > initialization.
> I could see that you are using _PAGE_TRANSIENT  to map memory 
> temporarily. However, I don't see this being translated to any of the 
> MPU hardware features (ie _PAGE_TRANSIENT does not seem to translate to 
> any of the attributes in PRBAR, PRLAR, PRENR, etc). Thus, how is it 
> different from mapping the memory in "non temporary" way ?
It is only software feature.
It is designed for implementing functions like ioremap_xxx(), or 
map_staticmem_pages_to_xen() here, which are always occuring with its 
reverse unmapping function nearby like iounmap(), or 
unmap_staticmem_pages_to_xen(), to map a chunk of memory *temporarily*, 
for a very short time.
I want to check this flag in the unmapping function, to ensure that we 
are unmapping the proper MPU region.
Fixed MPU regions are like Xen text section, Xen data section, etc.

> Please let me know what I am missing.
>
> - Ayan
>
> > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > ---
> > v3:
> > - new commit
> > ---
> >   xen/arch/arm/include/asm/mm.h |  2 ++
> >   xen/arch/arm/mmu/mm.c         | 10 ++++++++++
> >   xen/arch/arm/mpu/mm.c         | 10 ++++++++++
> >   xen/arch/arm/setup.c          | 21 +++++++++++++++++++++
> >   4 files changed, 43 insertions(+)
> >
> > diff --git a/xen/arch/arm/include/asm/mm.h 
> > b/xen/arch/arm/include/asm/mm.h
> > index 66d98b9a29..cffbf8a595 100644
> > --- a/xen/arch/arm/include/asm/mm.h
> > +++ b/xen/arch/arm/include/asm/mm.h
> > @@ -224,6 +224,8 @@ extern void mm_init_secondary_cpu(void);
> >   extern void setup_frametable_mappings(paddr_t ps, paddr_t pe);
> >   /* map a physical range in virtual memory */
> >   void __iomem *ioremap_attr(paddr_t start, size_t len, unsigned int 
> > attributes);
> > +extern int map_staticmem_pages_to_xen(paddr_t start, paddr_t end);
> > +extern int unmap_staticmem_pages_to_xen(paddr_t start, paddr_t end);
> >
> >   static inline void __iomem *ioremap_nocache(paddr_t start, size_t len)
> >   {
> > diff --git a/xen/arch/arm/mmu/mm.c b/xen/arch/arm/mmu/mm.c
> > index 2f29cb53fe..4196a55c32 100644
> > --- a/xen/arch/arm/mmu/mm.c
> > +++ b/xen/arch/arm/mmu/mm.c
> > @@ -1113,6 +1113,16 @@ int populate_pt_range(unsigned long virt, 
> > unsigned long nr_mfns)
> >       return xen_pt_update(virt, INVALID_MFN, nr_mfns, _PAGE_POPULATE);
> >   }
> >
> > +int __init map_staticmem_pages_to_xen(paddr_t start, paddr_t end)
> > +{
> > +    return 0;
> > +}
> > +
> > +int __init unmap_staticmem_pages_to_xen(paddr_t start, paddr_t end)
> > +{
> > +    return 0;
> > +}
> > +
> >   /*
> >    * Local variables:
> >    * mode: C
> > diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> > index a40055ae5e..9d5c1da39c 100644
> > --- a/xen/arch/arm/mpu/mm.c
> > +++ b/xen/arch/arm/mpu/mm.c
> > @@ -614,6 +614,16 @@ void __init setup_frametable_mappings(paddr_t ps, 
> > paddr_t pe)
> >              frametable_size - (nr_pdxs * sizeof(struct page_info)));
> >   }
> >
> > +int __init map_staticmem_pages_to_xen(paddr_t start, paddr_t end)
> > +{
> > +    return xen_mpumap_update(start, end, PAGE_HYPERVISOR | 
> > _PAGE_TRANSIENT);
> > +}
> > +
> > +int __init unmap_staticmem_pages_to_xen(paddr_t start, paddr_t end)
> > +{
> > +    return xen_mpumap_update(start, end, 0);
> > +}
> > +
> >   /*
> >    * Local variables:
> >    * mode: C
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index f42b53d17b..c21d1db763 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -637,12 +637,33 @@ void __init init_staticmem_pages(void)
> >               mfn_t bank_start = 
> > _mfn(PFN_UP(bootinfo.reserved_mem.bank[bank].start));
> >               unsigned long bank_pages = 
> > PFN_DOWN(bootinfo.reserved_mem.bank[bank].size);
> >               mfn_t bank_end = mfn_add(bank_start, bank_pages);
> > +            int res;
> >
> >               if ( mfn_x(bank_end) <= mfn_x(bank_start) )
> >                   return;
> >
> > +            /* Map temporarily before initialization */
> > +            res = map_staticmem_pages_to_xen(mfn_to_maddr(bank_start),
> > +                                             mfn_to_maddr(bank_end));
> > +            if ( res )
> > +            {
> > +                printk(XENLOG_ERR "Failed to map static memory to 
> > Xen: %d\n",
> > +                       res);
> > +                return;
> > +            }
> > +
> >               unprepare_staticmem_pages(mfn_to_page(bank_start),
> >                                         bank_pages, false);
> > +
> > +            /* Unmap immediately after initialization */
> > +            res = unmap_staticmem_pages_to_xen(mfn_to_maddr(bank_start),
> > +                                               mfn_to_maddr(bank_end));
> > +            if ( res )
> > +            {
> > +                printk(XENLOG_ERR "Failed to unmap static memory to 
> > Xen: %d\n",
> > +                       res);
> > +                return;
> > +            }
> >           }
> >       }
> >   #endif
> > --
> > 2.25.1