All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/9] populate/unpopulate memory when domain on static allocation
@ 2022-05-31  3:12 Penny Zheng
  2022-05-31  3:12 ` [PATCH v5 1/9] xen/arm: rename PGC_reserved to PGC_staticmem Penny Zheng
                   ` (8 more replies)
  0 siblings, 9 replies; 27+ messages in thread
From: Penny Zheng @ 2022-05-31  3:12 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.chen, Penny Zheng, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, Andrew Cooper,
	George Dunlap, Jan Beulich, Wei Liu

Today when a domain unpopulates the memory on runtime, they will always
hand the memory over to the heap allocator. And it will be a problem if it
is a static domain.
Pages used as guest RAM for static domain shall always be reserved to this
domain only, and not be used for any other purposes, so they shall never go
back to heap allocator.

This patch serie intends to fix this issue, by adding pages on the new list
resv_page_list after having taken them off the "normal" list, when unpopulating
memory, and retrieving pages from resv page list(resv_page_list) when
populating memory.

---
v5 changes:
- introduce three new commits
- In order to avoid stub functions, we #define PGC_staticmem to non-zero only
when CONFIG_STATIC_MEMORY
- use "unlikely()" around pg->count_info & PGC_staticmem
- remove pointless "if", since mark_page_free() is going to set count_info
to PGC_state_free and by consequence clear PGC_staticmem
- move #define PGC_staticmem 0 to mm.h
- guard "is_domain_using_staticmem" under CONFIG_STATIC_MEMORY
- #define is_domain_using_staticmem zero if undefined
- extract common codes for assigning pages into a helper assign_domstatic_pages
- refine commit message
- remove stub function acquire_reserved_page
- Alloc/free of memory can happen concurrently. So access to rsv_page_list
needs to be protected with a spinlock
---
v4 changes:
- commit message refinement
- miss dropping __init in acquire_domstatic_pages
- add the page back to the reserved list in case of error
- remove redundant printk
- refine log message and make it warn level
- guard "is_domain_using_staticmem" under CONFIG_STATIC_MEMORY
- #define is_domain_using_staticmem zero if undefined
---
v3 changes:
- fix possible racy issue in free_staticmem_pages()
- introduce a stub free_staticmem_pages() for the !CONFIG_STATIC_MEMORY case
- move the change to free_heap_pages() to cover other potential call sites
- change fixed width type uint32_t to unsigned int
- change "flags" to a more descriptive name "cdf"
- change name from "is_domain_static()" to "is_domain_using_staticmem"
- have page_list_del() just once out of the if()
- remove resv_pages counter
- make arch_free_heap_page be an expression, not a compound statement.
- move #ifndef is_domain_using_staticmem to the common header file
- remove #ifdef CONFIG_STATIC_MEMORY-ary
- remove meaningless page_to_mfn(page) in error log
---
v2 changes:
- let "flags" live in the struct domain. So other arch can take
advantage of it in the future
- change name from "is_domain_on_static_allocation" to "is_domain_static()"
- put reserved pages on resv_page_list after having taken them off
the "normal" list
- introduce acquire_reserved_page to retrieve reserved pages from
resv_page_list
- forbid non-zero-order requests in populate_physmap
- let is_domain_static return ((void)(d), false) on x86
- fix coding style

Penny Zheng (9):
  xen/arm: rename PGC_reserved to PGC_staticmem
  xen: do not free reserved memory into heap
  xen: update SUPPORT.md for static allocation
  xen: do not merge reserved pages in free_heap_pages()
  xen: add field "flags" to cover all internal CDF_XXX
  xen/arm: introduce CDF_staticmem
  xen/arm: unpopulate memory when domain is static
  xen: introduce prepare_staticmem_pages
  xen: retrieve reserved pages on populate_physmap

 SUPPORT.md                        |   7 ++
 xen/arch/arm/domain.c             |   2 -
 xen/arch/arm/domain_build.c       |   5 +-
 xen/arch/arm/include/asm/domain.h |   7 +-
 xen/arch/arm/include/asm/mm.h     |  20 +++-
 xen/common/domain.c               |   7 ++
 xen/common/memory.c               |  23 +++++
 xen/common/page_alloc.c           | 147 ++++++++++++++++++++----------
 xen/include/xen/domain.h          |   6 ++
 xen/include/xen/mm.h              |   7 +-
 xen/include/xen/sched.h           |   6 ++
 11 files changed, 180 insertions(+), 57 deletions(-)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v5 1/9] xen/arm: rename PGC_reserved to PGC_staticmem
  2022-05-31  3:12 [PATCH v5 0/9] populate/unpopulate memory when domain on static allocation Penny Zheng
@ 2022-05-31  3:12 ` Penny Zheng
  2022-05-31  8:32   ` Jan Beulich
  2022-05-31  3:12 ` [PATCH v5 2/9] xen: do not free reserved memory into heap Penny Zheng
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 27+ messages in thread
From: Penny Zheng @ 2022-05-31  3:12 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.chen, Penny Zheng, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, Andrew Cooper,
	George Dunlap, Jan Beulich, Wei Liu, Penny Zheng

PGC_reserved could be ambiguous, and we have to tell what the pages are
reserved for, so this commit intends to rename PGC_reserved to
PGC_staticmem, which clearly indicates the page is reserved for static
memory.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v5 changes:
- new commit
---
 xen/arch/arm/include/asm/mm.h |  6 +++---
 xen/common/page_alloc.c       | 20 ++++++++++----------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index 424aaf2823..1226700085 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -108,9 +108,9 @@ struct page_info
   /* Page is Xen heap? */
 #define _PGC_xen_heap     PG_shift(2)
 #define PGC_xen_heap      PG_mask(1, 2)
-  /* Page is reserved */
-#define _PGC_reserved     PG_shift(3)
-#define PGC_reserved      PG_mask(1, 3)
+  /* Page is static memory */
+#define _PGC_staticmem    PG_shift(3)
+#define PGC_staticmem     PG_mask(1, 3)
 /* ... */
 /* Page is broken? */
 #define _PGC_broken       PG_shift(7)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 319029140f..44600dd9cd 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -151,8 +151,8 @@
 #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
 #endif
 
-#ifndef PGC_reserved
-#define PGC_reserved 0
+#ifndef PGC_staticmem
+#define PGC_staticmem 0
 #endif
 
 /*
@@ -2286,7 +2286,7 @@ int assign_pages(
 
         for ( i = 0; i < nr; i++ )
         {
-            ASSERT(!(pg[i].count_info & ~(PGC_extra | PGC_reserved)));
+            ASSERT(!(pg[i].count_info & ~(PGC_extra | PGC_staticmem)));
             if ( pg[i].count_info & PGC_extra )
                 extra_pages++;
         }
@@ -2346,7 +2346,7 @@ int assign_pages(
         page_set_owner(&pg[i], d);
         smp_wmb(); /* Domain pointer must be visible before updating refcnt. */
         pg[i].count_info =
-            (pg[i].count_info & (PGC_extra | PGC_reserved)) | PGC_allocated | 1;
+            (pg[i].count_info & (PGC_extra | PGC_staticmem)) | PGC_allocated | 1;
 
         page_list_add_tail(&pg[i], page_to_list(d, &pg[i]));
     }
@@ -2652,8 +2652,8 @@ void __init free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
             scrub_one_page(pg);
         }
 
-        /* In case initializing page of static memory, mark it PGC_reserved. */
-        pg[i].count_info |= PGC_reserved;
+        /* In case initializing page of static memory, mark it PGC_staticmem. */
+        pg[i].count_info |= PGC_staticmem;
     }
 }
 
@@ -2683,7 +2683,7 @@ static struct page_info * __init acquire_staticmem_pages(mfn_t smfn,
     for ( i = 0; i < nr_mfns; i++ )
     {
         /* The page should be reserved and not yet allocated. */
-        if ( pg[i].count_info != (PGC_state_free | PGC_reserved) )
+        if ( pg[i].count_info != (PGC_state_free | PGC_staticmem) )
         {
             printk(XENLOG_ERR
                    "pg[%lu] Static MFN %"PRI_mfn" c=%#lx t=%#x\n",
@@ -2697,10 +2697,10 @@ static struct page_info * __init acquire_staticmem_pages(mfn_t smfn,
                                 &tlbflush_timestamp);
 
         /*
-         * Preserve flag PGC_reserved and change page state
+         * Preserve flag PGC_staticmem and change page state
          * to PGC_state_inuse.
          */
-        pg[i].count_info = PGC_reserved | PGC_state_inuse;
+        pg[i].count_info = PGC_staticmem | PGC_state_inuse;
         /* Initialise fields which have other uses for free pages. */
         pg[i].u.inuse.type_info = 0;
         page_set_owner(&pg[i], NULL);
@@ -2722,7 +2722,7 @@ static struct page_info * __init acquire_staticmem_pages(mfn_t smfn,
 
  out_err:
     while ( i-- )
-        pg[i].count_info = PGC_reserved | PGC_state_free;
+        pg[i].count_info = PGC_staticmem | PGC_state_free;
 
     spin_unlock(&heap_lock);
 
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 2/9] xen: do not free reserved memory into heap
  2022-05-31  3:12 [PATCH v5 0/9] populate/unpopulate memory when domain on static allocation Penny Zheng
  2022-05-31  3:12 ` [PATCH v5 1/9] xen/arm: rename PGC_reserved to PGC_staticmem Penny Zheng
