RE: [PATCH v6 7/9] xen/arm: unpopulate memory when domain is static

2022-06-08 Thread Penny Zheng
Hi Julien

> -Original Message-
> From: Julien Grall 
> Sent: Tuesday, June 7, 2022 5:20 PM
> To: Penny Zheng ; xen-devel@lists.xenproject.org
> Cc: Wei Chen ; Stefano Stabellini
> ; Bertrand Marquis ;
> Volodymyr Babchuk ; Andrew Cooper
> ; George Dunlap ;
> Jan Beulich ; Wei Liu 
> Subject: Re: [PATCH v6 7/9] xen/arm: unpopulate memory when domain is
> static
> 
> Hi Penny,
> 
> On 07/06/2022 08:30, 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 
> > Acked-by: Jan Beulich 
> > ---
> > v6 changes:
> > - refine in-code comment
> > - move PGC_static !CONFIG_STATIC_MEMORY definition to common header
> 
> I don't understand why this change is necessary for this patch. AFAICT, all 
> the
> users of PGC_static will be protected by #ifdef CONFIG_STATIC_MEMORY and
> therefore PGC_static should always be defined.
> 

True, I notice that arch_free_heap_page has already been guarded with
#ifdef CONFIG_STATIC_MEMORY. I'll revert the change.

> Cheers,
> 
> --
> Julien Grall


Re: [PATCH v6 7/9] xen/arm: unpopulate memory when domain is static

2022-06-07 Thread Julien Grall

Hi Penny,

On 07/06/2022 08:30, 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 
Acked-by: Jan Beulich 
---
v6 changes:
- refine in-code comment
- move PGC_static !CONFIG_STATIC_MEMORY definition to common header


I don't understand why this change is necessary for this patch. AFAICT, 
all the users of PGC_static will be protected by #ifdef 
CONFIG_STATIC_MEMORY and therefore PGC_static should always be defined.


Cheers,

--
Julien Grall



[PATCH v6 7/9] xen/arm: unpopulate memory when domain is static

2022-06-07 Thread 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 
Acked-by: Jan Beulich 
---
v6 changes:
- refine in-code comment
- move PGC_static !CONFIG_STATIC_MEMORY definition to common header
---
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/common/page_alloc.c   |  4 
 xen/include/xen/mm.h  |  4 
 xen/include/xen/sched.h   |  3 +++
 5 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index 7442893e77..2ce4d80796 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_static )  \
+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(>page_list);
 INIT_PAGE_LIST_HEAD(>extra_page_list);
 INIT_PAGE_LIST_HEAD(>xenpage_list);
+#ifdef CONFIG_STATIC_MEMORY
+INIT_PAGE_LIST_HEAD(>resv_page_list);
+#endif
+
 
 spin_lock_init(>node_affinity_lock);
 d->node_affinity = NODE_MASK_ALL;
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 7fb28e2e07..886b5d82a2 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_static
-#define PGC_static 0
-#endif
-
 /*
  * Comma-separated list of hexadecimal page numbers containing bad bytes.
  * e.g. 'badpage=0x3f45,0x8a321'.
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 1c4ddb336b..e80b4bdcde 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -210,6 +210,10 @@ extern struct domain *dom_cow;
 
 #include 
 
+#ifndef PGC_static
+#define PGC_static 0
+#endif
+
 static inline bool is_special_page(const struct page_info *page)
 {
 return is_xen_heap_page(page) || (page->count_info & PGC_extra);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 5191853c18..bd2782b3c5 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 */
+#endif
 
 /*
  * This field should only be directly accessed by domain_adjust_tot_pages()
-- 
2.25.1