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

Wed, 05 Jul 2023 06:33:56 -0700 


On 05/07/2023 11:16, Penny Zheng wrote:

Hi Ayan

Hi Penny,


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 understand that it is a software only feature. But why does the 
software need to know if the memory is mapped temporarily or not ? What 
difference does it make ?

I want to check this flag in the unmapping function, to ensure that we 
are unmapping the proper MPU region.



I had a look at unmap_staticmem_pages_to_xen() --> xen_mpumap_update() 
--> control_mpu_region_from_index() and I don't see this flag used anywhere.


- Ayan



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.zh...@arm.com>
Signed-off-by: Wei Chen <wei.c...@arm.com>
---
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



























					Reply via email to