@ 2022-05-31  3:12 ` Penny Zheng
  2022-05-31  8:36   ` Jan Beulich
  2022-05-31  3:12 ` [PATCH v5 3/9] xen: update SUPPORT.md for static allocation Penny Zheng
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 27+ messages in thread
From: Penny Zheng @ 2022-05-31  3:12 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.chen, Penny Zheng, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, Andrew Cooper,
	George Dunlap, Jan Beulich, Wei Liu, Penny Zheng

Pages used as guest RAM for static domain, shall be reserved to this
domain only.
So in case reserved pages being used for other purpose, users
shall not free them back to heap, even when last ref gets dropped.

free_staticmem_pages will be called by free_heap_pages in runtime
for static domain freeing memory resource, so let's drop the __init
flag.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v5 changes:
- In order to avoid stub functions, we #define PGC_staticmem to non-zero only
when CONFIG_STATIC_MEMORY
- use "unlikely()" around pg->count_info & PGC_staticmem
- remove pointless "if", since mark_page_free() is going to set count_info
to PGC_state_free and by consequence clear PGC_staticmem
- move #define PGC_staticmem 0 to mm.h
---
v4 changes:
- no changes
---
v3 changes:
- fix possible racy issue in free_staticmem_pages()
- introduce a stub free_staticmem_pages() for the !CONFIG_STATIC_MEMORY case
- move the change to free_heap_pages() to cover other potential call sites
- fix the indentation
---
v2 changes:
- new commit
---
 xen/arch/arm/include/asm/mm.h |  2 ++
 xen/common/page_alloc.c       | 16 +++++++++-------
 xen/include/xen/mm.h          |  6 +++++-
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index 1226700085..56d0939318 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -108,9 +108,11 @@ struct page_info
   /* Page is Xen heap? */
 #define _PGC_xen_heap     PG_shift(2)
 #define PGC_xen_heap      PG_mask(1, 2)
+#ifdef CONFIG_STATIC_MEMORY
   /* Page is static memory */
 #define _PGC_staticmem    PG_shift(3)
 #define PGC_staticmem     PG_mask(1, 3)
+#endif
 /* ... */
 /* Page is broken? */
 #define _PGC_broken       PG_shift(7)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 44600dd9cd..6425761116 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -151,10 +151,6 @@
 #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
 #endif
 
-#ifndef PGC_staticmem
-#define PGC_staticmem 0
-#endif
-
 /*
  * Comma-separated list of hexadecimal page numbers containing bad bytes.
  * e.g. 'badpage=0x3f45,0x8a321'.
@@ -1443,6 +1439,13 @@ static void free_heap_pages(
 
     ASSERT(order <= MAX_ORDER);
 
+    if ( unlikely(pg->count_info & PGC_staticmem) )
+    {
+        /* Pages of static memory shall not go back to the heap. */
+        free_staticmem_pages(pg, 1UL << order, need_scrub);
+        return;
+    }
+
     spin_lock(&heap_lock);
 
     for ( i = 0; i < (1 << order); i++ )
@@ -2636,8 +2639,8 @@ struct domain *get_pg_owner(domid_t domid)
 
 #ifdef CONFIG_STATIC_MEMORY
 /* Equivalent of free_heap_pages to free nr_mfns pages of static memory. */
-void __init free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
-                                 bool need_scrub)
+void free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
+                          bool need_scrub)
 {
     mfn_t mfn = page_to_mfn(pg);
     unsigned long i;
@@ -2652,7 +2655,6 @@ void __init free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
             scrub_one_page(pg);
         }
 
-        /* In case initializing page of static memory, mark it PGC_staticmem. */
         pg[i].count_info |= PGC_staticmem;
     }
 }
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 3be754da92..ca2c6f033e 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -85,10 +85,10 @@ bool scrub_free_pages(void);
 } while ( false )
 #define FREE_XENHEAP_PAGE(p) FREE_XENHEAP_PAGES(p, 0)
 
-#ifdef CONFIG_STATIC_MEMORY
 /* These functions are for static memory */
 void free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
                           bool need_scrub);
+#ifdef CONFIG_STATIC_MEMORY
 int acquire_domstatic_pages(struct domain *d, mfn_t smfn, unsigned int nr_mfns,
                             unsigned int memflags);
 #endif
@@ -212,6 +212,10 @@ extern struct domain *dom_cow;
 
 #include <asm/mm.h>
 
+#ifndef PGC_staticmem
+#define PGC_staticmem 0
+#endif
+
 static inline bool is_special_page(const struct page_info *page)
 {
     return is_xen_heap_page(page) || (page->count_info & PGC_extra);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 3/9] xen: update SUPPORT.md for static allocation
  2022-05-31  3:12 [PATCH v5 0/9] populate/unpopulate memory when domain on static allocation Penny Zheng
  2022-05-31  3:12 ` [PATCH v5 1/9] xen/arm: rename PGC_reserved to PGC_staticmem Penny Zheng
  2022-05-31  3:12 ` [PATCH v5 2/9] xen: do not free reserved memory into heap Penny Zheng
@ 2022-05-31  3:12 ` Penny Zheng
  2022-06-03  0:59   ` Stefano Stabellini
  2022-05-31  3:12 ` [PATCH v5 4/9] xen: do not merge reserved pages in free_heap_pages() Penny Zheng
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 27+ messages in thread
From: Penny Zheng @ 2022-05-31  3:12 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.chen, Penny Zheng, Andrew Cooper, George Dunlap, Jan Beulich,
	Julien Grall, Stefano Stabellini, Wei Liu, Penny Zheng

SUPPORT.md doesn't seem to explicitly say whether static memory is
supported, so this commit updates SUPPORT.md to add feature static
allocation tech preview for now.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v5 changes:
- new commit
---
 SUPPORT.md | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/SUPPORT.md b/SUPPORT.md
index ee2cd319e2..5980a82c4b 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -278,6 +278,13 @@ to boot with memory < maxmem.
 
     Status, x86 HVM: Supported
 
+### Static Allocation
+
+Static allocation refers to system or sub-system(domains) for which memory
+areas are pre-defined by configuration using physical address ranges.
+
+    Status, ARM: Tech Preview
+
 ### Memory Sharing
 
 Allow sharing of identical pages between guests
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 4/9] xen: do not merge reserved pages in free_heap_pages()
  2022-05-31  3:12 [PATCH v5 0/9] populate/unpopulate memory when domain on static allocation Penny Zheng
                   ` (2 preceding siblings ...)
  2022-05-31  3:12 ` [PATCH v5 3/9] xen: update SUPPORT.md for static allocation Penny Zheng
@ 2022-05-31  3:12 ` Penny Zheng
  2022-05-31  3:12 ` [PATCH v5 5/9] xen: add field "flags" to cover all internal CDF_XXX Penny Zheng
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 27+ messages in thread
From: Penny Zheng @ 2022-05-31  3:12 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.chen, Penny Zheng, Andrew Cooper, George Dunlap, Jan Beulich,
	Julien Grall, Stefano Stabellini, Wei Liu, Julien Grall,
	Penny Zheng

The code in free_heap_pages() will try to merge pages with the
successor/predecessor if pages are suitably aligned. So if the pages
reserved are right next to the pages given to the heap allocator,
free_heap_pages() will merge them, and give the reserved pages to heap
allocator accidently as a result.

So in order to avoid the above scenario, this commit updates free_heap_pages()
to check whether the predecessor and/or successor has PGC_reserved set,
when trying to merge the about-to-be-freed chunk with the predecessor
and/or successor.

