[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Ayan Kumar Halder <ayankuma@xxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Penny Zheng <penny.zheng@xxxxxxx>
  • Date: Thu, 13 Jul 2023 13:59:08 +0800
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 40.67.248.234) smtp.rcpttodomain=amd.com smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=none (message not signed); arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=fb21Prp8fY+sRO79D5+pwDnvhBbwbDsxvfdcKF5/o2o=; b=CfAMaPClnL+n/6Q9GWFs1wABvawGG7cFSFz8eNqs3g3etpyP99JFNu/E8eqXzgFUAaTCyZ73BhUIkKXMYTySg3z38L1dQdOzgnbxBp1MPcNJS7CX5JxGnoKoYH5dzSgDx0bffXNDFxtc2WFniuzGum28yEPSsP11bAQiJzRwABr3mPliDyHvm+NZxwFY1mCL3zacmagjADIZ6wsxm1GnCJE/K7rMmpJ/QHVaJ0SG/QgOXb+KPwq6UZ2TSnhbhC4VJiQNMxSvtCR4U9rtkzPwGDBFGqcAp0anZfeS3HHyeUU0kI5uQBQpM0oLB1m39i5ad8rmmPLVSPQrBzSJ2PmPdw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=eaivM5W/YR5q7hil1ZB3aCnEqpTDp9SqEhEoYC3szP7vb9KDjfRkOcitqJUyrleKAiFxmWngrplq8YMSi3HvRa+trxIy+R6zFw8nREFIywV5gPlAx02XDoGzH14wsVIE7MMrRfujqFLwgZgX9ODOintKzPU4JcP+IMDf99FjbYYOVB13o1XUPr8X9e2MH5I8GllSemdUy+/XcARFO7vIMZK3Wpdk1F6K3CLsdmfYbL9o/xu7wm+iikDSK0p8MJe7GtKqGkn/EIr/VXV1lzKbOkGXDH2eUjmJC0X1QQZNczUG6iy628u/5xGhVvJsjrt2FQjdvaN2qZrCSP7VDrPHAg==
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, "Volodymyr Babchuk" <Volodymyr_Babchuk@xxxxxxxx>, Wei Chen <wei.chen@xxxxxxx>
  • Delivery-date: Thu, 13 Jul 2023 05:59:46 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true

Hi Ayan

On 2023/7/5 21:33, Ayan Kumar Halder wrote:

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 ?

See this pair map_domain_page()/unmap_domain_page(), which is used to map/unmap page of guest RAM. vcpu in different mode is facing different scenario.

Taking usage in copy_guest() as example:
When vcpu in guest mode trying to access its own memory(e.g. copy
hypercall param), there is no need to do the mapping/unmapping in MPU,
as page is already mapped at both EL1/EL2. Checking if it is transient
region in unmap_domain_page() is definitely necessary to help avoid unmapping it here.
When vcpu in hypervisor mode at boot time copying kernel image to guest
memory, we need to map page as transient MPU region to do the copying and pasting.

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.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





 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.