Suggested-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
---
v5 changes:
- change PGC_reserved to adapt to PGC_staticmem
---
v4 changes:
- no changes
---
v3 changes:
- no changes
---
v2 changes:
- new commit
---
 xen/common/page_alloc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 6425761116..b1350fc238 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1482,6 +1482,7 @@ static void free_heap_pages(
             /* Merge with predecessor block? */
             if ( !mfn_valid(page_to_mfn(predecessor)) ||
                  !page_state_is(predecessor, free) ||
+                 (predecessor->count_info & PGC_staticmem) ||
                  (PFN_ORDER(predecessor) != order) ||
                  (phys_to_nid(page_to_maddr(predecessor)) != node) )
                 break;
@@ -1505,6 +1506,7 @@ static void free_heap_pages(
             /* Merge with successor block? */
             if ( !mfn_valid(page_to_mfn(successor)) ||
                  !page_state_is(successor, free) ||
+                 (successor->count_info & PGC_staticmem) ||
                  (PFN_ORDER(successor) != order) ||
                  (phys_to_nid(page_to_maddr(successor)) != node) )
                 break;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 5/9] xen: add field "flags" to cover all internal CDF_XXX
  2022-05-31  3:12 [PATCH v5 0/9] populate/unpopulate memory when domain on static allocation Penny Zheng
                   ` (3 preceding siblings ...)
  2022-05-31  3:12 ` [PATCH v5 4/9] xen: do not merge reserved pages in free_heap_pages() Penny Zheng
@ 2022-05-31  3:12 ` Penny Zheng
  2022-05-31  3:12 ` [PATCH v5 6/9] xen/arm: introduce CDF_staticmem Penny Zheng
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 27+ messages in thread
From: Penny Zheng @ 2022-05-31  3:12 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.chen, Penny Zheng, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, Andrew Cooper,
	George Dunlap, Jan Beulich, Wei Liu, Penny Zheng, Julien Grall

With more and more CDF_xxx internal flags in and to save the space, this
commit introduces a new field "flags" in struct domain to store CDF_*
internal flags directly.

Another new CDF_xxx will be introduced in the next patch.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
---
v5 changes:
- no change
---
v4 changes:
- no change
---
v3 changes:
- change fixed width type uint32_t to unsigned int
- change "flags" to a more descriptive name "cdf"
---
v2 changes:
- let "flags" live in the struct domain. So other arch can take
advantage of it in the future
- fix coding style
---
 xen/arch/arm/domain.c             | 2 --
 xen/arch/arm/include/asm/domain.h | 3 +--
 xen/common/domain.c               | 3 +++
 xen/include/xen/sched.h           | 3 +++
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 8110c1df86..74189d9878 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -709,8 +709,6 @@ int arch_domain_create(struct domain *d,
     ioreq_domain_init(d);
 #endif
 
-    d->arch.directmap = flags & CDF_directmap;
-
     /* p2m_init relies on some value initialized by the IOMMU subsystem */
     if ( (rc = iommu_domain_init(d, config->iommu_opts)) != 0 )
         goto fail;
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index ed63c2b6f9..fe7a029ebf 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -29,7 +29,7 @@ enum domain_type {
 #define is_64bit_domain(d) (0)
 #endif
 
-#define is_domain_direct_mapped(d) (d)->arch.directmap
+#define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap)
 
 /*
  * Is the domain using the host memory layout?
@@ -103,7 +103,6 @@ struct arch_domain
     void *tee;
 #endif
 
-    bool directmap;
 }  __cacheline_aligned;
 
 struct arch_vcpu
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 7570eae91a..a3ef991bd1 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -567,6 +567,9 @@ struct domain *domain_create(domid_t domid,
     /* Sort out our idea of is_system_domain(). */
     d->domain_id = domid;
 
+    /* Holding CDF_* internal flags. */
+    d->cdf = flags;
+
     /* Debug sanity. */
     ASSERT(is_system_domain(d) ? config == NULL : config != NULL);
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 463d41ffb6..5191853c18 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -596,6 +596,9 @@ struct domain
         struct ioreq_server     *server[MAX_NR_IOREQ_SERVERS];
     } ioreq_server;
 #endif
+
+    /* Holding CDF_* constant. Internal flags for domain creation. */
+    unsigned int cdf;
 };
 
 static inline struct page_list_head *page_to_list(
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 6/9] xen/arm: introduce CDF_staticmem
  2022-05-31  3:12 [PATCH v5 0/9] populate/unpopulate memory when domain on static allocation Penny Zheng
                   ` (4 preceding siblings ...)
  2022-05-31  3:12 ` [PATCH v5 5/9] xen: add field "flags" to cover all internal CDF_XXX Penny Zheng
@ 2022-05-31  3:12 ` Penny Zheng
  2022-05-31  8:40   ` Jan Beulich
  2022-06-03  1:05   ` Stefano Stabellini
  2022-05-31  3:12 ` [PATCH v5 7/9] xen/arm: unpopulate memory when domain is static Penny Zheng
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 27+ messages in thread
From: Penny Zheng @ 2022-05-31  3:12 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.chen, Penny Zheng, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, Andrew Cooper,
	George Dunlap, Jan Beulich, Wei Liu, Penny Zheng

In order to have an easy and quick way to find out whether this domain memory
is statically configured, this commit introduces a new flag CDF_staticmem and a
new helper is_domain_using_staticmem() to tell.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v5 changes:
- guard "is_domain_using_staticmem" under CONFIG_STATIC_MEMORY
- #define is_domain_using_staticmem zero if undefined
---
v4 changes:
- no changes
---
v3 changes:
- change name from "is_domain_static()" to "is_domain_using_staticmem"
---
v2 changes:
- change name from "is_domain_on_static_allocation" to "is_domain_static()"
---
 xen/arch/arm/domain_build.c       | 5 ++++-
 xen/arch/arm/include/asm/domain.h | 4 ++++
 xen/include/xen/domain.h          | 6 ++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 7ddd16c26d..f6e2e44c1e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -3287,9 +3287,12 @@ void __init create_domUs(void)
         if ( !dt_device_is_compatible(node, "xen,domain") )
             continue;
 
+        if ( dt_find_property(node, "xen,static-mem", NULL) )
+            flags |= CDF_staticmem;
+
         if ( dt_property_read_bool(node, "direct-map") )
         {
-            if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) || !dt_find_property(node, "xen,static-mem", NULL) )
+            if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) || !(flags & CDF_staticmem) )
                 panic("direct-map is not valid for domain %s without static allocation.\n",
                       dt_node_name(node));
 
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index fe7a029ebf..6bb999aff0 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -31,6 +31,10 @@ enum domain_type {
 
 #define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap)
 
+#ifdef CONFIG_STATIC_MEMORY
+#define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem)
+#endif
+
 /*
  * Is the domain using the host memory layout?
  *
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 1c3c88a14d..c613afa57e 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -34,6 +34,12 @@ void arch_get_domain_info(const struct domain *d,
 #ifdef CONFIG_ARM
 /* Should domain memory be directly mapped? */
 #define CDF_directmap            (1U << 1)
+/* Is domain memory on static allocation? */
+#define CDF_staticmem            (1U << 2)
+#endif
+
+#ifndef is_domain_using_staticmem
+#define is_domain_using_staticmem(d) ((void)(d), false)
 #endif
 
 /*
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 7/9] xen/arm: unpopulate memory when domain is static
  2022-05-31  3:12 [PATCH v5 0/9] populate/unpopulate memory when domain on static allocation Penny Zheng
                   ` (5 preceding siblings ...)
  2022-05-31  3:12 ` [PATCH v5 6/9] xen/arm: introduce CDF_staticmem Penny Zheng
@ 2022-05-31  3:12 ` Penny Zheng
  2022-05-31  8:46   ` Jan Beulich
  2022-06-03  1:11   ` Stefano Stabellini
  2022-05-31  3:12 ` [PATCH v5 8/9] xen: introduce prepare_staticmem_pages Penny Zheng
  2022-05-31  3:12 ` [PATCH v5 9/9] xen: retrieve reserved pages on populate_physmap Penny Zheng
  8 siblings, 2 replies; 27+ messages in thread
From: Penny Zheng @ 2022-05-31  3:12 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.chen, Penny Zheng, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, Andrew Cooper,
	George Dunlap, Jan Beulich, Wei Liu, Penny Zheng

Today when a domain unpopulates the memory on runtime, they will always
hand the memory back to the heap allocator. And it will be a problem if domain
is static.

Pages as guest RAM for static domain shall be reserved to only this domain
and not be used for any other purposes, so they shall never go back to heap
allocator.

This commit puts reserved pages on the new list resv_page_list only after
having taken them off the "normal" list, when the last ref dropped.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v5 changes:
- adapt this patch for PGC_staticmem
---
v4 changes:
- no changes
---
v3 changes:
- have page_list_del() just once out of the if()
- remove resv_pages counter
- make arch_free_heap_page be an expression, not a compound statement.
---
v2 changes:
- put reserved pages on resv_page_list after having taken them off
the "normal" list
---
 xen/arch/arm/include/asm/mm.h | 12 ++++++++++++
 xen/common/domain.c           |  4 ++++
 xen/include/xen/sched.h       |  3 +++
 3 files changed, 19 insertions(+)

diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index 56d0939318..ca384a3939 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -360,6 +360,18 @@ void clear_and_clean_page(struct page_info *page);
 
 unsigned int arch_get_dma_bitsize(void);
 
+/*
+ * Put free pages on the resv page list after having taken them
+ * off the "normal" page list, when pages from static memory
+ */
+#ifdef CONFIG_STATIC_MEMORY
+#define arch_free_heap_page(d, pg) ({                   \
+    page_list_del(pg, page_to_list(d, pg));             \
+    if ( (pg)->count_info & PGC_staticmem )              \
+        page_list_add_tail(pg, &(d)->resv_page_list);   \
+})
+#endif
+
 #endif /*  __ARCH_ARM_MM__ */
 /*
  * Local variables:
diff --git a/xen/common/domain.c b/xen/common/domain.c
index a3ef991bd1..a49574fa24 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -604,6 +604,10 @@ struct domain *domain_create(domid_t domid,
     INIT_PAGE_LIST_HEAD(&d->page_list);
     INIT_PAGE_LIST_HEAD(&d->extra_page_list);
     INIT_PAGE_LIST_HEAD(&d->xenpage_list);
+#ifdef CONFIG_STATIC_MEMORY
+    INIT_PAGE_LIST_HEAD(&d->resv_page_list);
+#endif
+
 
     spin_lock_init(&d->node_affinity_lock);
     d->node_affinity = NODE_MASK_ALL;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 5191853c18..3e22c77333 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -381,6 +381,9 @@ struct domain
     struct page_list_head page_list;  /* linked list */
     struct page_list_head extra_page_list; /* linked list (size extra_pages) */
     struct page_list_head xenpage_list; /* linked list (size xenheap_pages) */
+#ifdef CONFIG_STATIC_MEMORY
+    struct page_list_head resv_page_list; /* linked list (size resv_pages) */
+#endif
 
     /*
      * This field should only be directly accessed by domain_adjust_tot_pages()
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 8/9] xen: introduce prepare_staticmem_pages
  2022-05-31  3:12 [PATCH v5 0/9] populate/unpopulate memory when domain on static allocation Penny Zheng
                   ` (6 preceding siblings ...)
  2022-05-31  3:12 ` [PATCH v5 7/9] xen/arm: unpopulate memory when domain is static Penny Zheng
@ 2022-05-31  3:12 ` Penny Zheng
  2022-05-31  8:48   ` Jan Beulich
  2022-05-31  3:12 ` [PATCH v5 9/9] xen: retrieve reserved pages on populate_physmap Penny Zheng
  8 siblings, 1 reply; 27+ messages in thread
From: Penny Zheng @ 2022-05-31  3:12 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.chen, Penny Zheng, Andrew Cooper, George Dunlap, Jan Beulich,
	Julien Grall, Stefano Stabellini, Wei Liu, Penny Zheng

Later, we want to use acquire_domstatic_pages() for populating memory
for static domain on runtime, however, there are a lot of pointless work
(checking mfn_valid(), scrubbing the free part, cleaning the cache...)
considering we know the page is valid and belong to the guest.

This commit splits acquire_staticmem_pages() in two parts, and
introduces prepare_staticmem_pages to bypass all "pointless work".

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v5 changes:
- new commit
---
 xen/common/page_alloc.c | 61 ++++++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 25 deletions(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index b1350fc238..bdd2e62865 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2661,26 +2661,13 @@ void free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
     }
 }
 
-/*
- * Acquire nr_mfns contiguous reserved pages, starting at #smfn, of
- * static memory.
- * This function needs to be reworked if used outside of boot.
- */
-static struct page_info * __init acquire_staticmem_pages(mfn_t smfn,
-                                                         unsigned long nr_mfns,
-                                                         unsigned int memflags)
+static bool __init prepare_staticmem_pages(struct page_info *pg,
+                                           unsigned long nr_mfns,
+                                           unsigned int memflags)
 {
     bool need_tlbflush = false;
     uint32_t tlbflush_timestamp = 0;
     unsigned long i;
-    struct page_info *pg;
-
-    ASSERT(nr_mfns);
-    for ( i = 0; i < nr_mfns; i++ )
-        if ( !mfn_valid(mfn_add(smfn, i)) )
-            return NULL;
-
-    pg = mfn_to_page(smfn);
 
     spin_lock(&heap_lock);
 
@@ -2691,7 +2678,7 @@ static struct page_info * __init acquire_staticmem_pages(mfn_t smfn,
         {
             printk(XENLOG_ERR
                    "pg[%lu] Static MFN %"PRI_mfn" c=%#lx t=%#x\n",
-                   i, mfn_x(smfn) + i,
+                   i, mfn_x(page_to_mfn(pg)) + i,
                    pg[i].count_info, pg[i].tlbflush_timestamp);
             goto out_err;
         }
@@ -2715,6 +2702,38 @@ static struct page_info * __init acquire_staticmem_pages(mfn_t smfn,
     if ( need_tlbflush )
         filtered_flush_tlb_mask(tlbflush_timestamp);
 
+    return true;
+
+ out_err:
+    while ( i-- )
+        pg[i].count_info = PGC_staticmem | PGC_state_free;
+
+    spin_unlock(&heap_lock);
+
+    return false;
+}
+
+/*
+ * Acquire nr_mfns contiguous reserved pages, starting at #smfn, of
+ * static memory.
+ * This function needs to be reworked if used outside of boot.
+ */
+static struct page_info * __init acquire_staticmem_pages(mfn_t smfn,
+                                                         unsigned long nr_mfns,
+                                                         unsigned int memflags)
+{
+    unsigned long i;
+    struct page_info *pg;
+
+    ASSERT(nr_mfns);
+    for ( i = 0; i < nr_mfns; i++ )
+        if ( !mfn_valid(mfn_add(smfn, i)) )
+            return NULL;
+
+    pg = mfn_to_page(smfn);
+    if ( !prepare_staticmem_pages(pg, nr_mfns, memflags) )
+        return NULL;
+
     /*
      * Ensure cache and RAM are consistent for platforms where the guest
      * can control its own visibility of/through the cache.
@@ -2723,14 +2742,6 @@ static struct page_info * __init acquire_staticmem_pages(mfn_t smfn,
         flush_page_to_ram(mfn_x(smfn) + i, !(memflags & MEMF_no_icache_flush));
 
     return pg;
-
- out_err:
-    while ( i-- )
-        pg[i].count_info = PGC_staticmem | PGC_state_free;
-
-    spin_unlock(&heap_lock);
-
-    return NULL;
 }
 
 /*
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v5 9/9] xen: retrieve reserved pages on populate_physmap
  2022-05-31  3:12 [PATCH v5 0/9] populate/unpopulate memory when domain on static allocation Penny Zheng
                   ` (7 preceding siblings ...)
  2022-05-31  3:12 ` [PATCH v5 8/9] xen: introduce prepare_staticmem_pages Penny Zheng
@ 2022-05-31  3:12 ` Penny Zheng
  2022-05-31  8:54   ` Jan Beulich
  8 siblings, 1 reply; 27+ messages in thread
From: Penny Zheng @ 2022-05-31  3:12 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.chen, Penny Zheng, Andrew Cooper, George Dunlap, Jan Beulich,
	Julien Grall, Stefano Stabellini, Wei Liu, Penny Zheng

When a static domain populates memory through populate_physmap at runtime,
it shall retrieve reserved pages from resv_page_list to make sure that
guest RAM is still restricted in statically configured memory regions.
This commit also introduces a new helper acquire_reserved_page to make it work.

Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
v5 changes:
- extract common codes for assigning pages into a helper assign_domstatic_pages
- refine commit message
- remove stub function acquire_reserved_page
- Alloc/free of memory can happen concurrently. So access to rsv_page_list
needs to be protected with a spinlock
---
v4 changes:
- miss dropping __init in acquire_domstatic_pages
- add the page back to the reserved list in case of error
- remove redundant printk
- refine log message and make it warn level
---
v3 changes:
- move is_domain_using_staticmem to the common header file
- remove #ifdef CONFIG_STATIC_MEMORY-ary
- remove meaningless page_to_mfn(page) in error log
---
v2 changes:
- introduce acquire_reserved_page to retrieve reserved pages from
resv_page_list
- forbid non-zero-order requests in populate_physmap
- let is_domain_static return ((void)(d), false) on x86
---
 xen/common/memory.c     | 23 ++++++++++++++
 xen/common/page_alloc.c | 70 +++++++++++++++++++++++++++++++----------
 xen/include/xen/mm.h    |  1 +
 3 files changed, 77 insertions(+), 17 deletions(-)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index f2d009843a..cb330ce877 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -245,6 +245,29 @@ static void populate_physmap(struct memop_args *a)
 
                 mfn = _mfn(gpfn);
             }
+            else if ( is_domain_using_staticmem(d) )
+            {
+                /*
+                 * No easy way to guarantee the retrieved pages are contiguous,
+                 * so forbid non-zero-order requests here.
+                 */
+                if ( a->extent_order != 0 )
+                {
+                    gdprintk(XENLOG_WARNING,
+                             "Cannot allocate static order-%u pages for static %pd\n",
+                             a->extent_order, d);
+                    goto out;
+                }
+
+                mfn = acquire_reserved_page(d, a->memflags);
+                if ( mfn_eq(mfn, INVALID_MFN) )
+                {
+                    gdprintk(XENLOG_WARNING,
+                             "%pd: failed to retrieve a reserved page\n",
+                             d);
+                    goto out;
+                }
+            }
             else
             {
                 page = alloc_domheap_pages(d, a->extent_order, a->memflags);
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index bdd2e62865..9448552bab 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2661,9 +2661,8 @@ void free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
     }
 }
 
-static bool __init prepare_staticmem_pages(struct page_info *pg,
-                                           unsigned long nr_mfns,
-                                           unsigned int memflags)
+static bool prepare_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
+                                    unsigned int memflags)
 {
     bool need_tlbflush = false;
     uint32_t tlbflush_timestamp = 0;
@@ -2744,21 +2743,9 @@ static struct page_info * __init acquire_staticmem_pages(mfn_t smfn,
     return pg;
 }
 
-/*
- * Acquire nr_mfns contiguous pages, starting at #smfn, of static memory,
- * then assign them to one specific domain #d.
- */
-int __init acquire_domstatic_pages(struct domain *d, mfn_t smfn,
-                                   unsigned int nr_mfns, unsigned int memflags)
+static int assign_domstatic_pages(struct domain *d, struct page_info *pg,
+                                  unsigned int nr_mfns, unsigned int memflags)
 {
-    struct page_info *pg;
-
-    ASSERT(!in_irq());
-
-    pg = acquire_staticmem_pages(smfn, nr_mfns, memflags);
-    if ( !pg )
-        return -ENOENT;
-
     if ( !d || (memflags & (MEMF_no_owner | MEMF_no_refcount)) )
     {
         /*
@@ -2777,6 +2764,55 @@ int __init acquire_domstatic_pages(struct domain *d, mfn_t smfn,
 
     return 0;
 }
+
+/*
+ * Acquire nr_mfns contiguous pages, starting at #smfn, of static memory,
+ * then assign them to one specific domain #d.
+ */
+int __init acquire_domstatic_pages(struct domain *d, mfn_t smfn,
+                                   unsigned int nr_mfns, unsigned int memflags)
+{
+    struct page_info *pg;
+
+    ASSERT(!in_irq());
+
+    pg = acquire_staticmem_pages(smfn, nr_mfns, memflags);
+    if ( !pg )
+        return -ENOENT;
+
+    if ( assign_domstatic_pages(d, pg, nr_mfns, memflags) )
+        return -EINVAL;
+
+    return 0;
+}
+
+/*
+ * Acquire a page from reserved page list(resv_page_list), when populating
+ * memory for static domain on runtime.
+ */
+mfn_t acquire_reserved_page(struct domain *d, unsigned int memflags)
+{
+    struct page_info *page;
+
+    spin_lock(&d->page_alloc_lock);
+    /* Acquire a page from reserved page list(resv_page_list). */
+    page = page_list_remove_head(&d->resv_page_list);
+    if ( unlikely(!page) )
+        return INVALID_MFN;
+    spin_unlock(&d->page_alloc_lock);
+
+    if ( !prepare_staticmem_pages(page, 1, memflags) )
+        goto fail;
+
+    if ( assign_domstatic_pages(d, page, 1, memflags) )
+        goto fail;
+
+    return page_to_mfn(page);
+
+ fail:
+    page_list_add_tail(page, &d->resv_page_list);
+    return INVALID_MFN;
+}
 #endif
 
 /*
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index ca2c6f033e..4665bcdd25 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -92,6 +92,7 @@ void free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
 int acquire_domstatic_pages(struct domain *d, mfn_t smfn, unsigned int nr_mfns,
                             unsigned int memflags);
 #endif
+mfn_t acquire_reserved_page(struct domain *d, unsigned int memflags);
 
 /* Map machine page range in Xen virtual address space. */
 int map_pages_to_xen(
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 1/9] xen/arm: rename PGC_reserved to PGC_staticmem
  2022-05-31  3:12 ` [PATCH v5 1/9] xen/arm: rename PGC_reserved to PGC_staticmem Penny Zheng
@ 2022-05-31  8:32   ` Jan Beulich
  2022-06-02  2:04     ` Penny Zheng
  0 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2022-05-31  8:32 UTC (permalink / raw)
  To: Penny Zheng
  Cc: wei.chen, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
	xen-devel

On 31.05.2022 05:12, Penny Zheng wrote:
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -151,8 +151,8 @@
>  #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
>  #endif
>  
> -#ifndef PGC_reserved
> -#define PGC_reserved 0
> +#ifndef PGC_staticmem
> +#define PGC_staticmem 0
>  #endif

Just wondering: Is the "mem" part of the name really significant? Pages
always represent memory of some form, don't they?

Jan



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 2/9] xen: do not free reserved memory into heap
  2022-05-31  3:12 ` [PATCH v5 2/9] xen: do not free reserved memory into heap Penny Zheng
@ 2022-05-31  8:36   ` Jan Beulich
  2022-06-02  2:18     ` Penny Zheng
  0 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2022-05-31  8:36 UTC (permalink / raw)
  To: Penny Zheng
  Cc: wei.chen, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
	xen-devel

On 31.05.2022 05:12, Penny Zheng wrote:
> Pages used as guest RAM for static domain, shall be reserved to this
> domain only.
> So in case reserved pages being used for other purpose, users
> shall not free them back to heap, even when last ref gets dropped.
> 
> free_staticmem_pages will be called by free_heap_pages in runtime
> for static domain freeing memory resource, so let's drop the __init
> flag.
> 
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> ---
> v5 changes:
> - In order to avoid stub functions, we #define PGC_staticmem to non-zero only
> when CONFIG_STATIC_MEMORY
> - use "unlikely()" around pg->count_info & PGC_staticmem
> - remove pointless "if", since mark_page_free() is going to set count_info
> to PGC_state_free and by consequence clear PGC_staticmem
> - move #define PGC_staticmem 0 to mm.h
> ---
> v4 changes:
> - no changes
> ---
> v3 changes:
> - fix possible racy issue in free_staticmem_pages()
> - introduce a stub free_staticmem_pages() for the !CONFIG_STATIC_MEMORY case
> - move the change to free_heap_pages() to cover other potential call sites
> - fix the indentation
> ---
> v2 changes:
> - new commit
> ---
>  xen/arch/arm/include/asm/mm.h |  2 ++
>  xen/common/page_alloc.c       | 16 +++++++++-------
>  xen/include/xen/mm.h          |  6 +++++-
>  3 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
> index 1226700085..56d0939318 100644
> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -108,9 +108,11 @@ struct page_info
>    /* Page is Xen heap? */
>  #define _PGC_xen_heap     PG_shift(2)
>  #define PGC_xen_heap      PG_mask(1, 2)
> +#ifdef CONFIG_STATIC_MEMORY
>    /* Page is static memory */
>  #define _PGC_staticmem    PG_shift(3)
>  #define PGC_staticmem     PG_mask(1, 3)
> +#endif
>  /* ... */
>  /* Page is broken? */
>  #define _PGC_broken       PG_shift(7)
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 44600dd9cd..6425761116 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -151,10 +151,6 @@
>  #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
>  #endif
>  
> -#ifndef PGC_staticmem
> -#define PGC_staticmem 0
> -#endif
> -

Is the moving of this into the header really a necessary part of this
change? Afaics the symbol is still only ever used in this one C file.

> --- a/xen/include/xen/mm.h
> +++ b/xen/include/xen/mm.h
> @@ -85,10 +85,10 @@ bool scrub_free_pages(void);
>  } while ( false )
>  #define FREE_XENHEAP_PAGE(p) FREE_XENHEAP_PAGES(p, 0)
>  
> -#ifdef CONFIG_STATIC_MEMORY
>  /* These functions are for static memory */
>  void free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
>                            bool need_scrub);
> +#ifdef CONFIG_STATIC_MEMORY
>  int acquire_domstatic_pages(struct domain *d, mfn_t smfn, unsigned int nr_mfns,
>                              unsigned int memflags);
>  #endif

Is the #ifdef really worth retaining at this point? Code is generally
better readable without.

Jan



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 6/9] xen/arm: introduce CDF_staticmem
  2022-05-31  3:12 ` [PATCH v5 6/9] xen/arm: introduce CDF_staticmem Penny Zheng
@ 2022-05-31  8:40   ` Jan Beulich
  2022-06-02 10:07     ` Penny Zheng
  2022-06-03  1:05   ` Stefano Stabellini
  1 sibling, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2022-05-31  8:40 UTC (permalink / raw)
  To: Penny Zheng
  Cc: wei.chen, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
	xen-devel

On 31.05.2022 05:12, Penny Zheng wrote:
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -31,6 +31,10 @@ enum domain_type {
>  
>  #define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap)
>  
> +#ifdef CONFIG_STATIC_MEMORY
> +#define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem)
> +#endif

Why is this in the Arm header, rather than ...

> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -34,6 +34,12 @@ void arch_get_domain_info(const struct domain *d,
>  #ifdef CONFIG_ARM
>  /* Should domain memory be directly mapped? */
>  #define CDF_directmap            (1U << 1)
> +/* Is domain memory on static allocation? */
> +#define CDF_staticmem            (1U << 2)
> +#endif
> +
> +#ifndef is_domain_using_staticmem
> +#define is_domain_using_staticmem(d) ((void)(d), false)
>  #endif

... here (with what you have here now simply becoming the #else part)?
Once living here, I expect it also can be an inline function rather
than a macro, with the #ifdef merely inside its body.

Jan



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 7/9] xen/arm: unpopulate memory when domain is static
  2022-05-31  3:12 ` [PATCH v5 7/9] xen/arm: unpopulate memory when domain is static Penny Zheng
@ 2022-05-31  8:46   ` Jan Beulich
  2022-06-03  1:11   ` Stefano Stabellini
  1 sibling, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2022-05-31  8:46 UTC (permalink / raw)
  To: Penny Zheng
  Cc: wei.chen, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
	xen-devel

On 31.05.2022 05:12, Penny Zheng wrote:
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -381,6 +381,9 @@ struct domain
>      struct page_list_head page_list;  /* linked list */
>      struct page_list_head extra_page_list; /* linked list (size extra_pages) */
>      struct page_list_head xenpage_list; /* linked list (size xenheap_pages) */
> +#ifdef CONFIG_STATIC_MEMORY
> +    struct page_list_head resv_page_list; /* linked list (size resv_pages) */
> +#endif

The comment part in parentheses isn't applicable just yet. Please move
that to where that counter actually is introduced. While this may be
in this same series, there's no guarantee that all parts of this series
will be committed together. With that adjustment
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 8/9] xen: introduce prepare_staticmem_pages
  2022-05-31  3:12 ` [PATCH v5 8/9] xen: introduce prepare_staticmem_pages Penny Zheng
@ 2022-05-31  8:48   ` Jan Beulich
  0 siblings, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2022-05-31  8:48 UTC (permalink / raw)
  To: Penny Zheng
  Cc: wei.chen, Andrew Cooper, George Dunlap, Julien Grall,
	Stefano Stabellini, Wei Liu, xen-devel

On 31.05.2022 05:12, Penny Zheng wrote:
> Later, we want to use acquire_domstatic_pages() for populating memory
> for static domain on runtime, however, there are a lot of pointless work
> (checking mfn_valid(), scrubbing the free part, cleaning the cache...)
> considering we know the page is valid and belong to the guest.
> 
> This commit splits acquire_staticmem_pages() in two parts, and
> introduces prepare_staticmem_pages to bypass all "pointless work".
> 
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>

Acked-by: Jan Beulich <jbeulich@suse.com>



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 9/9] xen: retrieve reserved pages on populate_physmap
  2022-05-31  3:12 ` [PATCH v5 9/9] xen: retrieve reserved pages on populate_physmap Penny Zheng
@ 2022-05-31  8:54   ` Jan Beulich
  2022-05-31  9:35     ` Julien Grall
  0 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2022-05-31  8:54 UTC (permalink / raw)
  To: Penny Zheng
  Cc: wei.chen, Andrew Cooper, George Dunlap, Julien Grall,
	Stefano Stabellini, Wei Liu, xen-devel

On 31.05.2022 05:12, Penny Zheng wrote:
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -245,6 +245,29 @@ static void populate_physmap(struct memop_args *a)
>  
>                  mfn = _mfn(gpfn);
>              }
> +            else if ( is_domain_using_staticmem(d) )
> +            {
> +                /*
> +                 * No easy way to guarantee the retrieved pages are contiguous,
> +                 * so forbid non-zero-order requests here.
> +                 */
> +                if ( a->extent_order != 0 )
> +                {
> +                    gdprintk(XENLOG_WARNING,
> +                             "Cannot allocate static order-%u pages for static %pd\n",
> +                             a->extent_order, d);
> +                    goto out;
> +                }
> +
> +                mfn = acquire_reserved_page(d, a->memflags);
> +                if ( mfn_eq(mfn, INVALID_MFN) )
> +                {
> +                    gdprintk(XENLOG_WARNING,
> +                             "%pd: failed to retrieve a reserved page\n",
> +                             d);
> +                    goto out;
> +                }
> +            }

I'm not convinced of having these gdprintk()s here. The adjacent
is_domain_direct_mapped() code is somewhat different - iirc only Dom0
can be direct-mapped, and Dom0 having a problem can certainly be worth
a log message.

> +/*
> + * Acquire a page from reserved page list(resv_page_list), when populating
> + * memory for static domain on runtime.
> + */
> +mfn_t acquire_reserved_page(struct domain *d, unsigned int memflags)
> +{
> +    struct page_info *page;
> +
> +    spin_lock(&d->page_alloc_lock);
> +    /* Acquire a page from reserved page list(resv_page_list). */
> +    page = page_list_remove_head(&d->resv_page_list);
> +    if ( unlikely(!page) )
> +        return INVALID_MFN;
> +    spin_unlock(&d->page_alloc_lock);

You want to drop the lock before returning.

Jan



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 9/9] xen: retrieve reserved pages on populate_physmap
  2022-05-31  8:54   ` Jan Beulich
@ 2022-05-31  9:35     ` Julien Grall
  2022-05-31  9:40       ` Jan Beulich
  0 siblings, 1 reply; 27+ messages in thread
From: Julien Grall @ 2022-05-31  9:35 UTC (permalink / raw)
  To: Jan Beulich, Penny Zheng
  Cc: wei.chen, Andrew Cooper, George Dunlap, Stefano Stabellini,
	Wei Liu, xen-devel

Hi Jan,

On 31/05/2022 09:54, Jan Beulich wrote:
> On 31.05.2022 05:12, Penny Zheng wrote:
>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -245,6 +245,29 @@ static void populate_physmap(struct memop_args *a)
>>   
>>                   mfn = _mfn(gpfn);
>>               }
>> +            else if ( is_domain_using_staticmem(d) )
>> +            {
>> +                /*
>> +                 * No easy way to guarantee the retrieved pages are contiguous,
>> +                 * so forbid non-zero-order requests here.
>> +                 */
>> +                if ( a->extent_order != 0 )
>> +                {
>> +                    gdprintk(XENLOG_WARNING,
>> +                             "Cannot allocate static order-%u pages for static %pd\n",
>> +                             a->extent_order, d);
>> +                    goto out;
>> +                }
>> +
>> +                mfn = acquire_reserved_page(d, a->memflags);
>> +                if ( mfn_eq(mfn, INVALID_MFN) )
>> +                {
>> +                    gdprintk(XENLOG_WARNING,
>> +                             "%pd: failed to retrieve a reserved page\n",
>> +                             d);
>> +                    goto out;
>> +                }
>> +            }
> 
> I'm not convinced of having these gdprintk()s here. 

There are a number of time where I wished some error paths would contain 
debug printk(). Instead, I often end up to add them myself when I 
struggle to find the reason of a failure.

They are debug printk() and therefore there is no impact on the 
production build. So I would leave them around.

> The adjacent
> is_domain_direct_mapped() code is somewhat different - iirc only Dom0
> can be direct-mapped, and Dom0 having a problem can certainly be worth
> a log message.

There are plan to use direct-mapped domU.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 9/9] xen: retrieve reserved pages on populate_physmap
  2022-05-31  9:35     ` Julien Grall
@ 2022-05-31  9:40       ` Jan Beulich
  2022-05-31  9:50         ` Julien Grall
  0 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2022-05-31  9:40 UTC (permalink / raw)
  To: Julien Grall
  Cc: wei.chen, Andrew Cooper, George Dunlap, Stefano Stabellini,
	Wei Liu, xen-devel, Penny Zheng

On 31.05.2022 11:35, Julien Grall wrote:
> On 31/05/2022 09:54, Jan Beulich wrote:
>> On 31.05.2022 05:12, Penny Zheng wrote:
>>> --- a/xen/common/memory.c
>>> +++ b/xen/common/memory.c
>>> @@ -245,6 +245,29 @@ static void populate_physmap(struct memop_args *a)
>>>   
>>>                   mfn = _mfn(gpfn);
>>>               }
>>> +            else if ( is_domain_using_staticmem(d) )
>>> +            {
>>> +                /*
>>> +                 * No easy way to guarantee the retrieved pages are contiguous,
>>> +                 * so forbid non-zero-order requests here.
>>> +                 */
>>> +                if ( a->extent_order != 0 )
>>> +                {
>>> +                    gdprintk(XENLOG_WARNING,
>>> +                             "Cannot allocate static order-%u pages for static %pd\n",
>>> +                             a->extent_order, d);
>>> +                    goto out;
>>> +                }
>>> +
>>> +                mfn = acquire_reserved_page(d, a->memflags);
>>> +                if ( mfn_eq(mfn, INVALID_MFN) )
>>> +                {
>>> +                    gdprintk(XENLOG_WARNING,
>>> +                             "%pd: failed to retrieve a reserved page\n",
>>> +                             d);
>>> +                    goto out;
>>> +                }
>>> +            }
>>
>> I'm not convinced of having these gdprintk()s here. 
> 
> There are a number of time where I wished some error paths would contain 
> debug printk(). Instead, I often end up to add them myself when I 
> struggle to find the reason of a failure.

But this model doesn't scale - we don't want to have log messages on
each and every error path. I agree having such for very unlikely
errors, but order != 0 is clearly a call site mistake and memory
allocation requests failing also ought to not be entirely unexpected.

Jan



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 9/9] xen: retrieve reserved pages on populate_physmap
  2022-05-31  9:40       ` Jan Beulich
@ 2022-05-31  9:50         ` Julien Grall
  0 siblings, 0 replies; 27+ messages in thread
From: Julien Grall @ 2022-05-31  9:50 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.chen, Andrew Cooper, George Dunlap, Stefano Stabellini,
	Wei Liu, xen-devel, Penny Zheng

Hi,

On 31/05/2022 10:40, Jan Beulich wrote:
> On 31.05.2022 11:35, Julien Grall wrote:
>> On 31/05/2022 09:54, Jan Beulich wrote:
>>> On 31.05.2022 05:12, Penny Zheng wrote:
>>>> --- a/xen/common/memory.c
>>>> +++ b/xen/common/memory.c
>>>> @@ -245,6 +245,29 @@ static void populate_physmap(struct memop_args *a)
>>>>    
>>>>                    mfn = _mfn(gpfn);
>>>>                }
>>>> +            else if ( is_domain_using_staticmem(d) )
>>>> +            {
>>>> +                /*
>>>> +                 * No easy way to guarantee the retrieved pages are contiguous,
>>>> +                 * so forbid non-zero-order requests here.
>>>> +                 */
>>>> +                if ( a->extent_order != 0 )
>>>> +                {
>>>> +                    gdprintk(XENLOG_WARNING,
>>>> +                             "Cannot allocate static order-%u pages for static %pd\n",
>>>> +                             a->extent_order, d);
>>>> +                    goto out;
>>>> +                }
>>>> +
>>>> +                mfn = acquire_reserved_page(d, a->memflags);
>>>> +                if ( mfn_eq(mfn, INVALID_MFN) )
>>>> +                {
>>>> +                    gdprintk(XENLOG_WARNING,
>>>> +                             "%pd: failed to retrieve a reserved page\n",
>>>> +                             d);
>>>> +                    goto out;
>>>> +                }
>>>> +            }
>>>
>>> I'm not convinced of having these gdprintk()s here.
>>
>> There are a number of time where I wished some error paths would contain
>> debug printk(). Instead, I often end up to add them myself when I
>> struggle to find the reason of a failure.
> 
> But this model doesn't scale - we don't want to have log messages on
> each and every error path. I agree having such for very unlikely
> errors, but order != 0 is clearly a call site mistake and memory
> allocation requests failing also ought to not be entirely unexpected.
The problem is from the guest PoV, the error for both is the same. So it 
would be difficult (not impossible) for the developper to know what's 
the exact problem.

But note that we already have a gdprintk() for allocation failure in the 
non-direct map case. So I think they should be here for consistency.

If you want to drop the existing one, then this is a separate 
discussion. And, just so you know, I would strongly argue against 
removing them for the reason I stated above.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH v5 1/9] xen/arm: rename PGC_reserved to PGC_staticmem
  2022-05-31  8:32   ` Jan Beulich
@ 2022-06-02  2:04     ` Penny Zheng
  0 siblings, 0 replies; 27+ messages in thread
From: Penny Zheng @ 2022-06-02  2:04 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Wei Chen, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
	xen-devel

Hi jan 

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, May 31, 2022 4:33 PM
> To: Penny Zheng <Penny.Zheng@arm.com>
> Cc: Wei Chen <Wei.Chen@arm.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; Bertrand Marquis
> <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>;
> Wei Liu <wl@xen.org>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v5 1/9] xen/arm: rename PGC_reserved to PGC_staticmem
> 
> On 31.05.2022 05:12, Penny Zheng wrote:
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -151,8 +151,8 @@
> >  #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
> > #endif
> >
> > -#ifndef PGC_reserved
> > -#define PGC_reserved 0
> > +#ifndef PGC_staticmem
> > +#define PGC_staticmem 0
> >  #endif
> 
> Just wondering: Is the "mem" part of the name really significant? Pages always
> represent memory of some form, don't they?
> 

Sure, it seems redundant, I'll rename to PGC_static.

> Jan


^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH v5 2/9] xen: do not free reserved memory into heap
  2022-05-31  8:36   ` Jan Beulich
@ 2022-06-02  2:18     ` Penny Zheng
  2022-06-02  9:23       ` Jan Beulich
  0 siblings, 1 reply; 27+ messages in thread
From: Penny Zheng @ 2022-06-02  2:18 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Wei Chen, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
	xen-devel



> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, May 31, 2022 4:37 PM
> To: Penny Zheng <Penny.Zheng@arm.com>
> Cc: Wei Chen <Wei.Chen@arm.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; Bertrand Marquis
> <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>;
> Wei Liu <wl@xen.org>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v5 2/9] xen: do not free reserved memory into heap
> 
> On 31.05.2022 05:12, Penny Zheng wrote:
> > Pages used as guest RAM for static domain, shall be reserved to this
> > domain only.
> > So in case reserved pages being used for other purpose, users shall
> > not free them back to heap, even when last ref gets dropped.
> >
> > free_staticmem_pages will be called by free_heap_pages in runtime for
> > static domain freeing memory resource, so let's drop the __init flag.
> >
> > Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> > ---
> > v5 changes:
> > - In order to avoid stub functions, we #define PGC_staticmem to
> > non-zero only when CONFIG_STATIC_MEMORY
> > - use "unlikely()" around pg->count_info & PGC_staticmem
> > - remove pointless "if", since mark_page_free() is going to set
> > count_info to PGC_state_free and by consequence clear PGC_staticmem
> > - move #define PGC_staticmem 0 to mm.h
> > ---
> > v4 changes:
> > - no changes
> > ---
> > v3 changes:
> > - fix possible racy issue in free_staticmem_pages()
> > - introduce a stub free_staticmem_pages() for the
> > !CONFIG_STATIC_MEMORY case
> > - move the change to free_heap_pages() to cover other potential call
> > sites
> > - fix the indentation
> > ---
> > v2 changes:
> > - new commit
> > ---
> >  xen/arch/arm/include/asm/mm.h |  2 ++
> >  xen/common/page_alloc.c       | 16 +++++++++-------
> >  xen/include/xen/mm.h          |  6 +++++-
> >  3 files changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/xen/arch/arm/include/asm/mm.h
> > b/xen/arch/arm/include/asm/mm.h index 1226700085..56d0939318 100644
> > --- a/xen/arch/arm/include/asm/mm.h
> > +++ b/xen/arch/arm/include/asm/mm.h
> > @@ -108,9 +108,11 @@ struct page_info
> >    /* Page is Xen heap? */
> >  #define _PGC_xen_heap     PG_shift(2)
> >  #define PGC_xen_heap      PG_mask(1, 2)
> > +#ifdef CONFIG_STATIC_MEMORY
> >    /* Page is static memory */
> >  #define _PGC_staticmem    PG_shift(3)
> >  #define PGC_staticmem     PG_mask(1, 3)
> > +#endif
> >  /* ... */
> >  /* Page is broken? */
> >  #define _PGC_broken       PG_shift(7)
> > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index
> > 44600dd9cd..6425761116 100644
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -151,10 +151,6 @@
> >  #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
> > #endif
> >
> > -#ifndef PGC_staticmem
> > -#define PGC_staticmem 0
> > -#endif
> > -
> 
> Is the moving of this into the header really a necessary part of this change?
> Afaics the symbol is still only ever used in this one C file.

Later, in commit "xen/arm: unpopulate memory when domain is static", 
we will use this flag in xen/arch/arm/include/asm/mm.h

> > --- a/xen/include/xen/mm.h
> > +++ b/xen/include/xen/mm.h
> > @@ -85,10 +85,10 @@ bool scrub_free_pages(void);  } while ( false )
> > #define FREE_XENHEAP_PAGE(p) FREE_XENHEAP_PAGES(p, 0)
> >
> > -#ifdef CONFIG_STATIC_MEMORY
> >  /* These functions are for static memory */  void
> > free_staticmem_pages(struct page_info *pg, unsigned long nr_mfns,
> >                            bool need_scrub);
> > +#ifdef CONFIG_STATIC_MEMORY
> >  int acquire_domstatic_pages(struct domain *d, mfn_t smfn, unsigned int
> nr_mfns,
> >                              unsigned int memflags);  #endif
> 
> Is the #ifdef really worth retaining at this point? Code is generally better
> readable without.
> 

Sure, will remove

> Jan


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 2/9] xen: do not free reserved memory into heap
  2022-06-02  2:18     ` Penny Zheng
@ 2022-06-02  9:23       ` Jan Beulich
  0 siblings, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2022-06-02  9:23 UTC (permalink / raw)
  To: Penny Zheng
  Cc: Wei Chen, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
	xen-devel

On 02.06.2022 04:18, Penny Zheng wrote:
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Tuesday, May 31, 2022 4:37 PM
>>
>> On 31.05.2022 05:12, Penny Zheng wrote:
>>> --- a/xen/common/page_alloc.c
>>> +++ b/xen/common/page_alloc.c
>>> @@ -151,10 +151,6 @@
>>>  #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
>>> #endif
>>>
>>> -#ifndef PGC_staticmem
>>> -#define PGC_staticmem 0
>>> -#endif
>>> -
>>
>> Is the moving of this into the header really a necessary part of this change?
>> Afaics the symbol is still only ever used in this one C file.
> 
> Later, in commit "xen/arm: unpopulate memory when domain is static", 
> we will use this flag in xen/arch/arm/include/asm/mm.h

IOW you want to move this change there.

Jan



^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: [PATCH v5 6/9] xen/arm: introduce CDF_staticmem
  2022-05-31  8:40   ` Jan Beulich
@ 2022-06-02 10:07     ` Penny Zheng
  2022-06-02 10:34       ` Jan Beulich
  0 siblings, 1 reply; 27+ messages in thread
From: Penny Zheng @ 2022-06-02 10:07 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Wei Chen, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
	xen-devel

Hi Jan

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, May 31, 2022 4:41 PM
> To: Penny Zheng <Penny.Zheng@arm.com>
> Cc: Wei Chen <Wei.Chen@arm.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; Bertrand Marquis
> <Bertrand.Marquis@arm.com>; Volodymyr Babchuk
> <Volodymyr_Babchuk@epam.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>;
> Wei Liu <wl@xen.org>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v5 6/9] xen/arm: introduce CDF_staticmem
> 
> On 31.05.2022 05:12, Penny Zheng wrote:
> > --- a/xen/arch/arm/include/asm/domain.h
> > +++ b/xen/arch/arm/include/asm/domain.h
> > @@ -31,6 +31,10 @@ enum domain_type {
> >
> >  #define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap)
> >
> > +#ifdef CONFIG_STATIC_MEMORY
> > +#define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem)
> > +#endif
> 
> Why is this in the Arm header, rather than ...
> 
> > --- a/xen/include/xen/domain.h
> > +++ b/xen/include/xen/domain.h
> > @@ -34,6 +34,12 @@ void arch_get_domain_info(const struct domain *d,
> > #ifdef CONFIG_ARM
> >  /* Should domain memory be directly mapped? */
> >  #define CDF_directmap            (1U << 1)
> > +/* Is domain memory on static allocation? */
> > +#define CDF_staticmem            (1U << 2)
> > +#endif
> > +
> > +#ifndef is_domain_using_staticmem
> > +#define is_domain_using_staticmem(d) ((void)(d), false)
> >  #endif
> 
> ... here (with what you have here now simply becoming the #else part)?
> Once living here, I expect it also can be an inline function rather than a macro,
> with the #ifdef merely inside its body.
> 

In order to avoid bring the chicken and egg problem in xen/include/xen/domain.h,
I may need to move the static inline function to xen/include/xen/sched.h(which
has already included domain.h header).

> Jan


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 6/9] xen/arm: introduce CDF_staticmem
  2022-06-02 10:07     ` Penny Zheng
@ 2022-06-02 10:34       ` Jan Beulich
  0 siblings, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2022-06-02 10:34 UTC (permalink / raw)
  To: Penny Zheng
  Cc: Wei Chen, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, George Dunlap, Wei Liu,
	xen-devel

On 02.06.2022 12:07, Penny Zheng wrote:
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: Tuesday, May 31, 2022 4:41 PM
>>
>> On 31.05.2022 05:12, Penny Zheng wrote:
>>> --- a/xen/arch/arm/include/asm/domain.h
>>> +++ b/xen/arch/arm/include/asm/domain.h
>>> @@ -31,6 +31,10 @@ enum domain_type {
>>>
>>>  #define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap)
>>>
>>> +#ifdef CONFIG_STATIC_MEMORY
>>> +#define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem)
>>> +#endif
>>
>> Why is this in the Arm header, rather than ...
>>
>>> --- a/xen/include/xen/domain.h
>>> +++ b/xen/include/xen/domain.h
>>> @@ -34,6 +34,12 @@ void arch_get_domain_info(const struct domain *d,
>>> #ifdef CONFIG_ARM
>>>  /* Should domain memory be directly mapped? */
>>>  #define CDF_directmap            (1U << 1)
>>> +/* Is domain memory on static allocation? */
>>> +#define CDF_staticmem            (1U << 2)
>>> +#endif
>>> +
>>> +#ifndef is_domain_using_staticmem
>>> +#define is_domain_using_staticmem(d) ((void)(d), false)
>>>  #endif
>>
>> ... here (with what you have here now simply becoming the #else part)?
>> Once living here, I expect it also can be an inline function rather than a macro,
>> with the #ifdef merely inside its body.
>>
> 
> In order to avoid bring the chicken and egg problem in xen/include/xen/domain.h,
> I may need to move the static inline function to xen/include/xen/sched.h(which
> has already included domain.h header).

Hmm, yes, if made an inline function it would need to move to sched.h.
But as a macro it could live here (without one half needing to live
in the Arm header).

Jan



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 3/9] xen: update SUPPORT.md for static allocation
  2022-05-31  3:12 ` [PATCH v5 3/9] xen: update SUPPORT.md for static allocation Penny Zheng
@ 2022-06-03  0:59   ` Stefano Stabellini
  0 siblings, 0 replies; 27+ messages in thread
From: Stefano Stabellini @ 2022-06-03  0:59 UTC (permalink / raw)
  To: Penny Zheng
  Cc: xen-devel, wei.chen, Andrew Cooper, George Dunlap, Jan Beulich,
	Julien Grall, Stefano Stabellini, Wei Liu

On Tue, 31 May 2022, Penny Zheng wrote:
> SUPPORT.md doesn't seem to explicitly say whether static memory is
> supported, so this commit updates SUPPORT.md to add feature static
> allocation tech preview for now.
> 
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> ---
> v5 changes:
> - new commit
> ---
>  SUPPORT.md | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/SUPPORT.md b/SUPPORT.md
> index ee2cd319e2..5980a82c4b 100644
> --- a/SUPPORT.md
> +++ b/SUPPORT.md
> @@ -278,6 +278,13 @@ to boot with memory < maxmem.
>  
>      Status, x86 HVM: Supported
>  
> +### Static Allocation
> +
> +Static allocation refers to system or sub-system(domains) for which memory
> +areas are pre-defined by configuration using physical address ranges.

Although I completely understand what you mean I would rather not use
the word "sub-system" as we haven't used it before in a Xen context. So
instead I would only use domains. I would write it like this:


### Static Allocation

Static allocation refers to domains for which memory areas are
pre-defined by configuration using physical address ranges.

Status, ARM: Tech Preview


With that you can add

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> +    Status, ARM: Tech Preview
> +
>  ### Memory Sharing
>  
>  Allow sharing of identical pages between guests
> -- 
> 2.25.1
> 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 6/9] xen/arm: introduce CDF_staticmem
  2022-05-31  3:12 ` [PATCH v5 6/9] xen/arm: introduce CDF_staticmem Penny Zheng
  2022-05-31  8:40   ` Jan Beulich
@ 2022-06-03  1:05   ` Stefano Stabellini
  1 sibling, 0 replies; 27+ messages in thread
From: Stefano Stabellini @ 2022-06-03  1:05 UTC (permalink / raw)
  To: Penny Zheng
  Cc: xen-devel, wei.chen, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, Andrew Cooper,
	George Dunlap, Jan Beulich, Wei Liu

On Tue, 31 May 2022, Penny Zheng wrote:
> In order to have an easy and quick way to find out whether this domain memory
> is statically configured, this commit introduces a new flag CDF_staticmem and a
> new helper is_domain_using_staticmem() to tell.
> 
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>

I realize Jan asked for improvements but I just wanted to say that on my
side it looks good.


> ---
> v5 changes:
> - guard "is_domain_using_staticmem" under CONFIG_STATIC_MEMORY
> - #define is_domain_using_staticmem zero if undefined
> ---
> v4 changes:
> - no changes
> ---
> v3 changes:
> - change name from "is_domain_static()" to "is_domain_using_staticmem"
> ---
> v2 changes:
> - change name from "is_domain_on_static_allocation" to "is_domain_static()"
> ---
>  xen/arch/arm/domain_build.c       | 5 ++++-
>  xen/arch/arm/include/asm/domain.h | 4 ++++
>  xen/include/xen/domain.h          | 6 ++++++
>  3 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 7ddd16c26d..f6e2e44c1e 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -3287,9 +3287,12 @@ void __init create_domUs(void)
>          if ( !dt_device_is_compatible(node, "xen,domain") )
>              continue;
>  
> +        if ( dt_find_property(node, "xen,static-mem", NULL) )
> +            flags |= CDF_staticmem;
> +
>          if ( dt_property_read_bool(node, "direct-map") )
>          {
> -            if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) || !dt_find_property(node, "xen,static-mem", NULL) )
> +            if ( !IS_ENABLED(CONFIG_STATIC_MEMORY) || !(flags & CDF_staticmem) )
>                  panic("direct-map is not valid for domain %s without static allocation.\n",
>                        dt_node_name(node));
>  
> diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
> index fe7a029ebf..6bb999aff0 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -31,6 +31,10 @@ enum domain_type {
>  
>  #define is_domain_direct_mapped(d) ((d)->cdf & CDF_directmap)
>  
> +#ifdef CONFIG_STATIC_MEMORY
> +#define is_domain_using_staticmem(d) ((d)->cdf & CDF_staticmem)
> +#endif
> +
>  /*
>   * Is the domain using the host memory layout?
>   *
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index 1c3c88a14d..c613afa57e 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -34,6 +34,12 @@ void arch_get_domain_info(const struct domain *d,
>  #ifdef CONFIG_ARM
>  /* Should domain memory be directly mapped? */
>  #define CDF_directmap            (1U << 1)
> +/* Is domain memory on static allocation? */
> +#define CDF_staticmem            (1U << 2)
> +#endif
> +
> +#ifndef is_domain_using_staticmem
> +#define is_domain_using_staticmem(d) ((void)(d), false)
>  #endif
>  
>  /*
> -- 
> 2.25.1
> 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v5 7/9] xen/arm: unpopulate memory when domain is static
  2022-05-31  3:12 ` [PATCH v5 7/9] xen/arm: unpopulate memory when domain is static Penny Zheng
  2022-05-31  8:46   ` Jan Beulich
@ 2022-06-03  1:11   ` Stefano Stabellini
  1 sibling, 0 replies; 27+ messages in thread
From: Stefano Stabellini @ 2022-06-03  1:11 UTC (permalink / raw)
  To: Penny Zheng
  Cc: xen-devel, wei.chen, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Volodymyr Babchuk, Andrew Cooper,
	George Dunlap, Jan Beulich, Wei Liu

On Tue, 31 May 2022, Penny Zheng wrote:
> Today when a domain unpopulates the memory on runtime, they will always
> hand the memory back to the heap allocator. And it will be a problem if domain
> is static.
> 
> Pages as guest RAM for static domain shall be reserved to only this domain
> and not be used for any other purposes, so they shall never go back to heap
> allocator.
> 
> This commit puts reserved pages on the new list resv_page_list only after
> having taken them off the "normal" list, when the last ref dropped.
> 
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>

ARM bits:
Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> v5 changes:
> - adapt this patch for PGC_staticmem
> ---
> v4 changes:
> - no changes
> ---
> v3 changes:
> - have page_list_del() just once out of the if()
> - remove resv_pages counter
> - make arch_free_heap_page be an expression, not a compound statement.
> ---
> v2 changes:
> - put reserved pages on resv_page_list after having taken them off
> the "normal" list
> ---
>  xen/arch/arm/include/asm/mm.h | 12 ++++++++++++
>  xen/common/domain.c           |  4 ++++
>  xen/include/xen/sched.h       |  3 +++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
> index 56d0939318..ca384a3939 100644
> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -360,6 +360,18 @@ void clear_and_clean_page(struct page_info *page);
>  
>  unsigned int arch_get_dma_bitsize(void);
>  
> +/*
> + * Put free pages on the resv page list after having taken them
> + * off the "normal" page list, when pages from static memory
> + */
> +#ifdef CONFIG_STATIC_MEMORY
> +#define arch_free_heap_page(d, pg) ({                   \
> +    page_list_del(pg, page_to_list(d, pg));             \
> +    if ( (pg)->count_info & PGC_staticmem )              \
> +        page_list_add_tail(pg, &(d)->resv_page_list);   \
> +})
> +#endif
> +
>  #endif /*  __ARCH_ARM_MM__ */
>  /*
>   * Local variables:
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index a3ef991bd1..a49574fa24 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -604,6 +604,10 @@ struct domain *domain_create(domid_t domid,
>      INIT_PAGE_LIST_HEAD(&d->page_list);
>      INIT_PAGE_LIST_HEAD(&d->extra_page_list);
>      INIT_PAGE_LIST_HEAD(&d->xenpage_list);
> +#ifdef CONFIG_STATIC_MEMORY
> +    INIT_PAGE_LIST_HEAD(&d->resv_page_list);
> +#endif
> +
>  
>      spin_lock_init(&d->node_affinity_lock);
>      d->node_affinity = NODE_MASK_ALL;
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 5191853c18..3e22c77333 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -381,6 +381,9 @@ struct domain
>      struct page_list_head page_list;  /* linked list */
>      struct page_list_head extra_page_list; /* linked list (size extra_pages) */
>      struct page_list_head xenpage_list; /* linked list (size xenheap_pages) */
> +#ifdef CONFIG_STATIC_MEMORY
> +    struct page_list_head resv_page_list; /* linked list (size resv_pages) */
> +#endif
>  
>      /*
>       * This field should only be directly accessed by domain_adjust_tot_pages()
> -- 
> 2.25.1
> 


^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2022-06-03  1:12 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-31  3:12 [PATCH v5 0/9] populate/unpopulate memory when domain on static allocation Penny Zheng
2022-05-31  3:12 ` [PATCH v5 1/9] xen/arm: rename PGC_reserved to PGC_staticmem Penny Zheng
2022-05-31  8:32   ` Jan Beulich
2022-06-02  2:04     ` Penny Zheng
2022-05-31  3:12 ` [PATCH v5 2/9] xen: do not free reserved memory into heap Penny Zheng
2022-05-31  8:36   ` Jan Beulich
2022-06-02  2:18     ` Penny Zheng
2022-06-02  9:23       ` Jan Beulich
2022-05-31  3:12 ` [PATCH v5 3/9] xen: update SUPPORT.md for static allocation Penny Zheng
2022-06-03  0:59   ` Stefano Stabellini
2022-05-31  3:12 ` [PATCH v5 4/9] xen: do not merge reserved pages in free_heap_pages() Penny Zheng
2022-05-31  3:12 ` [PATCH v5 5/9] xen: add field "flags" to cover all internal CDF_XXX Penny Zheng
2022-05-31  3:12 ` [PATCH v5 6/9] xen/arm: introduce CDF_staticmem Penny Zheng
2022-05-31  8:40   ` Jan Beulich
2022-06-02 10:07     ` Penny Zheng
2022-06-02 10:34       ` Jan Beulich
2022-06-03  1:05   ` Stefano Stabellini
2022-05-31  3:12 ` [PATCH v5 7/9] xen/arm: unpopulate memory when domain is static Penny Zheng
2022-05-31  8:46   ` Jan Beulich
2022-06-03  1:11   ` Stefano Stabellini
2022-05-31  3:12 ` [PATCH v5 8/9] xen: introduce prepare_staticmem_pages Penny Zheng
2022-05-31  8:48   ` Jan Beulich
2022-05-31  3:12 ` [PATCH v5 9/9] xen: retrieve reserved pages on populate_physmap Penny Zheng
2022-05-31  8:54   ` Jan Beulich
2022-05-31  9:35     ` Julien Grall
2022-05-31  9:40       ` Jan Beulich
2022-05-31  9:50         ` Julien Grall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.